chore: add missing logic to release command (#3447)

Added missing logic in the release command:
1) Take into account skipPublish config  
2) Take into account Release.FilesToIgnore config. As part of this
change reused existing FilesChangedSince helper function to get all
files changed since last git release at one time.
3) Updated migrate command to populate the ignored-files field in
librarian.yaml
4) Added logic for rust releases to check if cargo version had already
been bumped since last release tag

For #3232

---------

Signed-off-by: ldetmer <1771267+ldetmer@users.noreply.github.com>

Author: ldetmer

internal/git/git.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"os/exec"
 	"slices"
-	"strconv"
 	"strings"
 
 	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
@@ -132,14 +131,3 @@ func MatchesBranchPoint(ctx context.Context, gitExe, remote, branch string) erro
 	}
 	return nil
 }
-
-// ChangesInDirectorySinceTag checks if there have been any changes in directory since tag.
-func ChangesInDirectorySinceTag(ctx context.Context, gitExe, tag, dir string) (int, error) {
-	delta := fmt.Sprintf("%s...HEAD", tag)
-	cmd := exec.CommandContext(ctx, gitExe, "rev-list", "--count", delta, "--", dir)
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return 0, fmt.Errorf("running %s failed: %w\noutput: %s", cmd.String(), err, string(output))
-	}
-	return strconv.Atoi(strings.TrimSpace(string(output)))
-}

internal/git/git_test.go

@@ -296,39 +296,6 @@ func TestMatchesDirtyCloneError(t *testing.T) {
 	}
 }
 
