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>

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
 						}
 					}

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)
 	})
 
 }

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))
+	}
+}