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 buf generate --clean flag #3124

Merged
merged 6 commits into from
Jul 4, 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
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions private/buf/bufgen/bufgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
34 changes: 33 additions & 1 deletion private/buf/bufgen/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -451,8 +483,8 @@ func validateResponses(
}

type generateOptions struct {
// plugin specific options:
baseOutDirPath string
deleteOuts bool
includeImportsOverride *bool
includeWellKnownTypesOverride *bool
}
Expand Down
14 changes: 14 additions & 0 deletions private/buf/cmd/buf/command/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const (
templateFlagName = "template"
baseOutDirPathFlagName = "output"
baseOutDirPathFlagShortName = "o"
deleteOutsFlagName = "clean"
errorFormatFlagName = "error-format"
configFlagName = "config"
pathsFlagName = "path"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
116 changes: 116 additions & 0 deletions private/buf/cmd/buf/command/generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"strings"
"testing"

"github.com/bufbuild/buf/private/buf/buftesting"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions private/bufpkg/bufprotoplugin/bufprotopluginos/bufprotopluginos.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading