Adding function to execute Buildozer commands on a single in-memory file (#1359)
A buildozer client are looking to execute the commands of buildozer on the content of a build file, in a service environment (without direct access to the file storage). The new function enables this by running the commands without accessing the file system.
Co-authored-by: Tim Malmström <oreflow@google.com>
diff --git a/buildozer/README.md b/buildozer/README.md
index 682108d..9d259e4 100644
--- a/buildozer/README.md
+++ b/buildozer/README.md
@@ -352,6 +352,25 @@
add deps //base //strings|-:foo|-:bar
```
+## Using Buildozer in-memory
+
+Some clients of Buildozer have the need to execute buildozer actions in memory
+(due to a service environment which does not have access to their file system).
+This can be done using, `edit.ExecuteCommandsOnInlineFile`, which accepts
+commands and BUILD file content as bytes, applies the changes and returns the
+raw file content.
+For more details and implementation, see [`/edit/buildozer.go`](../edit/buildozer.go)
+
+Some caveats of running Buildozer in-memory:
+
+* The function assumes (and validates to some extent) that all commands apply to
+ the same file and will return errors if there are commands affecting different
+ paths.
+* When referencing targets, the function will not reliably determine if targets
+ are local or remote. Hence redundant path references may be included in
+ output. (e.g. `add dep //package/path:bar|//package/path:foo` would add the dep
+ `//package/path:bar` instead of just `:bar`).
+
## Error code
The return code is:
diff --git a/edit/buildozer.go b/edit/buildozer.go
index c839204..9227878 100644
--- a/edit/buildozer.go
+++ b/edit/buildozer.go
@@ -1216,11 +1216,9 @@
}
var errs []error
changed := false
- for _, commands := range commandsForFile.commands {
- target := commands.target
- commands := commands.commands
- _, _, absPkg, rule := InterpretLabelForWorkspaceLocation(opts.RootDir, target)
- if label := labels.Parse(target); label.Package == stdinPackageName {
+ for _, cft := range commandsForFile.commands {
+ _, _, absPkg, rule := InterpretLabelForWorkspaceLocation(opts.RootDir, cft.target)
+ if label := labels.Parse(cft.target); label.Package == stdinPackageName {
// Special-case: This is already absolute
absPkg = stdinPackageName
}
@@ -1231,48 +1229,27 @@
targets, err := expandTargets(f, rule)
if err != nil {
- cerr := commandError(commands, target, err)
+ cerr := commandError(cft.commands, cft.target, err)
errs = append(errs, cerr)
if !opts.KeepGoing {
return &rewriteResult{file: name, errs: errs, records: records}
}
}
targets = filterRules(opts, targets)
- for _, cmd := range commands {
- cmdInfo := AllCommands[cmd.tokens[0]]
- // Depending on whether a transformation is rule-specific or not, it should be applied to
- // every rule that satisfies the filter or just once to the file.
- cmdTargets := targets
- if !cmdInfo.PerRule {
- cmdTargets = []*build.Rule{nil}
- }
- for _, r := range cmdTargets {
- record := &apipb.Output_Record{}
- newf, err := cmdInfo.Fn(opts, CmdEnvironment{f, r, vars, absPkg, cmd.tokens[1:], record})
- if len(record.Fields) != 0 {
- records = append(records, record)
- }
- if err != nil {
- cerr := commandError([]command{cmd}, target, err)
- if opts.KeepGoing {
- errs = append(errs, cerr)
- } else {
- return &rewriteResult{file: name, errs: []error{cerr}, records: records}
- }
- }
- if newf != nil {
- changed = true
- f = newf
- }
- }
+
+ newf, err := executeCommandsInFile(opts, f, cft, targets, &records, vars, absPkg, &errs)
+ if err != nil {
+ return &rewriteResult{file: name, errs: []error{err}, records: records}
+ }
+ if newf != nil {
+ changed = true
+ f = newf
}
}
if !changed {
return &rewriteResult{file: name, errs: errs, records: records}
}
- f = RemoveEmptyPackage(f)
- f = RemoveEmptyUseRepoCalls(f)
- ndata, err := buildifier.Buildify(opts, f)
+ ndata, err := cleanAndBuildify(opts, f)
if err != nil {
return &rewriteResult{file: name, errs: []error{fmt.Errorf("running buildifier: %v", err)}, records: records}
}
@@ -1297,6 +1274,58 @@
return &rewriteResult{file: name, errs: errs, modified: true, records: records}
}
+// executeCommandsInFile executes the provided commandsForTarget in the provided build.File.
+func executeCommandsInFile(
+ opts *Options,
+ f *build.File,
+ cft commandsForTarget,
+ rules []*build.Rule,
+ records *[]*apipb.Output_Record,
+ vars map[string]*build.AssignExpr,
+ absPkg string,
+ errs *[]error,
+) (*build.File, error) {
+ changed := false
+ for _, cmd := range cft.commands {
+ cmdInfo := AllCommands[cmd.tokens[0]]
+ // Depending on whether a transformation is rule-specific or not, it should be applied to
+ // every rule that satisfies the filter or just once to the file.
+ cmdTargets := rules
+ if !cmdInfo.PerRule {
+ cmdTargets = []*build.Rule{nil}
+ }
+ for _, r := range cmdTargets {
+ record := &apipb.Output_Record{}
+ newf, err := cmdInfo.Fn(opts, CmdEnvironment{f, r, vars, absPkg, cmd.tokens[1:], record})
+ if len(record.Fields) != 0 {
+ *records = append(*records, record)
+ }
+ if err != nil {
+ cerr := commandError([]command{cmd}, cft.target, err)
+ if opts.KeepGoing {
+ *errs = append(*errs, cerr)
+ } else {
+ return nil, cerr
+ }
+ }
+ if newf != nil {
+ f = newf
+ changed = true
+ }
+ }
+ }
+ if changed {
+ return f, nil
+ }
+ return nil, nil
+}
+
+func cleanAndBuildify(opts *Options, f *build.File) ([]byte, error) {
+ f = RemoveEmptyPackage(f)
+ f = RemoveEmptyUseRepoCalls(f)
+ return buildifier.Buildify(opts, f)
+}
+
// EditFile is a function that does any prework needed before editing a file.
// e.g. "checking out for write" from a locking source control repo.
var EditFile = func(fi os.FileInfo, name string) error {
@@ -1633,3 +1662,82 @@
}
return 0
}
+
+// ExecuteCommandsOnInlineFile executes the given commands on the given file content.
+// Returns the new file content after applying the commands.
+func ExecuteCommandsOnInlineFile(fileContent []byte, commands []string) ([]byte, error) {
+ opts := Options{}
+ commandsByTargetName, filename, err := groupCommandsForInlineFile(commands, opts)
+ if err != nil {
+ return nil, err
+ }
+ f, err := build.Parse(*filename, fileContent)
+ if err != nil {
+ return nil, err
+ }
+ if f.Type == build.TypeDefault {
+ // Buildozer is unable to infer the file type, fall back to BUILD by default.
+ f.Type = build.TypeBuild
+ }
+ for _, cft := range commandsByTargetName {
+ rules, err := expandTargets(f, cft.target)
+ if err != nil {
+ return nil, err
+ }
+ newf, err := executeCommandsInFile(
+ &opts,
+ f,
+ cft,
+ rules,
+ // Output records are ignored in inline file execution.
+ nil,
+ // Global variables not supported in inline file execution.
+ nil,
+ f.Pkg,
+ // Errors-list is ignored since opts.keepGoing is always false.
+ nil,
+ )
+ if err != nil {
+ return nil, err
+ }
+ if newf != nil {
+ f = newf
+ }
+ }
+ outputFileContent, err := cleanAndBuildify(&opts, f)
+ if err != nil {
+ return nil, err
+ }
+ return outputFileContent, nil
+}
+
+// groupCommandsForInlineFile groups the given commands by file and returns the commands for a
+// single file. Returns an error if the commands modify multiple files or if commands are invalid.
+func groupCommandsForInlineFile(commands []string, opts Options) ([]commandsForTarget, *string, error) {
+ commandsByFile := make(map[string][]commandsForTarget)
+ commandReader := strings.NewReader(strings.Join(commands, "\n"))
+ err := appendCommandsFromReader(&opts, commandReader, commandsByFile, nil)
+ if err != nil {
+ return nil, nil, fmt.Errorf("error parsing commands %s", err)
+ }
+ if len(commandsByFile) != 1 {
+ return nil, nil, fmt.Errorf("invalid input commands, expected all commands to reference a single file")
+ }
+ for filepath, commandsForTargets := range commandsByFile {
+ for i := range commandsForTargets {
+ cft := &commandsForTargets[i]
+ splitTarget := strings.Split(cft.target, ":")
+ switch len(splitTarget) {
+ case 1: // No-op
+ case 2:
+ // Only keeps target (what is after the ":" character) since path is redundant.
+ cft.target = splitTarget[1]
+ default:
+ return nil, nil, fmt.Errorf("invalid target name %q", cft.target)
+ }
+ }
+ _, filename := path.Split(filepath)
+ return commandsForTargets, &filename, nil
+ }
+ panic("unreachable")
+}
diff --git a/edit/buildozer_test.go b/edit/buildozer_test.go
index 34689b8..d6c81ee 100644
--- a/edit/buildozer_test.go
+++ b/edit/buildozer_test.go
@@ -17,6 +17,7 @@
package edit
import (
+ "fmt"
"os"
"path/filepath"
"reflect"
@@ -813,6 +814,170 @@
}
}
+func TestExecuteCommandsOnInlineFile(t *testing.T) {
+ tests := []struct {
+ name string
+ fileContent []byte
+ commands []string
+ wantOutput []byte
+ }{
+ {
+ name: "creating_new_target_and_adding_deps",
+ fileContent: nil,
+ commands: []string{
+ "new java_library foo|//package/path/BUILD",
+ "add deps :bar|//package/path:foo",
+ },
+ wantOutput: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "foo",`,
+ ` deps = [":bar"],`,
+ `)`,
+ ``}, "\n")),
+ },
+ {
+ name: "adding_deps_to_existing_targets",
+ fileContent: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "foo",`,
+ `)`,
+ ``,
+ `java_library(`,
+ ` name = "fruits",`,
+ ` deps = ["//package/fruits:apples"],`,
+ `)`,
+ ``}, "\n")),
+ commands: []string{
+ "add deps :bar|//package/path:foo",
+ "add deps //package/fruits:oranges|//package/path:fruits",
+ },
+ wantOutput: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "foo",`,
+ ` deps = [":bar"],`,
+ `)`,
+ ``,
+ `java_library(`,
+ ` name = "fruits",`,
+ ` deps = [`,
+ ` "//package/fruits:apples",`,
+ ` "//package/fruits:oranges",`,
+ ` ],`,
+ `)`,
+ ``}, "\n")),
+ },
+ {
+ name: "substituting_a_target",
+ fileContent: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "fruits",`,
+ ` deps = ["//package/fruits:apples"],`,
+ `)`,
+ ``}, "\n")),
+ commands: []string{
+ "replace deps //package/fruits:apples //package/fruits:oranges|//whatever/package/path:fruits",
+ },
+ wantOutput: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "fruits",`,
+ ` deps = ["//package/fruits:oranges"],`,
+ `)`,
+ ``}, "\n")),
+ },
+ {
+ name: "no_changes_does_not_return_any_diff",
+ fileContent: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "foo",`,
+ ` deps = [":bar"],`,
+ `)`,
+ ``}, "\n")),
+ commands: []string{
+ "add deps :bar |//whatever/package/path:foo",
+ },
+ wantOutput: []byte(strings.Join([]string{
+ `java_library(`,
+ ` name = "foo",`,
+ ` deps = [":bar"],`,
+ `)`,
+ ``}, "\n")),
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ output, err := ExecuteCommandsOnInlineFile(tc.fileContent, tc.commands)
+ if err != nil {
+ t.Fatalf("Error, got error %v", err)
+ }
+
+ if diff := cmp.Diff(tc.wantOutput, output); diff != "" {
+ t.Errorf("%s: (-want +got): %s", tc.name, diff)
+ }
+ })
+ }
+}
+
+func TestTestExecuteCommandsOnInlineFileFailed(t *testing.T) {
+ tests := []struct {
+ name string
+ fileContent []byte
+ commands []string
+ wantErr error
+ }{
+ {
+ name: "target_does_not_exist",
+ commands: []string{
+ "add deps :foo|//package/path:bar",
+ },
+ wantErr: fmt.Errorf("rule 'bar' not found"),
+ },
+ {
+ name: "invalid_input",
+ commands: []string{
+ "completely invalid command",
+ },
+ wantErr: fmt.Errorf("rule 'completely invalid command' not found"),
+ },
+ {
+ name: "missing_implementation",
+ commands: []string{
+ "extrapolate packages :foo|//package/path:bar",
+ },
+ wantErr: fmt.Errorf("invalid input commands, expected all commands to reference a single file"),
+ },
+ {
+ name: "commands_for_multiple_files",
+ commands: []string{
+ "add deps :foo|//package/path:bar",
+ "add deps :foo|//package2/path:bar",
+ },
+ wantErr: fmt.Errorf("invalid input commands, expected all commands to reference a single file"),
+ },
+ {
+ name: "command_with_unexpected_target_semicolons",
+ commands: []string{
+ "add deps :foo|//package:path:bar",
+ },
+ wantErr: fmt.Errorf("invalid target name \"//package:path:bar\""),
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ output, gotErr := ExecuteCommandsOnInlineFile(tc.fileContent, tc.commands)
+
+ if output != nil {
+ t.Fatalf("Error, got response for invalid input %v, expected error", output)
+ }
+
+ if diff := cmp.Diff(tc.wantErr.Error(), gotErr.Error()); diff != "" {
+ t.Errorf("%s: (-want +got): %s", tc.name, diff)
+ }
+ })
+ }
+}
+
func TestGetIgnoredPrefixes(t *testing.T) {
tmp, err := os.MkdirTemp("", "")
if err != nil {
