From 856507f4d21767d5c6dec66d86e8e88f2b66a40d Mon Sep 17 00:00:00 2001 From: MarkDacek Date: Fri, 5 Aug 2022 22:28:37 +0000 Subject: [PATCH] Add multiple property and replace functionality to bpmodify. Test: go run bpmodify.go -w -m=libcore-memory-metrics-tests -property=something,static_libs,deps,required,test_suites -replace-property=ahat:ahat_lib,general-tests:something -s ~/aosp-master-with-phones/libcore/metrictests/memory/host/Android.bp go test -v Change-Id: I005b6dd675beb205f544e89c729fe9191e6470c2 --- bpmodify/bpmodify.go | 227 +++++++++++++++++++++++--------------- bpmodify/bpmodify_test.go | 130 ++++++++++++++++++---- parser/modify.go | 18 +++ 3 files changed, 262 insertions(+), 113 deletions(-) diff --git a/bpmodify/bpmodify.go b/bpmodify/bpmodify.go index 9c88184..0ab9dde 100644 --- a/bpmodify/bpmodify.go +++ b/bpmodify/bpmodify.go @@ -2,13 +2,13 @@ // Copyright 2009 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. - package main import ( "bytes" "flag" "fmt" + "github.com/google/blueprint/parser" "io" "io/ioutil" "os" @@ -17,36 +17,36 @@ import ( "strings" "syscall" "unicode" - - "github.com/google/blueprint/parser" ) 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") - targetedModules = new(identSet) - targetedProperty = new(qualifiedProperty) - addIdents = new(identSet) - removeIdents = new(identSet) - removeProperty = flag.Bool("remove-property", false, "remove the property") - moveProperty = flag.Bool("move-property", false, "moves contents of property into new-location") - newLocation string - setString *string - addLiteral *string + 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) + targetedProperties = new(qualifiedProperties) + addIdents = new(identSet) + removeIdents = new(identSet) + removeProperty = flag.Bool("remove-property", false, "remove the property") + moveProperty = flag.Bool("move-property", false, "moves contents of property into newLocation") + newLocation string + setString *string + addLiteral *string + replaceProperty = new(replacements) ) 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(targetedProperties, "parameter", "alias to -property=`name1[,name2[,... […]") flag.StringVar(&newLocation, "new-location", "", " use with moveProperty to move contents of -property into a property with name -new-location ") + flag.Var(targetedProperties, "property", "comma-separated list of fully qualified `name`s of properties to modify (default \"deps\")") flag.Var(addIdents, "a", "comma or whitespace separated list of identifiers to add") flag.Var(stringPtrFlag{&addLiteral}, "add-literal", "a literal to add") flag.Var(removeIdents, "r", "comma or whitespace separated list of identifiers to remove") flag.Var(stringPtrFlag{&setString}, "str", "set a string property") + flag.Var(replaceProperty, "replace-property", "property names to be replaced, in the form of oldName1:newName1,oldName2:newName2") flag.Usage = usage } @@ -72,21 +72,16 @@ func processFile(filename string, in io.Reader, out io.Writer) error { return err } defer f.Close() - if *write { syscall.Flock(int(f.Fd()), syscall.LOCK_EX) } - in = f } - src, err := ioutil.ReadAll(in) if err != nil { return err } - r := bytes.NewBuffer(src) - file, errs := parser.Parse(filename, r, parser.NewScope(nil)) if len(errs) > 0 { for _, err := range errs { @@ -94,7 +89,6 @@ func processFile(filename string, in io.Reader, out io.Writer) error { } return fmt.Errorf("%d parsing errors", len(errs)) } - modified, errs := findModules(file) if len(errs) > 0 { for _, err := range errs { @@ -102,13 +96,11 @@ func processFile(filename string, in io.Reader, out io.Writer) error { } fmt.Fprintln(os.Stderr, "continuing...") } - if modified { res, err := parser.Print(file) if err != nil { return err } - if *list { fmt.Fprintln(out, filename) } @@ -126,23 +118,19 @@ func processFile(filename string, in io.Reader, out io.Writer) error { fmt.Printf("diff %s bpfmt/%s\n", filename, filename) out.Write(data) } - if !*list && !*write && !*doDiff { _, err = out.Write(res) } } - return err } - func findModules(file *parser.File) (modified bool, errs []error) { - for _, def := range file.Defs { if module, ok := def.(*parser.Module); ok { for _, prop := range module.Properties { - if prop.Name == "name" && prop.Value.Type() == parser.StringType { - if targetedModule(prop.Value.Eval().(*parser.String).Value) { - m, newErrs := processModule(module, prop.Name, file) + if prop.Name == "name" && prop.Value.Type() == parser.StringType && targetedModule(prop.Value.Eval().(*parser.String).Value) { + for _, p := range targetedProperties.properties { + m, newErrs := processModuleProperty(module, prop.Name, file, p) errs = append(errs, newErrs...) modified = modified || m } @@ -150,23 +138,22 @@ func findModules(file *parser.File) (modified bool, errs []error) { } } } - return modified, errs } -func processModule(module *parser.Module, moduleName string, - file *parser.File) (modified bool, errs []error) { - prop, parent, err := getRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes()) +func processModuleProperty(module *parser.Module, moduleName string, + file *parser.File, property qualifiedProperty) (modified bool, errs []error) { + prop, parent, err := getRecursiveProperty(module, property.name(), property.prefixes()) if err != nil { return false, []error{err} } if prop == nil { if len(addIdents.idents) > 0 || addLiteral != nil { // We are adding something to a non-existing list prop, so we need to create it first. - prop, modified, err = createRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes(), &parser.List{}) + prop, modified, err = createRecursiveProperty(module, property.name(), property.prefixes(), &parser.List{}) } else if setString != nil { // We setting a non-existent string property, so we need to create it first. - prop, modified, err = createRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes(), &parser.String{}) + prop, modified, err = createRecursiveProperty(module, property.name(), property.prefixes(), &parser.String{}) } else { // 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, @@ -183,22 +170,19 @@ func processModule(module *parser.Module, moduleName string, } else if *moveProperty { return parent.MovePropertyContents(prop.Name, newLocation), nil } - m, errs := processParameter(prop.Value, targetedProperty.String(), moduleName, file) + m, errs := processParameter(prop.Value, property.String(), moduleName, file) modified = modified || m return modified, errs } - func getRecursiveProperty(module *parser.Module, name string, prefixes []string) (prop *parser.Property, parent *parser.Map, err error) { prop, parent, _, err = getOrCreateRecursiveProperty(module, name, prefixes, nil) return prop, parent, err } - func createRecursiveProperty(module *parser.Module, name string, prefixes []string, empty parser.Expression) (prop *parser.Property, modified bool, err error) { prop, _, modified, err = getOrCreateRecursiveProperty(module, name, prefixes, empty) return prop, modified, err } - func getOrCreateRecursiveProperty(module *parser.Module, name string, prefixes []string, empty parser.Expression) (prop *parser.Property, parent *parser.Map, modified bool, err error) { m := &module.Map @@ -234,38 +218,40 @@ func getOrCreateRecursiveProperty(module *parser.Module, name string, prefixes [ return nil, nil, false, nil } } - func processParameter(value parser.Expression, paramName, moduleName string, file *parser.File) (modified bool, errs []error) { if _, ok := value.(*parser.Variable); ok { return false, []error{fmt.Errorf("parameter %s in module %s is a variable, unsupported", paramName, moduleName)} } - if _, ok := value.(*parser.Operator); ok { return false, []error{fmt.Errorf("parameter %s in module %s is an expression, unsupported", paramName, moduleName)} } + if (*replaceProperty).size() != 0 { + list, ok := value.Eval().(*parser.List) + if !ok { + return false, []error{fmt.Errorf("expected parameter %s in module %s to be a list, found %s", + paramName, moduleName, value.Type().String())} + } + return parser.ReplaceStringsInList(list, (*replaceProperty).oldNameToNewName), nil + } if len(addIdents.idents) > 0 || len(removeIdents.idents) > 0 { list, ok := value.(*parser.List) if !ok { return false, []error{fmt.Errorf("expected parameter %s in module %s to be list, found %s", - paramName, moduleName, value.Type().String())} + paramName, moduleName, value.Type())} } - wasSorted := parser.ListIsSorted(list) - for _, a := range addIdents.idents { m := parser.AddStringToList(list, a) modified = modified || m } - for _, r := range removeIdents.idents { m := parser.RemoveStringFromList(list, r) modified = modified || m } - if (wasSorted || *sortLists) && modified { parser.SortList(file, list) } @@ -290,14 +276,11 @@ func processParameter(value parser.Expression, paramName, moduleName string, return false, []error{fmt.Errorf("expected parameter %s in module %s to be string, found %s", paramName, moduleName, value.Type().String())} } - str.Value = *setString modified = true } - return modified, nil } - func targetedModule(name string) bool { if targetedModules.all { return true @@ -307,10 +290,8 @@ func targetedModule(name string) bool { return true } } - return false } - func visitFile(path string, f os.FileInfo, err error) error { if err == nil && f.Name() == "Blueprints" { err = processFile(path, nil, os.Stdout) @@ -320,11 +301,9 @@ func visitFile(path string, f os.FileInfo, err error) error { } return nil } - func walkDir(path string) { filepath.Walk(path, visitFile) } - func main() { defer func() { if err := recover(); err != nil { @@ -332,17 +311,16 @@ func main() { } os.Exit(exitCode) }() - flag.Parse() - if len(targetedProperty.parts) == 0 && *moveProperty { + + if len(targetedProperties.properties) == 0 && *moveProperty { report(fmt.Errorf("-move-property must specify property")) return } - if len(targetedProperty.parts) == 0 { - targetedProperty.Set("deps") + if len(targetedProperties.properties) == 0 { + targetedProperties.Set("deps") } - if flag.NArg() == 0 { if *write { report(fmt.Errorf("error: cannot use -w with standard input")) @@ -353,32 +331,27 @@ func main() { } return } - if len(targetedModules.idents) == 0 { report(fmt.Errorf("-m parameter is required")) return } - if len(addIdents.idents) == 0 && len(removeIdents.idents) == 0 && setString == nil && addLiteral == nil && !*removeProperty && !*moveProperty { - report(fmt.Errorf("-a, -add-literal, -r, -remove-property, -move-property, or -str parameter is required")) + if len(addIdents.idents) == 0 && len(removeIdents.idents) == 0 && setString == nil && addLiteral == nil && !*removeProperty && !*moveProperty && (*replaceProperty).size() == 0 { + report(fmt.Errorf("-a, -add-literal, -r, -remove-property, -move-property, replace-property or -str parameter is required")) return } - - if *removeProperty && (len(addIdents.idents) > 0 || len(removeIdents.idents) > 0 || setString != nil || addLiteral != nil) { + if *removeProperty && (len(addIdents.idents) > 0 || len(removeIdents.idents) > 0 || setString != nil || addLiteral != nil || (*replaceProperty).size() > 0) { report(fmt.Errorf("-remove-property cannot be used with other parameter(s)")) return } - - if *moveProperty && (len(addIdents.idents) > 0 || len(removeIdents.idents) > 0 || setString != nil || addLiteral != nil) { + if *moveProperty && (len(addIdents.idents) > 0 || len(removeIdents.idents) > 0 || setString != nil || addLiteral != nil || (*replaceProperty).size() > 0) { report(fmt.Errorf("-move-property cannot be used with other parameter(s)")) return } - if *moveProperty && newLocation == "" { report(fmt.Errorf("-move-property must specify -new-location")) return } - for i := 0; i < flag.NArg(); i++ { path := flag.Arg(i) switch dir, err := os.Stat(path); { @@ -393,7 +366,6 @@ func main() { } } } - func diff(b1, b2 []byte) (data []byte, err error) { f1, err := ioutil.TempFile("", "bpfmt") if err != nil { @@ -401,17 +373,14 @@ func diff(b1, b2 []byte) (data []byte, err error) { } defer os.Remove(f1.Name()) defer f1.Close() - f2, err := ioutil.TempFile("", "bpfmt") if err != nil { return } defer os.Remove(f2.Name()) defer f2.Close() - f1.Write(b1) f2.Write(b2) - data, err = exec.Command("diff", "-uw", f1.Name(), f2.Name()).CombinedOutput() if len(data) > 0 { // diff exits with a non-zero status when the files don't match. @@ -419,7 +388,6 @@ func diff(b1, b2 []byte) (data []byte, err error) { err = nil } return - } type stringPtrFlag struct { @@ -430,7 +398,6 @@ func (f stringPtrFlag) Set(s string) error { *f.s = &s return nil } - func (f stringPtrFlag) String() string { if f.s == nil || *f.s == nil { return "" @@ -438,6 +405,59 @@ func (f stringPtrFlag) String() string { return **f.s } +type replacements struct { + oldNameToNewName map[string]string +} + +func (m *replacements) String() string { + ret := "" + sep := "" + for k, v := range m.oldNameToNewName { + ret += sep + ret += k + ret += ":" + ret += v + sep = "," + } + return ret +} + +func (m *replacements) Set(s string) error { + usedNames := make(map[string]struct{}) + + pairs := strings.Split(s, ",") + length := len(pairs) + m.oldNameToNewName = make(map[string]string) + for i := 0; i < length; i++ { + + pair := strings.SplitN(pairs[i], ":", 2) + if len(pair) != 2 { + return fmt.Errorf("Invalid replacement pair %s", pairs[i]) + } + oldName := pair[0] + newName := pair[1] + if _, seen := usedNames[oldName]; seen { + return fmt.Errorf("Duplicated replacement name %s", oldName) + } + if _, seen := usedNames[newName]; seen { + return fmt.Errorf("Duplicated replacement name %s", newName) + } + usedNames[oldName] = struct{}{} + usedNames[newName] = struct{}{} + m.oldNameToNewName[oldName] = newName + } + return nil +} + +func (m *replacements) Get() interface{} { + //TODO(dacek): Remove Get() method from interface as it seems unused. + return m.oldNameToNewName +} + +func (m *replacements) size() (length int) { + return len(m.oldNameToNewName) +} + type identSet struct { idents []string all bool @@ -446,7 +466,6 @@ type identSet struct { func (m *identSet) String() string { return strings.Join(m.idents, ",") } - func (m *identSet) Set(s string) error { m.idents = strings.FieldsFunc(s, func(c rune) bool { return unicode.IsSpace(c) || c == ',' @@ -456,42 +475,70 @@ func (m *identSet) Set(s string) error { } return nil } - func (m *identSet) Get() interface{} { return m.idents } +type qualifiedProperties struct { + properties []qualifiedProperty +} + type qualifiedProperty struct { parts []string } -var _ flag.Getter = (*qualifiedProperty)(nil) +var _ flag.Getter = (*qualifiedProperties)(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 { +func parseQualifiedProperty(s string) (*qualifiedProperty, error) { + parts := strings.Split(s, ".") + if len(parts) == 0 { + return nil, fmt.Errorf("%q is not a valid property name", s) + } + for _, part := range parts { + if part == "" { + return nil, fmt.Errorf("%q is not a valid property name", s) + } + } + prop := qualifiedProperty{parts} + return &prop, nil + +} + +func (p *qualifiedProperties) Set(s string) error { + properties := strings.Split(s, ",") + if len(properties) == 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) + + p.properties = make([]qualifiedProperty, len(properties)) + for i := 0; i < len(properties); i++ { + tmp, err := parseQualifiedProperty(properties[i]) + if err != nil { + return err } + p.properties[i] = *tmp } return nil } -func (p *qualifiedProperty) Get() interface{} { - return p.parts +func (p *qualifiedProperties) String() string { + arrayLength := len(p.properties) + props := make([]string, arrayLength) + for i := 0; i < len(p.properties); i++ { + props[i] = p.properties[i].String() + } + return strings.Join(props, ",") +} +func (p *qualifiedProperties) Get() interface{} { + return p.properties } diff --git a/bpmodify/bpmodify_test.go b/bpmodify/bpmodify_test.go index 4340edb..3924358 100644 --- a/bpmodify/bpmodify_test.go +++ b/bpmodify/bpmodify_test.go @@ -4,34 +4,33 @@ // 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 +// 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" "github.com/google/blueprint/proptools" + "strings" + "testing" ) var testCases = []struct { - name string - input string - output string - property string - addSet string - removeSet string - addLiteral *string - setString *string - removeProperty bool + name string + input string + output string + property string + addSet string + removeSet string + addLiteral *string + setString *string + removeProperty bool + replaceProperty string }{ { name: "add", @@ -354,6 +353,48 @@ var testCases = []struct { `, property: "bar", removeProperty: true, + }, { + name: "replace property", + property: "deps", + input: ` + cc_foo { + name: "foo", + deps: ["baz", "unchanged"], + } + `, + output: ` + cc_foo { + name: "foo", + deps: [ + "baz_lib", + "unchanged", + ], + } + `, + replaceProperty: "baz:baz_lib,foobar:foobar_lib", + }, { + name: "replace property multiple modules", + property: "deps,required", + input: ` + cc_foo { + name: "foo", + deps: ["baz", "unchanged"], + unchanged: ["baz"], + required: ["foobar"], + } + `, + output: ` + cc_foo { + name: "foo", + deps: [ + "baz_lib", + "unchanged", + ], + unchanged: ["baz"], + required: ["foobar_lib"], + } + `, + replaceProperty: "baz:baz_lib,foobar:foobar_lib", }, } @@ -364,16 +405,16 @@ func simplifyModuleDefinition(def string) string { } return result } - func TestProcessModule(t *testing.T) { for i, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - targetedProperty.Set(testCase.property) + targetedProperties.Set(testCase.property) addIdents.Set(testCase.addSet) removeIdents.Set(testCase.removeSet) removeProperty = &testCase.removeProperty setString = testCase.setString addLiteral = testCase.addLiteral + replaceProperty.Set(testCase.replaceProperty) inAst, errs := parser.ParseAndEval("", strings.NewReader(testCase.input), parser.NewScope(nil)) if len(errs) > 0 { @@ -384,16 +425,18 @@ func TestProcessModule(t *testing.T) { t.Errorf("%+v", testCase) t.FailNow() } - if inModule, ok := inAst.Defs[0].(*parser.Module); !ok { t.Fatalf(" input must only contain a single module definition: %s", testCase.input) } else { - _, errs := processModule(inModule, "", inAst) - if len(errs) > 0 { - t.Errorf("test case %d:", i) - for _, err := range errs { - t.Errorf(" %s", err) + for _, p := range targetedProperties.properties { + _, errs := processModuleProperty(inModule, "", inAst, p) + 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) @@ -407,5 +450,46 @@ func TestProcessModule(t *testing.T) { } }) } - +} + +func TestReplacementsCycleError(t *testing.T) { + cycleString := "old1:new1,new1:old1" + err := replaceProperty.Set(cycleString) + + if err.Error() != "Duplicated replacement name new1" { + t.Errorf("Error message did not match") + t.Errorf("Expected ") + t.Errorf(" Duplicated replacement name new1") + t.Errorf("actual error:") + t.Errorf(" %s", err.Error()) + t.FailNow() + } +} + +func TestReplacementsDuplicatedError(t *testing.T) { + cycleString := "a:b,a:c" + err := replaceProperty.Set(cycleString) + + if err.Error() != "Duplicated replacement name a" { + t.Errorf("Error message did not match") + t.Errorf("Expected ") + t.Errorf(" Duplicated replacement name a") + t.Errorf("actual error:") + t.Errorf(" %s", err.Error()) + t.FailNow() + } +} + +func TestReplacementsMultipleReplacedToSame(t *testing.T) { + cycleString := "a:c,d:c" + err := replaceProperty.Set(cycleString) + + if err.Error() != "Duplicated replacement name c" { + t.Errorf("Error message did not match") + t.Errorf("Expected ") + t.Errorf(" Duplicated replacement name c") + t.Errorf("actual error:") + t.Errorf(" %s", err.Error()) + t.FailNow() + } } diff --git a/parser/modify.go b/parser/modify.go index 3051f66..a28fbe6 100644 --- a/parser/modify.go +++ b/parser/modify.go @@ -56,6 +56,24 @@ func RemoveStringFromList(list *List, s string) (modified bool) { return false } +func ReplaceStringsInList(list *List, replacements map[string]string) (replaced bool) { + modified := false + for i, v := range list.Values { + if v.Type() != StringType { + panic(fmt.Errorf("expected string in list, got %s", v.Type())) + } + if sv, ok := v.(*String); ok && replacements[sv.Value] != "" { + pos := list.Values[i].Pos() + list.Values[i] = &String{ + LiteralPos: pos, + Value: replacements[sv.Value], + } + modified = true + } + } + return modified +} + // A Patch represents a region of a text buffer to be replaced [Start, End) and its Replacement type Patch struct { Start, End int