diff --git a/CHANGELOG.md b/CHANGELOG.md index e4708d5741..27378e6c40 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/private/buf/bufgen/bufgen.go b/private/buf/bufgen/bufgen.go index a9d3262176..c349691e18 100644 --- a/private/buf/bufgen/bufgen.go +++ b/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 } } diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index 86273d7921..375420a4f0 100644 --- a/private/buf/bufgen/generator.go +++ b/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 } diff --git a/private/buf/cmd/buf/command/generate/generate.go b/private/buf/cmd/buf/command/generate/generate.go index ded5bfdb8d..970dd26f02 100644 --- a/private/buf/cmd/buf/command/generate/generate.go +++ b/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 { diff --git a/private/buf/cmd/buf/command/generate/generate_test.go b/private/buf/cmd/buf/command/generate/generate_test.go index a8986d930c..22758a0b10 100644 --- a/private/buf/cmd/buf/command/generate/generate_test.go +++ b/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) + } } } } diff --git a/private/bufpkg/bufconfig/buf_gen_yaml_file.go b/private/bufpkg/bufconfig/buf_gen_yaml_file.go index a35511689e..fb98bb62a4 100644 --- a/private/bufpkg/bufconfig/buf_gen_yaml_file.go +++ b/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"` } diff --git a/private/bufpkg/bufconfig/buf_gen_yaml_file_test.go b/private/bufpkg/bufconfig/buf_gen_yaml_file_test.go index b8d3b26e37..1ef6c16d96 100644 --- a/private/bufpkg/bufconfig/buf_gen_yaml_file_test.go +++ b/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 diff --git a/private/bufpkg/bufconfig/generate_config.go b/private/bufpkg/bufconfig/generate_config.go index 8c2bc51b59..dad48e8717 100644 --- a/private/bufpkg/bufconfig/generate_config.go +++ b/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 }