From 85a2e89a14ebf147a24f06e17c32109f43398e27 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 9 Jul 2018 09:45:06 -0700 Subject: [PATCH] Fix error reporting when parsing genrule cmd Report errors inside the android.Expand lambda and don't return an error so that processing can continue and multiple errors can be reported. Check for errors (for example incorrect syntax of variables) immediately after Expand returns. Avoids a misleading error message: specified depfile=true but did not include a reference to '${depfile}' in cmd Bug: 111219250 Test: m checkbuild Change-Id: Id9a16c1609f5fd9345bfa1a2191261cff72fd382 --- genrule/genrule.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/genrule/genrule.go b/genrule/genrule.go index d03d4ee2f..03d4ea6fd 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -223,13 +223,18 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { task := g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) 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 + reportError := func(fmt string, args ...interface{}) (string, error) { + ctx.PropertyErrorf("cmd", fmt, args...) + return "SOONG_ERROR", nil + } + switch name { case "location": if len(g.properties.Tools) == 0 && len(toolFiles) == 0 { - return "", fmt.Errorf("at least one `tools` or `tool_files` is required if $(location) is used") - } - - if len(g.properties.Tools) > 0 { + return reportError("at least one `tools` or `tool_files` is required if $(location) is used") + } else if len(g.properties.Tools) > 0 { return tools[g.properties.Tools[0]].String(), nil } else { return tools[toolFiles[0].Rel()].String(), nil @@ -241,7 +246,7 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { case "depfile": referencedDepfile = true if !Bool(g.properties.Depfile) { - return "", fmt.Errorf("$(depfile) used without depfile property") + return reportError("$(depfile) used without depfile property") } return "__SBOX_DEPFILE__", nil case "genDir": @@ -252,22 +257,22 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { if tool, ok := tools[label]; ok { return tool.String(), nil } else { - return "", fmt.Errorf("unknown location label %q", label) + return reportError("unknown location label %q", label) } } - return "", fmt.Errorf("unknown variable '$(%s)'", name) + return reportError("unknown variable '$(%s)'", name) } }) - if Bool(g.properties.Depfile) && !referencedDepfile { - ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") - } - if err != nil { ctx.PropertyErrorf("cmd", "%s", err.Error()) return } + if Bool(g.properties.Depfile) && !referencedDepfile { + ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") + } + // tell the sbox command which directory to use as its sandbox root buildDir := android.PathForOutput(ctx).String() sandboxPath := shared.TempDirForOutDir(buildDir)