Skip to content

Commit

Permalink
Merge "Revert "Fix non-determinism in prebuilt selection"" into main …
Browse files Browse the repository at this point in the history
…am: 5354483 am: 1e98107

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2984372

Change-Id: Ied7c0c39b23d6187523b31a92c0851c43349149a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
  • Loading branch information
dasspandan authored and android-build-merge-worker-robot committed Feb 29, 2024
2 parents d693e1c + 1e98107 commit 341f6c7
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 120 deletions.
27 changes: 1 addition & 26 deletions android/prebuilt.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,31 +534,6 @@ 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 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())
}
}
}
Expand Down Expand Up @@ -587,7 +562,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(name) {
if psi.IsSelected(*sdkLibrary.SdkLibraryName()) {
return false
}
}
Expand Down
4 changes: 2 additions & 2 deletions apex/apex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6199,7 +6199,7 @@ func TestBootDexJarsFromSourcesAndPrebuilts(t *testing.T) {
`)
})

t.Run("Co-existing unflagged apexes should create a duplicate module error", func(t *testing.T) {
t.Run("Co-existing unflagged apexes should create a duplicate deapexer error in hiddenapi processing", func(t *testing.T) {
bp := `
// Source
apex {
Expand Down Expand Up @@ -6273,7 +6273,7 @@ func TestBootDexJarsFromSourcesAndPrebuilts(t *testing.T) {
}
`

testDexpreoptWithApexes(t, bp, "Multiple prebuilt modules prebuilt_myapex.v1 and prebuilt_myapex.v2 have been marked as preferred for this source module", preparer, fragment)
testDexpreoptWithApexes(t, bp, "Multiple installable prebuilt APEXes provide ambiguous deapexers: prebuilt_myapex.v1 and prebuilt_myapex.v2", preparer, fragment)
})

}
Expand Down
92 changes: 0 additions & 92 deletions cc/prebuilt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,95 +610,3 @@ 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))
}
}

0 comments on commit 341f6c7

Please sign in to comment.