StrixOS/build_soong
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix missing genrule srcs and tools with ALLOW_MISSING_DEPENDENCIES=true

Browse Source 
Set the location label for missing srcs and tools to avoid
nonsensical errors when parsing the command.

Test: genrule_test.go
Test: paths_test.go
Test: unbundled branch with missing framework-res module needed by robolectric genrule
Change-Id: I9c1f1cd82a80f048c0e903b8e93910b1ae34b0b1
This commit is contained in:
Colin Cross
2019-03-18 12:12:48 -07:00
parent 7a41ebdf5f
commit ba71a3fb11
4 changed files with 137 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
android/paths.go
View File

@@ -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 {

35
android/paths_test.go
View File

@@ -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() {

37
genrule/genrule.go
View File

@@ -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)

37
genrule/genrule_test.go
View File

@@ -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)
}
})
}