Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clean to buf.gen.yaml #3130

Merged
merged 5 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions private/buf/bufgen/bufgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
8 changes: 6 additions & 2 deletions private/buf/bufgen/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -484,7 +488,7 @@ func validateResponses(

type generateOptions struct {
baseOutDirPath string
deleteOuts bool
deleteOuts *bool
includeImportsOverride *bool
includeWellKnownTypesOverride *bool
}
Expand Down
12 changes: 6 additions & 6 deletions private/buf/cmd/buf/command/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down
49 changes: 41 additions & 8 deletions private/buf/cmd/buf/command/generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions private/bufpkg/bufconfig/buf_gen_yaml_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"`
}
Expand Down
21 changes: 21 additions & 0 deletions private/bufpkg/bufconfig/buf_gen_yaml_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 19 additions & 8 deletions private/bufpkg/bufconfig/generate_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -38,6 +41,7 @@ type GenerateConfig interface {

// NewGenerateConfig returns a validated GenerateConfig.
func NewGenerateConfig(
cleanPluginOuts bool,
pluginConfigs []GeneratePluginConfig,
managedConfig GenerateManagedConfig,
typeConfig GenerateTypeConfig,
Expand All @@ -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(
Expand Down Expand Up @@ -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
}
Expand Down