diff options
| -rw-r--r-- | android/paths.go | 53 | ||||
| -rw-r--r-- | android/paths_test.go | 35 | ||||
| -rw-r--r-- | genrule/genrule.go | 37 | ||||
| -rw-r--r-- | genrule/genrule_test.go | 37 |
4 files changed, 137 insertions, 25 deletions
diff --git a/android/paths.go b/android/paths.go index cdcb7192..8cc7057a 100644 --- a/android/paths.go +++ b/android/paths.go @@ -217,13 +217,41 @@ func ExistentPathsForSources(ctx PathContext, paths []string) Paths { return ret } -// PathsForModuleSrc returns Paths rooted from the module's local source -// directory +// PathsForModuleSrc returns Paths rooted from the module's local source directory. It expands globs and references +// to SourceFileProducer modules using the ":name" syntax. Properties passed as the paths argument must have been +// annotated with struct tag `android:"path"` so that dependencies on SourceFileProducer modules will have already +// been handled by the path_properties mutator. If ctx.Config().AllowMissingDependencies() is true, then any missing +// SourceFileProducer dependencies will cause the module to be marked as having missing dependencies. func PathsForModuleSrc(ctx ModuleContext, paths []string) Paths { return PathsForModuleSrcExcludes(ctx, paths, nil) } +// PathsForModuleSrcExcludes returns Paths rooted from the module's local source directory, excluding paths listed in +// the excludes arguments. It expands globs and references to SourceFileProducer modules in both paths and excludes +// using the ":name" syntax. Properties passed as the paths or excludes argument must have been annotated with struct +// tag `android:"path"` so that dependencies on SourceFileProducer modules will have already been handled by the +// path_properties mutator. If ctx.Config().AllowMissingDependencies() is true, then any missing SourceFileProducer +// dependencies will cause the module to be marked as having missing dependencies. func PathsForModuleSrcExcludes(ctx ModuleContext, paths, excludes []string) Paths { + ret, missingDeps := PathsAndMissingDepsForModuleSrcExcludes(ctx, paths, excludes) + if ctx.Config().AllowMissingDependencies() { + ctx.AddMissingDependencies(missingDeps) + } else { + for _, m := range missingDeps { + ctx.ModuleErrorf(`missing dependency on %q, is the property annotated with android:"path"?`, m) + } + } + return ret +} + +// PathsAndMissingDepsForModuleSrcExcludes returns Paths rooted from the module's local source directory, excluding +// paths listed in the excludes arguments, and a list of missing dependencies. It expands globs and references to +// SourceFileProducer modules in both paths and excludes using the ":name" syntax. Properties passed as the paths or +// excludes argument must have been annotated with struct tag `android:"path"` so that dependencies on +// SourceFileProducer modules will have already been handled by the path_properties mutator. If +// ctx.Config().AllowMissingDependencies() is true, then any missing SourceFileProducer dependencies will be returned, +// and they will NOT cause the module to be marked as having missing dependencies. +func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleContext, paths, excludes []string) (Paths, []string) { prefix := pathForModuleSrc(ctx).String() var expandedExcludes []string @@ -231,15 +259,13 @@ func PathsForModuleSrcExcludes(ctx ModuleContext, paths, excludes []string) Path expandedExcludes = make([]string, 0, len(excludes)) } + var missingExcludeDeps []string + for _, e := range excludes { if m := SrcIsModule(e); m != "" { module := ctx.GetDirectDepWithTag(m, SourceDepTag) if module == nil { - if ctx.Config().AllowMissingDependencies() { - ctx.AddMissingDependencies([]string{m}) - } else { - ctx.ModuleErrorf(`missing dependency on %q, is the property annotated with android:"path"?`, m) - } + missingExcludeDeps = append(missingExcludeDeps, m) continue } if srcProducer, ok := module.(SourceFileProducer); ok { @@ -253,24 +279,23 @@ func PathsForModuleSrcExcludes(ctx ModuleContext, paths, excludes []string) Path } if paths == nil { - return nil + return nil, missingExcludeDeps } + var missingDeps []string + expandedSrcFiles := make(Paths, 0, len(paths)) for _, s := range paths { srcFiles, err := expandOneSrcPath(ctx, s, expandedExcludes) if depErr, ok := err.(missingDependencyError); ok { - if ctx.Config().AllowMissingDependencies() { - ctx.AddMissingDependencies(depErr.missingDeps) - } else { - ctx.ModuleErrorf(`%s, is the property annotated with android:"path"?`, depErr.Error()) - } + missingDeps = append(missingDeps, depErr.missingDeps...) } else if err != nil { reportPathError(ctx, err) } expandedSrcFiles = append(expandedSrcFiles, srcFiles...) } - return expandedSrcFiles + + return expandedSrcFiles, append(missingDeps, missingExcludeDeps...) } type missingDependencyError struct { diff --git a/android/paths_test.go b/android/paths_test.go index 2e0e0e86..b52d7133 100644 --- a/android/paths_test.go +++ b/android/paths_test.go @@ -714,6 +714,8 @@ type pathForModuleSrcTestModule struct { Exclude_srcs []string `android:"path"` Src *string `android:"path"` + + Module_handles_missing_deps bool } src string @@ -733,7 +735,12 @@ func pathForModuleSrcTestModuleFactory() Module { } func (p *pathForModuleSrcTestModule) GenerateAndroidBuildActions(ctx ModuleContext) { - srcs := PathsForModuleSrcExcludes(ctx, p.props.Srcs, p.props.Exclude_srcs) + var srcs Paths + if p.props.Module_handles_missing_deps { + srcs, p.missingDeps = PathsAndMissingDepsForModuleSrcExcludes(ctx, p.props.Srcs, p.props.Exclude_srcs) + } else { + srcs = PathsForModuleSrcExcludes(ctx, p.props.Srcs, p.props.Exclude_srcs) + } p.srcs = srcs.Strings() for _, src := range srcs { @@ -748,7 +755,9 @@ func (p *pathForModuleSrcTestModule) GenerateAndroidBuildActions(ctx ModuleConte } } - p.missingDeps = ctx.GetMissingDependencies() + if !p.props.Module_handles_missing_deps { + p.missingDeps = ctx.GetMissingDependencies() + } } type pathForModuleSrcTestCase struct { @@ -957,6 +966,13 @@ func TestPathsForModuleSrc_AllowMissingDependencies(t *testing.T) { exclude_srcs: [":b"], src: ":c", } + + test { + name: "bar", + srcs: [":d"], + exclude_srcs: [":e"], + module_handles_missing_deps: true, + } ` mockFS := map[string][]byte{ @@ -974,17 +990,26 @@ func TestPathsForModuleSrc_AllowMissingDependencies(t *testing.T) { foo := ctx.ModuleForTests("foo", "").Module().(*pathForModuleSrcTestModule) if g, w := foo.missingDeps, []string{"a", "b", "c"}; !reflect.DeepEqual(g, w) { - t.Errorf("want missing deps %q, got %q", w, g) + t.Errorf("want foo missing deps %q, got %q", w, g) } if g, w := foo.srcs, []string{}; !reflect.DeepEqual(g, w) { - t.Errorf("want srcs %q, got %q", w, g) + t.Errorf("want foo srcs %q, got %q", w, g) } if g, w := foo.src, ""; g != w { - t.Errorf("want src %q, got %q", w, g) + t.Errorf("want foo src %q, got %q", w, g) } + bar := ctx.ModuleForTests("bar", "").Module().(*pathForModuleSrcTestModule) + + if g, w := bar.missingDeps, []string{"d", "e"}; !reflect.DeepEqual(g, w) { + t.Errorf("want bar missing deps %q, got %q", w, g) + } + + if g, w := bar.srcs, []string{}; !reflect.DeepEqual(g, w) { + t.Errorf("want bar srcs %q, got %q", w, g) + } } func ExampleOutputPath_ReplaceExtension() { diff --git a/genrule/genrule.go b/genrule/genrule.go index e259b1d9..87e6747e 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -189,6 +189,8 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } if len(g.properties.Tools) > 0 { + seenTools := make(map[string]bool) + ctx.VisitDirectDepsBlueprint(func(module blueprint.Module) { switch tag := ctx.OtherModuleDependencyTag(module).(type) { case hostToolDependencyTag: @@ -220,11 +222,25 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { if path.Valid() { g.deps = append(g.deps, path.Path()) addLocationLabel(tag.label, []string{path.Path().String()}) + seenTools[tag.label] = true } else { ctx.ModuleErrorf("host tool %q missing output file", tool) } } }) + + // If AllowMissingDependencies is enabled, the build will not have stopped when + // AddFarVariationDependencies was called on a missing tool, which will result in nonsensical + // "cmd: unknown location label ..." errors later. Add a dummy file to the local label. The + // command that uses this dummy file will never be executed because the rule will be replaced with + // an android.Error rule reporting the missing dependencies. + if ctx.Config().AllowMissingDependencies() { + for _, tool := range g.properties.Tools { + if !seenTools[tool] { + addLocationLabel(tool, []string{"***missing tool " + tool + "***"}) + } + } + } } if ctx.Failed() { @@ -239,9 +255,24 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { var srcFiles android.Paths for _, in := range g.properties.Srcs { - paths := android.PathsForModuleSrcExcludes(ctx, []string{in}, g.properties.Exclude_srcs) - srcFiles = append(srcFiles, paths...) - addLocationLabel(in, paths.Strings()) + paths, missingDeps := android.PathsAndMissingDepsForModuleSrcExcludes(ctx, []string{in}, g.properties.Exclude_srcs) + if len(missingDeps) > 0 { + if !ctx.Config().AllowMissingDependencies() { + panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator", + missingDeps)) + } + + // If AllowMissingDependencies is enabled, the build will not have stopped when + // the dependency was added on a missing SourceFileProducer module, which will result in nonsensical + // "cmd: label ":..." has no files" errors later. Add a dummy file to the local label. The + // command that uses this dummy file will never be executed because the rule will be replaced with + // an android.Error rule reporting the missing dependencies. + ctx.AddMissingDependencies(missingDeps) + addLocationLabel(in, []string{"***missing srcs " + in + "***"}) + } else { + srcFiles = append(srcFiles, paths...) + addLocationLabel(in, paths.Strings()) + } } task := g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 5cb51b8b..0b6952f3 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -17,11 +17,13 @@ package genrule import ( "io/ioutil" "os" + "reflect" "strings" "testing" "android/soong/android" - "reflect" + + "github.com/google/blueprint/proptools" ) var buildDir string @@ -123,6 +125,8 @@ func TestGenruleCmd(t *testing.T) { name string prop string + allowMissingDependencies bool + err string expect string }{ @@ -425,6 +429,30 @@ func TestGenruleCmd(t *testing.T) { `, err: "must have at least one output file", }, + { + name: "srcs allow missing dependencies", + prop: ` + srcs: [":missing"], + out: ["out"], + cmd: "cat $(location :missing) > $(out)", + `, + + allowMissingDependencies: true, + + expect: "cat ***missing srcs :missing*** > __SBOX_OUT_FILES__", + }, + { + name: "tool allow missing dependencies", + prop: ` + tools: [":missing"], + out: ["out"], + cmd: "$(location :missing) > $(out)", + `, + + allowMissingDependencies: true, + + expect: "***missing tool :missing*** > __SBOX_OUT_FILES__", + }, } for _, test := range testcases { @@ -435,7 +463,10 @@ func TestGenruleCmd(t *testing.T) { bp += test.prop bp += "}\n" + config.TestProductVariables.Allow_missing_dependencies = proptools.BoolPtr(test.allowMissingDependencies) + ctx := testContext(config, bp, nil) + ctx.SetAllowMissingDependencies(test.allowMissingDependencies) _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) if errs == nil { @@ -460,8 +491,8 @@ func TestGenruleCmd(t *testing.T) { } gen := ctx.ModuleForTests("gen", "").Module().(*Module) - if gen.rawCommand != "'"+test.expect+"'" { - t.Errorf("want %q, got %q", test.expect, gen.rawCommand) + if g, w := gen.rawCommand, "'"+test.expect+"'"; w != g { + t.Errorf("want %q, got %q", w, g) } }) } |