Generalize the handling of dist tags

Previously in order to make use of dist entries with tag properties it
was necessary for each module type to be modified to store the results
of GenerateTaggedDistFiles(module) in the AndroidMkEntry.DistFiles
property. This change generalizes that mechanism to work across all
module types.

This change does improve the behavior of a couple of tests where
previously a dist with tag property was ignored because tagged dist
files were not available now it is used correctly.

Some module types do not implement OutputFileProducer interface and so
they cannot handle dist with tag properties. This change makes it an
error to attempt to do that. That necessitated adding OutputFiles(tag)
method to customModule in queryview_test.go as some of the tests in
that file used dist entries with tag properties.

Follow up changes will remove the code that was made redundant by this.

Test: m nothing
      m dist sdk - before and after this change, compare result to
      make sure that there are no significant differences.
Bug: 174226317
Change-Id: Ifc54d67db10ce0d0fe8179c05b97a2be2113be4e

android/androidmk.go

@@ -238,6 +238,15 @@ func (a *AndroidMkEntries) getDistContributions(mod blueprint.Module) *distContr
 		availableTaggedDists = availableTaggedDists.addPathsForTag(DefaultDistTag, a.OutputFile.Path())
 	}
 
+	// If the distFiles created by GenerateTaggedDistFiles contains paths for the
+	// DefaultDistTag then that takes priority so delete any existing paths.
+	if _, ok := amod.distFiles[DefaultDistTag]; ok {
+		delete(availableTaggedDists, DefaultDistTag)
+	}
+
+	// Finally, merge the distFiles created by GenerateTaggedDistFiles.
+	availableTaggedDists = availableTaggedDists.merge(amod.distFiles)
+
 	if len(availableTaggedDists) == 0 {
 		// Nothing dist-able for this module.
 		return nil

android/androidmk_test.go

@@ -601,8 +601,6 @@ func TestGetDistContributions(t *testing.T) {
 					{
 						targets: ["my_goal"],
 					},
-					// The following is silently ignored because the dist files do not
-					// contain the tagged files.
 					{
 						targets: ["my_goal"],
 						tag: ".multiple",
@@ -617,6 +615,13 @@ func TestGetDistContributions(t *testing.T) {
 					distCopyForTest("default-dist.out", "default-dist.out"),
 				},
 			},
+			{
+				goals: "my_goal",
+				copies: []distCopy{
+					distCopyForTest("two.out", "two.out"),
+					distCopyForTest("three/four.out", "four.out"),
+				},
+			},
 		},
 	})
 
@@ -631,15 +636,23 @@ func TestGetDistContributions(t *testing.T) {
 					{
 						targets: ["my_goal"],
 					},
-					// The following is silently ignored because the dist files do not
-					// contain the tagged files.
 					{
 						targets: ["my_goal"],
 						tag: ".multiple",
 					},
 				],
 			}
