feat(librarian): change publish/tag library flags to release-commit (#4141)

Instead of specifying the library that should be used to find a release
commit, after this change the release commit is specified directly. This
is a lot less confusing, as otherwise there was an implication that
*only* that library would be published/tagged.

Fixes #4053.

Author: jskeet

cmd/librarian/doc.go

@@ -169,12 +169,12 @@ USAGE:
 
 OPTIONS:
 
-	--execute             fully publish (default is to only perform a dry run)
-	--library string      library to find a release commit for; default finds latest release commit for any library
-	--dry-run             print commands without executing (legacy Rust-only flag)
-	--dry-run-keep-going  print commands without executing, don't stop on error (legacy Rust-only flag)
-	--skip-semver-checks  skip semantic versioning checks (legacy Rust-only flag)
-	--help, -h            show help
+	--execute                fully publish (default is to only perform a dry run)
+	--release-commit string  the release commit to publish; default finds latest release commit
+	--dry-run                print commands without executing (legacy Rust-only flag)
+	--dry-run-keep-going     print commands without executing, don't stop on error (legacy Rust-only flag)
+	--skip-semver-checks     skip semantic versioning checks (legacy Rust-only flag)
+	--help, -h               show help
 
 GLOBAL OPTIONS:
 
@@ -192,8 +192,8 @@ USAGE:
 
 OPTIONS:
 
-	--library string  library to find a release commit for; default finds latest release commit for any library
-	--help, -h        show help
+	--release-commit string  the release commit to tag; default finds latest release commit
+	--help, -h               show help
 
 GLOBAL OPTIONS:
 

internal/librarian/bump.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 	"strings"
 
 	"github.com/googleapis/librarian/internal/command"
@@ -309,12 +308,12 @@ func findReleasedLibraries(cfgBefore, cfgAfter *config.Config) ([]string, error)
 }
 
 // findLatestReleaseCommitHash finds the latest (most recent) commit hash
