From ce679d29ec735058a0f655cd2dc5948dd521dd5c Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Wed, 23 Sep 2020 04:30:02 +0000 Subject: [PATCH] Add symlink_outputs support to Soong. This CL adds symlink_outputs to various locations in Soong that creates actions that creates symlink outputs, and explicitly mark them as such. Test: m Bug: 160568334 Change-Id: I322751bada52a9f49011c74731d84761586e03e7 --- android/defs.go | 5 ++- android/module.go | 38 +++++++++++++++- android/module_test.go | 46 +++++++++++++++++++ android/rule_builder.go | 87 ++++++++++++++++++++++++++++++++---- android/rule_builder_test.go | 39 ++++++++++++++-- android/singleton.go | 4 ++ ui/build/kati.go | 1 + 7 files changed, 206 insertions(+), 14 deletions(-) diff --git a/android/defs.go b/android/defs.go index 83daa0368..2b1bd85c9 100644 --- a/android/defs.go +++ b/android/defs.go @@ -69,8 +69,9 @@ var ( // A symlink rule. Symlink = pctx.AndroidStaticRule("Symlink", blueprint.RuleParams{ - Command: "rm -f $out && ln -f -s $fromPath $out", - Description: "symlink $out", + Command: "rm -f $out && ln -f -s $fromPath $out", + Description: "symlink $out", + SymlinkOutputs: []string{"$out"}, }, "fromPath") diff --git a/android/module.go b/android/module.go index 822e5bdf9..70a343e47 100644 --- a/android/module.go +++ b/android/module.go @@ -43,6 +43,8 @@ type BuildParams struct { Description string Output WritablePath Outputs WritablePaths + SymlinkOutput WritablePath + SymlinkOutputs WritablePaths ImplicitOutput WritablePath ImplicitOutputs WritablePaths Input Path @@ -1763,6 +1765,27 @@ func (m *moduleContext) ModuleBuild(pctx PackageContext, params ModuleBuildParam m.Build(pctx, BuildParams(params)) } +func validateBuildParams(params blueprint.BuildParams) error { + // Validate that the symlink outputs are declared outputs or implicit outputs + allOutputs := map[string]bool{} + for _, output := range params.Outputs { + allOutputs[output] = true + } + for _, output := range params.ImplicitOutputs { + allOutputs[output] = true + } + for _, symlinkOutput := range params.SymlinkOutputs { + if !allOutputs[symlinkOutput] { + return fmt.Errorf( + "Symlink output %s is not a declared output or implicit output", + symlinkOutput) + } + } + return nil +} + +// Convert build parameters from their concrete Android types into their string representations, +// and combine the singular and plural fields of the same type (e.g. Output and Outputs). func convertBuildParams(params BuildParams) blueprint.BuildParams { bparams := blueprint.BuildParams{ Rule: params.Rule, @@ -1770,6 +1793,7 @@ func convertBuildParams(params BuildParams) blueprint.BuildParams { Deps: params.Deps, Outputs: params.Outputs.Strings(), ImplicitOutputs: params.ImplicitOutputs.Strings(), + SymlinkOutputs: params.SymlinkOutputs.Strings(), Inputs: params.Inputs.Strings(), Implicits: params.Implicits.Strings(), OrderOnly: params.OrderOnly.Strings(), @@ -1784,6 +1808,9 @@ func convertBuildParams(params BuildParams) blueprint.BuildParams { if params.Output != nil { bparams.Outputs = append(bparams.Outputs, params.Output.String()) } + if params.SymlinkOutput != nil { + bparams.SymlinkOutputs = append(bparams.SymlinkOutputs, params.SymlinkOutput.String()) + } if params.ImplicitOutput != nil { bparams.ImplicitOutputs = append(bparams.ImplicitOutputs, params.ImplicitOutput.String()) } @@ -1799,6 +1826,7 @@ func convertBuildParams(params BuildParams) blueprint.BuildParams { bparams.Outputs = proptools.NinjaEscapeList(bparams.Outputs) bparams.ImplicitOutputs = proptools.NinjaEscapeList(bparams.ImplicitOutputs) + bparams.SymlinkOutputs = proptools.NinjaEscapeList(bparams.SymlinkOutputs) bparams.Inputs = proptools.NinjaEscapeList(bparams.Inputs) bparams.Implicits = proptools.NinjaEscapeList(bparams.Implicits) bparams.OrderOnly = proptools.NinjaEscapeList(bparams.OrderOnly) @@ -1855,7 +1883,15 @@ func (m *moduleContext) Build(pctx PackageContext, params BuildParams) { m.buildParams = append(m.buildParams, params) } - m.bp.Build(pctx.PackageContext, convertBuildParams(params)) + bparams := convertBuildParams(params) + err := validateBuildParams(bparams) + if err != nil { + m.ModuleErrorf( + "%s: build parameter validation failed: %s", + m.ModuleName(), + err.Error()) + } + m.bp.Build(pctx.PackageContext, bparams) } func (m *moduleContext) Phony(name string, deps ...Path) { diff --git a/android/module_test.go b/android/module_test.go index 6e648d7bb..3a039e29a 100644 --- a/android/module_test.go +++ b/android/module_test.go @@ -187,3 +187,49 @@ func TestErrorDependsOnDisabledModule(t *testing.T) { _, errs = ctx.PrepareBuildActions(config) FailIfNoMatchingErrors(t, `module "foo": depends on disabled module "bar"`, errs) } + +func TestValidateCorrectBuildParams(t *testing.T) { + config := TestConfig(buildDir, nil, "", nil) + pathContext := PathContextForTesting(config) + bparams := convertBuildParams(BuildParams{ + // Test with Output + Output: PathForOutput(pathContext, "undeclared_symlink"), + SymlinkOutput: PathForOutput(pathContext, "undeclared_symlink"), + }) + + err := validateBuildParams(bparams) + if err != nil { + t.Error(err) + } + + bparams = convertBuildParams(BuildParams{ + // Test with ImplicitOutput + ImplicitOutput: PathForOutput(pathContext, "undeclared_symlink"), + SymlinkOutput: PathForOutput(pathContext, "undeclared_symlink"), + }) + + err = validateBuildParams(bparams) + if err != nil { + t.Error(err) + } +} + +func TestValidateIncorrectBuildParams(t *testing.T) { + config := TestConfig(buildDir, nil, "", nil) + pathContext := PathContextForTesting(config) + params := BuildParams{ + Output: PathForOutput(pathContext, "regular_output"), + Outputs: PathsForOutput(pathContext, []string{"out1", "out2"}), + ImplicitOutput: PathForOutput(pathContext, "implicit_output"), + ImplicitOutputs: PathsForOutput(pathContext, []string{"i_out1", "_out2"}), + SymlinkOutput: PathForOutput(pathContext, "undeclared_symlink"), + } + + bparams := convertBuildParams(params) + err := validateBuildParams(bparams) + if err != nil { + FailIfNoMatchingErrors(t, "undeclared_symlink is not a declared output or implicit output", []error{err}) + } else { + t.Errorf("Expected build params to fail validation: %+v", bparams) + } +} diff --git a/android/rule_builder.go b/android/rule_builder.go index afb5f4e4a..8dc9d6a42 100644 --- a/android/rule_builder.go +++ b/android/rule_builder.go @@ -246,6 +246,41 @@ func (r *RuleBuilder) Outputs() WritablePaths { return outputList } +func (r *RuleBuilder) symlinkOutputSet() map[string]WritablePath { + symlinkOutputs := make(map[string]WritablePath) + for _, c := range r.commands { + for _, symlinkOutput := range c.symlinkOutputs { + symlinkOutputs[symlinkOutput.String()] = symlinkOutput + } + } + return symlinkOutputs +} + +// SymlinkOutputs returns the list of paths that the executor (Ninja) would +// verify, after build edge completion, that: +// +// 1) Created output symlinks match the list of paths in this list exactly (no more, no fewer) +// 2) Created output files are *not* declared in this list. +// +// These symlink outputs are expected to be a subset of outputs or implicit +// outputs, or they would fail validation at build param construction time +// later, to support other non-rule-builder approaches for constructing +// statements. +func (r *RuleBuilder) SymlinkOutputs() WritablePaths { + symlinkOutputs := r.symlinkOutputSet() + + var symlinkOutputList WritablePaths + for _, symlinkOutput := range symlinkOutputs { + symlinkOutputList = append(symlinkOutputList, symlinkOutput) + } + + sort.Slice(symlinkOutputList, func(i, j int) bool { + return symlinkOutputList[i].String() < symlinkOutputList[j].String() + }) + + return symlinkOutputList +} + func (r *RuleBuilder) depFileSet() map[string]WritablePath { depFiles := make(map[string]WritablePath) for _, c := range r.commands { @@ -467,6 +502,7 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string Implicits: r.Inputs(), Output: output, ImplicitOutputs: implicitOutputs, + SymlinkOutputs: r.SymlinkOutputs(), Depfile: depFile, Deps: depFormat, Description: desc, @@ -478,14 +514,15 @@ func (r *RuleBuilder) Build(pctx PackageContext, ctx BuilderContext, name string // RuleBuilderCommand, so they can be used chained or unchained. All methods that add text implicitly add a single // space as a separator from the previous method. type RuleBuilderCommand struct { - buf strings.Builder - inputs Paths - implicits Paths - orderOnlys Paths - outputs WritablePaths - depFiles WritablePaths - tools Paths - rspFileInputs Paths + buf strings.Builder + inputs Paths + implicits Paths + orderOnlys Paths + outputs WritablePaths + symlinkOutputs WritablePaths + depFiles WritablePaths + tools Paths + rspFileInputs Paths // spans [start,end) of the command that should not be ninja escaped unescapedSpans [][2]int @@ -715,6 +752,40 @@ func (c *RuleBuilderCommand) ImplicitOutputs(paths WritablePaths) *RuleBuilderCo return c } +// ImplicitSymlinkOutput declares the specified path as an implicit output that +// will be a symlink instead of a regular file. Does not modify the command +// line. +func (c *RuleBuilderCommand) ImplicitSymlinkOutput(path WritablePath) *RuleBuilderCommand { + c.symlinkOutputs = append(c.symlinkOutputs, path) + return c.ImplicitOutput(path) +} + +// ImplicitSymlinkOutputs declares the specified paths as implicit outputs that +// will be a symlinks instead of regular files. Does not modify the command +// line. +func (c *RuleBuilderCommand) ImplicitSymlinkOutputs(paths WritablePaths) *RuleBuilderCommand { + for _, path := range paths { + c.ImplicitSymlinkOutput(path) + } + return c +} + +// SymlinkOutput declares the specified path as an output that will be a symlink +// instead of a regular file. Modifies the command line. +func (c *RuleBuilderCommand) SymlinkOutput(path WritablePath) *RuleBuilderCommand { + c.symlinkOutputs = append(c.symlinkOutputs, path) + return c.Output(path) +} + +// SymlinkOutputsl declares the specified paths as outputs that will be symlinks +// instead of regular files. Modifies the command line. +func (c *RuleBuilderCommand) SymlinkOutputs(paths WritablePaths) *RuleBuilderCommand { + for _, path := range paths { + c.SymlinkOutput(path) + } + return c +} + // ImplicitDepFile adds the specified depfile path to the paths returned by RuleBuilder.DepFiles without modifying // the command line, and causes RuleBuilder.Build file to set the depfile flag for ninja. If multiple depfiles // are added to commands in a single RuleBuilder then RuleBuilder.Build will add an extra command to merge the diff --git a/android/rule_builder_test.go b/android/rule_builder_test.go index c41b06734..0d1070da2 100644 --- a/android/rule_builder_test.go +++ b/android/rule_builder_test.go @@ -35,6 +35,7 @@ func pathContext() PathContext { "a": nil, "b": nil, "ls": nil, + "ln": nil, "turbine": nil, "java": nil, "javac": nil, @@ -67,6 +68,32 @@ func ExampleRuleBuilder() { // outputs: ["out/linked"] } +func ExampleRuleBuilder_SymlinkOutputs() { + rule := NewRuleBuilder() + + ctx := pathContext() + + rule.Command(). + Tool(PathForSource(ctx, "ln")). + FlagWithInput("-s ", PathForTesting("a.o")). + SymlinkOutput(PathForOutput(ctx, "a")) + rule.Command().Text("cp out/a out/b"). + ImplicitSymlinkOutput(PathForOutput(ctx, "b")) + + fmt.Printf("commands: %q\n", strings.Join(rule.Commands(), " && ")) + fmt.Printf("tools: %q\n", rule.Tools()) + fmt.Printf("inputs: %q\n", rule.Inputs()) + fmt.Printf("outputs: %q\n", rule.Outputs()) + fmt.Printf("symlink_outputs: %q\n", rule.SymlinkOutputs()) + + // Output: + // commands: "ln -s a.o out/a && cp out/a out/b" + // tools: ["ln"] + // inputs: ["a.o"] + // outputs: ["out/a" "out/b"] + // symlink_outputs: ["out/a" "out/b"] +} + func ExampleRuleBuilder_Temporary() { rule := NewRuleBuilder() @@ -293,6 +320,8 @@ func TestRuleBuilder(t *testing.T) { Input(PathForSource(ctx, "Input")). Output(PathForOutput(ctx, "Output")). OrderOnly(PathForSource(ctx, "OrderOnly")). + SymlinkOutput(PathForOutput(ctx, "SymlinkOutput")). + ImplicitSymlinkOutput(PathForOutput(ctx, "ImplicitSymlinkOutput")). Text("Text"). Tool(PathForSource(ctx, "Tool")) @@ -318,17 +347,18 @@ func TestRuleBuilder(t *testing.T) { } wantInputs := PathsForSource(ctx, []string{"Implicit", "Input", "input", "input2", "input3"}) - wantOutputs := PathsForOutput(ctx, []string{"ImplicitOutput", "Output", "output", "output2", "output3"}) + wantOutputs := PathsForOutput(ctx, []string{"ImplicitOutput", "ImplicitSymlinkOutput", "Output", "SymlinkOutput", "output", "output2", "output3"}) wantDepFiles := PathsForOutput(ctx, []string{"DepFile", "depfile", "ImplicitDepFile", "depfile2"}) wantTools := PathsForSource(ctx, []string{"Tool", "tool2"}) wantOrderOnlys := PathsForSource(ctx, []string{"OrderOnly", "OrderOnlys"}) + wantSymlinkOutputs := PathsForOutput(ctx, []string{"ImplicitSymlinkOutput", "SymlinkOutput"}) t.Run("normal", func(t *testing.T) { rule := NewRuleBuilder() addCommands(rule) wantCommands := []string{ - "out/DepFile Flag FlagWithArg=arg FlagWithDepFile=out/depfile FlagWithInput=input FlagWithOutput=out/output Input out/Output Text Tool after command2 old cmd", + "out/DepFile Flag FlagWithArg=arg FlagWithDepFile=out/depfile FlagWithInput=input FlagWithOutput=out/output Input out/Output out/SymlinkOutput Text Tool after command2 old cmd", "command2 out/depfile2 input2 out/output2 tool2", "command3 input3 out/output2 out/output3", } @@ -345,6 +375,9 @@ func TestRuleBuilder(t *testing.T) { if g, w := rule.Outputs(), wantOutputs; !reflect.DeepEqual(w, g) { t.Errorf("\nwant rule.Outputs() = %#v\n got %#v", w, g) } + if g, w := rule.SymlinkOutputs(), wantSymlinkOutputs; !reflect.DeepEqual(w, g) { + t.Errorf("\nwant rule.SymlinkOutputs() = %#v\n got %#v", w, g) + } if g, w := rule.DepFiles(), wantDepFiles; !reflect.DeepEqual(w, g) { t.Errorf("\nwant rule.DepFiles() = %#v\n got %#v", w, g) } @@ -365,7 +398,7 @@ func TestRuleBuilder(t *testing.T) { addCommands(rule) wantCommands := []string{ - "__SBOX_OUT_DIR__/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_OUT_DIR__/depfile FlagWithInput=input FlagWithOutput=__SBOX_OUT_DIR__/output Input __SBOX_OUT_DIR__/Output Text Tool after command2 old cmd", + "__SBOX_OUT_DIR__/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_OUT_DIR__/depfile FlagWithInput=input FlagWithOutput=__SBOX_OUT_DIR__/output Input __SBOX_OUT_DIR__/Output __SBOX_OUT_DIR__/SymlinkOutput Text Tool after command2 old cmd", "command2 __SBOX_OUT_DIR__/depfile2 input2 __SBOX_OUT_DIR__/output2 tool2", "command3 input3 __SBOX_OUT_DIR__/output2 __SBOX_OUT_DIR__/output3", } diff --git a/android/singleton.go b/android/singleton.go index 9832378d5..bb6614dad 100644 --- a/android/singleton.go +++ b/android/singleton.go @@ -168,6 +168,10 @@ func (s *singletonContextAdaptor) Build(pctx PackageContext, params BuildParams) s.buildParams = append(s.buildParams, params) } bparams := convertBuildParams(params) + err := validateBuildParams(bparams) + if err != nil { + s.Errorf("%s: build parameter validation failed: %s", s.Name(), err.Error()) + } s.SingletonContext.Build(pctx.PackageContext, bparams) } diff --git a/ui/build/kati.go b/ui/build/kati.go index f6d3a5741..f6c0f52a3 100644 --- a/ui/build/kati.go +++ b/ui/build/kati.go @@ -68,6 +68,7 @@ func runKati(ctx Context, config Config, extraSuffix string, args []string, envF "--ninja_suffix=" + config.KatiSuffix() + extraSuffix, "--no_ninja_prelude", "--use_ninja_phony_output", + "--use_ninja_symlink_outputs", "--regen", "--ignore_optional_include=" + filepath.Join(config.OutDir(), "%.P"), "--detect_android_echo",