-func TestChangesInDirectorySinceTag(t *testing.T) {
-	for _, test := range []struct {
-		name string
-		dir  string
-		want int
-	}{
-		{
-			name: "changes exist in directory",
-			dir:  "src/storage",
-			want: 1,
-		},
-		{
-			name: "changes do not exist in directory",
-			dir:  "src/gax-internal",
-			want: 0,
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			testhelper.RequireCommand(t, "git")
-			tag := "v1.2.3"
-			remoteDir := testhelper.SetupRepoWithChange(t, tag)
-			testhelper.CloneRepository(t, remoteDir)
-			got, err := ChangesInDirectorySinceTag(t.Context(), "git", tag, test.dir)
-			if err != nil {
-				t.Fatal(err)
-			}
-			if got != test.want {
-				t.Errorf("ChangesInDirectorySinceTag() = %d, want %d", got, test.want)
-			}
-		})
-	}
-}
-
 func TestShowFile(t *testing.T) {
 	testhelper.RequireCommand(t, "git")
 	remoteDir := testhelper.SetupRepo(t)

internal/librarian/release.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/googleapis/librarian/internal/command"
 	"github.com/googleapis/librarian/internal/config"
@@ -82,8 +83,16 @@ func runRelease(ctx context.Context, cmd *cli.Command) error {
 		return err
 	}
 
+	if cfg.Release == nil {
+		return errReleaseConfigEmpty
+	}
+	lastTag, err := git.GetLastTag(ctx, gitExe, cfg.Release.Remote, cfg.Release.Branch)
+	if err != nil {
+		return err
+	}
+
 	if all {
-		err = releaseAll(ctx, cfg)
+		err = releaseAll(ctx, cfg, lastTag, gitExe)
 	} else {
 		libConfg, err := libraryByName(cfg, libraryName)
 		if err != nil {
@@ -93,7 +102,7 @@ func runRelease(ctx context.Context, cmd *cli.Command) error {
 		if err != nil {
 			return err
 		}
-		err = releaseLibrary(ctx, cfg, libConfg, srcPath)
+		err = releaseLibrary(ctx, cfg, libConfg, srcPath, lastTag, gitExe)
 		if err != nil {
 			return err
 		}
@@ -104,25 +113,41 @@ func runRelease(ctx context.Context, cmd *cli.Command) error {
 	return yaml.Write(librarianConfigPath, cfg)
 }
 
-func releaseAll(ctx context.Context, cfg *config.Config) error {
+func releaseAll(ctx context.Context, cfg *config.Config, lastTag, gitExe string) error {
+	filesChanged, err := git.FilesChangedSince(ctx, lastTag, gitExe, cfg.Release.IgnoredChanges)
+	if err != nil {
+		return err
+	}
 	for _, library := range cfg.Libraries {
 		srcPath, err := getSrcPathForLanguage(cfg, library)
 		if err != nil {
 			return err
 		}
-		release, err := shouldReleaseLibrary(ctx, cfg, srcPath)
-		if err != nil {
-			return err
-		}
-		if release {
-			if err := releaseLibrary(ctx, cfg, library, srcPath); err != nil {
+		if shouldRelease(library, filesChanged, srcPath) {
+			if err := releaseLibrary(ctx, cfg, library, srcPath, lastTag, gitExe); err != nil {
 				return err
 			}
 		}
 	}
 	return nil
 }
 
+func shouldRelease(library *config.Library, filesChanged []string, srcPath string) bool {
+	if library.SkipPublish {
+		return false
+	}
+	pathWithTrailingSlash := srcPath
+	if !strings.HasSuffix(pathWithTrailingSlash, "/") {
+		pathWithTrailingSlash = pathWithTrailingSlash + "/"
+	}
+	for _, path := range filesChanged {
+		if strings.Contains(path, pathWithTrailingSlash) {
+			return true
+		}
+	}
+	return false
+}
+
 func getSrcPathForLanguage(cfg *config.Config, libConfig *config.Library) (string, error) {
 	srcPath := ""
 	switch cfg.Language {
@@ -137,11 +162,18 @@ func getSrcPathForLanguage(cfg *config.Config, libConfig *config.Library) (strin
 	return srcPath, nil
 }
 
-func releaseLibrary(ctx context.Context, cfg *config.Config, libConfig *config.Library, srcPath string) error {
+func releaseLibrary(ctx context.Context, cfg *config.Config, libConfig *config.Library, srcPath, lastTag, gitExe string) error {
 	switch cfg.Language {
 	case languageFake:
 		return fakeReleaseLibrary(libConfig)
 	case languageRust:
+		release, err := rust.ManifestVersionNeedsBump(gitExe, lastTag, srcPath+"/Cargo.toml")
+		if err != nil {
+			return err
+		}
+		if !release {
+			return nil
+		}
 		if err := rust.ReleaseLibrary(libConfig, srcPath); err != nil {
 			return err
 		}
@@ -166,22 +198,3 @@ func libraryByName(c *config.Config, name string) (*config.Library, error) {
 	}
 	return nil, errLibraryNotFound
 }
-
-// shouldReleaseLibrary looks up last release tag and returns true if any commits have been made
-// in the provided path since then.
-func shouldReleaseLibrary(ctx context.Context, cfg *config.Config, path string) (bool, error) {
-	if cfg.Release == nil {
-		return false, errReleaseConfigEmpty
-	}
-	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
-	}
-	numberOfChanges, err := git.ChangesInDirectorySinceTag(ctx, gitExe, lastTag, path)
-	if err != nil {
-		return false, err
-	}
-
-	return numberOfChanges > 0, nil
-}

internal/librarian/release_test.go

@@ -99,6 +99,19 @@ func TestReleaseCommand(t *testing.T) {
 			args:           []string{"librarian", "release", "--all"},
 			dirtyGitStatus: true,
 		},
+		{
+			name: "release config empty",
+			args: []string{"librarian", "release", "--all"},
+			srcPaths: map[string]string{
+				testlib:  "src/storage",
+				testlib2: "src/storage",
+			},
+			wantVersions: map[string]string{
+				testlib:  fakeReleaseVersion,
+				testlib2: fakeReleaseVersion,
+			},
+			wantErr: errReleaseConfigEmpty,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			testhelper.RequireCommand(t, "git")
@@ -125,6 +138,9 @@ func TestReleaseCommand(t *testing.T) {
 					},
 				},
 			}
+			if test.wantErr == errReleaseConfigEmpty {
+				cfg.Release = nil
+			}
 			if !test.skipYamlCreation {
 				if err := yaml.Write(configPath, cfg); err != nil {
 					t.Fatal(err)
@@ -229,6 +245,7 @@ func TestRelease(t *testing.T) {
 		name    string
 		srcPath string
 		version string
+		lastTag string
 	}{
 		{
 			name:    "library released",
@@ -250,7 +267,7 @@ func TestRelease(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			libConfg := &config.Library{}
-			err := releaseLibrary(t.Context(), cfg, libConfg, test.srcPath)
+			err := releaseLibrary(t.Context(), cfg, libConfg, test.srcPath, test.lastTag, "git")
 			if err != nil {
 				t.Fatalf("releaseLibrary() error = %v", err)
 			}
@@ -262,11 +279,72 @@ func TestRelease(t *testing.T) {
 	}
 }
 
-func TestMissingReleaseConfig(t *testing.T) {
-	cfg := &config.Config{}
-	_, err := shouldReleaseLibrary(t.Context(), cfg, "")
-	if !errors.Is(err, errReleaseConfigEmpty) {
-		t.Fatalf("Run() error = %v, wantErr %v", err, errReleaseConfigEmpty)
-	}
+func TestReleaseAll(t *testing.T) {
 
+	for _, test := range []struct {
+		name        string
+		libName     string
+		dir         string
+		skipPublish bool
+		wantVersion string
+	}{
+		{
+			name:        "library has changes",
+			libName:     "google-cloud-storage",
+			dir:         "src/storage",
+			wantVersion: "1.2.3",
+			skipPublish: false,
+		},
+		{
+			name:        "library does not have any changes",
+			libName:     "gax-internal",
+			dir:         "src/gax-internal",
+			wantVersion: "1.2.2",
+			skipPublish: false,
+		},
+		{
+			name:        "library does not have any changes on shared directory prefix",
+			libName:     "gax-internal",
+			dir:         "src/stor",
+			wantVersion: "1.2.2",
+			skipPublish: false,
+		},
+		{
+			name:        "library has changes but skipPublish is true",
+			libName:     "google-cloud-storage",
+			dir:         "src/storage",
+			wantVersion: "1.2.2",
+			skipPublish: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			testhelper.RequireCommand(t, "git")
+			tag := "v1.2.3"
+			config := &config.Config{
+				Language: languageFake,
+				Libraries: []*config.Library{
+					{
+						Name:        test.libName,
+						Version:     "1.2.2",
+						Output:      test.dir,
+						SkipPublish: test.skipPublish,
+					},
+				},
+				Release: &config.Release{
+					Remote:         "origin",
+					Branch:         "main",
+					IgnoredChanges: []string{},
+				},
+			}
+			remoteDir := testhelper.SetupRepoWithChange(t, tag)
+			testhelper.CloneRepository(t, remoteDir)
+			err := releaseAll(t.Context(), config, tag, "git")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if config.Libraries[0].Version != test.wantVersion {
+				t.Errorf("got version %s, want %s", config.Libraries[0].Version, test.wantVersion)
+			}
+		})
+	}
 }

internal/librarian/rust/update_manifest.go

@@ -39,7 +39,7 @@ type Cargo struct {
 
 // UpdateManifest bumps the version of a crate if it has changed since the last tag.
 func UpdateManifest(gitExe, lastTag, manifest string) ([]string, error) {
-	needsBump, err := manifestVersionNeedsBump(gitExe, lastTag, manifest)
+	needsBump, err := ManifestVersionNeedsBump(gitExe, lastTag, manifest)
 	if err != nil {
 		return nil, err
 	}
@@ -99,7 +99,9 @@ func UpdateCargoVersion(path, newVersion string) error {
 	return os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0644)
 }
 
-func manifestVersionNeedsBump(gitExe, lastTag, manifest string) (bool, error) {
+// ManifestVersionNeedsBump checks if the manifest version needs to be bumped.
+// It returns false if the version has already been updated since the last tag.
+func ManifestVersionNeedsBump(gitExe, lastTag, manifest string) (bool, error) {
 	delta := fmt.Sprintf("%s..HEAD", lastTag)
 	cmd := exec.Command(gitExe, "diff", delta, "--", manifest)
 	cmd.Dir = "."

internal/librarian/rust/update_manifest_test.go

@@ -215,7 +215,7 @@ func TestManifestVersionNeedsBumpSuccess(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	needsBump, err := manifestVersionNeedsBump("git", tag, name)
+	needsBump, err := ManifestVersionNeedsBump("git", tag, name)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -238,7 +238,7 @@ func TestManifestVersionNeedsBumpNewCrate(t *testing.T) {
 	}
 	name := path.Join("src", "new", "Cargo.toml")
 
-	needsBump, err := manifestVersionNeedsBump("git", tag, name)
+	needsBump, err := ManifestVersionNeedsBump("git", tag, name)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -252,7 +252,7 @@ func TestManifestVersionNeedsBumpNoChange(t *testing.T) {
 	testhelper.RequireCommand(t, "git")
 	testhelper.SetupForVersionBump(t, tag)
 	name := path.Join("src", "storage", "Cargo.toml")
-	needsBump, err := manifestVersionNeedsBump("git", tag, name)
+	needsBump, err := ManifestVersionNeedsBump("git", tag, name)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -266,7 +266,7 @@ func TestManifestVersionNeedsBumpBadDiff(t *testing.T) {
 	testhelper.RequireCommand(t, "git")
 	testhelper.SetupForVersionBump(t, tag)
 	name := path.Join("src", "storage", "Cargo.toml")
-	if updated, err := manifestVersionNeedsBump("git", "not-a-valid-tag", name); err == nil {
+	if updated, err := ManifestVersionNeedsBump("git", "not-a-valid-tag", name); err == nil {
 		t.Errorf("expected an error with an valid tag, got=%v", updated)
 	}
 }