diff --git a/CHANGELOG.md b/CHANGELOG.md index e441325e32..fff5e79144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,14 @@ ## [Unreleased] -- No changes yet. +- Add `buf generate --clean` flag that will delete the directories, jar files, or zip files that the + plugins will write to, prior to generation. Allows cleaning of existing assets without having + to call `rm -rf`. ## [v1.34.0] - 2024-06-21 - Add `buf config ls-modules` command to list configured modules. -- Fix issue where `buf generate` would succeed on missing insertion points and +- Fix issue where `buf generate` would succeed on missing insertion points and panic on empty insertion point files. - Update `buf generate` to allow the use of Editions syntax when doing local code generation by proxying to a `protoc` binary (for languages where code gen is diff --git a/private/buf/bufgen/bufgen.go b/private/buf/bufgen/bufgen.go index 40a1568a95..a9d3262176 100644 --- a/private/buf/bufgen/bufgen.go +++ b/private/buf/bufgen/bufgen.go @@ -118,6 +118,14 @@ func GenerateWithBaseOutDirPath(baseOutDirPath string) GenerateOption { } } +// GenerateWithDeleteOuts returns a new GenerateOption that results in the +// output directories, zip files, or jar files being deleted before generation is run. +func GenerateWithDeleteOuts() GenerateOption { + return func(generateOptions *generateOptions) { + generateOptions.deleteOuts = true + } +} + // GenerateWithIncludeImportsOverride is a strict override on whether imports are // generated. This overrides IncludeImports from the GeneratePluginConfig. // diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index af4acc532d..86273d7921 100644 --- a/private/buf/bufgen/generator.go +++ b/private/buf/bufgen/generator.go @@ -34,6 +34,7 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/connectclient" + "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/thread" "github.com/bufbuild/buf/private/pkg/tracing" @@ -103,6 +104,17 @@ func (g *generator) Generate( if err := bufimagemodify.Modify(image, config.GenerateManagedConfig()); err != nil { return err } + } + if generateOptions.deleteOuts { + if err := g.deleteOuts( + ctx, + generateOptions.baseOutDirPath, + config.GeneratePluginConfigs(), + ); err != nil { + return err + } + } + for _, image := range images { if err := g.generateCode( ctx, container, @@ -118,6 +130,26 @@ func (g *generator) Generate( return nil } +func (g *generator) deleteOuts( + ctx context.Context, + baseOutDir string, + pluginConfigs []bufconfig.GeneratePluginConfig, +) error { + return bufprotopluginos.NewCleaner(g.storageosProvider).DeleteOuts( + ctx, + slicesext.Map( + pluginConfigs, + func(pluginConfig bufconfig.GeneratePluginConfig) string { + out := pluginConfig.Out() + if baseOutDir != "" && baseOutDir != "." { + return filepath.Join(baseOutDir, out) + } + return out + }, + ), + ) +} + func (g *generator) generateCode( ctx context.Context, container app.EnvStdioContainer, @@ -451,8 +483,8 @@ func validateResponses( } type generateOptions struct { - // plugin specific options: baseOutDirPath string + deleteOuts bool includeImportsOverride *bool includeWellKnownTypesOverride *bool } diff --git a/private/buf/cmd/buf/command/generate/generate.go b/private/buf/cmd/buf/command/generate/generate.go index 600619a17f..daa8297ead 100644 --- a/private/buf/cmd/buf/command/generate/generate.go +++ b/private/buf/cmd/buf/command/generate/generate.go @@ -42,6 +42,7 @@ const ( templateFlagName = "template" baseOutDirPathFlagName = "output" baseOutDirPathFlagShortName = "o" + deleteOutsFlagName = "clean" errorFormatFlagName = "error-format" configFlagName = "config" pathsFlagName = "path" @@ -372,6 +373,7 @@ Insertion points are processed in the order the plugins are specified in the tem type flags struct { Template string BaseOutDirPath string + DeleteOuts bool ErrorFormat string Files []string Config string @@ -425,6 +427,12 @@ func (f *flags) Bind(flagSet *pflag.FlagSet) { ".", `The base directory to generate to. This is prepended to the out directories in the generation template`, ) + flagSet.BoolVar( + &f.DeleteOuts, + deleteOutsFlagName, + false, + `Prior to generation, delete the directories, jar files, or zip files that the plugins will write to. Allows cleaning of existing assets without having to call rm -rf`, + ) flagSet.StringVar( &f.ErrorFormat, errorFormatFlagName, @@ -514,6 +522,12 @@ func run( generateOptions := []bufgen.GenerateOption{ bufgen.GenerateWithBaseOutDirPath(flags.BaseOutDirPath), } + if flags.DeleteOuts { + generateOptions = append( + generateOptions, + bufgen.GenerateWithDeleteOuts(), + ) + } if flags.IncludeImportsOverride != nil { generateOptions = append( generateOptions, diff --git a/private/buf/cmd/buf/command/generate/generate_test.go b/private/buf/cmd/buf/command/generate/generate_test.go index 44516fdce1..a8986d930c 100644 --- a/private/buf/cmd/buf/command/generate/generate_test.go +++ b/private/buf/cmd/buf/command/generate/generate_test.go @@ -20,8 +20,10 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "os" "path/filepath" + "strings" "testing" "github.com/bufbuild/buf/private/buf/buftesting" @@ -30,6 +32,8 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/normalpath" + "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storagearchive" "github.com/bufbuild/buf/private/pkg/storage/storagemem" @@ -544,6 +548,24 @@ func TestGenerateInsertionPointMixedPathsFail(t *testing.T) { testGenerateInsertionPointMixedPathsFailV2(t, wd, ".") } +func TestGenerateDeleteOutDir(t *testing.T) { + t.Parallel() + testGenerateDeleteOuts(t, "", "foo") + testGenerateDeleteOuts(t, "base", "foo") + testGenerateDeleteOuts(t, "", "foo", "bar") + testGenerateDeleteOuts(t, "", "foo", "bar", "foo") + testGenerateDeleteOuts(t, "base", "foo", "bar") + testGenerateDeleteOuts(t, "base", "foo", "bar", "foo") + testGenerateDeleteOuts(t, "", "foo.jar") + testGenerateDeleteOuts(t, "", "foo.zip") + testGenerateDeleteOuts(t, "", "foo/bar.jar") + testGenerateDeleteOuts(t, "", "foo/bar.zip") + testGenerateDeleteOuts(t, "base", "foo.jar") + testGenerateDeleteOuts(t, "base", "foo.zip") + testGenerateDeleteOuts(t, "base", "foo/bar.jar") + testGenerateDeleteOuts(t, "base", "foo/bar.zip") +} + func TestBoolPointerFlagTrue(t *testing.T) { t.Parallel() expected := true @@ -882,6 +904,100 @@ func testRunSuccess(t *testing.T, args ...string) { ) } +func testGenerateDeleteOuts( + t *testing.T, + baseOutDirPath string, + outputPaths ...string, +) { + // Just add more builtins to the plugins slice below if this goes off + require.True(t, len(outputPaths) < 4, "we want to have unique plugins to work with and this test is only set up for three plugins max right now") + fullOutputPaths := outputPaths + if baseOutDirPath != "" && baseOutDirPath != "." { + fullOutputPaths = slicesext.Map( + outputPaths, + func(outputPath string) string { + return normalpath.Join(baseOutDirPath, outputPath) + }, + ) + } + ctx := context.Background() + tmpDirPath := t.TempDir() + storageBucket, err := storageos.NewProvider().NewReadWriteBucket(tmpDirPath) + require.NoError(t, err) + for _, fullOutputPath := range fullOutputPaths { + switch normalpath.Ext(fullOutputPath) { + case ".jar", ".zip": + // Write a one-byte file to the location. We'll compare the size below as a simple test. + require.NoError( + t, + storage.PutPath( + ctx, + storageBucket, + fullOutputPath, + []byte(`1`), + ), + ) + default: + // Write a file that won't be generated to the location. + require.NoError( + t, + storage.PutPath( + ctx, + storageBucket, + normalpath.Join(fullOutputPath, "foo.txt"), + []byte(`1`), + ), + ) + } + } + var templateBuilder strings.Builder + _, _ = templateBuilder.WriteString(`version: v2 +plugins: +`) + + plugins := []string{"java", "cpp", "ruby"} + for i, outputPath := range outputPaths { + _, _ = templateBuilder.WriteString(` - protoc_builtin: `) + _, _ = templateBuilder.WriteString(plugins[i]) + _, _ = templateBuilder.WriteString("\n") + _, _ = templateBuilder.WriteString(` out: `) + _, _ = templateBuilder.WriteString(outputPath) + _, _ = templateBuilder.WriteString("\n") + } + testRunStdoutStderr( + t, + nil, + 0, + ``, + ``, + filepath.Join("testdata", "simple"), + "--template", + templateBuilder.String(), + "-o", + filepath.Join(tmpDirPath, normalpath.Unnormalize(baseOutDirPath)), + "--clean", + ) + for _, fullOutputPath := range fullOutputPaths { + switch normalpath.Ext(fullOutputPath) { + case ".jar", ".zip": + data, err := storage.ReadPath( + ctx, + storageBucket, + fullOutputPath, + ) + require.NoError(t, err) + require.True(t, len(data) > 1, "expected non-fake data at %q", fullOutputPath) + default: + _, err := storage.ReadPath( + ctx, + storageBucket, + normalpath.Join(fullOutputPath, "foo.txt"), + ) + require.ErrorIs(t, err, fs.ErrNotExist) + } + } +} + func testRunStdoutStderr(t *testing.T, stdin io.Reader, expectedExitCode int, expectedStdout string, expectedStderr string, args ...string) { appcmdtesting.RunCommandExitCodeStdoutStderr( t, diff --git a/private/bufpkg/bufprotoplugin/bufprotopluginos/bufprotopluginos.go b/private/bufpkg/bufprotoplugin/bufprotopluginos/bufprotopluginos.go index 94d594b6cc..33a8e9d0ad 100644 --- a/private/bufpkg/bufprotoplugin/bufprotopluginos/bufprotopluginos.go +++ b/private/bufpkg/bufprotoplugin/bufprotopluginos/bufprotopluginos.go @@ -66,3 +66,16 @@ func ResponseWriterWithCreateOutDirIfNotExists() ResponseWriterOption { responseWriterOptions.createOutDirIfNotExists = true } } + +// Cleaner deletes output locations prior to generation. +// +// This must be done before any interaction with ResponseWriters, as multiple plugins may output to a single +// location. +type Cleaner interface { + DeleteOuts(ctx context.Context, pluginOuts []string) error +} + +// NewCleaner returns a new Cleaner. +func NewCleaner(storageosProvider storageos.Provider) Cleaner { + return newCleaner(storageosProvider) +} diff --git a/private/bufpkg/bufprotoplugin/bufprotopluginos/cleaner.go b/private/bufpkg/bufprotoplugin/bufprotopluginos/cleaner.go new file mode 100644 index 0000000000..693ec997f9 --- /dev/null +++ b/private/bufpkg/bufprotoplugin/bufprotopluginos/cleaner.go @@ -0,0 +1,124 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// 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 bufprotopluginos + +import ( + "context" + "errors" + "io/fs" + "path/filepath" + + "github.com/bufbuild/buf/private/pkg/filepathext" + "github.com/bufbuild/buf/private/pkg/normalpath" + "github.com/bufbuild/buf/private/pkg/osext" + "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/bufbuild/buf/private/pkg/syserror" +) + +type cleaner struct { + storageosProvider storageos.Provider +} + +func newCleaner( + storageosProvider storageos.Provider, +) *cleaner { + return &cleaner{ + storageosProvider: storageosProvider, + } +} + +func (c *cleaner) DeleteOuts( + ctx context.Context, + pluginOuts []string, +) error { + pwd, err := osext.Getwd() + if err != nil { + return err + } + pwd, err = reallyCleanPath(pwd) + if err != nil { + return err + } + for _, pluginOut := range pluginOuts { + if err := validatePluginOut(pwd, pluginOut); err != nil { + return err + } + } + for _, pluginOut := range pluginOuts { + if err := c.deleteOut(ctx, pluginOut); err != nil { + return err + } + } + return nil +} + +func (c *cleaner) deleteOut( + ctx context.Context, + pluginOut string, +) error { + dirPath := pluginOut + removePath := "." + switch filepath.Ext(pluginOut) { + case ".jar", ".zip": + dirPath = normalpath.Dir(pluginOut) + removePath = normalpath.Base(pluginOut) + default: + // Assume output is a directory. + } + bucket, err := c.storageosProvider.NewReadWriteBucket( + dirPath, + storageos.ReadWriteBucketWithSymlinksIfSupported(), + ) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil + } + return err + } + return bucket.DeleteAll(ctx, removePath) +} + +func validatePluginOut(pwd string, pluginOut string) error { + if pluginOut == "" { + // This is just triple-making sure. + return syserror.New("got empty pluginOut in bufprotopluginos.Cleaner") + } + if pluginOut == "." { + // This is just a really defensive safety check. We can't see a reason you'd want to delete + // your current working directory other than something like a (cd proto && buf generate), so + // until and unless someone complains, we're just going to outlaw this. + return errors.New("cannot use --clean if your plugin will output to the current directory") + } + cleanedPluginOut, err := reallyCleanPath(pluginOut) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil + } + return err + } + if cleanedPluginOut == pwd { + // Same thing, more defense for now. + return errors.New("cannot use --clean if your plugin will output to the current directory") + } + return nil +} + +func reallyCleanPath(path string) (string, error) { + path, err := filepathext.RealClean(path) + if err != nil { + return "", err + } + return filepath.EvalSymlinks(path) +}