From d878f780905d41ed3a1e48a27bb6eb904154e848 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 22 Apr 2026 11:55:05 -0500 Subject: [PATCH 1/2] Use appcmd for changed-plugins --- internal/cmd/changed-plugins/main.go | 42 +++++++++++++++++----------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/internal/cmd/changed-plugins/main.go b/internal/cmd/changed-plugins/main.go index 915ef02c6..363369aa7 100644 --- a/internal/cmd/changed-plugins/main.go +++ b/internal/cmd/changed-plugins/main.go @@ -2,38 +2,46 @@ package main import ( "context" - "flag" "fmt" - "log" - "os" "strings" + "buf.build/go/app/appcmd" + "buf.build/go/app/appext" + "github.com/bufbuild/plugins/internal/plugin" ) func main() { - flag.Parse() - if len(flag.Args()) != 1 { - flag.Usage() - os.Exit(2) + appcmd.Main(context.Background(), newRootCommand("changed-plugins")) +} + +func newRootCommand(name string) *appcmd.Command { + builder := appext.NewBuilder(name) + return &appcmd.Command{ + Use: name + " ", + Short: "Outputs plugins that changed relative to a base git ref.", + Args: appcmd.ExactArgs(1), + Run: builder.NewRunFunc(run), + BindPersistentFlags: builder.BindRoot, } - basedir := flag.Args()[0] +} - plugins, err := plugin.FindAll(basedir) +func run(ctx context.Context, container appext.Container) error { + plugins, err := plugin.FindAll(container.Arg(0)) if err != nil { - log.Fatalf("failed to find plugins: %v", err) + return fmt.Errorf("find plugins: %w", err) } - // Filter by changed plugins (for PR builds) - includedPlugins, err := plugin.FilterByBaseRefDiff(context.Background(), plugins) + includedPlugins, err := plugin.FilterByBaseRefDiff(ctx, plugins) if err != nil { - log.Fatalf("failed to filter plugins by changed files: %v", err) + return fmt.Errorf("filter plugins by changed files: %w", err) } var sb strings.Builder - for _, includedPlugin := range includedPlugins { - sb.WriteString(strings.TrimPrefix(includedPlugin.Name, "buf.build/")) + for _, p := range includedPlugins { + sb.WriteString(strings.TrimPrefix(p.Name, "buf.build/")) sb.WriteByte(':') - sb.WriteString(includedPlugin.PluginVersion) + sb.WriteString(p.PluginVersion) sb.WriteByte(' ') } - fmt.Println(strings.TrimSpace(sb.String())) //nolint:forbidigo + fmt.Fprintln(container.Stdout(), strings.TrimSpace(sb.String())) + return nil } From 7e344b836fcc4e912690dd03d664498bc88f2dfc Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Wed, 22 Apr 2026 12:08:40 -0500 Subject: [PATCH 2/2] Prefer flags for --dir, --base-ref, --include-testdata --- .github/workflows/ci.yml | 4 +-- .github/workflows/pr.yml | 5 +--- internal/cmd/changed-plugins/main.go | 28 ++++++++++++++----- internal/plugin/plugin.go | 40 +++------------------------- 4 files changed, 28 insertions(+), 49 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4b445530..76bbeb999 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,10 +47,8 @@ jobs: check-latest: true - name: Calculate changed plugins and set PLUGINS env var from last successful commit push if: ${{ inputs.plugins == '' }} - env: - BASE_REF: ${{ steps.last_successful_commit_push.outputs.base }} run: | - val=`go run ./internal/cmd/changed-plugins .` + val=`go run ./internal/cmd/changed-plugins --base-ref '${{ steps.last_successful_commit_push.outputs.base }}'` if [[ -n "${val}" && -z "${PLUGINS}" ]]; then echo "PLUGINS=${val}" >> $GITHUB_ENV fi diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 33e14f524..70e487b97 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -41,11 +41,8 @@ jobs: check-latest: true - name: Calculate changed plugins and set PLUGINS env var from base branch if: ${{ inputs.plugins == '' }} - env: - BASE_REF: 'origin/${{ github.base_ref }}' # we can use a remote ref because we fetch everything back in the checkout step - INCLUDE_TESTDATA: 'true' run: | - val=`go run ./internal/cmd/changed-plugins .` + val=`go run ./internal/cmd/changed-plugins --base-ref 'origin/${{ github.base_ref }}' --include-testdata` if [[ -n "${val}" && -z "${PLUGINS}" ]]; then echo "PLUGINS=${val}" >> $GITHUB_ENV fi diff --git a/internal/cmd/changed-plugins/main.go b/internal/cmd/changed-plugins/main.go index 363369aa7..8d0927d5c 100644 --- a/internal/cmd/changed-plugins/main.go +++ b/internal/cmd/changed-plugins/main.go @@ -7,6 +7,7 @@ import ( "buf.build/go/app/appcmd" "buf.build/go/app/appext" + "github.com/spf13/pflag" "github.com/bufbuild/plugins/internal/plugin" ) @@ -17,21 +18,36 @@ func main() { func newRootCommand(name string) *appcmd.Command { builder := appext.NewBuilder(name) + f := &flags{} return &appcmd.Command{ - Use: name + " ", + Use: name, Short: "Outputs plugins that changed relative to a base git ref.", - Args: appcmd.ExactArgs(1), - Run: builder.NewRunFunc(run), + Args: appcmd.NoArgs, + Run: builder.NewRunFunc(func(ctx context.Context, container appext.Container) error { return run(ctx, container, f) }), + BindFlags: f.Bind, BindPersistentFlags: builder.BindRoot, } } -func run(ctx context.Context, container appext.Container) error { - plugins, err := plugin.FindAll(container.Arg(0)) +type flags struct { + dir string + baseRef string + includeTestdata bool +} + +func (f *flags) Bind(flagSet *pflag.FlagSet) { + flagSet.StringVar(&f.dir, "dir", ".", "directory path to plugins") + flagSet.StringVar(&f.baseRef, "base-ref", "", "base git ref to diff against") + flagSet.BoolVar(&f.includeTestdata, "include-testdata", false, "include testdata plugin paths in the diff") + _ = appcmd.MarkFlagRequired(flagSet, "base-ref") +} + +func run(ctx context.Context, container appext.Container, f *flags) error { + plugins, err := plugin.FindAll(f.dir) if err != nil { return fmt.Errorf("find plugins: %w", err) } - includedPlugins, err := plugin.FilterByBaseRefDiff(ctx, plugins) + includedPlugins, err := plugin.FilterByBaseRefDiff(ctx, plugins, f.baseRef, f.includeTestdata) if err != nil { return fmt.Errorf("filter plugins by changed files: %w", err) } diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index 7b56b6cc3..c30d82c65 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -4,7 +4,6 @@ import ( "bytes" "cmp" "context" - "errors" "fmt" "io/fs" "log" @@ -12,7 +11,6 @@ import ( "os/exec" "path/filepath" "slices" - "strconv" "strings" "sync" "unicode" @@ -205,16 +203,12 @@ func FilterByPluginsEnv(plugins []*Plugin, pluginsEnv string) ([]*Plugin, error) // FilterByBaseRefDiff filters the passed plugins to the ones that changed from a base Git ref to // diff against. It calculates the changed files from that ref, and filters the relevant files in // the plugins directory(ies) to determine which plugins changed from the ones passed. -func FilterByBaseRefDiff(ctx context.Context, plugins []*Plugin) ([]*Plugin, error) { - diffEnv, err := readDiffEnv() +func FilterByBaseRefDiff(ctx context.Context, plugins []*Plugin, baseRef string, includeTestdata bool) ([]*Plugin, error) { + allChangedFiles, err := git.ChangedFilesFrom(ctx, baseRef) if err != nil { - return nil, fmt.Errorf("get diff env: %w", err) + return nil, fmt.Errorf("calculate changed files from base ref %q: %w", baseRef, err) } - allChangedFiles, err := git.ChangedFilesFrom(ctx, diffEnv.baseRef) - if err != nil { - return nil, fmt.Errorf("calculate changed files from base ref %q: %w", diffEnv.baseRef, err) - } - return filterPluginsByChangedFiles(plugins, allChangedFiles, diffEnv.includeTestdata) + return filterPluginsByChangedFiles(plugins, allChangedFiles, includeTestdata) } func filterPluginsByChangedFiles(plugins []*Plugin, allChangedFiles []string, includeTestdata bool) ([]*Plugin, error) { @@ -346,32 +340,6 @@ func calculateGitModified(ctx context.Context, pluginYamlPath string) (bool, err return strings.TrimSpace(output.String()) != "", nil } -type diffEnv struct { - baseRef string - includeTestdata bool -} - -func readDiffEnv() (*diffEnv, error) { - baseRef, ok := os.LookupEnv("BASE_REF") - if !ok { - return nil, errors.New("missing BASE_REF") - } else if baseRef == "" { - return nil, errors.New("empty BASE_REF") - } - var includeTestdata bool // default false - if includeTestdataStr, _ := os.LookupEnv("INCLUDE_TESTDATA"); includeTestdataStr != "" { - var err error - includeTestdata, err = strconv.ParseBool(includeTestdataStr) - if err != nil { - return nil, fmt.Errorf("invalid INCLUDE_TESTDATA value %s: %w", includeTestdataStr, err) - } - } - return &diffEnv{ - baseRef: baseRef, - includeTestdata: includeTestdata, - }, nil -} - // filterPluginPaths returns the filepaths that are considered to be a relevant plugin file, based // on the plugins dir(s). func filterPluginPaths(filePaths []string, includeTestdata bool) []string {