feat(internal/librarian): remove name argument from add command (#3795)

Change the add command interface from:

`librarian add <name> [apis...]` to `librarian add <apis...>`.
 
The library name is now derived from the first API path using
language-specific logic:

Update tidy to only remove derivable API paths when there is exactly one
API. When there are multiple APIs, all paths are preserved.

Fixes #3778

Author: julieqiu

internal/librarian/add.go

@@ -21,58 +21,71 @@ import (
 	"slices"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/googleapis/librarian/internal/config"
+	"github.com/googleapis/librarian/internal/librarian/python"
+	"github.com/googleapis/librarian/internal/librarian/rust"
 	"github.com/urfave/cli/v3"
 )
 
 var (
 	errLibraryAlreadyExists = errors.New("library already exists in config")
-	errMissingLibraryName   = errors.New("must provide library name")
+	errMissingAPI           = errors.New("must provide at least one API")
 	errConfigNotFound       = errors.New("librarian.yaml not found")
 )
 
 func addCommand() *cli.Command {
 	return &cli.Command{
 		Name:      "add",
 		Usage:     "add a new client library to librarian.yaml",
-		UsageText: "librarian add <library> [apis...] [flags]",
+		UsageText: "librarian add <apis...> [flags]",
 		Action: func(ctx context.Context, c *cli.Command) error {
-			args := c.Args()
-			name := args.First()
-			if name == "" {
-				return errMissingLibraryName
-			}
-			var apis []string
-			if len(args.Slice()) > 1 {
-				apis = args.Slice()[1:]
+			apis := c.Args().Slice()
+			if len(apis) == 0 {
+				return errMissingAPI
 			}
 			cfg, err := loadConfig()
 			if err != nil {
 				return err
 			}
-			return runAdd(ctx, cfg, name, apis...)
+			return runAdd(ctx, cfg, apis...)
 		},
 	}
 }
 
-func runAdd(ctx context.Context, cfg *config.Config, name string, api ...string) error {
-	// check for existing libraries, if it exists return an error
+func runAdd(ctx context.Context, cfg *config.Config, apis ...string) error {
+	name := deriveLibraryName(cfg.Language, apis[0])
 	exists := slices.ContainsFunc(cfg.Libraries, func(lib *config.Library) bool {
 		return lib.Name == name
 	})
 	if exists {
 		return fmt.Errorf("%w: %s", errLibraryAlreadyExists, name)
 	}
 
-	cfg = addLibraryToLibrarianConfig(cfg, name, api...)
+	cfg = addLibraryToLibrarianConfig(cfg, name, apis...)
 	if err := RunTidyOnConfig(ctx, cfg); err != nil {
 		return err
 	}
 	return nil
 }
 
+// deriveLibraryName derives a library name from an API path.
+// The derivation is language-specific.
+func deriveLibraryName(language, api string) string {
+	switch language {
+	case languageFake:
+		return fakeDefaultLibraryName(api)
+	case languagePython:
+		return python.DefaultLibraryName(api)
+	case languageRust:
+		return rust.DefaultLibraryName(api)
+	default:
+		return strings.ReplaceAll(api, "/", "-")
+	}
+}
+
 func addLibraryToLibrarianConfig(cfg *config.Config, name string, api ...string) *config.Config {
 	lib := &config.Library{
 		Name:          name,

internal/librarian/add_test.go

@@ -32,38 +32,38 @@ func TestAddLibrary(t *testing.T) {
 	copyrightYear := strconv.Itoa(time.Now().Year())
 	for _, test := range []struct {
 		name                   string
-		libName                string
+		apis                   []string
 		initialLibraries       []*config.Library
 		wantFinalLibraries     []*config.Library
 		wantGeneratedOutputDir string
 		wantError              error
 	}{
 		{
 			name:                   "create new library",
-			libName:                "google-cloud-secretmanager",
+			apis:                   []string{"google/cloud/secretmanager/v1"},
 			initialLibraries:       []*config.Library{},
 			wantGeneratedOutputDir: "newlib-output",
 			wantFinalLibraries: []*config.Library{
 				{
-					Name:          "google-cloud-secretmanager",
+					Name:          "google-cloud-secretmanager-v1",
 					CopyrightYear: copyrightYear,
 				},
 			},
 		},
 		{
-			name:    "fail create existing library",
-			libName: "google-cloud-secretmanager",
+			name: "fail create existing library",
+			apis: []string{"google/cloud/secretmanager/v1"},
 			initialLibraries: []*config.Library{
 				{
-					Name: "google-cloud-secretmanager",
+					Name: "google-cloud-secretmanager-v1",
 				},
 			},
 			wantGeneratedOutputDir: "existing-output",
 			wantError:              errLibraryAlreadyExists,
 		},
 		{
-			name:    "create new library and tidy existing",
-			libName: "google-cloud-secretmanager",
+			name: "create new library and tidy existing",
+			apis: []string{"google/cloud/orgpolicy/v1"},
 			initialLibraries: []*config.Library{
 				{
 					Name: "existinglib",
@@ -81,7 +81,7 @@ func TestAddLibrary(t *testing.T) {
 					},
 				},
 				{
-					Name:          "google-cloud-secretmanager",
+					Name:          "google-cloud-orgpolicy-v1",
 					CopyrightYear: copyrightYear,
 				},
 			},
@@ -102,7 +102,7 @@ func TestAddLibrary(t *testing.T) {
 			if err := yaml.Write(librarianConfigPath, cfg); err != nil {
 				t.Fatal(err)
 			}
-			err = runAdd(t.Context(), cfg, test.libName)
+			err = runAdd(t.Context(), cfg, test.apis...)
 			if test.wantError != nil {
 				if !errors.Is(err, test.wantError) {
 					t.Errorf("expected error %v, got %v", test.wantError, err)
@@ -135,60 +135,34 @@ func TestAddCommand(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	testName := "google-cloud-secret-manager"
 	for _, test := range []struct {
 		name     string
-		args     []string
-		wantErr  error
+		apis     []string
+		wantName string
 		wantAPIs []*config.API
+		wantErr  error
 	}{
 		{
 			name:    "no args",
-			args:    []string{"librarian", "add"},
-			wantErr: errMissingLibraryName,
+			wantErr: errMissingAPI,
 		},
 		{
-			name: "library name only",
-			args: []string{
-				"librarian",
-				"add",
-				testName,
-			},
+			name:     "single API",
+			apis:     []string{"google/cloud/secretmanager/v1"},
+			wantName: "google-cloud-secretmanager-v1",
 		},
 		{
-			name: "library with single API",
-			args: []string{
-				"librarian",
-				"add",
-				testName,
-				"google/cloud/secretmanager/v1",
-			},
-			wantAPIs: []*config.API{
-				{
-					Path: "google/cloud/secretmanager/v1",
-				},
-			},
-		},
-		{
-			name: "library with multiple APIs",
-			args: []string{
-				"librarian",
-				"add",
-				testName,
+			name: "multiple APIs",
+			apis: []string{
 				"google/cloud/secretmanager/v1",
 				"google/cloud/secretmanager/v1beta2",
 				"google/cloud/secrets/v1beta1",
 			},
+			wantName: "google-cloud-secretmanager-v1",
 			wantAPIs: []*config.API{
-				{
-					Path: "google/cloud/secretmanager/v1",
-				},
-				{
-					Path: "google/cloud/secretmanager/v1beta2",
-				},
-				{
-					Path: "google/cloud/secrets/v1beta1",
-				},
+				{Path: "google/cloud/secretmanager/v1"},
+				{Path: "google/cloud/secretmanager/v1beta2"},
+				{Path: "google/cloud/secrets/v1beta1"},
 			},
 		},
 	} {
@@ -203,7 +177,8 @@ func TestAddCommand(t *testing.T) {
 			if err := yaml.Write(librarianConfigPath, cfg); err != nil {
 				t.Fatal(err)
 			}
-			err := Run(t.Context(), test.args...)
+			args := append([]string{"librarian", "add"}, test.apis...)
+			err := Run(t.Context(), args...)
 			if test.wantErr != nil {
 				if !errors.Is(err, test.wantErr) {
 					t.Fatalf("want error %v, got %v", test.wantErr, err)
@@ -218,14 +193,12 @@ func TestAddCommand(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			got, err := findLibrary(gotCfg, testName)
+			got, err := findLibrary(gotCfg, test.wantName)
 			if err != nil {
 				t.Fatal(err)
 			}
-			if test.wantAPIs != nil {
-				if diff := cmp.Diff(test.wantAPIs, got.APIs); diff != "" {
-					t.Errorf("apis mismatch (-want +got):\n%s", diff)
-				}
+			if diff := cmp.Diff(test.wantAPIs, got.APIs); diff != "" {
+				t.Errorf("apis mismatch (-want +got):\n%s", diff)
 			}
 		})
 	}
@@ -238,10 +211,6 @@ func TestAddLibraryToLibrarianYaml(t *testing.T) {
 		apis        []string
 		want        []*config.API
 	}{
-		{
-			name:        "library with no specification-source",
-			libraryName: "newlib",
-		},
 		{
 			name:        "library with single API",
 			libraryName: "newlib",
@@ -305,3 +274,27 @@ func TestAddLibraryToLibrarianYaml(t *testing.T) {
 		})
 	}
 }
+
+func TestDeriveLibraryName(t *testing.T) {
+	for _, test := range []struct {
+		language string
+		apiPath  string
+		want     string
+	}{
+		{"python", "google/cloud/secretmanager/v1", "google-cloud-secretmanager"},
+		{"python", "google/cloud/secretmanager/v1beta2", "google-cloud-secretmanager"},
+		{"python", "google/cloud/storage/v2alpha", "google-cloud-storage"},
+		{"python", "google/maps/addressvalidation/v1", "google-maps-addressvalidation"},
+		{"python", "google/api/v1", "google-api"},
+		{"rust", "google/cloud/secretmanager/v1", "google-cloud-secretmanager-v1"},
+		{"rust", "google/cloud/secretmanager/v1beta2", "google-cloud-secretmanager-v1beta2"},
+		{"fake", "google/cloud/secretmanager/v1", "google-cloud-secretmanager-v1"},
+	} {
+		t.Run(test.language+"/"+test.apiPath, func(t *testing.T) {
+			got := deriveLibraryName(test.language, test.apiPath)
+			if got != test.want {
+				t.Errorf("deriveLibraryName(%q, %q) = %q, want %q", test.language, test.apiPath, got, test.want)
+			}
+		})
+	}
+}

internal/librarian/fake.go

@@ -79,3 +79,9 @@ func fakeCreateSkeleton(library *config.Library) error {
 	starterPath := filepath.Join(library.Output, "STARTER.md")
 	return os.WriteFile(starterPath, []byte(content), 0644)
 }
+
+// fakeDefaultLibraryName derives a library name from an API path by
+// replacing "/" with "-".
+func fakeDefaultLibraryName(api string) string {
+	return strings.ReplaceAll(api, "/", "-")
+}

internal/librarian/python/generate.go

@@ -297,3 +297,16 @@ func cleanUpFilesAfterPostProcessing(repoRoot string) error {
 func DefaultOutputByName(name, defaultOutput string) string {
 	return filepath.Join(defaultOutput, name)
 }
+
+// DefaultLibraryName derives a library name from an API path by stripping
+// the version suffix and replacing "/" with "-".
+// For example: "google/cloud/secretmanager/v1" ->
+// "google-cloud-secretmanager".
+func DefaultLibraryName(api string) string {
+	path := api
+	if v := filepath.Base(api); len(v) > 1 && v[0] == 'v' && v[1] >= '0' && v[1] <= '9' {
+		// Strip version suffix (v1, v1beta2, v2alpha, etc.).
+		path = filepath.Dir(api)
+	}
+	return strings.ReplaceAll(path, "/", "-")
+}

internal/librarian/python/generate_test.go

@@ -382,6 +382,27 @@ func TestDefaultOutputByName(t *testing.T) {
 	}
 }
 
+func TestDefaultLibraryName(t *testing.T) {
+	for _, test := range []struct {
+		api  string
+		want string
+	}{
+		{"google/cloud/secretmanager/v1", "google-cloud-secretmanager"},
+		{"google/cloud/secretmanager/v1beta2", "google-cloud-secretmanager"},
+		{"google/cloud/storage/v2alpha", "google-cloud-storage"},
+		{"google/maps/addressvalidation/v1", "google-maps-addressvalidation"},
+		{"google/api/v1", "google-api"},
+		{"google/cloud/vision", "google-cloud-vision"},
+	} {
+		t.Run(test.api, func(t *testing.T) {
+			got := DefaultLibraryName(test.api)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func requirePythonModule(t *testing.T, module string) {
 	t.Helper()
 	cmd := exec.Command("python3", "-c", fmt.Sprintf("import %s", module))

internal/librarian/rust/generate.go

@@ -160,9 +160,9 @@ func Keep(library *config.Library) ([]string, error) {
 	return keep, nil
 }
 
-// defaultLibraryName derives a library name from a api path.
+// DefaultLibraryName derives a library name from a api path.
 // For example: google/cloud/secretmanager/v1 -> google-cloud-secretmanager-v1.
-func defaultLibraryName(api string) string {
+func DefaultLibraryName(api string) string {
 	return strings.ReplaceAll(api, "/", "-")
 }
 

internal/librarian/rust/generate_test.go

@@ -303,7 +303,7 @@ func TestDefaultLibraryName(t *testing.T) {
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			got := defaultLibraryName(test.api)
+			got := DefaultLibraryName(test.api)
 			if diff := cmp.Diff(test.want, got); diff != "" {
 				t.Errorf("mismatch (-want +got):\n%s", diff)
 			}

internal/librarian/tidy.go

@@ -73,10 +73,10 @@ func tidyLibrary(cfg *config.Config, lib *config.Library) error {
 		// Veneers are never generated, so ensure skip_generate is false.
 		lib.SkipGenerate = false
 	}
-	for _, ch := range lib.APIs {
-		if isDerivableAPIPath(cfg.Language, lib.Name, ch.Path) {
-			ch.Path = ""
-		}
+	// Only remove derivable API paths when there's exactly one API.
+	// When there are multiple APIs, preserve all of them.
+	if len(lib.APIs) == 1 && isDerivableAPIPath(cfg.Language, lib.Name, lib.APIs[0].Path) {
+		lib.APIs[0].Path = ""
 	}
 	lib.APIs = slices.DeleteFunc(lib.APIs, func(ch *config.API) bool {
 		return ch.Path == ""