Merge "Generalize the handling of dist tags" am: 5e95589fe9

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1511913

Change-Id: If811b96f5ae891e85f7fbca7a7cf642568e2d808

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