Use appcmd.Command in changed-plugins

.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

.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

internal/cmd/changed-plugins/main.go

@@ -2,38 +2,62 @@ package main
 
 import (
 	"context"
-	"flag"
 	"fmt"
-	"log"
-	"os"
 	"strings"
 
+	"buf.build/go/app/appcmd"
+	"buf.build/go/app/appext"
+	"github.com/spf13/pflag"
+
 	"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)
+	f := &flags{}
+	return &appcmd.Command{
+		Use:                 name,
+		Short:               "Outputs plugins that changed relative to a base git ref.",
+		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,
 	}
-	basedir := flag.Args()[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")
+}
 
-	plugins, err := plugin.FindAll(basedir)
+func run(ctx context.Context, container appext.Container, f *flags) error {
+	plugins, err := plugin.FindAll(f.dir)
 	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, f.baseRef, f.includeTestdata)
 	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
 }

internal/plugin/plugin.go

@@ -4,15 +4,13 @@ import (
 	"bytes"
 	"cmp"
 	"context"
-	"errors"
 	"fmt"
 	"io/fs"
 	"log"
 	"os"
 	"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 {