From f98da19a07203f01521c92067aeac336910bfb16 Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Mon, 15 Apr 2024 11:15:41 +0000 Subject: Refactor the contruction of the manifest check inputs. This is a no-op change for a majority of cases. Before this change, the contruction of the manifest check inputs is confusing. It mutates uses_libs properties in place just for the manifest check, by replacing module names with library names for direct dependencies and merging library names from CLC for both direct denpendencies and transitive denpendencies, and then constructs manifest check inputs from those mutated uses_libs properties. This is error-prone and leads to insistency: the goal is to check that the CLC matches the manifest, but the inputs to the check don't reflect the CLC. After this change, we no longer mutate uses_libs properties in place. Instead, we maintain a separate list of missing denpendencies, and then construct manifest check inputs directly from the CLC for all existing libraries, no matter they are direct or transtive, and from the separate list of missing libraries. This change makes the logic more consistent and straightforward, and it also allows us to easily do the next change, which is to propagate transtive missing denpendencies. In fact, this change revealed several bugs around library optionality and order in CLC construction, and fixed them. Bug: 331528424 Test: m --no-skip-soong-tests Ignore-AOSP-First: Depends on internal changes. Will cherry-pick once merged. Merged-In: I0de82e76c47995b54aba9efd41538d950256a95f Change-Id: I0de82e76c47995b54aba9efd41538d950256a95f --- java/app_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'java/app_test.go') diff --git a/java/app_test.go b/java/app_test.go index 8262777b2..d5c1cc75e 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -3280,7 +3280,7 @@ func TestUsesLibraries(t *testing.T) { sdk_version: "current", optional_uses_libs: [ "bar", - "baz", + "missing-lib-b", ], } @@ -3295,7 +3295,7 @@ func TestUsesLibraries(t *testing.T) { ], optional_uses_libs: [ "bar", - "baz", + "missing-lib-b", ], } ` @@ -3317,10 +3317,10 @@ func TestUsesLibraries(t *testing.T) { // propagated from dependencies. actualManifestFixerArgs := app.Output("manifest_fixer/AndroidManifest.xml").Args["args"] expectManifestFixerArgs := `--extract-native-libs=true ` + - `--uses-library qux ` + - `--uses-library quuz ` + `--uses-library foo ` + `--uses-library com.non.sdk.lib ` + + `--uses-library qux ` + + `--uses-library quuz ` + `--uses-library runtime-library ` + `--uses-library runtime-required-x ` + `--uses-library runtime-required-y ` + @@ -3339,9 +3339,9 @@ func TestUsesLibraries(t *testing.T) { `--uses-library runtime-required-x ` + `--uses-library runtime-required-y ` + `--optional-uses-library bar ` + - `--optional-uses-library baz ` + `--optional-uses-library runtime-optional-x ` + - `--optional-uses-library runtime-optional-y ` + `--optional-uses-library runtime-optional-y ` + + `--missing-optional-uses-library missing-lib-b ` android.AssertStringDoesContain(t, "verify cmd args", verifyCmd, verifyArgs) // Test that all libraries are verified for an APK (library order matters). @@ -3350,7 +3350,7 @@ func TestUsesLibraries(t *testing.T) { `--uses-library com.non.sdk.lib ` + `--uses-library android.test.runner ` + `--optional-uses-library bar ` + - `--optional-uses-library baz ` + `--missing-optional-uses-library missing-lib-b ` android.AssertStringDoesContain(t, "verify apk cmd args", verifyApkCmd, verifyApkArgs) // Test that necessary args are passed for constructing CLC in Ninja phase. -- cgit v1.2.3-59-g8ed1b From 369370818f5cfbe5b2d6f9dfbc9be2b2309d9f71 Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Mon, 15 Apr 2024 11:15:50 +0000 Subject: Propagate transitive missing optional_uses_libs. Bug: 331528424 Test: m --no-skip-soong-tests Ignore-AOSP-First: Depends on internal changes. Will cherry-pick once merged. Merged-In: Ied2821f11b6a5056ecf577e1e25765bc6dd212c0 Change-Id: Ied2821f11b6a5056ecf577e1e25765bc6dd212c0 --- java/aar.go | 13 ++++++++++--- java/app.go | 1 + java/app_test.go | 8 ++++++-- java/base.go | 1 + java/java.go | 16 ++++++++++++++++ 5 files changed, 34 insertions(+), 5 deletions(-) (limited to 'java/app_test.go') diff --git a/java/aar.go b/java/aar.go index 037342a21..752d6839f 100644 --- a/java/aar.go +++ b/java/aar.go @@ -356,12 +356,13 @@ type aaptBuildActionOptions struct { forceNonFinalResourceIDs bool extraLinkFlags []string aconfigTextFiles android.Paths + usesLibrary *usesLibrary } func (a *aapt) buildActions(ctx android.ModuleContext, opts aaptBuildActionOptions) { staticResourcesNodesDepSet, sharedResourcesNodesDepSet, staticRRODirsDepSet, staticManifestsDepSet, sharedExportPackages, libFlags := - aaptLibs(ctx, opts.sdkContext, opts.classLoaderContexts) + aaptLibs(ctx, opts.sdkContext, opts.classLoaderContexts, opts.usesLibrary) // Exclude any libraries from the supplied list. opts.classLoaderContexts = opts.classLoaderContexts.ExcludeLibs(opts.excludedLibs) @@ -703,7 +704,8 @@ func (t transitiveAarDeps) assets() android.Paths { } // aaptLibs collects libraries from dependencies and sdk_version and converts them into paths -func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, classLoaderContexts dexpreopt.ClassLoaderContextMap) ( +func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, + classLoaderContexts dexpreopt.ClassLoaderContextMap, usesLibrary *usesLibrary) ( staticResourcesNodes, sharedResourcesNodes *android.DepSet[*resourcesNode], staticRRODirs *android.DepSet[rroDir], staticManifests *android.DepSet[android.Path], sharedLibs android.Paths, flags []string) { @@ -753,6 +755,9 @@ func aaptLibs(ctx android.ModuleContext, sdkContext android.SdkContext, classLoa } addCLCFromDep(ctx, module, classLoaderContexts) + if usesLibrary != nil { + addMissingOptionalUsesLibsFromDep(ctx, module, usesLibrary) + } }) // AAPT2 overlays are in lowest to highest priority order, the topological order will be reversed later. @@ -829,6 +834,7 @@ func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) classLoaderContexts: a.classLoaderContexts, enforceDefaultTargetSdkVersion: false, aconfigTextFiles: getAconfigFilePaths(ctx), + usesLibrary: &a.usesLibrary, }, ) @@ -1212,7 +1218,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { linkDeps = append(linkDeps, a.manifest) staticResourcesNodesDepSet, sharedResourcesNodesDepSet, staticRRODirsDepSet, staticManifestsDepSet, sharedLibs, libFlags := - aaptLibs(ctx, android.SdkContext(a), nil) + aaptLibs(ctx, android.SdkContext(a), nil, nil) _ = sharedResourcesNodesDepSet _ = staticRRODirsDepSet @@ -1282,6 +1288,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } addCLCFromDep(ctx, module, a.classLoaderContexts) + addMissingOptionalUsesLibsFromDep(ctx, module, &a.usesLibrary) }) if len(staticJars) > 0 { diff --git a/java/app.go b/java/app.go index a02ada12c..61e3235bc 100755 --- a/java/app.go +++ b/java/app.go @@ -527,6 +527,7 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { forceNonFinalResourceIDs: nonFinalIds, extraLinkFlags: aaptLinkFlags, aconfigTextFiles: getAconfigFilePaths(ctx), + usesLibrary: &a.usesLibrary, }, ) diff --git a/java/app_test.go b/java/app_test.go index d5c1cc75e..c8bc7706c 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -3244,7 +3244,10 @@ func TestUsesLibraries(t *testing.T) { name: "static-y", srcs: ["a.java"], uses_libs: ["runtime-required-y"], - optional_uses_libs: ["runtime-optional-y"], + optional_uses_libs: [ + "runtime-optional-y", + "missing-lib-a", + ], sdk_version: "current", } @@ -3341,7 +3344,8 @@ func TestUsesLibraries(t *testing.T) { `--optional-uses-library bar ` + `--optional-uses-library runtime-optional-x ` + `--optional-uses-library runtime-optional-y ` + - `--missing-optional-uses-library missing-lib-b ` + `--missing-optional-uses-library missing-lib-b ` + + `--missing-optional-uses-library missing-lib-a` android.AssertStringDoesContain(t, "verify cmd args", verifyCmd, verifyArgs) // Test that all libraries are verified for an APK (library order matters). diff --git a/java/base.go b/java/base.go index 241030eb7..abe1e69c4 100644 --- a/java/base.go +++ b/java/base.go @@ -2385,6 +2385,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } addCLCFromDep(ctx, module, j.classLoaderContexts) + addMissingOptionalUsesLibsFromDep(ctx, module, &j.usesLibrary) }) return deps diff --git a/java/java.go b/java/java.go index f5238476a..963651a21 100644 --- a/java/java.go +++ b/java/java.go @@ -3151,6 +3151,22 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module, } } +func addMissingOptionalUsesLibsFromDep(ctx android.ModuleContext, depModule android.Module, + usesLibrary *usesLibrary) { + + dep, ok := depModule.(ModuleWithUsesLibrary) + if !ok { + return + } + + for _, lib := range dep.UsesLibrary().usesLibraryProperties.Missing_optional_uses_libs { + if !android.InList(lib, usesLibrary.usesLibraryProperties.Missing_optional_uses_libs) { + usesLibrary.usesLibraryProperties.Missing_optional_uses_libs = + append(usesLibrary.usesLibraryProperties.Missing_optional_uses_libs, lib) + } + } +} + type JavaApiContributionImport struct { JavaApiContribution -- cgit v1.2.3-59-g8ed1b