diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 8a9a5f8..64fa8e5 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -180,8 +180,10 @@ func toolDir(config interface{}) string { func pluginDeps(ctx blueprint.BottomUpMutatorContext) { if pkg, ok := ctx.Module().(*goPackage); ok { - for _, plugin := range pkg.properties.PluginFor { - ctx.AddReverseDependency(ctx.Module(), nil, plugin) + if ctx.PrimaryModule() == ctx.Module() { + for _, plugin := range pkg.properties.PluginFor { + ctx.AddReverseDependency(ctx.Module(), nil, plugin) + } } } } @@ -211,7 +213,7 @@ func isGoPluginFor(name string) func(blueprint.Module) bool { } } -func isBootstrapModule(module blueprint.Module) bool { +func IsBootstrapModule(module blueprint.Module) bool { _, isPackage := module.(*goPackage) _, isBinary := module.(*goBinary) return isPackage || isBinary @@ -268,6 +270,9 @@ func newGoPackageModuleFactory(config *Config) func() (blueprint.Module, []inter } func (g *goPackage) DynamicDependencies(ctx blueprint.DynamicDependerModuleContext) []string { + if ctx.Module() != ctx.PrimaryModule() { + return nil + } return g.properties.Deps } @@ -297,6 +302,16 @@ func (g *goPackage) IsPluginFor(name string) bool { } func (g *goPackage) GenerateBuildActions(ctx blueprint.ModuleContext) { + // Allow the primary builder to create multiple variants. Any variants after the first + // will copy outputs from the first. + if ctx.Module() != ctx.PrimaryModule() { + primary := ctx.PrimaryModule().(*goPackage) + g.pkgRoot = primary.pkgRoot + g.archiveFile = primary.archiveFile + g.testResultFile = primary.testResultFile + return + } + var ( name = ctx.ModuleName() hasPlugins = false @@ -386,6 +401,9 @@ func newGoBinaryModuleFactory(config *Config, tooldir bool) func() (blueprint.Mo } func (g *goBinary) DynamicDependencies(ctx blueprint.DynamicDependerModuleContext) []string { + if ctx.Module() != ctx.PrimaryModule() { + return nil + } return g.properties.Deps } @@ -395,6 +413,14 @@ func (g *goBinary) InstallPath() string { } func (g *goBinary) GenerateBuildActions(ctx blueprint.ModuleContext) { + // Allow the primary builder to create multiple variants. Any variants after the first + // will copy outputs from the first. + if ctx.Module() != ctx.PrimaryModule() { + primary := ctx.PrimaryModule().(*goBinary) + g.installPath = primary.installPath + return + } + var ( name = ctx.ModuleName() objDir = moduleObjDir(ctx, g.config) @@ -653,13 +679,15 @@ func (s *singleton) GenerateBuildActions(ctx blueprint.SingletonContext) { var blueprintTools []string ctx.VisitAllModulesIf(isBootstrapBinaryModule, func(module blueprint.Module) { - binaryModule := module.(*goBinary) + if ctx.PrimaryModule(module) == module { + binaryModule := module.(*goBinary) - if binaryModule.properties.Tool_dir { - blueprintTools = append(blueprintTools, binaryModule.InstallPath()) - } - if binaryModule.properties.PrimaryBuilder { - primaryBuilders = append(primaryBuilders, binaryModule) + if binaryModule.properties.Tool_dir { + blueprintTools = append(blueprintTools, binaryModule.InstallPath()) + } + if binaryModule.properties.PrimaryBuilder { + primaryBuilders = append(primaryBuilders, binaryModule) + } } }) diff --git a/bpmodify/bpmodify.go b/bpmodify/bpmodify.go index 29e97d1..0190f19 100644 --- a/bpmodify/bpmodify.go +++ b/bpmodify/bpmodify.go @@ -22,18 +22,20 @@ import ( var ( // main operation modes - list = flag.Bool("l", false, "list files that would be modified by bpmodify") - write = flag.Bool("w", false, "write result to (source) file instead of stdout") - doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") - sortLists = flag.Bool("s", false, "sort touched lists, even if they were unsorted") - parameter = flag.String("parameter", "deps", "name of parameter to modify on each module") - targetedModules = new(identSet) - addIdents = new(identSet) - removeIdents = new(identSet) + list = flag.Bool("l", false, "list files that would be modified by bpmodify") + write = flag.Bool("w", false, "write result to (source) file instead of stdout") + doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") + sortLists = flag.Bool("s", false, "sort touched lists, even if they were unsorted") + targetedModules = new(identSet) + targetedProperty = new(qualifiedProperty) + addIdents = new(identSet) + removeIdents = new(identSet) ) func init() { flag.Var(targetedModules, "m", "comma or whitespace separated list of modules on which to operate") + flag.Var(targetedProperty, "parameter", "alias to -property=`name`") + flag.Var(targetedProperty, "property", "fully qualified `name` of property to modify (default \"deps\")") flag.Var(addIdents, "a", "comma or whitespace separated list of identifiers to add") flag.Var(removeIdents, "r", "comma or whitespace separated list of identifiers to remove") flag.Usage = usage @@ -140,24 +142,74 @@ func findModules(file *parser.File) (modified bool, errs []error) { func processModule(module *parser.Module, moduleName string, file *parser.File) (modified bool, errs []error) { - - for _, prop := range module.Properties { - if prop.Name == *parameter { - modified, errs = processParameter(prop.Value, *parameter, moduleName, file) - return + prop, err := getRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes()) + if err != nil { + return false, []error{err} + } + if prop == nil { + if len(addIdents.idents) == 0 { + // We cannot find an existing prop, and we aren't adding anything to the prop, + // which means we must be removing something from a non-existing prop, + // which means this is a noop. + return false, nil + } + // Else we are adding something to a non-existing prop, so we need to create it first. + prop, modified, err = createRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes()) + if err != nil { + // Here should be unreachable, but still handle it for completeness. + return false, []error{err} } } - - prop := parser.Property{Name: *parameter, Value: &parser.List{}} - modified, errs = processParameter(prop.Value, *parameter, moduleName, file) - - if modified { - module.Properties = append(module.Properties, &prop) - } - + m, errs := processParameter(prop.Value, targetedProperty.String(), moduleName, file) + modified = modified || m return modified, errs } +func getRecursiveProperty(module *parser.Module, name string, prefixes []string) (prop *parser.Property, err error) { + prop, _, err = getOrCreateRecursiveProperty(module, name, prefixes, false) + return prop, err +} + +func createRecursiveProperty(module *parser.Module, name string, prefixes []string) (prop *parser.Property, modified bool, err error) { + return getOrCreateRecursiveProperty(module, name, prefixes, true) +} + +func getOrCreateRecursiveProperty(module *parser.Module, name string, prefixes []string, + createIfNotFound bool) (prop *parser.Property, modified bool, err error) { + m := &module.Map + for i, prefix := range prefixes { + if prop, found := m.GetProperty(prefix); found { + if mm, ok := prop.Value.Eval().(*parser.Map); ok { + m = mm + } else { + // We've found a property in the AST and such property is not of type + // *parser.Map, which must mean we didn't modify the AST. + return nil, false, fmt.Errorf("Expected property %q to be a map, found %s", + strings.Join(prefixes[:i+1], "."), prop.Value.Type()) + } + } else if createIfNotFound { + mm := &parser.Map{} + m.Properties = append(m.Properties, &parser.Property{Name: prefix, Value: mm}) + m = mm + // We've created a new node in the AST. This means the m.GetProperty(name) + // check after this for loop must fail, because the node we inserted is an + // empty parser.Map, thus this function will return |modified| is true. + } else { + return nil, false, nil + } + } + if prop, found := m.GetProperty(name); found { + // We've found a property in the AST, which must mean we didn't modify the AST. + return prop, false, nil + } else if createIfNotFound { + prop = &parser.Property{Name: name, Value: &parser.List{}} + m.Properties = append(m.Properties, prop) + return prop, true, nil + } else { + return nil, false, nil + } +} + func processParameter(value parser.Expression, paramName, moduleName string, file *parser.File) (modified bool, errs []error) { if _, ok := value.(*parser.Variable); ok { @@ -232,6 +284,10 @@ func main() { flag.Parse() + if len(targetedProperty.parts) == 0 { + targetedProperty.Set("deps") + } + if flag.NArg() == 0 { if *write { report(fmt.Errorf("error: cannot use -w with standard input")) @@ -318,3 +374,38 @@ func (m *identSet) Set(s string) error { func (m *identSet) Get() interface{} { return m.idents } + +type qualifiedProperty struct { + parts []string +} + +var _ flag.Getter = (*qualifiedProperty)(nil) + +func (p *qualifiedProperty) name() string { + return p.parts[len(p.parts)-1] +} + +func (p *qualifiedProperty) prefixes() []string { + return p.parts[:len(p.parts)-1] +} + +func (p *qualifiedProperty) String() string { + return strings.Join(p.parts, ".") +} + +func (p *qualifiedProperty) Set(s string) error { + p.parts = strings.Split(s, ".") + if len(p.parts) == 0 { + return fmt.Errorf("%q is not a valid property name", s) + } + for _, part := range p.parts { + if part == "" { + return fmt.Errorf("%q is not a valid property name", s) + } + } + return nil +} + +func (p *qualifiedProperty) Get() interface{} { + return p.parts +} diff --git a/bpmodify/bpmodify_test.go b/bpmodify/bpmodify_test.go new file mode 100644 index 0000000..a498a14 --- /dev/null +++ b/bpmodify/bpmodify_test.go @@ -0,0 +1,259 @@ +// Copyright 2020 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "strings" + "testing" + + "github.com/google/blueprint/parser" +) + +var testCases = []struct { + input string + output string + property string + addSet string + removeSet string +}{ + { + ` + cc_foo { + name: "foo", + } + `, + ` + cc_foo { + name: "foo", + deps: ["bar"], + } + `, + "deps", + "bar", + "", + }, + { + ` + cc_foo { + name: "foo", + deps: ["bar"], + } + `, + ` + cc_foo { + name: "foo", + deps: [], + } + `, + "deps", + "", + "bar", + }, + { + ` + cc_foo { + name: "foo", + } + `, + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "dep2", + "nested_dep",], + }, + }, + } + `, + "arch.arm.deps", + "nested_dep,dep2", + "", + }, + { + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "dep2", + "nested_dep", + ], + }, + }, + } + `, + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + ], + }, + }, + } + `, + "arch.arm.deps", + "", + "nested_dep,dep2", + }, + { + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "nested_dep", + "dep2", + ], + }, + }, + } + `, + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "nested_dep", + "dep2", + ], + }, + }, + } + `, + "arch.arm.deps", + "dep2,dep2", + "", + }, + { + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "nested_dep", + "dep2", + ], + }, + }, + } + `, + ` + cc_foo { + name: "foo", + arch: { + arm: { + deps: [ + "nested_dep", + "dep2", + ], + }, + }, + } + `, + "arch.arm.deps", + "", + "dep3,dep4", + }, + { + ` + cc_foo { + name: "foo", + } + `, + ` + cc_foo { + name: "foo", + } + `, + "deps", + "", + "bar", + }, + { + ` + cc_foo { + name: "foo", + arch: {}, + } + `, + ` + cc_foo { + name: "foo", + arch: {}, + } + `, + "arch.arm.deps", + "", + "dep3,dep4", + }, +} + +func simplifyModuleDefinition(def string) string { + var result string + for _, line := range strings.Split(def, "\n") { + result += strings.TrimSpace(line) + } + return result +} + +func TestProcessModule(t *testing.T) { + for i, testCase := range testCases { + targetedProperty.Set(testCase.property) + addIdents.Set(testCase.addSet) + removeIdents.Set(testCase.removeSet) + + inAst, errs := parser.ParseAndEval("", strings.NewReader(testCase.input), parser.NewScope(nil)) + if len(errs) > 0 { + t.Errorf("test case %d:", i) + for _, err := range errs { + t.Errorf(" %s", err) + } + t.Errorf("failed to parse:") + t.Errorf("%+v", testCase) + continue + } + + if inModule, ok := inAst.Defs[0].(*parser.Module); !ok { + t.Errorf("test case %d:", i) + t.Errorf(" input must only contain a single module definition: %s", testCase.input) + continue + } else { + _, errs := processModule(inModule, "", inAst) + if len(errs) > 0 { + t.Errorf("test case %d:", i) + for _, err := range errs { + t.Errorf(" %s", err) + } + } + inModuleText, _ := parser.Print(inAst) + inModuleString := string(inModuleText) + if simplifyModuleDefinition(inModuleString) != simplifyModuleDefinition(testCase.output) { + t.Errorf("test case %d:", i) + t.Errorf("expected module definition:") + t.Errorf(" %s", testCase.output) + t.Errorf("actual module definition:") + t.Errorf(" %s", inModuleString) + } + } + } +} diff --git a/module_ctx.go b/module_ctx.go index 28578e0..c992c0a 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -246,6 +246,24 @@ type BaseModuleContext interface { // invalidated by future mutators. WalkDeps(visit func(Module, Module) bool) + // PrimaryModule returns the first variant of the current module. Variants of a module are always visited in + // order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from the + // Module returned by PrimaryModule without data races. This can be used to perform singleton actions that are + // only done once for all variants of a module. + PrimaryModule() Module + + // FinalModule returns the last variant of the current module. Variants of a module are always visited in + // order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from all + // variants using VisitAllModuleVariants if the current module == FinalModule(). This can be used to perform + // singleton actions that are only done once for all variants of a module. + FinalModule() Module + + // VisitAllModuleVariants calls visit for each variant of the current module. Variants of a module are always + // visited in order by mutators and GenerateBuildActions, so the data created by the current mutator can be read + // from all variants if the current module == FinalModule(). Otherwise, care must be taken to not access any + // data modified by the current mutator. + VisitAllModuleVariants(visit func(Module)) + // OtherModuleName returns the name of another Module. See BaseModuleContext.ModuleName for more information. // It is intended for use inside the visit functions of Visit* and WalkDeps. OtherModuleName(m Module) string @@ -309,24 +327,6 @@ type ModuleContext interface { // Build creates a new ninja build statement. Build(pctx PackageContext, params BuildParams) - // PrimaryModule returns the first variant of the current module. Variants of a module are always visited in - // order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from the - // Module returned by PrimaryModule without data races. This can be used to perform singleton actions that are - // only done once for all variants of a module. - PrimaryModule() Module - - // FinalModule returns the last variant of the current module. Variants of a module are always visited in - // order by mutators and GenerateBuildActions, so the data created by the current mutator can be read from all - // variants using VisitAllModuleVariants if the current module == FinalModule(). This can be used to perform - // singleton actions that are only done once for all variants of a module. - FinalModule() Module - - // VisitAllModuleVariants calls visit for each variant of the current module. Variants of a module are always - // visited in order by mutators and GenerateBuildActions, so the data created by the current mutator can be read - // from all variants if the current module == FinalModule(). Otherwise, care must be taken to not access any - // data modified by the current mutator. - VisitAllModuleVariants(visit func(Module)) - // GetMissingDependencies returns the list of dependencies that were passed to AddDependencies or related methods, // but do not exist. It can be used with Context.SetAllowMissingDependencies to allow the primary builder to // handle missing dependencies on its own instead of having Blueprint treat them as an error. @@ -642,6 +642,18 @@ func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) { m.visitingDep = depInfo{} } +func (m *baseModuleContext) PrimaryModule() Module { + return m.module.group.modules[0].logicModule +} + +func (m *baseModuleContext) FinalModule() Module { + return m.module.group.modules[len(m.module.group.modules)-1].logicModule +} + +func (m *baseModuleContext) VisitAllModuleVariants(visit func(Module)) { + m.context.visitAllModuleVariants(m.module, visit) +} + func (m *baseModuleContext) AddNinjaFileDeps(deps ...string) { m.ninjaFileDeps = append(m.ninjaFileDeps, deps...) } @@ -695,18 +707,6 @@ func (m *moduleContext) Build(pctx PackageContext, params BuildParams) { m.actionDefs.buildDefs = append(m.actionDefs.buildDefs, def) } -func (m *moduleContext) PrimaryModule() Module { - return m.module.group.modules[0].logicModule -} - -func (m *moduleContext) FinalModule() Module { - return m.module.group.modules[len(m.module.group.modules)-1].logicModule -} - -func (m *moduleContext) VisitAllModuleVariants(visit func(Module)) { - m.context.visitAllModuleVariants(m.module, visit) -} - func (m *moduleContext) GetMissingDependencies() []string { m.handledMissingDeps = true return m.module.missingDeps