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 <[email protected]>
Co-authored-by: Oliver Sun <[email protected]>
Co-authored-by: bufdev <[email protected]>
Co-authored-by: bufdev <[email protected]>

Author: 3 people

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
 }