Remove depfile support from genrules (55492574) · Commits · e / os / android_build_soong

genrule/allowlists.go

+0 −6

Original line number	Diff line number	Diff line
		@@ -15,12 +15,6 @@
		package genrule

		var (
		DepfileAllowList = []string{
		// go/keep-sorted start
		"depfile_allowed_for_test",
		// go/keep-sorted end
		}

		SandboxingDenyModuleList = []string{
		// go/keep-sorted start
		"aidl_camera_build_version",

genrule/genrule.go

+13 −81

Original line number	Diff line number	Diff line
		@@ -124,14 +124,10 @@ type generatorProperties struct {
		// $(locations <label>): the paths to the tools, tool_files, inputs or outputs with name <label>. Use $(locations) if <label> refers to a rule that outputs two or more files.
		// $(in): one or more input files.
		// $(out): a single output file.
		// $(depfile): a file to which dependencies will be written, if the depfile property is set to true.
		// $(genDir): the sandbox directory for this tool; contains $(out).
		// $$: a literal $
		Cmd *string

		// Enable reading a file containing dependencies in gcc format after the command completes
		Depfile *bool

		// name of the modules (if any) that produces the host executable. Leave empty for
		// prebuilts or scripts that do not need a module to build them.
		Tools []string
		@@ -194,10 +190,8 @@ type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles androi
		type generateTask struct {
		in android.Paths
		out android.WritablePaths
		depFile android.WritablePath
		copyTo android.WritablePaths // For gensrcs to set on gensrcsMerge rule.
		genDir android.WritablePath
		extraTools android.Paths // dependencies on tools used by the generator
		extraInputs map[string][]string

		cmd string
		@@ -448,8 +442,6 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
		addLocationLabel(out.Rel(), outputLocation{out})
		}

		referencedDepfile := false

		rawCommand, err := android.Expand(task.cmd, func(name string) (string, error) {
		// report the error directly without returning an error to android.Expand to catch multiple errors in a
		// single run
		@@ -481,12 +473,6 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
		sandboxOuts = append(sandboxOuts, cmd.PathForOutput(out))
		}
		return strings.Join(proptools.ShellEscapeList(sandboxOuts), " "), nil
		case "depfile":
		referencedDepfile = true
		if !Bool(g.properties.Depfile) {
		return reportError("$(depfile) used without depfile property")
		}
		return "__SBOX_DEPFILE__", nil
		case "genDir":
		return proptools.ShellEscape(cmd.PathForOutput(task.genDir)), nil
		default:
		@@ -526,10 +512,6 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
		return
		}

		if Bool(g.properties.Depfile) && !referencedDepfile {
		ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd")
		return
		}
		g.rawCommands = append(g.rawCommands, rawCommand)

		cmd.Text(rawCommand)
		@@ -538,11 +520,7 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
		cmd.ImplicitOutputs(task.out)
		cmd.Implicits(task.in)
		cmd.ImplicitTools(tools)
		cmd.ImplicitTools(task.extraTools)
		cmd.ImplicitPackagedTools(packagedTools)
		if Bool(g.properties.Depfile) {
		cmd.ImplicitDepFile(task.depFile)
		}

		// Create the rule to run the genrule command inside sbox.
		rule.Build(name, desc)
		@@ -583,19 +561,6 @@ func (g *Module) generateCommonBuildActions(ctx android.ModuleContext) {
		}

		func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
		// Allowlist genrule to use depfile until we have a solution to remove it.
		// TODO(b/307824623): Remove depfile property
		if Bool(g.properties.Depfile) {
		sandboxingAllowlistSets := getSandboxingAllowlistSets(ctx)
		if ctx.DeviceConfig().GenruleSandboxing() && !sandboxingAllowlistSets.depfileAllowSet[g.Name()] {
		ctx.PropertyErrorf(
		"depfile",
		"Deprecated because with genrule sandboxing, dependencies must be known before the action is run "+
		"in order to add them to the sandbox. "+
		"Please specify the dependencies explicitly so that there is no need to use depfile.")
		}
		}

		g.generateCommonBuildActions(ctx)

		// For <= 6 outputs, just embed those directly in the users. Right now, that covers >90% of
		@@ -721,7 +686,6 @@ func NewGenSrcs() *Module {
		for i, shard := range shards {
		var commands []string
		var outFiles android.WritablePaths
		var commandDepFiles []string
		var copyTo android.WritablePaths

		// When sharding is enabled (i.e. len(shards) > 1), the sbox rules for each
		@@ -761,12 +725,6 @@ func NewGenSrcs() *Module {
		return in.String(), nil
		case "out":
		return rule.Command().PathForOutput(outFile), nil
		case "depfile":
		// Generate a depfile for each output file. Store the list for
		// later in order to combine them all into a single depfile.
		depFile := rule.Command().PathForOutput(outFile.ReplaceExtension(ctx, "d"))
		commandDepFiles = append(commandDepFiles, depFile)
		return depFile, nil
		default:
		return "$(" + name + ")", nil
		}
		@@ -781,30 +739,14 @@ func NewGenSrcs() *Module {
		}
		fullCommand := strings.Join(commands, " && ")

		var outputDepfile android.WritablePath
		var extraTools android.Paths
		if len(commandDepFiles) > 0 {
		// Each command wrote to a depfile, but ninja can only handle one
		// depfile per rule. Use the dep_fixer tool at the end of the
		// command to combine all the depfiles into a single output depfile.
		outputDepfile = android.PathForModuleGen(ctx, genSubDir, "gensrcs.d")
		depFixerTool := ctx.Config().HostToolPath(ctx, "dep_fixer")
		fullCommand += fmt.Sprintf(" && %s -o $(depfile) %s",
		rule.Command().PathForTool(depFixerTool),
		strings.Join(commandDepFiles, " "))
		extraTools = append(extraTools, depFixerTool)
		}

		generateTasks = append(generateTasks, generateTask{
		in: shard,
		out: outFiles,
		depFile: outputDepfile,
		copyTo: copyTo,
		genDir: genDir,
		cmd: fullCommand,
		shard: i,
		shards: len(shards),
		extraTools: extraTools,
		extraInputs: map[string][]string{
		"data": properties.Data,
		},
		@@ -843,18 +785,12 @@ func NewGenRule() *Module {

		taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask {
		outs := make(android.WritablePaths, len(properties.Out))
		var depFile android.WritablePath
		for i, out := range properties.Out {
		outPath := android.PathForModuleGen(ctx, out)
		if i == 0 {
		depFile = outPath.ReplaceExtension(ctx, "d")
		}
		outs[i] = outPath
		outs[i] = android.PathForModuleGen(ctx, out)
		}
		return []generateTask{{
		in: srcFiles,
		out: outs,
		depFile: depFile,
		genDir: android.PathForModuleGen(ctx),
		cmd: rawCommand,
		}}
		@@ -907,22 +843,18 @@ var sandboxingAllowlistKey = android.NewOnceKey("genruleSandboxingAllowlistKey")
		type sandboxingAllowlistSets struct {
		sandboxingDenyModuleSet map[string]bool
		sandboxingDenyPathSet map[string]bool
		depfileAllowSet map[string]bool
		}

		func getSandboxingAllowlistSets(ctx android.PathContext) *sandboxingAllowlistSets {
		return ctx.Config().Once(sandboxingAllowlistKey, func() interface{} {
		sandboxingDenyModuleSet := map[string]bool{}
		sandboxingDenyPathSet := map[string]bool{}
		depfileAllowSet := map[string]bool{}

		android.AddToStringSet(sandboxingDenyModuleSet, append(DepfileAllowList, SandboxingDenyModuleList...))
		android.AddToStringSet(sandboxingDenyModuleSet, SandboxingDenyModuleList)
		android.AddToStringSet(sandboxingDenyPathSet, SandboxingDenyPathList)
		android.AddToStringSet(depfileAllowSet, DepfileAllowList)
		return &sandboxingAllowlistSets{
		sandboxingDenyModuleSet: sandboxingDenyModuleSet,
		sandboxingDenyPathSet: sandboxingDenyPathSet,
		depfileAllowSet: depfileAllowSet,
		}
		}).(*sandboxingAllowlistSets)
		}

genrule/genrule_test.go

+0 −82

Original line number	Diff line number	Diff line
		@@ -286,16 +286,6 @@ func TestGenruleCmd(t *testing.T) {
		`,
		expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out2",
		},
		{
		name: "depfile",
		moduleName: "depfile_allowed_for_test",
		prop: `
		out: ["out"],
		depfile: true,
		cmd: "echo foo > $(out) && touch $(depfile)",
		`,
		expect: "echo foo > __SBOX_SANDBOX_DIR__/out/out && touch __SBOX_DEPFILE__",
		},
		{
		name: "gendir",
		prop: `
		@@ -391,24 +381,6 @@ func TestGenruleCmd(t *testing.T) {
		`,
		err: `unknown variable '$(foo)'`,
		},
		{
		name: "error depfile",
		prop: `
		out: ["out"],
		cmd: "echo foo > $(out) && touch $(depfile)",
		`,
		err: "$(depfile) used without depfile property",
		},
		{
		name: "error no depfile",
		moduleName: "depfile_allowed_for_test",
		prop: `
		out: ["out"],
		depfile: true,
		cmd: "echo foo > $(out)",
		`,
		err: "specified depfile=true but did not include a reference to '${depfile}' in cmd",
		},
		{
		name: "error no out",
		prop: `
		@@ -695,60 +667,6 @@ func TestGenSrcs(t *testing.T) {
		}
		}

		func TestGenruleAllowlistingDepfile(t *testing.T) {
		tests := []struct {
		name string
		prop string
		err string
		moduleName string
		}{
		{
		name: `error when module is not allowlisted`,
		prop: `
		depfile: true,
		cmd: "cat $(in) > $(out) && cat $(depfile)",
		`,
		err: "depfile: Deprecated because with genrule sandboxing, dependencies must be known before the action is run in order to add them to the sandbox",
		},
		{
		name: `no error when module is allowlisted`,
		prop: `
		depfile: true,
		cmd: "cat $(in) > $(out) && cat $(depfile)",
		`,
		moduleName: `depfile_allowed_for_test`,
		},
		}
		for _, test := range tests {
		t.Run(test.name, func(t *testing.T) {
		moduleName := "foo"
		if test.moduleName != "" {
		moduleName = test.moduleName
		}
		bp := fmt.Sprintf(`
		gensrcs {
		name: "%s",
		srcs: ["data.txt"],
		%s
		}`, moduleName, test.prop)

		var expectedErrors []string
		if test.err != "" {
		expectedErrors = append(expectedErrors, test.err)
		}
		android.GroupFixturePreparers(
		prepareForGenRuleTest,
		android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) {
		variables.GenruleSandboxing = proptools.BoolPtr(true)
		}),
		).
		ExtendWithErrorHandler(android.FixtureExpectsAllErrorsToMatchAPattern(expectedErrors)).
		RunTestWithBp(t, bp)
		})

		}
		}

		func TestGenruleDefaults(t *testing.T) {
		bp := `
		genrule_defaults {