From 78210f6c9b607d411a5466d90c68db52d546aabf Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Wed, 2 Dec 2020 13:06:47 +0000 Subject: [PATCH 1/2] Make error message more precise. Bug: 132357300 Test: m nothing Change-Id: Iaac6aaf61a3bbf8476f0b11cf2c01d7d96a79920 --- dexpreopt/class_loader_context.go | 6 +++++- dexpreopt/class_loader_context_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dexpreopt/class_loader_context.go b/dexpreopt/class_loader_context.go index 375921729..deaf77fe4 100644 --- a/dexpreopt/class_loader_context.go +++ b/dexpreopt/class_loader_context.go @@ -437,7 +437,11 @@ func validateClassLoaderContextRec(sdkVer int, clcs []*ClassLoaderContext) (bool if sdkVer == AnySdkVersion { // Return error if dexpreopt doesn't know paths to one of the // dependencies. In the future we may need to relax this and just disable dexpreopt. - return false, fmt.Errorf("invalid path for \"%s\"", clc.Name) + if clc.Host == nil { + return false, fmt.Errorf("invalid build path for \"%s\"", clc.Name) + } else { + return false, fmt.Errorf("invalid install path for \"%s\"", clc.Name) + } } else { // No error for compatibility libraries, as Soong doesn't know if they are needed // (this depends on the targetSdkVersion in the manifest), but the CLC is invalid. diff --git a/dexpreopt/class_loader_context_test.go b/dexpreopt/class_loader_context_test.go index df6856390..be7d4c63b 100644 --- a/dexpreopt/class_loader_context_test.go +++ b/dexpreopt/class_loader_context_test.go @@ -195,7 +195,7 @@ func TestCLCMaybeAdd(t *testing.T) { // But class loader context in such cases should raise an error on validation. t.Run("validate", func(t *testing.T) { _, err := validateClassLoaderContext(m) - checkError(t, err, "invalid path for \"a\"") + checkError(t, err, "invalid build path for \"a\"") }) } From 65b031910bc36bd079759d002ab7b3d66668f132 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Thu, 3 Dec 2020 16:50:22 +0000 Subject: [PATCH 2/2] Do not propagate deps through static SDK component libs. If some Java library/app depends on an SDK component library (e.g. stubs library), then it transitively depends on the SDK library itself (because the component library has a dependency on its SDK library). Previously having this transitive dependency resulted in adding the SDK library to the dependencies of the library/app. However, this doesn't make sense if the app has a *static* dependency on the component library. This patch stops adding dependency in that case. Bug: 132357300 Test: m nothing Test: added new Soong test that would previously fail with an error: invalid build path for "fred" Change-Id: I697a65e461037c95ec56b6c321afa4ec52ccbbec --- java/aar.go | 11 ++++------- java/app_test.go | 14 +++++++++++++- java/java.go | 41 ++++++++++++++++++++++++++++++++++------- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/java/aar.go b/java/aar.go index 1940d7f7b..3b6b34e27 100644 --- a/java/aar.go +++ b/java/aar.go @@ -407,6 +407,7 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext, classLoaderConte ctx.VisitDirectDeps(func(module android.Module) { depName := ctx.OtherModuleName(module) + depTag := ctx.OtherModuleDependencyTag(module) var exportPackage android.Path aarDep, _ := module.(AndroidLibraryDependency) @@ -414,7 +415,7 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext, classLoaderConte exportPackage = aarDep.ExportPackage() } - switch ctx.OtherModuleDependencyTag(module) { + switch depTag { case instrumentationForTag: // Nothing, instrumentationForTag is treated as libTag for javac but not for aapt2. case libTag: @@ -439,7 +440,6 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext, classLoaderConte transitiveStaticLibs = append(transitiveStaticLibs, aarDep.ExportedStaticPackages()...) transitiveStaticLibs = append(transitiveStaticLibs, exportPackage) transitiveStaticLibManifests = append(transitiveStaticLibManifests, aarDep.ExportedManifests()...) - classLoaderContexts.AddContextMap(aarDep.ClassLoaderContexts(), depName) if aarDep.ExportedAssets().Valid() { assets = append(assets, aarDep.ExportedAssets().Path()) } @@ -458,11 +458,8 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext, classLoaderConte } } - // Add nested dependencies after processing the direct dependency: if it is a , - // nested context is added as its subcontext, and should not be re-added at the top-level. - if dep, ok := module.(Dependency); ok { - classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), depName) - } + // Merge dep's CLC after processing the dep itself (which may add its own ). + maybeAddCLCFromDep(module, depTag, depName, classLoaderContexts) }) deps = append(deps, sharedLibs...) diff --git a/java/app_test.go b/java/app_test.go index 6efb0dcb3..8523b874f 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2729,6 +2729,13 @@ func TestUsesLibraries(t *testing.T) { sdk_version: "current", } + java_sdk_library { + name: "fred", + srcs: ["a.java"], + api_packages: ["fred"], + sdk_version: "current", + } + java_sdk_library { name: "bar", srcs: ["a.java"], @@ -2753,7 +2760,12 @@ func TestUsesLibraries(t *testing.T) { name: "app", srcs: ["a.java"], libs: ["qux", "quuz.stubs"], - static_libs: ["static-runtime-helper"], + static_libs: [ + "static-runtime-helper", + // statically linked component libraries should not pull their SDK libraries, + // so "fred" should not be added to class loader context + "fred.stubs", + ], uses_libs: ["foo"], sdk_version: "current", optional_uses_libs: [ diff --git a/java/java.go b/java/java.go index 8738e00cf..a582bfa2d 100644 --- a/java/java.go +++ b/java/java.go @@ -1050,7 +1050,6 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { switch tag { case libTag: deps.classpath = append(deps.classpath, dep.SdkHeaderJars(ctx, j.sdkVersion())...) - // names of sdk libs that are directly depended are exported j.classLoaderContexts.MaybeAddContext(ctx, dep.OptionalImplicitSdkLibrary(), dep.DexJarBuildPath(), dep.DexJarInstallPath()) case staticLibTag: @@ -1062,7 +1061,6 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { deps.bootClasspath = append(deps.bootClasspath, dep.HeaderJars()...) case libTag, instrumentationForTag: deps.classpath = append(deps.classpath, dep.HeaderJars()...) - // sdk lib names from dependencies are re-exported j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName) deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...) pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins() @@ -1075,8 +1073,6 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { deps.staticJars = append(deps.staticJars, dep.ImplementationJars()...) deps.staticHeaderJars = append(deps.staticHeaderJars, dep.HeaderJars()...) deps.staticResourceJars = append(deps.staticResourceJars, dep.ResourceJars()...) - // sdk lib names from dependencies are re-exported - j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName) deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...) pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins() addPlugins(&deps, pluginJars, pluginClasses...) @@ -1151,6 +1147,9 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { deps.systemModules = &systemModules{outputDir, outputDeps} } } + + // Merge dep's CLC after processing the dep itself (which may add its own ). + maybeAddCLCFromDep(module, tag, otherName, j.classLoaderContexts) }) return deps @@ -2781,8 +2780,6 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) { switch tag { case libTag, staticLibTag: flags.classpath = append(flags.classpath, dep.HeaderJars()...) - // sdk lib names from dependencies are re-exported - j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName) case bootClasspathTag: flags.bootClasspath = append(flags.bootClasspath, dep.HeaderJars()...) } @@ -2790,10 +2787,12 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) { switch tag { case libTag: flags.classpath = append(flags.classpath, dep.SdkHeaderJars(ctx, j.sdkVersion())...) - // names of sdk libs that are directly depended are exported j.classLoaderContexts.AddContext(ctx, otherName, dep.DexJarBuildPath(), dep.DexJarInstallPath()) } } + + // Merge dep's CLC after processing the dep itself (which may add its own ). + maybeAddCLCFromDep(module, tag, otherName, j.classLoaderContexts) }) var installFile android.Path @@ -3214,3 +3213,31 @@ var Bool = proptools.Bool var BoolDefault = proptools.BoolDefault var String = proptools.String var inList = android.InList + +// Add class loader context of a given dependency to the given class loader context, provided that +// all the necessary conditions are met. +func maybeAddCLCFromDep(depModule android.Module, depTag blueprint.DependencyTag, + depName string, clcMap dexpreopt.ClassLoaderContextMap) { + + if dep, ok := depModule.(Dependency); ok { + if depTag == libTag { + // Ok, propagate through non-static library dependencies. + } else if depTag == staticLibTag { + // Propagate through static library dependencies, unless it is a + // component library (such as stubs). Component libraries have a dependency on their + // SDK library, which should not be pulled just because of a static component library. + if comp, isComp := depModule.(SdkLibraryComponentDependency); isComp { + if compName := comp.OptionalImplicitSdkLibrary(); compName != nil { + dep = nil + } + } + } else { + // Don't propagate for other dependency tags. + dep = nil + } + + if dep != nil { + clcMap.AddContextMap(dep.ClassLoaderContexts(), depName) + } + } +}