Fix missing genrule srcs and tools with ALLOW_MISSING_DEPENDENCIES=true (ba71a3fb) · Commits · e / os / android_build_soong

android/paths.go

+39 −14

Original line number	Diff line number	Diff line
		@@ -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 {

android/paths_test.go

+30 −5

Original line number	Diff line number	Diff line
		@@ -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,8 +755,10 @@ func (p *pathForModuleSrcTestModule) GenerateAndroidBuildActions(ctx ModuleConte
		}
		}

		if !p.props.Module_handles_missing_deps {
		p.missingDeps = ctx.GetMissingDependencies()
		}
		}

		type pathForModuleSrcTestCase struct {
		name string
		@@ -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() {

genrule/genrule.go

+34 −3

Original line number	Diff line number	Diff line
		@@ -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,10 +255,25 @@ 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)
		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)

genrule/genrule_test.go

+34 −3

Original line number	Diff line number	Diff line
		@@ -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)
		}
		})
		}