Remove SymlinkOutputs

symlink_outputs was added so bazel could run ninja files, but we
abanoned that approach in roboleaf, and then roboleaf was cancelled
entirely. Remove this feature so we're more compatible with upstream
ninja / n2.

Bug: 160568334
Test: Presubmits
Change-Id: Ic368c48dd01b68e51c471c3fe90d0c02c55956e9
This commit is contained in:
Cole Faust
2024-01-18 20:12:02 +00:00
parent f29b80c51f
commit 9a346f6da3
8 changed files with 23 additions and 222 deletions

View File

@@ -88,7 +88,6 @@ var (
blueprint.RuleParams{
Command: "rm -f $out && ln -f -s $fromPath $out",
Description: "symlink $out",
SymlinkOutputs: []string{"$out"},
},
"fromPath")

View File

@@ -16,11 +16,12 @@ package android
import (
"fmt"
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
"path"
"path/filepath"
"strings"
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
)
// BuildParameters describes the set of potential parameters to build a Ninja rule.
@@ -44,10 +45,6 @@ type BuildParams struct {
// Outputs is a slice of output file of the action. When using this field, references to $out in
// the Ninja command will refer to these files.
Outputs WritablePaths
// SymlinkOutput is an output file specifically that is a symlink.
SymlinkOutput WritablePath
// SymlinkOutputs is a slice of output files specifically that is a symlink.
SymlinkOutputs WritablePaths
// ImplicitOutput is an output file generated by the action. Note: references to `$out` in the
// Ninja command will NOT include references to this file.
ImplicitOutput WritablePath
@@ -255,25 +252,6 @@ 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 {
@@ -283,7 +261,6 @@ 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(),
@@ -298,9 +275,6 @@ 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())
}
@@ -316,7 +290,6 @@ 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)
@@ -374,13 +347,6 @@ func (m *moduleContext) Build(pctx PackageContext, params BuildParams) {
}
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)
}

View File

