Improve error reporting when depending on prebuilt implementation jar
The sdk snapshot must not be including implementation code for boot libraries, the implementation is provided by dex jars within the corresponding APEX. However, the snapshot does need a module for each boot library so that the build can seamlessly access the dex files from the APEX. A java_library boot library (like core-oj) is represented in the snapshot by a java_import module which requires a jar file to be provided, otherwise it is disabled. However, that is provided purely to keep Soong happy and should never be used. Previously, the snapshot would contain an empty file for the jar. As an empty file is an invalid jar any tool (like compiler) that tried to consume it would fail which was the correct behavior. Unfortunately, the error message that was produced was not very helpful, it was just some variant on `invalid file` which lead to a lot of bugs being raised. This change replaces that empty file with a reference to the output from a genrule which runs a script which produces a more useful error message, with information on how to fix the issue, and fails the build. It also adds a Name() method to the SdkMemberProperties type as that is needed in AddInternalModule() to construct the name of the additional module. Tested as follows: In AOSP/master make the following changes: 1. Temporarily set visibility on core-oj and core-libart to //visibility:public. 2. Run packages/modules/common/build/mainline_modules_sdks.py to create the snapshots. For each of the S, T and latest snapshots I did the following in the s-aml-prebuilt-test, t-aml-prebuilt-test and aosp/master branches: 1. Created an Android.bp file containing the following: java_library { name: "broken", static_libs: [ "prebuilt_core-libart", "prebuilt_core-oj", ], } 2. Fix the visibility issues and run `m broken` where it fails with an invalid file. 3. Delete the contents of the prebuilts/module_sdk/art/current/sdk directory. 4. Unpack the relevant version of the art-module-sdk snapshot into the directory. 5. Run `m broken` where it fails with the helpful message. 6. Test the instructions on how to use the ninja -t path tool to identify the cause of the problem and fix it. Bug: 257969510 Test: See above. Change-Id: I125bde2d7202afff84c97daebcef37e21c548a3a
This commit is contained in:
80
java/java.go
80
java/java.go
@@ -86,11 +86,11 @@ func RegisterJavaSdkMemberTypes() {
|
||||
var (
|
||||
// Supports adding java header libraries to module_exports and sdk.
|
||||
javaHeaderLibsSdkMemberType = &librarySdkMemberType{
|
||||
android.SdkMemberTypeBase{
|
||||
SdkMemberTypeBase: android.SdkMemberTypeBase{
|
||||
PropertyName: "java_header_libs",
|
||||
SupportsSdk: true,
|
||||
},
|
||||
func(_ android.SdkMemberContext, j *Library) android.Path {
|
||||
jarToExportGetter: func(_ android.SdkMemberContext, j *Library) android.Path {
|
||||
headerJars := j.HeaderJars()
|
||||
if len(headerJars) != 1 {
|
||||
panic(fmt.Errorf("there must be only one header jar from %q", j.Name()))
|
||||
@@ -98,8 +98,8 @@ var (
|
||||
|
||||
return headerJars[0]
|
||||
},
|
||||
sdkSnapshotFilePathForJar,
|
||||
copyEverythingToSnapshot,
|
||||
snapshotPathGetter: sdkSnapshotFilePathForJar,
|
||||
onlyCopyJarToSnapshot: copyEverythingToSnapshot,
|
||||
}
|
||||
|
||||
// Export implementation classes jar as part of the sdk.
|
||||
@@ -113,12 +113,12 @@ var (
|
||||
|
||||
// Supports adding java implementation libraries to module_exports but not sdk.
|
||||
javaLibsSdkMemberType = &librarySdkMemberType{
|
||||
android.SdkMemberTypeBase{
|
||||
SdkMemberTypeBase: android.SdkMemberTypeBase{
|
||||
PropertyName: "java_libs",
|
||||
},
|
||||
exportImplementationClassesJar,
|
||||
sdkSnapshotFilePathForJar,
|
||||
copyEverythingToSnapshot,
|
||||
jarToExportGetter: exportImplementationClassesJar,
|
||||
snapshotPathGetter: sdkSnapshotFilePathForJar,
|
||||
onlyCopyJarToSnapshot: copyEverythingToSnapshot,
|
||||
}
|
||||
|
||||
snapshotRequiresImplementationJar = func(ctx android.SdkMemberContext) bool {
|
||||
@@ -143,11 +143,11 @@ var (
|
||||
// necessary. The java_boot_libs property to allow those modules to be exported as part of the
|
||||
// sdk/module_exports without exposing any unnecessary information.
|
||||
javaBootLibsSdkMemberType = &librarySdkMemberType{
|
||||
android.SdkMemberTypeBase{
|
||||
SdkMemberTypeBase: android.SdkMemberTypeBase{
|
||||
PropertyName: "java_boot_libs",
|
||||
SupportsSdk: true,
|
||||
},
|
||||
func(ctx android.SdkMemberContext, j *Library) android.Path {
|
||||
jarToExportGetter: func(ctx android.SdkMemberContext, j *Library) android.Path {
|
||||
if snapshotRequiresImplementationJar(ctx) {
|
||||
return exportImplementationClassesJar(ctx, j)
|
||||
}
|
||||
@@ -156,9 +156,9 @@ var (
|
||||
// jar for use by dexpreopting and boot jars package check. They do not need to provide an
|
||||
// actual implementation jar but the java_import will need a file that exists so just copy an
|
||||
// empty file. Any attempt to use that file as a jar will cause a build error.
|
||||
return ctx.SnapshotBuilder().EmptyFile()
|
||||
return nil
|
||||
},
|
||||
func(ctx android.SdkMemberContext, osPrefix, name string) string {
|
||||
snapshotPathGetter: func(ctx android.SdkMemberContext, osPrefix, name string) string {
|
||||
if snapshotRequiresImplementationJar(ctx) {
|
||||
return sdkSnapshotFilePathForJar(ctx, osPrefix, name)
|
||||
}
|
||||
@@ -168,7 +168,7 @@ var (
|
||||
// TODO(b/175714559): Provide a proper error message in Soong not ninja.
|
||||
return filepath.Join(osPrefix, "java_boot_libs", "snapshot", "jars", "are", "invalid", name+jarFileSuffix)
|
||||
},
|
||||
onlyCopyJarToSnapshot,
|
||||
onlyCopyJarToSnapshot: onlyCopyJarToSnapshot,
|
||||
}
|
||||
|
||||
// Supports adding java systemserver libraries to module_exports and sdk.
|
||||
@@ -182,27 +182,27 @@ var (
|
||||
// necessary. The java_systemserver_libs property to allow those modules to be exported as part of
|
||||
// the sdk/module_exports without exposing any unnecessary information.
|
||||
javaSystemserverLibsSdkMemberType = &librarySdkMemberType{
|
||||
android.SdkMemberTypeBase{
|
||||
SdkMemberTypeBase: android.SdkMemberTypeBase{
|
||||
PropertyName: "java_systemserver_libs",
|
||||
SupportsSdk: true,
|
||||
|
||||
// This was only added in Tiramisu.
|
||||
SupportedBuildReleaseSpecification: "Tiramisu+",
|
||||
},
|
||||
func(ctx android.SdkMemberContext, j *Library) android.Path {
|
||||
jarToExportGetter: func(ctx android.SdkMemberContext, j *Library) android.Path {
|
||||
// Java systemserver libs are only provided in the SDK to provide access to their dex
|
||||
// implementation jar for use by dexpreopting. They do not need to provide an actual
|
||||
// implementation jar but the java_import will need a file that exists so just copy an empty
|
||||
// file. Any attempt to use that file as a jar will cause a build error.
|
||||
return ctx.SnapshotBuilder().EmptyFile()
|
||||
return nil
|
||||
},
|
||||
func(_ android.SdkMemberContext, osPrefix, name string) string {
|
||||
snapshotPathGetter: func(_ android.SdkMemberContext, osPrefix, name string) string {
|
||||
// Create a special name for the implementation jar to try and provide some useful information
|
||||
// to a developer that attempts to compile against this.
|
||||
// TODO(b/175714559): Provide a proper error message in Soong not ninja.
|
||||
return filepath.Join(osPrefix, "java_systemserver_libs", "snapshot", "jars", "are", "invalid", name+jarFileSuffix)
|
||||
},
|
||||
onlyCopyJarToSnapshot,
|
||||
onlyCopyJarToSnapshot: onlyCopyJarToSnapshot,
|
||||
}
|
||||
|
||||
// Supports adding java test libraries to module_exports but not sdk.
|
||||
@@ -232,7 +232,7 @@ type JavaInfo struct {
|
||||
ImplementationAndResourcesJars android.Paths
|
||||
|
||||
// ImplementationJars is a list of jars that contain the implementations of classes in the
|
||||
//module.
|
||||
// module.
|
||||
ImplementationJars android.Paths
|
||||
|
||||
// ResourceJars is a list of jars that contain the resources included in the module.
|
||||
@@ -718,7 +718,8 @@ type librarySdkMemberType struct {
|
||||
android.SdkMemberTypeBase
|
||||
|
||||
// Function to retrieve the appropriate output jar (implementation or header) from
|
||||
// the library.
|
||||
// the library, if this returns nil then it is assumed that the snapshot must not provide access
|
||||
// to the jar.
|
||||
jarToExportGetter func(ctx android.SdkMemberContext, j *Library) android.Path
|
||||
|
||||
// Function to compute the snapshot relative path to which the named library's
|
||||
@@ -755,7 +756,11 @@ func (mt *librarySdkMemberType) CreateVariantPropertiesStruct() android.SdkMembe
|
||||
type librarySdkMemberProperties struct {
|
||||
android.SdkMemberPropertiesBase
|
||||
|
||||
JarToExport android.Path `android:"arch_variant"`
|
||||
JarToExport android.Path `android:"arch_variant"`
|
||||
|
||||
// The path to a script to use when the jar is invalid.
|
||||
InvalidJarScript android.Path
|
||||
|
||||
AidlIncludeDirs android.Paths
|
||||
|
||||
// The list of permitted packages that need to be passed to the prebuilts as they are used to
|
||||
@@ -766,7 +771,15 @@ type librarySdkMemberProperties struct {
|
||||
func (p *librarySdkMemberProperties) PopulateFromVariant(ctx android.SdkMemberContext, variant android.Module) {
|
||||
j := variant.(*Library)
|
||||
|
||||
p.JarToExport = ctx.MemberType().(*librarySdkMemberType).jarToExportGetter(ctx, j)
|
||||
memberType := ctx.MemberType().(*librarySdkMemberType)
|
||||
p.JarToExport = memberType.jarToExportGetter(ctx, j)
|
||||
|
||||
// If no jar was provided for export then disallow access to it completely.
|
||||
if p.JarToExport == nil {
|
||||
// Copy the script to prevent access to the jar into the snapshot.
|
||||
p.InvalidJarScript = android.PathForSource(ctx.SdkModuleContext(),
|
||||
"build/soong/java/invalid_implementation_jar.sh")
|
||||
}
|
||||
|
||||
p.AidlIncludeDirs = j.AidlIncludeDirs()
|
||||
|
||||
@@ -789,6 +802,21 @@ func (p *librarySdkMemberProperties) AddToPropertySet(ctx android.SdkMemberConte
|
||||
propertySet.AddProperty("jars", []string{snapshotRelativeJavaLibPath})
|
||||
}
|
||||
|
||||
if scriptSrc := p.InvalidJarScript; scriptSrc != nil {
|
||||
// Copy the script to prevent access to the jar into the snapshot.
|
||||
scriptDest := filepath.Join("scripts", scriptSrc.Base())
|
||||
builder.CopyToSnapshot(scriptSrc, scriptDest)
|
||||
|
||||
// Generate a genrule module that will invoke the script passing in the module name.
|
||||
genrule := builder.AddInternalModule(p, "genrule", "error")
|
||||
genRuleName := genrule.Name()
|
||||
genrule.AddProperty("out", []string{"this-file-will-never-be-created.jar"})
|
||||
genrule.AddProperty("tool_files", []string{scriptDest})
|
||||
genrule.AddProperty("cmd", fmt.Sprintf("$(location %s) %s", scriptDest, p.Name()))
|
||||
|
||||
propertySet.AddPropertyWithTag("jars", []string{":" + genRuleName}, builder.SdkMemberReferencePropertyTag(true))
|
||||
}
|
||||
|
||||
if len(p.PermittedPackages) > 0 {
|
||||
propertySet.AddProperty("permitted_packages", p.PermittedPackages)
|
||||
}
|
||||
@@ -1650,7 +1678,7 @@ func (j *Import) DepsMutator(ctx android.BottomUpMutatorContext) {
|
||||
}
|
||||
|
||||
func (j *Import) commonBuildActions(ctx android.ModuleContext) {
|
||||
//TODO(b/231322772) these should come from Bazel once available
|
||||
// TODO(b/231322772) these should come from Bazel once available
|
||||
j.sdkVersion = j.SdkVersion(ctx)
|
||||
j.minSdkVersion = j.MinSdkVersion(ctx)
|
||||
|
||||
@@ -2253,7 +2281,7 @@ func (m *Library) convertJavaResourcesAttributes(ctx android.TopDownMutatorConte
|
||||
resources.Append(android.BazelLabelForModuleSrc(ctx, m.properties.Java_resources))
|
||||
}
|
||||
|
||||
//TODO(b/179889880) handle case where glob includes files outside package
|
||||
// TODO(b/179889880) handle case where glob includes files outside package
|
||||
resDeps := ResourceDirsToFiles(
|
||||
ctx,
|
||||
m.properties.Java_resource_dirs,
|
||||
@@ -2401,7 +2429,7 @@ func (m *Library) convertLibraryAttrsBp2Build(ctx android.TopDownMutatorContext)
|
||||
}
|
||||
|
||||
epEnabled := m.properties.Errorprone.Enabled
|
||||
//TODO(b/227504307) add configuration that depends on RUN_ERROR_PRONE environment variable
|
||||
// TODO(b/227504307) add configuration that depends on RUN_ERROR_PRONE environment variable
|
||||
if Bool(epEnabled) {
|
||||
javacopts = append(javacopts, m.properties.Errorprone.Javacflags...)
|
||||
}
|
||||
@@ -2637,7 +2665,7 @@ func (i *Import) ProcessBazelQueryResponse(ctx android.ModuleContext) {
|
||||
HeaderJars: android.PathsIfNonNil(i.combinedClasspathFile),
|
||||
ImplementationAndResourcesJars: android.PathsIfNonNil(i.combinedClasspathFile),
|
||||
ImplementationJars: android.PathsIfNonNil(i.combinedClasspathFile),
|
||||
//TODO(b/240308299) include AIDL information from Bazel
|
||||
// TODO(b/240308299) include AIDL information from Bazel
|
||||
})
|
||||
|
||||
i.maybeInstall(ctx, jarName, outputFile)
|
||||
|
Reference in New Issue
Block a user