Add clean to buf.gen.yaml (#3130)

As a quick follow-up to #3124 and in response to
#3124 (comment),
this adds a `clean` config key to `v2` `buf.gen.yaml`, for example:

```yaml
version: v2
clean: true
plugins:
  - local: custom-gen-go
    out: gen/go
    opt: paths=source_relative
    strategy: directory
  - protoc_builtin: java
    out: gen/java
```

When running `buf generate` with the above configs, the outs set to each
plugin
(e.g. `gen/go` and `gen/java`) will be removed before code generation is
run.

If `buf generate --clean` flag is set, then that will always take
precedence, even if
`clean: false` in the configuration. And likewise, if `buf generate
--clean=false`,
and `clean: true` in the configuration, then we would not delete the out
directories.

---------

Co-authored-by: Oliver Sun <73540835+oliversun9@users.noreply.github.com>
Co-authored-by: Oliver Sun <osun@buf.build>
Co-authored-by: bufdev <4228796+bufdev@users.noreply.github.com>
Co-authored-by: bufdev <bufdev-github@buf.build>

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## [Unreleased]
 
+- Add `clean` as a top-level option in `buf.gen.yaml`, matching the `buf generate --clean` flag. If
+ set to true, this will delete the directories, jar files, or zip files set to `out` for each
+ plugin.
 - Fix git input handling of annotated tags.
 - Update `buf registry login` to complete the login flow in the browser by default. This allows
  users to login with their browser and have the token automatically provided to the CLI.

private/buf/bufgen/bufgen.go

@@ -120,9 +120,9 @@ 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 {
+func GenerateWithDeleteOuts(deleteOuts bool) GenerateOption {
  return func(generateOptions *generateOptions) {
- generateOptions.deleteOuts = true
+ generateOptions.deleteOuts = &deleteOuts
  }
 }
 

private/buf/bufgen/generator.go

@@ -105,7 +105,11 @@ func (g *generator) Generate(
  return err
  }
  }
- if generateOptions.deleteOuts {
+ shouldDeleteOuts := config.CleanPluginOuts()
+ if generateOptions.deleteOuts != nil {
+ shouldDeleteOuts = *generateOptions.deleteOuts
+ }
+ if shouldDeleteOuts {
  if err := g.deleteOuts(
  ctx,
  generateOptions.baseOutDirPath,
@@ -484,7 +488,7 @@ func validateResponses(
 
 type generateOptions struct {
  baseOutDirPath string
- deleteOuts bool
+ deleteOuts *bool
  includeImportsOverride *bool
  includeWellKnownTypesOverride *bool
 }

private/buf/cmd/buf/command/generate/generate.go

@@ -373,7 +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
+ DeleteOuts *bool
  ErrorFormat string
  Files []string
  Config string
@@ -427,10 +427,10 @@ 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,
+ bindBoolPointer(
+ flagSet,
  deleteOutsFlagName,
- false,
+ &f.DeleteOuts,
  `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(
@@ -522,10 +522,10 @@ func run(
  generateOptions := []bufgen.GenerateOption{
  bufgen.GenerateWithBaseOutDirPath(flags.BaseOutDirPath),
  }
- if flags.DeleteOuts {
+ if flags.DeleteOuts != nil {
  generateOptions = append(
  generateOptions,
- bufgen.GenerateWithDeleteOuts(),
+ bufgen.GenerateWithDeleteOuts(*flags.DeleteOuts),
  )
  }
  if flags.IncludeImportsOverride != nil {

private/buf/cmd/buf/command/generate/generate_test.go

@@ -908,6 +908,24 @@ func testGenerateDeleteOuts(
  t *testing.T,
  baseOutDirPath string,
  outputPaths ...string,
+) {
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, nil, true, true, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, nil, false, false, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean"}, false, true, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean"}, true, true, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean=true"}, true, true, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean=true"}, false, true, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean=false"}, true, false, outputPaths)
+ testGenerateDeleteOutsWithArgAndConfig(t, baseOutDirPath, []string{"--clean=false"}, false, false, outputPaths)
+}
+
+func testGenerateDeleteOutsWithArgAndConfig(
+ t *testing.T,
+ baseOutDirPath string,
+ cleanArgs []string,
+ cleanOptionInConfig bool,
+ expectedClean bool,
+ 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")
@@ -964,18 +982,25 @@ plugins:
  _, _ = templateBuilder.WriteString(outputPath)
  _, _ = templateBuilder.WriteString("\n")
  }
+ if cleanOptionInConfig {
+ templateBuilder.WriteString("clean: true\n")
+ }
  testRunStdoutStderr(
  t,
  nil,
  0,
  ``,
  ``,
- filepath.Join("testdata", "simple"),
- "--template",
- templateBuilder.String(),
- "-o",
- filepath.Join(tmpDirPath, normalpath.Unnormalize(baseOutDirPath)),
- "--clean",
+ append(
+ []string{
+ filepath.Join("testdata", "simple"),
+ "--template",
+ templateBuilder.String(),
+ "-o",
+ filepath.Join(tmpDirPath, normalpath.Unnormalize(baseOutDirPath)),
+ },
+ cleanArgs...,
+ )...,
  )
  for _, fullOutputPath := range fullOutputPaths {
  switch normalpath.Ext(fullOutputPath) {
@@ -986,14 +1011,22 @@ plugins:
  fullOutputPath,
  )
  require.NoError(t, err)
+ // Always expect non-fake data, because the existing ".jar" or ".zip"
+ // file is always replaced by the output. This is the existing and correct
+ // behavior.
  require.True(t, len(data) > 1, "expected non-fake data at %q", fullOutputPath)
  default:
- _, err := storage.ReadPath(
+ data, err := storage.ReadPath(
  ctx,
  storageBucket,
  normalpath.Join(fullOutputPath, "foo.txt"),
  )
- require.ErrorIs(t, err, fs.ErrNotExist)
+ if expectedClean {
+ require.ErrorIs(t, err, fs.ErrNotExist)
+ } else {
+ require.NoError(t, err, "expected foo.txt at %q", fullOutputPath)
+ require.NotNil(t, data, "expected foo.txt at %q", fullOutputPath)
+ }
  }
  }
 }

private/bufpkg/bufconfig/buf_gen_yaml_file.go

@@ -258,6 +258,7 @@ func writeBufGenYAMLFile(writer io.Writer, bufGenYAMLFile BufGenYAMLFile) error
  }
  externalBufGenYAMLFileV2 := externalBufGenYAMLFileV2{
  Version: FileVersionV2.String(),
+ Clean: bufGenYAMLFile.GenerateConfig().CleanPluginOuts(),
  Plugins: externalPluginConfigsV2,
  Managed: externalManagedConfigV2,
  Inputs: externalInputConfigsV2,
@@ -481,8 +482,11 @@ type externalTypesConfigV1 struct {
 
 // externalBufGenYAMLFileV2 represents the v2 buf.gen.yaml file.
 type externalBufGenYAMLFileV2 struct {
- Version string `json:"version,omitempty" yaml:"version,omitempty"`
- Managed externalGenerateManagedConfigV2 `json:"managed,omitempty" yaml:"managed,omitempty"`
+ Version string `json:"version,omitempty" yaml:"version,omitempty"`
+ Managed externalGenerateManagedConfigV2 `json:"managed,omitempty" yaml:"managed,omitempty"`
+ // Clean, if set to true, will delete the output directories, zip files, or jar files
+ // before generation is run.
+ Clean bool `json:"clean,omitempty" yaml:"clean,omitempty"`
  Plugins []externalGeneratePluginConfigV2 `json:"plugins,omitempty" yaml:"plugins,omitempty"`
  Inputs []externalInputConfigV2 `json:"inputs,omitempty" yaml:"inputs,omitempty"`
 }

private/bufpkg/bufconfig/buf_gen_yaml_file_test.go

@@ -106,6 +106,27 @@ plugins:
  t,
  // input
  `version: v2
+clean: true
+plugins:
+ - local: custom-gen-go
+ out: gen/go
+ opt: paths=source_relative
+ strategy: directory
+`,
+ // expected output
+ `version: v2
+clean: true
+plugins:
+ - local: custom-gen-go
+ out: gen/go
+ opt: paths=source_relative
+ strategy: directory
+`,
+ )
+ testReadWriteBufGenYAMLFileRoundTrip(
+ t,
+ // input
+ `version: v2
 managed:
  disable:
  - module: buf.build/googleapis/googleapis

private/bufpkg/bufconfig/generate_config.go

@@ -22,6 +22,9 @@ import (
 
 // GenerateConfig is a generation configuration.
 type GenerateConfig interface {
+ // CleanPluginOuts is whether to delete the output directories, zip files, or jar files before
+ // generation is run.
+ CleanPluginOuts() bool
  // GeneratePluginConfigs returns the plugin configurations. This will always be
  // non-empty. Zero plugin configs will cause an error at construction time.
  GeneratePluginConfigs() []GeneratePluginConfig
@@ -38,6 +41,7 @@ type GenerateConfig interface {
 
 // NewGenerateConfig returns a validated GenerateConfig.
 func NewGenerateConfig(
+ cleanPluginOuts bool,
  pluginConfigs []GeneratePluginConfig,
  managedConfig GenerateManagedConfig,
  typeConfig GenerateTypeConfig,
@@ -46,18 +50,20 @@ func NewGenerateConfig(
  return nil, newNoPluginsError()
  }
  return &generateConfig{
- pluginConfigs: pluginConfigs,
- managedConfig: managedConfig,
- typeConfig: typeConfig,
+ cleanPluginOuts: cleanPluginOuts,
+ pluginConfigs: pluginConfigs,
+ managedConfig: managedConfig,
+ typeConfig: typeConfig,
  }, nil
 }
 
 // *** PRIVATE ***
 
 type generateConfig struct {
- pluginConfigs []GeneratePluginConfig
- managedConfig GenerateManagedConfig
- typeConfig GenerateTypeConfig
+ cleanPluginOuts bool
+ pluginConfigs []GeneratePluginConfig
+ managedConfig GenerateManagedConfig
+ typeConfig GenerateTypeConfig
 }
 
 func newGenerateConfigFromExternalFileV1Beta1(
@@ -122,11 +128,16 @@ func newGenerateConfigFromExternalFileV2(
  return nil, err
  }
  return &generateConfig{
- managedConfig: managedConfig,
- pluginConfigs: pluginConfigs,
+ cleanPluginOuts: externalFile.Clean,
+ managedConfig: managedConfig,
+ pluginConfigs: pluginConfigs,
  }, nil
 }
 
+func (g *generateConfig) CleanPluginOuts() bool {
+ return g.cleanPluginOuts
+}
+
 func (g *generateConfig) GeneratePluginConfigs() []GeneratePluginConfig {
  return g.pluginConfigs
 }