chore: refactor to remove service config from librarian yaml if it can find it in googleapis (#3370)

Update logic in RunTidy, to only remove the service config entry from
librarian.yaml if it can find it in googleapis repo. (Safer
implementation).

This includes a refactor to make FetchSource available outside of
generate command.

[Generated librarian yaml
file](https://paste.googleplex.com/5634686921605120)

Fixes #3358

---------

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

Author: ldetmer

devtools/cmd/migrate-librarian/main.go

@@ -135,7 +135,7 @@ func run(ctx context.Context, args []string) error {
 		return fmt.Errorf("failed to write config: %w", err)
 	}
 
-	if err := librarian.RunTidy(); err != nil {
+	if err := librarian.RunTidy(ctx); err != nil {
 		return errTidyFailed
 	}
 

devtools/cmd/migrate-sidekick/main.go

@@ -16,6 +16,7 @@
 package main
 
 import (
+	"context"
 	"errors"
 	"flag"
 	"fmt"
@@ -75,13 +76,13 @@ func readCargoConfig(dir string) (*rustrelease.Cargo, error) {
 }
 
 func main() {
-	if err := run(os.Args[1:]); err != nil {
+	if err := run(context.Background(), os.Args[1:]); err != nil {
 		slog.Error("migrate-sidekick failed", "error", err)
 		os.Exit(1)
 	}
 }
 
-func run(args []string) error {
+func run(ctx context.Context, args []string) error {
 	flagSet := flag.NewFlagSet("migrate-sidekick", flag.ContinueOnError)
 	outputPath := flagSet.String("output", "./librarian.yaml", "Output file path (default: ./librarian.yaml)")
 	if err := flagSet.Parse(args); err != nil {
@@ -136,7 +137,7 @@ func run(args []string) error {
 	}
 	slog.Info("Wrote config to output file", "path", *outputPath)
 
-	if err := librarian.RunTidy(); err != nil {
+	if err := librarian.RunTidy(ctx); err != nil {
 		slog.Error(errTidyFailed.Error(), "error", err)
 		return errTidyFailed
 	}

devtools/cmd/migrate-sidekick/main_test.go

@@ -840,7 +840,7 @@ func TestRunMigrateCommand(t *testing.T) {
 			wantReleaseBranch := "main"
 			wantReleaseRemote := "upstream"
 
-			if err := run([]string{test.path}); err != nil {
+			if err := run(t.Context(), []string{test.path}); err != nil {
 				if test.wantErr == nil {
 					t.Fatal(err)
 				}

devtools/cmd/migrate-sidekick/testdata/run/success/.sidekick.toml

@@ -0,0 +1,3 @@
+[source]
+googleapis-root             = 'https://github.com/googleapis/googleapis/archive/fe58211356a91f4140ed51893703910db05ade91.tar.gz'
+googleapis-sha256           = 'c809264b7973949c157aaaacfbab015263fd8cea3a262a9297a676f4b92dd0bd'

internal/librarian/generate.go

@@ -24,7 +24,6 @@ import (
 	"strings"
 
 	"github.com/googleapis/librarian/internal/config"
-	"github.com/googleapis/librarian/internal/fetch"
 	"github.com/googleapis/librarian/internal/librarian/internal/python"
 	"github.com/googleapis/librarian/internal/librarian/internal/rust"
 	"github.com/googleapis/librarian/internal/serviceconfig"
@@ -326,21 +325,6 @@ func formatLibrary(ctx context.Context, language string, library *config.Library
 	return fmt.Errorf("format not implemented for %q", language)
 }
 
-func fetchSource(ctx context.Context, source *config.Source, repo string) (string, error) {
-	if source == nil {
-		return "", nil
-	}
-	if source.Dir != "" {
-		return source.Dir, nil
-	}
-
-	dir, err := fetch.RepoDir(ctx, repo, source.Commit, source.SHA256)
-	if err != nil {
-		return "", fmt.Errorf("failed to fetch %s: %w", repo, err)
-	}
-	return dir, nil
-}
-
 // cleanOutput removes all files in dir except those in keep. The keep list
 // should contain paths relative to dir. It returns an error if the directory
 // does not exist or any file in keep does not exist.

internal/librarian/source.go

@@ -0,0 +1,39 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package librarian
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/googleapis/librarian/internal/config"
+	"github.com/googleapis/librarian/internal/fetch"
+)
+
+// fetchSource fetches a repository source.
+func fetchSource(ctx context.Context, source *config.Source, repo string) (string, error) {
+	if source == nil {
+		return "", nil
+	}
+	if source.Dir != "" {
+		return source.Dir, nil
+	}
+
+	dir, err := fetch.RepoDir(ctx, repo, source.Commit, source.SHA256)
+	if err != nil {
+		return "", fmt.Errorf("failed to fetch %s: %w", repo, err)
+	}
+	return dir, nil
+}

internal/librarian/tidy.go

@@ -22,13 +22,15 @@ import (
 	"strings"
 
 	"github.com/googleapis/librarian/internal/config"
+	"github.com/googleapis/librarian/internal/serviceconfig"
 	"github.com/googleapis/librarian/internal/yaml"
 	"github.com/urfave/cli/v3"
 )
 
 var (
-	errDuplicateLibraryName = errors.New("duplicate library name")
-	errDuplicateChannelPath = errors.New("duplicate channel path")
+	errDuplicateLibraryName  = errors.New("duplicate library name")
+	errDuplicateChannelPath  = errors.New("duplicate channel path")
+	errNoGoogleapiSourceInfo = errors.New("googleapis source information is not provided in Librarian.yaml file")
 )
 
 func tidyCommand() *cli.Command {
@@ -37,13 +39,13 @@ func tidyCommand() *cli.Command {
 		Usage:     "format and validate librarian.yaml",
 		UsageText: "librarian tidy [path]",
 		Action: func(ctx context.Context, cmd *cli.Command) error {
-			return RunTidy()
+			return RunTidy(ctx)
 		},
 	}
 }
 
 // RunTidy formats and validates the librarian configuration file.
-func RunTidy() error {
+func RunTidy(ctx context.Context) error {
 	cfg, err := yaml.Read[config.Config](librarianConfigPath)
 	if err != nil {
 		return err
@@ -52,6 +54,14 @@ func RunTidy() error {
 		return err
 	}
 
+	if cfg.Sources == nil || cfg.Sources.Googleapis == nil {
+		return errNoGoogleapiSourceInfo
+	}
+	googleapisDir, err := fetchSource(ctx, cfg.Sources.Googleapis, googleapisRepo)
+	if err != nil {
+		return err
+	}
+
 	for _, lib := range cfg.Libraries {
 		if lib.Output != "" && len(lib.Channels) == 1 && isDerivableOutput(cfg, lib) {
 			lib.Output = ""
@@ -60,7 +70,7 @@ func RunTidy() error {
 			if isDerivableChannelPath(cfg.Language, lib, ch) {
 				ch.Path = ""
 			}
-			if isDerivableServiceConfig(cfg.Language, lib, ch) {
+			if isDerivableServiceConfig(cfg.Language, lib, ch, googleapisDir) {
 				ch.ServiceConfig = ""
 			}
 		}
@@ -82,38 +92,19 @@ func isDerivableChannelPath(language string, lib *config.Library, ch *config.Cha
 	return ch.Path == deriveChannelPath(language, lib)
 }
 
-func isDerivableServiceConfig(language string, lib *config.Library, ch *config.Channel) bool {
+func isDerivableServiceConfig(language string, lib *config.Library, ch *config.Channel, googleapisDir string) bool {
+	if ch.ServiceConfig == "" {
+		return false
+	}
 	path := ch.Path
 	if path == "" {
 		path = deriveChannelPath(language, lib)
 	}
-	return ch.ServiceConfig != "" && ch.ServiceConfig == deriveServiceConfig(path)
-}
-
-// deriveServiceConfig returns the conventionally derived service config
-// path for a given channel. For example, if resolved_path is
-// "google/cloud/speech/v1", it derives to
-// "google/cloud/speech/v1/speech_v1.yaml".
-//
-// It returns an empty string if the resolved path does not contain sufficient
-// components or if the version component does not start with 'v'.
-//
-// This is only used by librarian tidy as a heuristic for serviceconfig.Find,
-// since that command currently does not have access to the googleapis
-// directory.
-//
-// TODO(https://github.com/googleapis/librarian/issues/3358): use
-// serviceconfig.Find instead.
-func deriveServiceConfig(resolvedPath string) string {
-	parts := strings.Split(resolvedPath, "/")
-	if len(parts) >= 2 {
-		version := parts[len(parts)-1]
-		service := parts[len(parts)-2]
-		if strings.HasPrefix(version, "v") {
-			return fmt.Sprintf("%s/%s_%s.yaml", resolvedPath, service, version)
-		}
+	derived, err := serviceconfig.Find(googleapisDir, path)
+	if err != nil {
+		return false
 	}
-	return ""
+	return ch.ServiceConfig == derived
 }
 
 func validateLibraries(cfg *config.Config) error {

internal/librarian/tidy_test.go

@@ -95,7 +95,8 @@ func TestTidyCommand(t *testing.T) {
 	configContent := `language: rust
 sources:
   googleapis:
-    commit: abc123
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-storage-v1
     version: "1.0.0"
@@ -139,6 +140,10 @@ func TestTidy_DerivableFields(t *testing.T) {
 		{
 			name: "derivable fields removed",
 			configContent: `
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-accessapproval-v1
     channels:
@@ -153,34 +158,28 @@ libraries:
 		{
 			name: "non-derivable path not removed",
 			configContent: `
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
-  - name: google-cloud-api-servicecontrol-v1
-    channels:
-      - path: google/api/servicecontrol/v1
-        service_config: google/api/servicecontrol/v1/servicecontrol.yaml
-`,
-			wantPath:     "google/api/servicecontrol/v1",
-			wantSvcCfg:   "google/api/servicecontrol/v1/servicecontrol.yaml",
-			wantNumLibs:  1,
-			wantNumChnls: 1,
-		},
-		{
-			name: "only derivable service config removed",
-			configContent: `
-libraries:
-  - name: google-cloud-vision-v1
+  - name: google-cloud-aiplatform-v1-schema-predict-instance
     channels:
-      - path: google/some/other/domain/vision/v1
-        service_config: google/some/other/domain/vision/v1/vision_v1.yaml
+      - path: src/generated/cloud/aiplatform/schema/predict/instance
+        service_config: google/cloud/aiplatform/v1/schema/aiplatform_v1.yaml
 `,
-			wantPath:     "google/some/other/domain/vision/v1",
-			wantSvcCfg:   "",
+			wantPath:     "src/generated/cloud/aiplatform/schema/predict/instance",
+			wantSvcCfg:   "google/cloud/aiplatform/v1/schema/aiplatform_v1.yaml",
 			wantNumLibs:  1,
 			wantNumChnls: 1,
 		},
 		{
 			name: "path needs to be resolved",
 			configContent: `
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-vision-v1
     channels:
@@ -194,6 +193,10 @@ libraries:
 		{
 			name: "service config not derivable (no version at end of path)",
 			configContent: `
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-speech
     channels:
@@ -208,6 +211,10 @@ libraries:
 		{
 			name: "channel removed if service config does not exist",
 			configContent: `
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-orgpolicy-v1
     channels:
@@ -288,6 +295,10 @@ func TestTidy_DerivableOutput(t *testing.T) {
 language: rust
 default:
   output: generated/
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 libraries:
   - name: google-cloud-secretmanager-v1
     output: generated/cloud/secretmanager/v1
@@ -297,7 +308,7 @@ libraries:
 	if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
 		t.Fatal(err)
 	}
-	if err := RunTidy(); err != nil {
+	if err := RunTidy(t.Context()); err != nil {
 		t.Fatal(err)
 	}
 	cfg, err := yaml.Read[config.Config](configPath)
@@ -323,6 +334,10 @@ func TestTidyLanguageConfig_Rust(t *testing.T) {
 			name: "empty_module_removed",
 			configContent: `
 language: rust
+sources:
+  googleapis:
+    commit: 94ccedca05acb0bb60780789e93371c9e4100ddc
+    sha256: fff40946e897d96bbdccd566cb993048a87029b7e08eacee3fe99eac792721ba
 default:
   output: generated/
 libraries:
@@ -367,3 +382,26 @@ libraries:
 		})
 	}
 }
+
+func TestTidyCommandMissingGoogleApisSource(t *testing.T) {
+	tempDir := t.TempDir()
+	t.Chdir(tempDir)
+	configPath := filepath.Join(tempDir, librarianConfigPath)
+	configContent := `language: rust
+libraries:
+  - name: google-cloud-storage-v1
+    version: "1.0.0"
+  - name: google-cloud-bigquery-v1
+    version: "2.0.0"
+`
+	if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
+		t.Fatal(err)
+	}
+	err := Run(t.Context(), "librarian", "tidy")
+	if err == nil {
+		t.Fatalf("expected error, got %v", nil)
+	}
+	if !errors.Is(err, errNoGoogleapiSourceInfo) {
+		t.Errorf("mismatch error want %v got %v", errNoGoogleapiSourceInfo, err)
+	}
+}