diff options
author | Spandan Das <spandandas@google.com> | 2024-03-04 10:07:19 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2024-03-04 10:07:19 +0000 |
commit | d1816e09f0a1ec71532a711731ddccc774ae799b (patch) | |
tree | 56670c02295ad8eba83b6c848cdd750f552d6a7b | |
parent | 5d7d4ba90be7737e24e76d94c43224ecde2a7395 (diff) | |
parent | f2c105758659078460effa2564854db8e9c372ca (diff) | |
download | soong-d1816e09f0a1ec71532a711731ddccc774ae799b.tar.gz |
Fix non-determinism in prebuilt selection am: f2c1057586
Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2984036
Change-Id: Id4f52ec49aad78751373840c6e1dc252990db0f1
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r-- | android/prebuilt.go | 31 | ||||
-rw-r--r-- | apex/apex_test.go | 4 | ||||
-rw-r--r-- | cc/prebuilt_test.go | 92 |
3 files changed, 124 insertions, 3 deletions
diff --git a/android/prebuilt.go b/android/prebuilt.go index 13cda9dad..734c1101f 100644 --- a/android/prebuilt.go +++ b/android/prebuilt.go @@ -534,6 +534,35 @@ func hideUnflaggedModules(ctx BottomUpMutatorContext, psi PrebuiltSelectionInfoM for _, moduleInFamily := range allModulesInFamily { if moduleInFamily.Name() != selectedModuleInFamily.Name() { moduleInFamily.HideFromMake() + // If this is a prebuilt module, unset properties.UsePrebuilt + // properties.UsePrebuilt might evaluate to true via soong config var fallback mechanism + // Set it to false explicitly so that the following mutator does not replace rdeps to this unselected prebuilt + if p := GetEmbeddedPrebuilt(moduleInFamily); p != nil { + p.properties.UsePrebuilt = false + } + } + } + } + // Do a validation pass to make sure that multiple prebuilts of a specific module are not selected. + // This might happen if the prebuilts share the same soong config var namespace. + // This should be an error, unless one of the prebuilts has been explicitly declared in apex_contributions + var selectedPrebuilt Module + for _, moduleInFamily := range allModulesInFamily { + // Skip if this module is in a different namespace + if !moduleInFamily.ExportedToMake() { + continue + } + // Skip for the top-level java_sdk_library_(_import). This has some special cases that need to be addressed first. + // This does not run into non-determinism because PrebuiltPostDepsMutator also has the special case + if sdkLibrary, ok := moduleInFamily.(interface{ SdkLibraryName() *string }); ok && sdkLibrary.SdkLibraryName() != nil { + continue + } + if p := GetEmbeddedPrebuilt(moduleInFamily); p != nil && p.properties.UsePrebuilt { + if selectedPrebuilt == nil { + selectedPrebuilt = moduleInFamily + } else { + ctx.ModuleErrorf("Multiple prebuilt modules %v and %v have been marked as preferred for this source module. "+ + "Please add the appropriate prebuilt module to apex_contributions for this release config.", selectedPrebuilt.Name(), moduleInFamily.Name()) } } } @@ -562,7 +591,7 @@ func PrebuiltPostDepsMutator(ctx BottomUpMutatorContext) { // If we do not special-case this here, rdeps referring to a java_sdk_library in next builds via libs // will get prebuilt stubs // TODO (b/308187268): Remove this after the apexes have been added to apex_contributions - if psi.IsSelected(*sdkLibrary.SdkLibraryName()) { + if psi.IsSelected(name) { return false } } diff --git a/apex/apex_test.go b/apex/apex_test.go index 97b9c84f0..54d2d08ff 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -6199,7 +6199,7 @@ func TestBootDexJarsFromSourcesAndPrebuilts(t *testing.T) { `) }) - t.Run("Co-existing unflagged apexes should create a duplicate deapexer error in hiddenapi processing", func(t *testing.T) { + t.Run("Co-existing unflagged apexes should create a duplicate module error", func(t *testing.T) { bp := ` // Source apex { @@ -6273,7 +6273,7 @@ func TestBootDexJarsFromSourcesAndPrebuilts(t *testing.T) { } ` - testDexpreoptWithApexes(t, bp, "Multiple installable prebuilt APEXes provide ambiguous deapexers: prebuilt_myapex.v1 and prebuilt_myapex.v2", preparer, fragment) + testDexpreoptWithApexes(t, bp, "Multiple prebuilt modules prebuilt_myapex.v1 and prebuilt_myapex.v2 have been marked as preferred for this source module", preparer, fragment) }) } diff --git a/cc/prebuilt_test.go b/cc/prebuilt_test.go index f6b5ed56c..7dfe6422f 100644 --- a/cc/prebuilt_test.go +++ b/cc/prebuilt_test.go @@ -610,3 +610,95 @@ func TestMultiplePrebuilts(t *testing.T) { android.AssertStringEquals(t, "unexpected LOCAL_MODULE", "libbar", entries.EntryMap["LOCAL_MODULE"][0]) } } + +// Setting prefer on multiple prebuilts is an error, unless one of them is also listed in apex_contributions +func TestMultiplePrebuiltsPreferredUsingLegacyFlags(t *testing.T) { + bp := ` + // an rdep + cc_library { + name: "libfoo", + shared_libs: ["libbar"], + } + + // multiple variations of dep + // source + cc_library { + name: "libbar", + } + // prebuilt "v1" + cc_prebuilt_library_shared { + name: "libbar", + srcs: ["libbar.so"], + prefer: true, + } + // prebuilt "v2" + cc_prebuilt_library_shared { + name: "libbar.v2", + stem: "libbar", + source_module_name: "libbar", + srcs: ["libbar.so"], + prefer: true, + } + + // selectors + apex_contributions { + name: "myapex_contributions", + contents: [%v], + } + all_apex_contributions {name: "all_apex_contributions"} + ` + hasDep := func(ctx *android.TestContext, m android.Module, wantDep android.Module) bool { + t.Helper() + var found bool + ctx.VisitDirectDeps(m, func(dep blueprint.Module) { + if dep == wantDep { + found = true + } + }) + return found + } + + testCases := []struct { + desc string + selectedDependencyName string + expectedDependencyName string + expectedErr string + }{ + { + desc: "Multiple prebuilts have prefer: true", + expectedErr: "Multiple prebuilt modules prebuilt_libbar and prebuilt_libbar.v2 have been marked as preferred for this source module", + }, + { + desc: "Multiple prebuilts have prefer: true. The prebuilt listed in apex_contributions wins.", + selectedDependencyName: `"prebuilt_libbar"`, + expectedDependencyName: "prebuilt_libbar", + }, + } + + for _, tc := range testCases { + preparer := android.GroupFixturePreparers( + android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) { + android.RegisterApexContributionsBuildComponents(ctx) + }), + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.BuildFlags = map[string]string{ + "RELEASE_APEX_CONTRIBUTIONS_ADSERVICES": "myapex_contributions", + } + }), + ) + if tc.expectedErr != "" { + preparer = preparer.ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(tc.expectedErr)) + } + + ctx := testPrebuilt(t, fmt.Sprintf(bp, tc.selectedDependencyName), map[string][]byte{ + "libbar.so": nil, + "crtx.o": nil, + }, preparer) + if tc.expectedErr != "" { + return // the fixture will assert that the excepted err has been raised + } + libfoo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_shared").Module() + expectedDependency := ctx.ModuleForTests(tc.expectedDependencyName, "android_arm64_armv8-a_shared").Module() + android.AssertBoolEquals(t, fmt.Sprintf("expected dependency from %s to %s\n", libfoo.Name(), tc.expectedDependencyName), true, hasDep(ctx, libfoo, expectedDependency)) + } +} |