From 092ef811a056bd891d500788e5fcb412ff4e2f8a Mon Sep 17 00:00:00 2001
From: Jooyung Han <jooyung@google.com>
Date: Wed, 10 Mar 2021 15:40:34 +0900
Subject: [PATCH] Fix PackagingBase.CopyDepsToZip

CopyDepsToZip() zips direct dependencies with tags implementing
PackagingItem interface.

Previously, it relied on InstallNeededDependencyTag which has a
different meaning.
- InstallNeededDependencyTag tells whether a dependency is required to
  be installed together.
- PackagingItem tells whether a dependency (of PackagingBase) is
  required to be packaged.

With the separation of InstallNeededDependencyTag and PackagingItem,
PackagingBase module can distinguish cases which were not available
before.(I = InstallNeededDependencyTag, P = PackagingItem)

   a (PackagingBase module)
   |
   |`--(I)--> b
   |
   |`--(P)--> c --(I)--> d
   |
    `--(I/P)--> e

a's CopyDepsToZip(): [c, d, e]

Test: m nothing (packaging_test)
Change-Id: I71fce29b19b0f00dc394981bcf4240e9c1041c7a
---
 android/packaging.go      | 30 ++++++++++++++++++++++++------
 android/packaging_test.go | 31 +++++++++++++++++++++++++++++--
 filesystem/filesystem.go  |  2 +-
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/android/packaging.go b/android/packaging.go
index 9b901ce28..72c0c1777 100644
--- a/android/packaging.go
+++ b/android/packaging.go
@@ -59,7 +59,8 @@ type PackageModule interface {
 	packagingBase() *PackagingBase
 
 	// AddDeps adds dependencies to the `deps` modules. This should be called in DepsMutator.
-	// When adding the dependencies, depTag is used as the tag.
+	// When adding the dependencies, depTag is used as the tag. If `deps` modules are meant to
+	// be copied to a zip in CopyDepsToZip, `depTag` should implement PackagingItem marker interface.
 	AddDeps(ctx BottomUpMutatorContext, depTag blueprint.DependencyTag)
 
 	// CopyDepsToZip zips the built artifacts of the dependencies into the given zip file and
@@ -167,6 +168,24 @@ func (p *PackagingBase) getSupportedTargets(ctx BaseModuleContext) []Target {
 	return ret
 }
 
+// PackagingItem is a marker interface for dependency tags.
+// Direct dependencies with a tag implementing PackagingItem are packaged in CopyDepsToZip().
+type PackagingItem interface {
+	// IsPackagingItem returns true if the dep is to be packaged
+	IsPackagingItem() bool
+}
+
+// DepTag provides default implementation of PackagingItem interface.
+// PackagingBase-derived modules can define their own dependency tag by embedding this, which
+// can be passed to AddDeps() or AddDependencies().
+type PackagingItemAlwaysDepTag struct {
+}
+
+// IsPackagingItem returns true if the dep is to be packaged
+func (PackagingItemAlwaysDepTag) IsPackagingItem() bool {
+	return true
+}
+
 // See PackageModule.AddDeps
 func (p *PackagingBase) AddDeps(ctx BottomUpMutatorContext, depTag blueprint.DependencyTag) {
 	for _, t := range p.getSupportedTargets(ctx) {
@@ -182,16 +201,15 @@ func (p *PackagingBase) AddDeps(ctx BottomUpMutatorContext, depTag blueprint.Dep
 // See PackageModule.CopyDepsToZip
 func (p *PackagingBase) CopyDepsToZip(ctx ModuleContext, zipOut WritablePath) (entries []string) {
 	m := make(map[string]PackagingSpec)
-	ctx.WalkDeps(func(child Module, parent Module) bool {
-		if !IsInstallDepNeeded(ctx.OtherModuleDependencyTag(child)) {
-			return false
+	ctx.VisitDirectDeps(func(child Module) {
+		if pi, ok := ctx.OtherModuleDependencyTag(child).(PackagingItem); !ok || !pi.IsPackagingItem() {
+			return
 		}
-		for _, ps := range child.PackagingSpecs() {
+		for _, ps := range child.TransitivePackagingSpecs() {
 			if _, ok := m[ps.relPathInPackage]; !ok {
 				m[ps.relPathInPackage] = ps
 			}
 		}
-		return true
 	})
 
 	builder := NewRuleBuilder(pctx, ctx)
diff --git a/android/packaging_test.go b/android/packaging_test.go
index 2c99b97a2..e5a06a26b 100644
--- a/android/packaging_test.go
+++ b/android/packaging_test.go
@@ -57,7 +57,9 @@ func (m *componentTestModule) GenerateAndroidBuildActions(ctx ModuleContext) {
 type packageTestModule struct {
 	ModuleBase
 	PackagingBase
-
+	properties struct {
+		Install_deps []string `android:`
+	}
 	entries []string
 }
 
@@ -65,6 +67,7 @@ func packageMultiTargetTestModuleFactory() Module {
 	module := &packageTestModule{}
 	InitPackageModule(module)
 	InitAndroidMultiTargetsArchModule(module, DeviceSupported, MultilibCommon)
+	module.AddProperties(&module.properties)
 	return module
 }
 
@@ -72,11 +75,18 @@ func packageTestModuleFactory() Module {
 	module := &packageTestModule{}
 	InitPackageModule(module)
 	InitAndroidArchModule(module, DeviceSupported, MultilibBoth)
+	module.AddProperties(&module.properties)
 	return module
 }
 
+type packagingDepTag struct {
+	blueprint.BaseDependencyTag
+	PackagingItemAlwaysDepTag
+}
+
 func (m *packageTestModule) DepsMutator(ctx BottomUpMutatorContext) {
-	m.AddDeps(ctx, installDepTag{})
+	m.AddDeps(ctx, packagingDepTag{})
+	ctx.AddDependency(ctx.Module(), installDepTag{}, m.properties.Install_deps...)
 }
 
 func (m *packageTestModule) GenerateAndroidBuildActions(ctx ModuleContext) {
@@ -341,4 +351,21 @@ func TestPackagingBaseSingleTarget(t *testing.T) {
 			},
 		}
 		`, []string{"lib64/foo", "lib64/bar"})
+
+	runPackagingTest(t, multiTarget,
+		`
+		component {
+			name: "foo",
+		}
+
+		component {
+			name: "bar",
+		}
+
+		package_module {
+			name: "package",
+			deps: ["foo"],
+			install_deps: ["bar"],
+		}
+		`, []string{"lib64/foo"})
 }
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index 3b0a7ae5a..699402661 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -88,7 +88,7 @@ func filesystemFactory() android.Module {
 
 var dependencyTag = struct {
 	blueprint.BaseDependencyTag
-	android.InstallAlwaysNeededDependencyTag
+	android.PackagingItemAlwaysDepTag
 }{}
 
 func (f *filesystem) DepsMutator(ctx android.BottomUpMutatorContext) {