Check for duplicate bundled files from different modules.

A single file generated by different modules is almost never correct;
make it an error.

The check is skipped for some error cases, which will be handled in follow-ups.

Bug: 191770320
Test: m (soong test)
Change-Id: Ib3c6efd16bdf13b59f79be66d4f9dba49100f6cc

apex/apex.go

@@ -2066,15 +2066,23 @@ type visitorContext struct {
 	requireNativeLibs []string
 
 	handleSpecialLibs bool
+
+	// if true, raise error on duplicate apexFile
+	checkDuplicate bool
 }
 
-func (vctx *visitorContext) normalizeFileInfo() {
+func (vctx *visitorContext) normalizeFileInfo(mctx android.ModuleContext) {
 	encountered := make(map[string]apexFile)
 	for _, f := range vctx.filesInfo {
 		dest := filepath.Join(f.installDir, f.builtFile.Base())
 		if e, ok := encountered[dest]; !ok {
 			encountered[dest] = f
 		} else {
+			if vctx.checkDuplicate && f.builtFile.String() != e.builtFile.String() {
+				mctx.ModuleErrorf("apex file %v is provided by two different files %v and %v",
+					dest, e.builtFile, f.builtFile)
+				return
+			}
 			// If a module is directly included and also transitively depended on
 			// consider it as directly included.
 			e.transitiveDep = e.transitiveDep && f.transitiveDep
@@ -2433,6 +2441,25 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext,
 	return false
 }
 
+func (a *apexBundle) shouldCheckDuplicate(ctx android.ModuleContext) bool {
+	// TODO(b/263308293) remove this
+	if a.properties.IsCoverageVariant {
+		return false
+	}
+	// TODO(b/263308515) remove this
+	if a.testApex {
+		return false
+	}
+	// TODO(b/263309864) remove this
+	if a.Host() {
+		return false
+	}
+	if a.Device() && ctx.DeviceConfig().DeviceArch() == "" {
+		return false
+	}
+	return true
+}
+
 // Creates build rules for an APEX. It consists of the following major steps:
 //
 // 1) do some validity checks such as apex_available, min_sdk_version, etc.
@@ -2453,9 +2480,12 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 
 	// TODO(jiyong): do this using WalkPayloadDeps
 	// TODO(jiyong): make this clean!!!
-	vctx := visitorContext{handleSpecialLibs: !android.Bool(a.properties.Ignore_system_library_special_case)}
+	vctx := visitorContext{
+		handleSpecialLibs: !android.Bool(a.properties.Ignore_system_library_special_case),
+		checkDuplicate:    a.shouldCheckDuplicate(ctx),
+	}
 	ctx.WalkDepsBlueprint(func(child, parent blueprint.Module) bool { return a.depVisitor(&vctx, ctx, child, parent) })
-	vctx.normalizeFileInfo()
+	vctx.normalizeFileInfo(ctx)
 	if a.privateKeyFile == nil {
 		ctx.PropertyErrorf("key", "private_key for %q could not be found", String(a.overridableProperties.Key))
 		return

apex/apex_test.go

@@ -6992,6 +6992,42 @@ func TestCompatConfig(t *testing.T) {
 	})
 }
 
+func TestNoDupeApexFiles(t *testing.T) {
+	android.GroupFixturePreparers(
+		android.PrepareForTestWithAndroidBuildComponents,
+		PrepareForTestWithApexBuildComponents,
+		prepareForTestWithMyapex,
+		prebuilt_etc.PrepareForTestWithPrebuiltEtc,
+	).
+		ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern("is provided by two different files")).
+		RunTestWithBp(t, `
+			apex {
+				name: "myapex",
+				key: "myapex.key",
+				prebuilts: ["foo", "bar"],
+				updatable: false,
+			}
+
+			apex_key {
+				name: "myapex.key",
+				public_key: "testkey.avbpubkey",
+				private_key: "testkey.pem",
+			}
+
+			prebuilt_etc {
+				name: "foo",
+				src: "myprebuilt",
+				filename_from_src: true,
+			}
+
+			prebuilt_etc {
+				name: "bar",
+				src: "myprebuilt",
+				filename_from_src: true,
+			}
+		`)
+}
+
 func TestRejectNonInstallableJavaLibrary(t *testing.T) {
 	testApexError(t, `"myjar" is not configured to be compiled into dex`, `
 		apex {

apex/bootclasspath_fragment_test.go

@@ -71,10 +71,6 @@ func TestBootclasspathFragments(t *testing.T) {
 			name: "com.android.art",
 			key: "com.android.art.key",
 			bootclasspath_fragments: ["art-bootclasspath-fragment"],
- 			java_libs: [
-				"baz",
-				"quuz",
-			],
 			updatable: false,
 		}
 
@@ -301,11 +297,7 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) {
 				"mybootclasspathfragment",
 			],
 			// bar (like foo) should be transitively included in this apex because it is part of the
-			// mybootclasspathfragment bootclasspath_fragment. However, it is kept here to ensure that the
-			// apex dedups the files correctly.
-			java_libs: [
-				"bar",
-			],
+			// mybootclasspathfragment bootclasspath_fragment.
 			updatable: false,
 		}
 
@@ -445,7 +437,6 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) {
 		})
 
 		java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{
-			`bar`,
 			`com.android.art.key`,
 			`mybootclasspathfragment`,
 		})
@@ -559,7 +550,6 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) {
 		})
 
 		java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{
-			`bar`,
 			`com.android.art.key`,
 			`mybootclasspathfragment`,
 			`prebuilt_com.android.art`,
@@ -1105,10 +1095,6 @@ func TestBootclasspathFragment_AndroidNonUpdatable(t *testing.T) {
 			name: "com.android.art",
 			key: "com.android.art.key",
 			bootclasspath_fragments: ["art-bootclasspath-fragment"],
- 			java_libs: [
-				"baz",
-				"quuz",
-			],
 			updatable: false,
 		}
 
@@ -1270,10 +1256,6 @@ func TestBootclasspathFragment_AndroidNonUpdatable_AlwaysUsePrebuiltSdks(t *test
 			name: "com.android.art",
 			key: "com.android.art.key",
 			bootclasspath_fragments: ["art-bootclasspath-fragment"],
- 			java_libs: [
-				"baz",
-				"quuz",
-			],
 			updatable: false,
 		}