-// which released the library named by libraryName, or which released any libraries
-// if libraryName is empty. (See findReleasedLibraries for the definition of what it
-// means for a commit to release a library.) Importantly, it does this *without*
-// using tags, as it's used in circumstances where the full release process has
-// not yet been completed (e.g. to find which commit *should* be tagged).
-func findLatestReleaseCommitHash(ctx context.Context, gitExe, libraryName string) (string, error) {
+// which released any libraries. (See findReleasedLibraries for the definition
+// of what it means for a commit to release a library.) Importantly, it does
+// this *without* using tags, as it's used in circumstances where the full
+// release process has not yet been completed (e.g. to find which commit
+// *should* be tagged).
+func findLatestReleaseCommitHash(ctx context.Context, gitExe string) (string, error) {
 	commits, err := git.FindCommitsForPath(ctx, gitExe, librarianConfigPath)
 	if err != nil {
 		return "", err
@@ -342,7 +341,7 @@ func findLatestReleaseCommitHash(ctx context.Context, gitExe, libraryName string
 			if err != nil {
 				return "", err
 			}
-			if len(released) > 0 && (libraryName == "" || slices.Contains(released, libraryName)) {
+			if len(released) > 0 {
 				return candidateCommit, nil
 			}
 		}

internal/librarian/bump_test.go

@@ -915,12 +915,11 @@ func TestFindLatestReleaseCommitHash(t *testing.T) {
 	for _, test := range []struct {
 		name            string
 		setup           func(cfg *config.Config)
-		libraryName     string
 		wantCommitCount int
 		wantCommitIndex int // Commit index in the log: HEAD=0, HEAD~=1 etc
 	}{
 		{
-			name: "HEAD commit releases, match any release",
+			name: "HEAD commit releases",
 			setup: func(cfg *config.Config) {
 				// 2 commits in addition to the two in Setup:
 				// - Chore commit with a modified readme
@@ -933,7 +932,7 @@ func TestFindLatestReleaseCommitHash(t *testing.T) {
 			wantCommitIndex: 0,
 		},
 		{
-			name: "HEAD~ commit, match any release",
+			name: "HEAD~ commit",
 			setup: func(cfg *config.Config) {
 				// 3 commits in addition to the two in Setup:
 				// - Chore commit with a modified readme
@@ -947,26 +946,6 @@ func TestFindLatestReleaseCommitHash(t *testing.T) {
 			wantCommitCount: 5,
 			wantCommitIndex: 1,
 		},
-		{
-			name: "match specific library",
-			setup: func(cfg *config.Config) {
-				// 4 commits in addition to the two in Setup:
-				// - Chore commit with a modified readme
-				// - Release commit with the first library version bumped
-				// - Chore commit with another modified readme
-				// - Release commit with the second library version bumped
-				// (We're looking for the first library, so effectively HEAD~2)
-				writeReadmeAndCommit(t, "modified readme")
-				cfg.Libraries[0].Version = "1.1.0"
-				writeConfigAndCommit(t, cfg)
-				writeReadmeAndCommit(t, "modified readme again")
-				cfg.Libraries[1].Version = "1.3.0"
-				writeConfigAndCommit(t, cfg)
-			},
-			libraryName:     sample.Lib1Name,
-			wantCommitCount: 6,
-			wantCommitIndex: 2,
-		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			cfg := &config.Config{
@@ -988,7 +967,7 @@ func TestFindLatestReleaseCommitHash(t *testing.T) {
 			if test.wantCommitCount != len(commits) {
 				t.Fatalf("expected setup to create %d commits; got %d", test.wantCommitCount, len(commits))
 			}
-			got, err := findLatestReleaseCommitHash(t.Context(), "git", test.libraryName)
+			got, err := findLatestReleaseCommitHash(t.Context(), "git")
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1005,7 +984,6 @@ func TestFindLatestReleaseCommitHash_Error(t *testing.T) {
 	for _, test := range []struct {
 		name                      string
 		setup                     func(cfg *config.Config)
-		libraryName               string
 		wantReleaseCommitNotFound bool
 	}{
 		{
@@ -1017,24 +995,6 @@ func TestFindLatestReleaseCommitHash_Error(t *testing.T) {
 			},
 			wantReleaseCommitNotFound: true,
 		},
-		{
-			name: "no library with given name",
-			setup: func(cfg *config.Config) {
-				cfg.Libraries[0].Version = "1.1.0"
-				writeConfigAndCommit(t, cfg)
-			},
-			libraryName:               "nonexistent",
-			wantReleaseCommitNotFound: true,
-		},
-		{
-			name: "release, but not for the specified library",
-			setup: func(cfg *config.Config) {
-				cfg.Libraries[0].Version = "1.1.0"
-				writeConfigAndCommit(t, cfg)
-			},
-			libraryName:               sample.Lib2Name,
-			wantReleaseCommitNotFound: true,
-		},
 		{
 			name: "invalid release",
 			setup: func(cfg *config.Config) {
@@ -1080,7 +1040,7 @@ func TestFindLatestReleaseCommitHash_Error(t *testing.T) {
 			}
 			testhelper.Setup(t, opts)
 			test.setup(cfg)
-			got, err := findLatestReleaseCommitHash(t.Context(), "git", test.libraryName)
+			got, err := findLatestReleaseCommitHash(t.Context(), "git")
 			if err == nil {
 				t.Errorf("expected error; succeeded with hash %s", got)
 			}

internal/librarian/publish.go

@@ -37,8 +37,8 @@ func publishCommand() *cli.Command {
 				Usage: "fully publish (default is to only perform a dry run)",
 			},
 			&cli.StringFlag{
-				Name:  "library",
-				Usage: "library to find a release commit for; default finds latest release commit for any library",
+				Name:  "release-commit",
+				Usage: "the release commit to publish; default finds latest release commit",
 			},
 			&cli.BoolFlag{
 				Name:  "dry-run",
@@ -61,7 +61,7 @@ func publishCommand() *cli.Command {
 			if cfg.Language == languageRust {
 				return legacyRustPublish(ctx, cfg, cmd)
 			}
-			return publish(ctx, cfg, cmd.String("library"), cmd.Bool("execute"))
+			return publish(ctx, cfg, cmd.String("release-commit"), cmd.Bool("execute"))
 		},
 	}
 }
@@ -81,22 +81,25 @@ func legacyRustPublish(ctx context.Context, cfg *config.Config, cmd *cli.Command
 // at HEAD, just to find the git executable to use, after which it finds the
 // release commit to publish. The configuration at the release commit is used
 // for all further operations (and the repo will be checked out at that commit).
-// The library flag allows a user to identify a specific release to publish, in
-// case of overlapping releases being performed. The execute flag says whether to
-// actually publish (true) or just perform a dry run (false).
-func publish(ctx context.Context, cfg *config.Config, library string, execute bool) error {
+// The releaseCommit flag allows a user to identify a specific release commit to
+// publish, in case of overlapping releases being performed. The execute flag
+// says whether to actually publish (true) or just perform a dry run (false).
+func publish(ctx context.Context, cfg *config.Config, releaseCommit string, execute bool) error {
 	gitExe := "git"
 	if cfg.Release != nil {
 		gitExe = command.GetExecutablePath(cfg.Release.Preinstalled, "git")
 	}
 	if err := git.AssertGitStatusClean(ctx, gitExe); err != nil {
 		return err
 	}
-	releaseCommitHash, err := findLatestReleaseCommitHash(ctx, gitExe, library)
-	if err != nil {
-		return err
+	var err error
+	if releaseCommit == "" {
+		releaseCommit, err = findLatestReleaseCommitHash(ctx, gitExe)
+		if err != nil {
+			return err
+		}
 	}
-	if err := git.Checkout(ctx, gitExe, releaseCommitHash); err != nil {
+	if err := git.Checkout(ctx, gitExe, releaseCommit); err != nil {
 		return err
 	}
 	// Reload the config after checking out the release commit.
@@ -121,6 +124,9 @@ func publish(ctx context.Context, cfg *config.Config, library string, execute bo
 	if err != nil {
 		return err
 	}
+	if len(librariesToPublish) == 0 {
+		return fmt.Errorf("error publishing %s: %w", releaseCommit, errNoLibrariesAtReleaseCommit)
+	}
 
 	switch cfg.Language {
 	case languageFake:

internal/librarian/publish_test.go

@@ -15,12 +15,14 @@
 package librarian
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/librarian/internal/config"
+	"github.com/googleapis/librarian/internal/git"
 	"github.com/googleapis/librarian/internal/sample"
 	"github.com/googleapis/librarian/internal/testhelper"
 )
@@ -29,11 +31,11 @@ func TestPublish(t *testing.T) {
 	// Each test starts (before setup) with Lib1Name with a version of 1.0.0 and
 	// Lib2Name with a version of 1.2.0.
 	for _, test := range []struct {
-		name    string
-		setup   func(cfg *config.Config)
-		library string
-		execute bool
-		want    string
+		name          string
+		setup         func(cfg *config.Config)
+		releaseCommit string
+		execute       bool
+		want          string
 	}{
 		{
 			name: "publish Lib1Name and Lib2Name",
@@ -63,23 +65,26 @@ func TestPublish(t *testing.T) {
 			want: fmt.Sprintf("libraries=%s; execute=false", sample.Lib1Name),
 		},
 		{
-			name: "publish Lib1Name, specified in flags, with a later release of Lib2Name ignored",
+			name: "publish Lib1Name due to release commit specified in flags, with a later release of Lib2Name ignored",
 			setup: func(cfg *config.Config) {
 				cfg.Libraries[0].Version = "1.1.0"
 				writeConfigAndCommit(t, cfg)
 				cfg.Libraries[1].Version = "1.3.0"
 				writeConfigAndCommit(t, cfg)
 			},
-			library: sample.Lib1Name,
-			want:    fmt.Sprintf("libraries=%s; execute=false", sample.Lib1Name),
+			// Fortunately this doesn't have to be an actual commit, just a
+			// committish, so we don't need to know the hash of the HEAD~
+			// commit when creating the test data.
+			releaseCommit: "HEAD~",
+			want:          fmt.Sprintf("libraries=%s; execute=false", sample.Lib1Name),
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			cfg := sample.Config()
 			cfg.Libraries[1].Version = "1.2.0"
 			testhelper.Setup(t, testhelper.SetupOptions{Config: cfg})
 			test.setup(cfg)
-			if err := publish(t.Context(), cfg, test.library, test.execute); err != nil {
+			if err := publish(t.Context(), cfg, test.releaseCommit, test.execute); err != nil {
 				t.Fatal(err)
 			}
 			got, err := os.ReadFile(fakePublishedFile)
@@ -95,9 +100,10 @@ func TestPublish(t *testing.T) {
 
 func TestPublish_Error(t *testing.T) {
 	for _, test := range []struct {
-		name    string
-		setup   func(cfg *config.Config)
-		library string
+		name          string
+		setup         func(cfg *config.Config)
+		releaseCommit string
+		wantErr       error
 	}{
 		{
 			name: "custom tool specified for git and doesn't exist",
@@ -111,6 +117,7 @@ func TestPublish_Error(t *testing.T) {
 				}
 				writeConfigAndCommit(t, cfg)
 			},
+			// Can't easily check this error.
 		},
 		{
 			name: "repo is dirty",
@@ -122,6 +129,7 @@ func TestPublish_Error(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
+			wantErr: git.ErrGitStatusUnclean,
 		},
 		{
 			name: "language isn't supported",
@@ -131,30 +139,42 @@ func TestPublish_Error(t *testing.T) {
 				cfg.Language = "unsupported-for-publish"
 				writeConfigAndCommit(t, cfg)
 			},
+			// Can't easily check this error.
 		},
 		{
 			name: "no release commit",
 			setup: func(cfg *config.Config) {
 			},
 		},
 		{
-			name: "no release commit for specified library",
+			name: "specified release commit is invalid",
 			setup: func(cfg *config.Config) {
-				cfg.Libraries[1].Version = "1.3.0"
-				writeConfigAndCommit(t, cfg)
 			},
-			library: sample.Lib1Name,
+			releaseCommit: "not a commit",
+			// Can't easily check this error
+		},
+		{
+			name: "release commit does not release anything",
+			setup: func(cfg *config.Config) {
+				writeFileAndCommit(t, "README.txt", []byte("Just a readme"), "Modified config")
+			},
+			releaseCommit: "HEAD",
+			wantErr:       errNoLibrariesAtReleaseCommit,
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			cfg := sample.Config()
 			cfg.Libraries[1].Version = "1.2.0"
 			testhelper.Setup(t, testhelper.SetupOptions{Config: cfg})
 			test.setup(cfg)
-			err := publish(t.Context(), cfg, test.library, false)
+			err := publish(t.Context(), cfg, test.releaseCommit, false)
 			if err == nil {
-				t.Errorf("publish(): expected error, got none")
+				t.Fatal("expected error, got nil")
 			}
+			if test.wantErr != nil && !errors.Is(err, test.wantErr) {
+				t.Errorf("expected %v, got %v", test.wantErr, err)
+			}
+
 		})
 	}
 }
@@ -170,7 +190,7 @@ func TestPublishCommand(t *testing.T) {
 	cfg.Libraries[1].Version = "1.3.0"
 	writeConfigAndCommit(t, cfg)
 
-	if err := Run(t.Context(), "librarian", "publish", "--library", sample.Lib1Name, "--execute"); err != nil {
+	if err := Run(t.Context(), "librarian", "publish", "--release-commit", "HEAD~", "--execute"); err != nil {
 		t.Fatal(err)
 	}
 	want := fmt.Sprintf("libraries=%s; execute=true", sample.Lib1Name)