chore: move GetExecutablePath from config to command (#3383)

Migrates GetExecutablePath from config package to command package, to
keep config as only a struct container.

Fixes #3377

---------

Signed-off-by: ldetmer <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Author: ldetmer

internal/command/command.go

@@ -45,3 +45,12 @@ func RunWithEnv(ctx context.Context, env map[string]string, command string, arg
 	}
 	return nil
 }
+
+// GetExecutablePath finds the path for a given command, checking for an
+// override in the provided commandOverrides map first.
+func GetExecutablePath(commandOverrides map[string]string, commandName string) string {
+	if exe, ok := commandOverrides[commandName]; ok {
+		return exe
+	}
+	return commandName
+}

internal/command/command_test.go

@@ -18,6 +18,9 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/googleapis/librarian/internal/config"
 )
 
 func TestRun(t *testing.T) {
@@ -60,3 +63,48 @@ func TestRunWithEnv_VariableNotSetFailsValidation(t *testing.T) {
 		t.Fatalf("RunWithEnv() = %v, want non-nil", err)
 	}
 }
+
+func TestGetExecutablePath(t *testing.T) {
+	tests := []struct {
+		name           string
+		releaseConfig  *config.Release
+		executableName string
+		want           string
+	}{
+		{
+			name: "Preinstalled tool found",
+			releaseConfig: &config.Release{
+				Preinstalled: map[string]string{
+					"cargo": "/usr/bin/cargo",
+					"git":   "/usr/bin/git",
+				},
+			},
+			executableName: "cargo",
+			want:           "/usr/bin/cargo",
+		},
+		{
+			name: "Preinstalled tool not found",
+			releaseConfig: &config.Release{
+				Preinstalled: map[string]string{
+					"git": "/usr/bin/git",
+				},
+			},
+			executableName: "cargo",
+			want:           "cargo",
+		},
+		{
+			name:           "No preinstalled section",
+			releaseConfig:  &config.Release{},
+			executableName: "cargo",
+			want:           "cargo",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := GetExecutablePath(test.releaseConfig.Preinstalled, test.executableName)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

internal/config/config.go

@@ -64,17 +64,6 @@ type Release struct {
 	Tools map[string][]Tool `yaml:"tools,omitempty"`
 }
 
-// GetExecutablePath finds the path for a given command, checking for an
-// override in the configuration first.
-func (r *Release) GetExecutablePath(commandName string) string {
-	if r != nil && r.Preinstalled != nil {
-		if exe, ok := r.Preinstalled[commandName]; ok {
-			return exe
-		}
-	}
-	return commandName
-}
-
 // Tool defines the configuration required to install helper tools.
 type Tool struct {
 	// Name is the name of the tool e.g. nox.

internal/config/config_test.go

@@ -137,54 +137,3 @@ func TestWrite(t *testing.T) {
 		t.Errorf("mismatch (-want +got):\n%s", diff)
 	}
 }
-
-func TestReleaseGetExecutablePath(t *testing.T) {
-	tests := []struct {
-		name           string
-		releaseConfig  *Release
-		executableName string
-		want           string
-	}{
-		{
-			name: "Preinstalled tool found",
-			releaseConfig: &Release{
-				Preinstalled: map[string]string{
-					"cargo": "/usr/bin/cargo",
-					"git":   "/usr/bin/git",
-				},
-			},
-			executableName: "cargo",
-			want:           "/usr/bin/cargo",
-		},
-		{
-			name: "Preinstalled tool not found",
-			releaseConfig: &Release{
-				Preinstalled: map[string]string{
-					"git": "/usr/bin/git",
-				},
-			},
-			executableName: "cargo",
-			want:           "cargo",
-		},
-		{
-			name:           "No preinstalled section",
-			releaseConfig:  &Release{},
-			executableName: "cargo",
-			want:           "cargo",
-		},
-		{
-			name:           "Nil release config",
-			releaseConfig:  nil,
-			executableName: "cargo",
-			want:           "cargo",
-		},
-	}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			got := test.releaseConfig.GetExecutablePath(test.executableName)
-			if diff := cmp.Diff(test.want, got); diff != "" {
-				t.Errorf("GetExecutablePath() mismatch (-want +got):\n%s", diff)
-			}
-		})
-	}
-}

internal/git/git_test.go

@@ -39,7 +39,7 @@ func TestGetLastTag(t *testing.T) {
 		Remote: "origin",
 		Branch: "main",
 	}
-	got, err := GetLastTag(t.Context(), cfg.GetExecutablePath("git"), cfg.Remote, cfg.Branch)
+	got, err := GetLastTag(t.Context(), command.GetExecutablePath(cfg.Preinstalled, "git"), cfg.Remote, cfg.Branch)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -54,7 +54,7 @@ func TestLastTagGitError(t *testing.T) {
 		Remote: "origin",
 		Branch: "main",
 	}
-	_, err := GetLastTag(t.Context(), cfg.GetExecutablePath("git"), cfg.Remote, cfg.Branch)
+	_, err := GetLastTag(t.Context(), command.GetExecutablePath(cfg.Preinstalled, "git"), cfg.Remote, cfg.Branch)
 	if err == nil {
 		t.Fatal("expected an error but got none")
 	}
@@ -77,7 +77,7 @@ func TestIsNewFileSuccess(t *testing.T) {
 		t.Fatal(err)
 	}
 	cfg := &config.Release{}
