From c6a773df6493ac4697d9e6ad244028a54eceabec Mon Sep 17 00:00:00 2001
From: Jiyong Park <jiyong@google.com>
Date: Tue, 14 May 2024 21:49:11 +0900
Subject: [PATCH] Filter-out deps of unsupported arch

When a common-arch module (ex: phony) has the "required" dependencies on
native modules (ex: cc_library), both 32 and 64-bit variants of the
native modules are added as dependencies. This by itself is fine and
actually is intended, otherwise there's no way for us to install both
arch variants via required deps.

However, this imposes a problem when the common-arch module is depended
on by a filesystem module with compile_multilib: "first". Here, the
expectation is that only the first variant (64-bit) of the native module
is instaled, but in reality both variants are installed.

To handle this situation, make sure that the packaging routine filters
out packaging specs from unsupported architecture.

Bug: N/A
Test: go test ./... under soong/filesyste

Change-Id: Ie1ad5ace2e5d88e00183a115f4a76e5df87a8166
---
 android/module_context.go     |  3 ++
 android/packaging.go          | 23 ++++++++++++
 filesystem/filesystem_test.go | 67 +++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)

diff --git a/android/module_context.go b/android/module_context.go
index 3c1e30a6c..18adb3002 100644
--- a/android/module_context.go
+++ b/android/module_context.go
@@ -497,6 +497,7 @@ func (m *moduleContext) packageFile(fullInstallPath InstallPath, srcPath Path, e
 		partition:             fullInstallPath.partition,
 		skipInstall:           m.skipInstall(),
 		aconfigPaths:          m.getAconfigPaths(),
+		archType:              m.target.Arch.ArchType,
 	}
 	m.packagingSpecs = append(m.packagingSpecs, spec)
 	return spec
@@ -622,6 +623,7 @@ func (m *moduleContext) InstallSymlink(installPath InstallPath, name string, src
 		partition:        fullInstallPath.partition,
 		skipInstall:      m.skipInstall(),
 		aconfigPaths:     m.getAconfigPaths(),
+		archType:         m.target.Arch.ArchType,
 	})
 
 	return fullInstallPath
@@ -665,6 +667,7 @@ func (m *moduleContext) InstallAbsoluteSymlink(installPath InstallPath, name str
 		partition:        fullInstallPath.partition,
 		skipInstall:      m.skipInstall(),
 		aconfigPaths:     m.getAconfigPaths(),
+		archType:         m.target.Arch.ArchType,
 	})
 
 	return fullInstallPath
diff --git a/android/packaging.go b/android/packaging.go
index a7260a641..383f828bd 100644
--- a/android/packaging.go
+++ b/android/packaging.go
@@ -51,6 +51,9 @@ type PackagingSpec struct {
 
 	// Paths of aconfig files for the built artifact
 	aconfigPaths *Paths
+
+	// ArchType of the module which produced this packaging spec
+	archType ArchType
 }
 
 func (p *PackagingSpec) Equals(other *PackagingSpec) bool {
@@ -260,11 +263,31 @@ func (p *PackagingBase) AddDeps(ctx BottomUpMutatorContext, depTag blueprint.Dep
 
 func (p *PackagingBase) GatherPackagingSpecsWithFilter(ctx ModuleContext, filter func(PackagingSpec) bool) map[string]PackagingSpec {
 	m := make(map[string]PackagingSpec)
+
+	var arches []ArchType
+	for _, target := range p.getSupportedTargets(ctx) {
+		arches = append(arches, target.Arch.ArchType)
+	}
+
+	// filter out packaging specs for unsupported architecture
+	filterArch := func(ps PackagingSpec) bool {
+		for _, arch := range arches {
+			if arch == ps.archType {
+				return true
+			}
+		}
+		return false
+	}
+
 	ctx.VisitDirectDeps(func(child Module) {
 		if pi, ok := ctx.OtherModuleDependencyTag(child).(PackagingItem); !ok || !pi.IsPackagingItem() {
 			return
 		}
 		for _, ps := range child.TransitivePackagingSpecs() {
+			if !filterArch(ps) {
+				continue
+			}
+
 			if filter != nil {
 				if !filter(ps) {
 					continue
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 861918f16..015d39aab 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -497,3 +497,70 @@ func TestTrackPhonyAsRequiredDep(t *testing.T) {
 		android.AssertStringListContains(t, "missing entry", fs.entries, e)
 	}
 }
+
+func TestFilterOutUnsupportedArches(t *testing.T) {
+	result := fixture.RunTestWithBp(t, `
+		android_filesystem {
+			name: "fs_64_only",
+			deps: ["foo"],
+		}
+
+		android_filesystem {
+			name: "fs_64_32",
+			compile_multilib: "both",
+			multilib: {
+				first: {
+					deps: ["foo"],
+				},
+			},
+		}
+
+		cc_binary {
+			name: "foo",
+			required: ["phony"],
+		}
+
+		phony {
+			name: "phony",
+			required: [
+				"libbar",
+				"app",
+			],
+		}
+
+		cc_library {
+			name: "libbar",
+		}
+
+		android_app {
+			name: "app",
+			srcs: ["a.java"],
+			platform_apis: true,
+		}
+	`)
+	testcases := []struct {
+		fsName     string
+		expected   []string
+		unexpected []string
+	}{
+		{
+			fsName:     "fs_64_only",
+			expected:   []string{"app/app/app.apk", "bin/foo", "lib64/libbar.so"},
+			unexpected: []string{"lib/libbar.so"},
+		},
+		{
+			fsName:     "fs_64_32",
+			expected:   []string{"app/app/app.apk", "bin/foo", "lib64/libbar.so", "lib/libbar.so"},
+			unexpected: []string{},
+		},
+	}
+	for _, c := range testcases {
+		fs := result.ModuleForTests(c.fsName, "android_common").Module().(*filesystem)
+		for _, e := range c.expected {
+			android.AssertStringListContains(t, "missing entry", fs.entries, e)
+		}
+		for _, e := range c.unexpected {
+			android.AssertStringListDoesNotContain(t, "unexpected entry", fs.entries, e)
+		}
+	}
+}