diff --git a/android/androidmk.go b/android/androidmk.go index 7afc9ded8..42593bac4 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -222,13 +222,23 @@ func (a *AndroidMkEntries) getDistContributions(mod blueprint.Module) *distContr amod := mod.(Module).base() name := amod.BaseModuleName() + // Collate the set of associated tag/paths available for copying to the dist. + // Start with an empty (nil) set. var availableTaggedDists TaggedDistFiles + // Then merge in any that are provided explicitly by the module. if a.DistFiles != nil { - availableTaggedDists = a.DistFiles - } else if a.OutputFile.Valid() { - availableTaggedDists = MakeDefaultDistFiles(a.OutputFile.Path()) - } else { + // Merge the DistFiles into the set. + availableTaggedDists = availableTaggedDists.merge(a.DistFiles) + } + + // If no paths have been provided for the DefaultDistTag and the output file is + // valid then add that as the default dist path. + if _, ok := availableTaggedDists[DefaultDistTag]; !ok && a.OutputFile.Valid() { + availableTaggedDists = availableTaggedDists.addPathsForTag(DefaultDistTag, a.OutputFile.Path()) + } + + if len(availableTaggedDists) == 0 { // Nothing dist-able for this module. return nil } @@ -245,7 +255,7 @@ func (a *AndroidMkEntries) getDistContributions(mod blueprint.Module) *distContr var tag string if dist.Tag == nil { // If the dist struct does not specify a tag, use the default output files tag. - tag = "" + tag = DefaultDistTag } else { tag = *dist.Tag } @@ -259,10 +269,10 @@ func (a *AndroidMkEntries) getDistContributions(mod blueprint.Module) *distContr } if len(tagPaths) > 1 && (dist.Dest != nil || dist.Suffix != nil) { - errorMessage := "Cannot apply dest/suffix for more than one dist " + - "file for %s goals in module %s. The list of dist files, " + + errorMessage := "%s: Cannot apply dest/suffix for more than one dist " + + "file for %q goals tag %q in module %s. The list of dist files, " + "which should have a single element, is:\n%s" - panic(fmt.Errorf(errorMessage, goals, name, tagPaths)) + panic(fmt.Errorf(errorMessage, mod, goals, tag, name, tagPaths)) } copiesForGoals := distContributions.getCopiesForGoals(goals) diff --git a/android/androidmk_test.go b/android/androidmk_test.go index 5d05d917c..ae9de552c 100644 --- a/android/androidmk_test.go +++ b/android/androidmk_test.go @@ -35,6 +35,10 @@ type customModule struct { data AndroidMkData distFiles TaggedDistFiles outputFile OptionalPath + + // The paths that will be used as the default dist paths if no tag is + // specified. + defaultDistPaths Paths } const ( @@ -48,7 +52,14 @@ func (m *customModule) GenerateAndroidBuildActions(ctx ModuleContext) { // If the dist_output_file: true then create an output file that is stored in // the OutputFile property of the AndroidMkEntry. if proptools.BoolDefault(m.properties.Dist_output_file, true) { - m.outputFile = OptionalPathForPath(PathForTesting("dist-output-file.out")) + path := PathForTesting("dist-output-file.out") + m.outputFile = OptionalPathForPath(path) + + // Previous code would prioritize the DistFiles property over the OutputFile + // property in AndroidMkEntry when determining the default dist paths. + // Setting this first allows it to be overridden based on the + // default_dist_files setting replicating that previous behavior. + m.defaultDistPaths = Paths{path} } // Based on the setting of the default_dist_files property possibly create a @@ -60,9 +71,22 @@ func (m *customModule) GenerateAndroidBuildActions(ctx ModuleContext) { // Do nothing case defaultDistFiles_Default: - m.distFiles = MakeDefaultDistFiles(PathForTesting("default-dist.out")) + path := PathForTesting("default-dist.out") + m.defaultDistPaths = Paths{path} + m.distFiles = MakeDefaultDistFiles(path) case defaultDistFiles_Tagged: + // Module types that set AndroidMkEntry.DistFiles to the result of calling + // GenerateTaggedDistFiles(ctx) relied on no tag being treated as "" which + // meant that the default dist paths would be whatever was returned by + // OutputFiles(""). In order to preserve that behavior when treating no tag + // as being equal to DefaultDistTag this ensures that + // OutputFiles(DefaultDistTag) will return the same as OutputFiles(""). + m.defaultDistPaths = PathsForTesting("one.out") + + // This must be called after setting defaultDistPaths/outputFile as + // GenerateTaggedDistFiles calls into OutputFiles(tag) which may use those + // fields. m.distFiles = m.GenerateTaggedDistFiles(ctx) } } @@ -77,6 +101,12 @@ func (m *customModule) AndroidMk() AndroidMkData { func (m *customModule) OutputFiles(tag string) (Paths, error) { switch tag { + case DefaultDistTag: + if m.defaultDistPaths != nil { + return m.defaultDistPaths, nil + } else { + return nil, fmt.Errorf("default dist tag is not available") + } case "": return PathsForTesting("one.out"), nil case ".multiple": diff --git a/android/module.go b/android/module.go index 0716b5dac..d64b5163c 100644 --- a/android/module.go +++ b/android/module.go @@ -503,8 +503,12 @@ type Dist struct { // A suffix to add to the artifact file name (before any extension). Suffix *string `android:"arch_variant"` - // A string tag to select the OutputFiles associated with the tag. Defaults to the - // the empty "" string. + // A string tag to select the OutputFiles associated with the tag. + // + // If no tag is specified then it will select the default dist paths provided + // by the module type. If a tag of "" is specified then it will return the + // default output files provided by the modules, i.e. the result of calling + // OutputFiles(""). Tag *string `android:"arch_variant"` } @@ -733,9 +737,45 @@ type distProperties struct { Dists []Dist `android:"arch_variant"` } +// The key to use in TaggedDistFiles when a Dist structure does not specify a +// tag property. This intentionally does not use "" as the default because that +// would mean that an empty tag would have a different meaning when used in a dist +// structure that when used to reference a specific set of output paths using the +// :module{tag} syntax, which passes tag to the OutputFiles(tag) method. +const DefaultDistTag = "" + // A map of OutputFile tag keys to Paths, for disting purposes. type TaggedDistFiles map[string]Paths +// addPathsForTag adds a mapping from the tag to the paths. If the map is nil +// then it will create a map, update it and then return it. If a mapping already +// exists for the tag then the paths are appended to the end of the current list +// of paths, ignoring any duplicates. +func (t TaggedDistFiles) addPathsForTag(tag string, paths ...Path) TaggedDistFiles { + if t == nil { + t = make(TaggedDistFiles) + } + + for _, distFile := range paths { + if distFile != nil && !t[tag].containsPath(distFile) { + t[tag] = append(t[tag], distFile) + } + } + + return t +} + +// merge merges the entries from the other TaggedDistFiles object into this one. +// If the TaggedDistFiles is nil then it will create a new instance, merge the +// other into it, and then return it. +func (t TaggedDistFiles) merge(other TaggedDistFiles) TaggedDistFiles { + for tag, paths := range other { + t = t.addPathsForTag(tag, paths...) + } + + return t +} + func MakeDefaultDistFiles(paths ...Path) TaggedDistFiles { for _, path := range paths { if path == nil { @@ -744,7 +784,7 @@ func MakeDefaultDistFiles(paths ...Path) TaggedDistFiles { } // The default OutputFile tag is the empty "" string. - return TaggedDistFiles{"": paths} + return TaggedDistFiles{DefaultDistTag: paths} } type hostAndDeviceProperties struct { @@ -1071,24 +1111,24 @@ func (m *ModuleBase) Dists() []Dist { } func (m *ModuleBase) GenerateTaggedDistFiles(ctx BaseModuleContext) TaggedDistFiles { - distFiles := make(TaggedDistFiles) + var distFiles TaggedDistFiles for _, dist := range m.Dists() { - var tag string - var distFilesForTag Paths - if dist.Tag == nil { - tag = "" - } else { - tag = *dist.Tag - } + // If no tag is specified then it means to use the default dist paths so use + // 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 err != nil { + + // 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 } - for _, distFile := range distFilesForTag { - if distFile != nil && !distFiles[tag].containsPath(distFile) { - distFiles[tag] = append(distFiles[tag], distFile) - } - } + + distFiles = distFiles.addPathsForTag(tag, distFilesForTag...) } return distFiles diff --git a/apex/apex.go b/apex/apex.go index d0e015663..ffc29b7ab 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1096,7 +1096,8 @@ var _ android.OutputFileProducer = (*apexBundle)(nil) // Implements android.OutputFileProducer func (a *apexBundle) OutputFiles(tag string) (android.Paths, error) { switch tag { - case "": + case "", android.DefaultDistTag: + // This is the default dist path. return android.Paths{a.outputFile}, nil default: return nil, fmt.Errorf("unsupported module reference tag %q", tag) diff --git a/java/java.go b/java/java.go index bf139a80b..42fb95574 100644 --- a/java/java.go +++ b/java/java.go @@ -486,6 +486,8 @@ func (j *Module) OutputFiles(tag string) (android.Paths, error) { switch tag { case "": return append(android.Paths{j.outputFile}, j.extraOutputFiles...), nil + case android.DefaultDistTag: + return android.Paths{j.outputFile}, nil case ".jar": return android.Paths{j.implementationAndResourcesJar}, nil case ".proguard_map":