-	gitExe := cfg.GetExecutablePath("git")
+	gitExe := command.GetExecutablePath(cfg.Preinstalled, "git")
 
 	newName := path.Join("src", "storage", "src", "new.rs")
 	if err := os.MkdirAll(path.Dir(newName), 0755); err != nil {
@@ -105,7 +105,7 @@ func TestIsNewFileDiffError(t *testing.T) {
 	t.Chdir(t.TempDir())
 	testhelper.SetupForVersionBump(t, wantTag)
 	cfg := &config.Release{}
-	gitExe := cfg.GetExecutablePath("git")
+	gitExe := command.GetExecutablePath(cfg.Preinstalled, "git")
 	existingName := path.Join("src", "storage", "src", "lib.rs")
 	if IsNewFile(t.Context(), gitExe, "invalid-tag", existingName) {
 		t.Errorf("diff errors should return false for isNewFile(): %s", existingName)
@@ -114,14 +114,10 @@ func TestIsNewFileDiffError(t *testing.T) {
 
 func TestFilesChangedSuccess(t *testing.T) {
 	const wantTag = "release-2001-02-03"
-	release := &config.Release{
-		Remote: "origin",
-		Branch: "main",
-	}
 	remoteDir := testhelper.SetupRepoWithChange(t, wantTag)
 	testhelper.CloneRepository(t, remoteDir)
 
-	got, err := FilesChangedSince(t.Context(), wantTag, release.GetExecutablePath("git"), release.IgnoredChanges)
+	got, err := FilesChangedSince(t.Context(), wantTag, command.GetExecutablePath(nil, "git"), nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -133,13 +129,9 @@ func TestFilesChangedSuccess(t *testing.T) {
 
 func TestFilesBadRef(t *testing.T) {
 	const wantTag = "release-2002-03-04"
-	release := &config.Release{
-		Remote: "origin",
-		Branch: "main",
-	}
 	remoteDir := testhelper.SetupRepoWithChange(t, wantTag)
 	testhelper.CloneRepository(t, remoteDir)
-	if got, err := FilesChangedSince(t.Context(), "--invalid--", release.GetExecutablePath("git"), release.IgnoredChanges); err == nil {
+	if got, err := FilesChangedSince(t.Context(), "--invalid--", command.GetExecutablePath(nil, "git"), nil); err == nil {
 		t.Errorf("expected an error with invalid tag, got=%v", got)
 	}
 }
@@ -245,7 +237,7 @@ func TestAssertGitStatusClean(t *testing.T) {
 			tmpDir := t.TempDir()
 			t.Chdir(tmpDir)
 			test.setup(t)
-			err := AssertGitStatusClean(t.Context(), cfg.GetExecutablePath("git"))
+			err := AssertGitStatusClean(t.Context(), command.GetExecutablePath(cfg.Preinstalled, "git"))
 			if (err != nil) != test.wantErr {
 				t.Errorf("AssertGitStatusClean() error = %v, wantErr %v", err, test.wantErr)
 			}

internal/librarian/publish.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/googleapis/librarian/internal/command"
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/git"
 	"github.com/googleapis/librarian/internal/librarian/rust"
@@ -62,7 +63,10 @@ func publish(ctx context.Context, cfg *config.Config, dryRun bool, skipSemverChe
 	if err := verifyRequiredTools(ctx, cfg.Language, cfg.Release); err != nil {
 		return err
 	}
-	gitExe := cfg.Release.GetExecutablePath("git")
+	gitExe := "git"
+	if cfg.Release != nil {
+		gitExe = command.GetExecutablePath(cfg.Release.Preinstalled, "git")
+	}
 	if err := git.AssertGitStatusClean(ctx, gitExe); err != nil {
 		return err
 	}
@@ -86,7 +90,10 @@ func publish(ctx context.Context, cfg *config.Config, dryRun bool, skipSemverChe
 
 // verifyRequiredTools verifies all the necessary language-agnostic tools are installed.
 func verifyRequiredTools(ctx context.Context, language string, cfg *config.Release) error {
-	gitExe := cfg.GetExecutablePath("git")
+	gitExe := "git"
+	if cfg != nil {
+		gitExe = command.GetExecutablePath(cfg.Preinstalled, "git")
+	}
 	if err := git.GitVersion(ctx, gitExe); err != nil {
 		return err
 	}

internal/librarian/release.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/googleapis/librarian/internal/command"
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/git"
 	"github.com/googleapis/librarian/internal/librarian/rust"
@@ -78,7 +79,10 @@ func runRelease(ctx context.Context, cmd *cli.Command) error {
 	if err != nil {
 		return err
 	}
-	gitExe := cfg.Release.GetExecutablePath("git")
+	gitExe := "git"
+	if cfg.Release != nil {
+		gitExe = command.GetExecutablePath(cfg.Release.Preinstalled, "git")
+	}
 	if err := git.AssertGitStatusClean(ctx, gitExe); err != nil {
 		return err
 	}
@@ -174,7 +178,7 @@ func shouldReleaseLibrary(ctx context.Context, cfg *config.Config, path string)
 	if cfg.Release == nil {
 		return false, errReleaseConfigEmpty
 	}
-	gitExe := cfg.Release.GetExecutablePath("git")
+	gitExe := command.GetExecutablePath(cfg.Release.Preinstalled, "git")
 	lastTag, err := git.GetLastTag(ctx, gitExe, cfg.Release.Remote, cfg.Release.Branch)
 	if err != nil {
 		return false, err