@@ -15,10 +15,11 @@
package android
import (
"github.com/google/blueprint"
"path/filepath"
"runtime"
"testing"
"github.com/google/blueprint"
)
func TestSrcIsModule(t *testing.T) {
@@ -244,52 +245,6 @@ func TestErrorDependsOnDisabledModule(t *testing.T) {
RunTestWithBp(t, bp)
}
func TestValidateCorrectBuildParams(t *testing.T) {
config := TestConfig(t.TempDir(), 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(t.TempDir(), 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)
}
}
func TestDistErrorChecking(t *testing.T) {
bp := `
deps {

View File

@@ -336,41 +336,6 @@ 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 {
@@ -775,7 +740,6 @@ func (r *RuleBuilder) build(name string, desc string, ninjaEscapeCommandString b
Validations: r.Validations(),
Output: output,
ImplicitOutputs: implicitOutputs,
SymlinkOutputs: r.SymlinkOutputs(),
Depfile: depFile,
Deps: depFormat,
Description: desc,
@@ -795,7 +759,6 @@ type RuleBuilderCommand struct {
orderOnlys Paths
validations Paths
outputs WritablePaths
symlinkOutputs WritablePaths
depFiles WritablePaths
tools Paths
packagedTools []PackagingSpec
@@ -1223,42 +1186,6 @@ 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 {
checkPathNotNil(path)
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 {
checkPathNotNil(path)
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

View File

@@ -48,7 +48,6 @@ func builderContext() BuilderContext {
"a": nil,
"b": nil,
"ls": nil,
"ln": nil,
"turbine": nil,
"java": nil,
"javac": nil,
@@ -81,32 +80,6 @@ func ExampleRuleBuilder() {
// outputs: ["out/soong/linked"]
}
func ExampleRuleBuilder_SymlinkOutputs() {
ctx := builderContext()
rule := NewRuleBuilder(pctx, ctx)
rule.Command().
Tool(PathForSource(ctx, "ln")).
FlagWithInput("-s ", PathForTesting("a.o")).
SymlinkOutput(PathForOutput(ctx, "a"))
rule.Command().Text("cp out/soong/a out/soong/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/soong/a && cp out/soong/a out/soong/b"
// tools: ["ln"]
// inputs: ["a.o"]
// outputs: ["out/soong/a" "out/soong/b"]
// symlink_outputs: ["out/soong/a" "out/soong/b"]
}
func ExampleRuleBuilder_Temporary() {
ctx := builderContext()
@@ -334,8 +307,6 @@ func TestRuleBuilder(t *testing.T) {
Output(PathForOutput(ctx, "module/Output")).
OrderOnly(PathForSource(ctx, "OrderOnly")).
Validation(PathForSource(ctx, "Validation")).
SymlinkOutput(PathForOutput(ctx, "module/SymlinkOutput")).
ImplicitSymlinkOutput(PathForOutput(ctx, "module/ImplicitSymlinkOutput")).
Text("Text").
Tool(PathForSource(ctx, "Tool"))
@@ -367,15 +338,13 @@ func TestRuleBuilder(t *testing.T) {
wantRspFileInputs := Paths{PathForSource(ctx, "RspInput"),
PathForOutput(ctx, "other/RspOutput2")}
wantOutputs := PathsForOutput(ctx, []string{
"module/ImplicitOutput", "module/ImplicitSymlinkOutput", "module/Output", "module/SymlinkOutput",
"module/output", "module/output2", "module/output3"})
"module/ImplicitOutput", "module/Output", "module/output", "module/output2",
"module/output3"})
wantDepFiles := PathsForOutput(ctx, []string{
"module/DepFile", "module/depfile", "module/ImplicitDepFile", "module/depfile2"})
wantTools := PathsForSource(ctx, []string{"Tool", "tool2"})
wantOrderOnlys := PathsForSource(ctx, []string{"OrderOnly", "OrderOnlys"})
wantValidations := PathsForSource(ctx, []string{"Validation", "Validations"})
wantSymlinkOutputs := PathsForOutput(ctx, []string{
"module/ImplicitSymlinkOutput", "module/SymlinkOutput"})
t.Run("normal", func(t *testing.T) {
rule := NewRuleBuilder(pctx, ctx)
@@ -384,7 +353,7 @@ func TestRuleBuilder(t *testing.T) {
wantCommands := []string{
"out_local/soong/module/DepFile Flag FlagWithArg=arg FlagWithDepFile=out_local/soong/module/depfile " +
"FlagWithInput=input FlagWithOutput=out_local/soong/module/output FlagWithRspFileInputList=out_local/soong/rsp " +
"Input out_local/soong/module/Output out_local/soong/module/SymlinkOutput Text Tool after command2 old cmd",
"Input out_local/soong/module/Output Text Tool after command2 old cmd",
"command2 out_local/soong/module/depfile2 input2 out_local/soong/module/output2 tool2",
"command3 input3 out_local/soong/module/output2 out_local/soong/module/output3 input3 out_local/soong/module/output2",
}
@@ -397,7 +366,6 @@ func TestRuleBuilder(t *testing.T) {
AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs())
AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs())
AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs())
AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs())
AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles())
AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools())
AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys())
@@ -415,7 +383,7 @@ func TestRuleBuilder(t *testing.T) {
"__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile " +
"FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output " +
"FlagWithRspFileInputList=out_local/soong/rsp Input __SBOX_SANDBOX_DIR__/out/Output " +
"__SBOX_SANDBOX_DIR__/out/SymlinkOutput Text Tool after command2 old cmd",
"Text Tool after command2 old cmd",
"command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 tool2",
"command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3 input3 __SBOX_SANDBOX_DIR__/out/output2",
}
@@ -427,7 +395,6 @@ func TestRuleBuilder(t *testing.T) {
AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs())
AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs())
AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs())
AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs())
AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles())
AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools())
AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys())
@@ -445,7 +412,7 @@ func TestRuleBuilder(t *testing.T) {
"__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile " +
"FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output " +
"FlagWithRspFileInputList=out_local/soong/rsp Input __SBOX_SANDBOX_DIR__/out/Output " +
"__SBOX_SANDBOX_DIR__/out/SymlinkOutput Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd",
"Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd",
"command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/tools/src/tool2",
"command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3 input3 __SBOX_SANDBOX_DIR__/out/output2",
}
@@ -457,7 +424,6 @@ func TestRuleBuilder(t *testing.T) {
AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs())
AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs())
AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs())
AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs())
AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles())
AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools())
AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys())
@@ -475,7 +441,7 @@ func TestRuleBuilder(t *testing.T) {
"__SBOX_SANDBOX_DIR__/out/DepFile Flag FlagWithArg=arg FlagWithDepFile=__SBOX_SANDBOX_DIR__/out/depfile " +
"FlagWithInput=input FlagWithOutput=__SBOX_SANDBOX_DIR__/out/output " +
"FlagWithRspFileInputList=__SBOX_SANDBOX_DIR__/out/soong/rsp Input __SBOX_SANDBOX_DIR__/out/Output " +
"__SBOX_SANDBOX_DIR__/out/SymlinkOutput Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd",
"Text __SBOX_SANDBOX_DIR__/tools/src/Tool after command2 old cmd",
"command2 __SBOX_SANDBOX_DIR__/out/depfile2 input2 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/tools/src/tool2",
"command3 input3 __SBOX_SANDBOX_DIR__/out/output2 __SBOX_SANDBOX_DIR__/out/output3 input3 __SBOX_SANDBOX_DIR__/out/output2",
}
@@ -487,7 +453,6 @@ func TestRuleBuilder(t *testing.T) {
AssertDeepEquals(t, "rule.Inputs()", wantInputs, rule.Inputs())
AssertDeepEquals(t, "rule.RspfileInputs()", wantRspFileInputs, rule.RspFileInputs())
AssertDeepEquals(t, "rule.Outputs()", wantOutputs, rule.Outputs())
AssertDeepEquals(t, "rule.SymlinkOutputs()", wantSymlinkOutputs, rule.SymlinkOutputs())
AssertDeepEquals(t, "rule.DepFiles()", wantDepFiles, rule.DepFiles())
AssertDeepEquals(t, "rule.Tools()", wantTools, rule.Tools())
AssertDeepEquals(t, "rule.OrderOnlys()", wantOrderOnlys, rule.OrderOnlys())

View File

@@ -170,12 +170,7 @@ 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)
}
func (s *singletonContextAdaptor) Phony(name string, deps ...Path) {

View File

@@ -725,7 +725,6 @@ type TestingBuildParams struct {
// - Depfile
// - Rspfile
// - RspfileContent
// - SymlinkOutputs
// - CommandDeps
// - CommandOrderOnly
//
@@ -747,8 +746,6 @@ func (p TestingBuildParams) RelativeToTop() TestingBuildParams {
bparams.Depfile = normalizeWritablePathRelativeToTop(bparams.Depfile)
bparams.Output = normalizeWritablePathRelativeToTop(bparams.Output)
bparams.Outputs = bparams.Outputs.RelativeToTop()
bparams.SymlinkOutput = normalizeWritablePathRelativeToTop(bparams.SymlinkOutput)
bparams.SymlinkOutputs = bparams.SymlinkOutputs.RelativeToTop()
bparams.ImplicitOutput = normalizeWritablePathRelativeToTop(bparams.ImplicitOutput)
bparams.ImplicitOutputs = bparams.ImplicitOutputs.RelativeToTop()
bparams.Input = normalizePathRelativeToTop(bparams.Input)
@@ -766,7 +763,6 @@ func (p TestingBuildParams) RelativeToTop() TestingBuildParams {
rparams.Depfile = normalizeStringRelativeToTop(p.config, rparams.Depfile)
rparams.Rspfile = normalizeStringRelativeToTop(p.config, rparams.Rspfile)
rparams.RspfileContent = normalizeStringRelativeToTop(p.config, rparams.RspfileContent)
rparams.SymlinkOutputs = normalizeStringArrayRelativeToTop(p.config, rparams.SymlinkOutputs)
rparams.CommandDeps = normalizeStringArrayRelativeToTop(p.config, rparams.CommandDeps)
rparams.CommandOrderOnly = normalizeStringArrayRelativeToTop(p.config, rparams.CommandOrderOnly)

View File

@@ -100,8 +100,6 @@ func runKati(ctx Context, config Config, extraSuffix string, args []string, envF
"--no_ninja_prelude",
// Support declaring phony outputs in AOSP Ninja.
"--use_ninja_phony_output",
// Support declaring symlink outputs in AOSP Ninja.
"--use_ninja_symlink_outputs",
// Regenerate the Ninja file if environment inputs have changed. e.g.
// CLI flags, .mk file timestamps, env vars, $(wildcard ..) and some
// $(shell ..) results.