-`, nil)
+`, &distContributions{
+		copiesForGoals: []*copiesForGoals{
+			{
+				goals: "my_goal",
+				copies: []distCopy{
+					distCopyForTest("two.out", "two.out"),
+					distCopyForTest("three/four.out", "four.out"),
+				},
+			},
+		},
+	})
 
 	testHelper(t, "tagged-dist-files-default-output", `
 			custom {
@@ -683,8 +696,6 @@ func TestGetDistContributions(t *testing.T) {
 					{
 						targets: ["my_goal"],
 					},
-					// The following is silently ignored because the dist files do not
-					// contain the tagged files.
 					{
 						targets: ["my_goal"],
 						tag: ".multiple",
@@ -699,6 +710,13 @@ func TestGetDistContributions(t *testing.T) {
 					distCopyForTest("default-dist.out", "default-dist.out"),
 				},
 			},
+			{
+				goals: "my_goal",
+				copies: []distCopy{
+					distCopyForTest("two.out", "two.out"),
+					distCopyForTest("three/four.out", "four.out"),
+				},
+			},
 		},
 	})
 
@@ -711,8 +729,6 @@ func TestGetDistContributions(t *testing.T) {
 					{
 						targets: ["my_goal"],
 					},
-					// The following is silently ignored because the dist files do not
-					// contain the tagged files.
 					{
 						targets: ["my_goal"],
 						tag: ".multiple",
@@ -727,6 +743,13 @@ func TestGetDistContributions(t *testing.T) {
 					distCopyForTest("dist-output-file.out", "dist-output-file.out"),
 				},
 			},
+			{
+				goals: "my_goal",
+				copies: []distCopy{
+					distCopyForTest("two.out", "two.out"),
+					distCopyForTest("three/four.out", "four.out"),
+				},
+			},
 		},
 	})
 }

android/module.go

@@ -1010,6 +1010,9 @@ type ModuleBase struct {
 	noticeFiles        Paths
 	phonies            map[string]Paths
 
+	// The files to copy to the dist as explicitly specified in the .bp file.
+	distFiles TaggedDistFiles
+
 	// Used by buildTargetSingleton to create checkbuild and per-directory build targets
 	// Only set on the final variant of each module
 	installTarget    WritablePath
@@ -1117,18 +1120,25 @@ func (m *ModuleBase) GenerateTaggedDistFiles(ctx BaseModuleContext) TaggedDistFi
 		// the special tag name which represents that.
 		tag := proptools.StringDefault(dist.Tag, DefaultDistTag)
 
-		// Call the OutputFiles(tag) method to get the paths associated with the tag.
-		distFilesForTag, err := m.base().module.(OutputFileProducer).OutputFiles(tag)
+		if outputFileProducer, ok := m.module.(OutputFileProducer); ok {
+			// Call the OutputFiles(tag) method to get the paths associated with the tag.
+			distFilesForTag, err := outputFileProducer.OutputFiles(tag)
 
-		// If the tag was not supported and is not DefaultDistTag then it is an error.
-		// Failing to find paths for DefaultDistTag is not an error. It just means
-		// that the module type requires the legacy behavior.
-		if err != nil && tag != DefaultDistTag {
-			ctx.PropertyErrorf("dist.tag", "%s", err.Error())
-			continue
+			// If the tag was not supported and is not DefaultDistTag then it is an error.
+			// Failing to find paths for DefaultDistTag is not an error. It just means
+			// that the module type requires the legacy behavior.
+			if err != nil && tag != DefaultDistTag {
+				ctx.PropertyErrorf("dist.tag", "%s", err.Error())
+			}
+
+			distFiles = distFiles.addPathsForTag(tag, distFilesForTag...)
+		} else if tag != DefaultDistTag {
+			// If the tag was specified then it is an error if the module does not
+			// implement OutputFileProducer because there is no other way of accessing
+			// the paths for the specified tag.
+			ctx.PropertyErrorf("dist.tag",
+				"tag %s not supported because the module does not implement OutputFileProducer", tag)
 		}
-
-		distFiles = distFiles.addPathsForTag(tag, distFilesForTag...)
 	}
 
 	return distFiles
@@ -1658,6 +1668,15 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 			return
 		}
 
+		// Create the set of tagged dist files after calling GenerateAndroidBuildActions
+		// as GenerateTaggedDistFiles() calls OutputFiles(tag) and so relies on the
+		// output paths being set which must be done before or during
+		// GenerateAndroidBuildActions.
+		m.distFiles = m.GenerateTaggedDistFiles(ctx)
+		if ctx.Failed() {
+			return
+		}
+
 		m.installFiles = append(m.installFiles, ctx.installFiles...)
 		m.checkbuildFiles = append(m.checkbuildFiles, ctx.checkbuildFiles...)
 		m.packagingSpecs = append(m.packagingSpecs, ctx.packagingSpecs...)

cmd/soong_build/queryview_test.go

@@ -53,6 +53,12 @@ type customModule struct {
 	android.ModuleBase
 }
 
+// OutputFiles is needed because some instances of this module use dist with a
+// tag property which requires the module implements OutputFileProducer.
+func (m *customModule) OutputFiles(tag string) (android.Paths, error) {
+	return android.PathsForTesting("path" + tag), nil
+}
+
 func (m *customModule) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	// nothing for now.
 }