Skip to content

Commit 016bdf9

Browse files
authored
feat(librarian): remove channel from librarian.yaml if can be derived from library name (#3221)
Let `librarian tidy` remove redundant fields in Channel if can be derived from library name. When generating, `prepareLibrary` should derive channel path and service_config from defaults. Also fix test setups for `TestGenerateCommand` and `TestGenerateSkip` that Channel path and service config file should be present. This was not an issue before because no `channel` was explicitly specified in test config. Rules used go derive Channel: - path: replacing all '-' in the library's name with '/'. - service_config: '[resolved_path]/[service_name]_[version].yaml'. - If both the path and service_config fields of a channel are removed, the entire channel entry will be removed from the configuration. Assumption made that may need adjustments in the future: - Rules for deriving service_config from path is the same across language. - Derivation is on "best-effort" , and for simplicity I assumed the [service_name] and [version] are the last 2 parts from `path`. For #3139
1 parent 627b4d7 commit 016bdf9

File tree

5 files changed

+274
-49
lines changed

5 files changed

+274
-49
lines changed

internal/librarian/generate.go

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"io/fs"
2222
"os"
2323
"path/filepath"
24+
"strings"
2425

2526
"github.com/googleapis/librarian/internal/config"
2627
"github.com/googleapis/librarian/internal/fetch"
@@ -168,6 +169,35 @@ func defaultOutput(language, channel, defaultOut string) string {
168169
}
169170
}
170171

172+
func deriveChannelPath(language string, lib *config.Library) string {
173+
switch language {
174+
case "rust":
175+
return rust.DeriveChannelPath(lib.Name)
176+
default:
177+
return strings.ReplaceAll(lib.Name, "-", "/")
178+
}
179+
}
180+
181+
// deriveServiceConfig returns the conventionally derived service config path for a given channel.
182+
//
183+
// The final service config path is constructed using the pattern: "[resolved_path]/[service_name]_[version].yaml".
184+
//
185+
// For example, if resolved_path is "google/cloud/speech/v1", it derives to "google/cloud/speech/v1/speech_v1.yaml".
186+
//
187+
// It returns an empty string if the resolved path does not contain sufficient components
188+
// (e.g., missing version or service name) or if the version component does not start with 'v'.
189+
func deriveServiceConfig(resolvedPath string) string {
190+
parts := strings.Split(resolvedPath, "/")
191+
if len(parts) >= 2 {
192+
version := parts[len(parts)-1]
193+
service := parts[len(parts)-2]
194+
if strings.HasPrefix(version, "v") {
195+
return fmt.Sprintf("%s/%s_%s.yaml", resolvedPath, service, version)
196+
}
197+
}
198+
return ""
199+
}
200+
171201
func dirExists(path string) bool {
172202
info, err := os.Stat(path)
173203
if err != nil {
@@ -210,13 +240,23 @@ func generateLibrary(ctx context.Context, cfg *config.Config, libraryName string
210240
// For Rust libraries without an explicit output path, it derives the output
211241
// from the first channel path.
212242
func prepareLibrary(language string, lib *config.Library, defaults *config.Default) (*config.Library, error) {
243+
if len(lib.Channels) == 0 {
244+
// If no channels are specified, create an empty channel first
245+
lib.Channels = append(lib.Channels, &config.Channel{})
246+
}
247+
for _, ch := range lib.Channels {
248+
if ch.Path == "" {
249+
ch.Path = deriveChannelPath(language, lib)
250+
}
251+
if ch.ServiceConfig == "" {
252+
ch.ServiceConfig = deriveServiceConfig(ch.Path)
253+
}
254+
}
255+
213256
if lib.Output == "" {
214257
if lib.Veneer {
215258
return nil, fmt.Errorf("veneer %q requires an explicit output path", lib.Name)
216259
}
217-
if len(lib.Channels) == 0 {
218-
return nil, fmt.Errorf("library %q has no channels, cannot determine default output", lib.Name)
219-
}
220260
lib.Output = defaultOutput(language, lib.Channels[0].Path, defaults.Output)
221261
}
222262
return fillDefaults(lib, defaults), nil

internal/librarian/generate_test.go

Lines changed: 86 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,30 +33,28 @@ func TestGenerateCommand(t *testing.T) {
3333
lib2 = "library-two"
3434
lib2Output = "output2"
3535
)
36-
wd, err := os.Getwd()
37-
if err != nil {
38-
t.Fatal(err)
39-
}
40-
googleapisDir := filepath.Join(wd, "testdata", "googleapis")
41-
4236
tempDir := t.TempDir()
4337
t.Chdir(tempDir)
38+
googleapisDir := createGoogleapisServiceConfigs(t, tempDir, map[string]string{
39+
"google/cloud/speech/v1": "speech_v1.yaml",
40+
"grafeas/v1": "grafeas_v1.yaml",
41+
"google/cloud/texttospeech/v1": "texttospeech_v1.yaml",
42+
})
4443
configPath := filepath.Join(tempDir, librarianConfigPath)
45-
4644
configContent := fmt.Sprintf(`language: testhelper
4745
sources:
4846
googleapis:
4947
dir: %s
5048
libraries:
5149
- name: %s
5250
output: %s
53-
apis:
51+
channels:
5452
- path: google/cloud/speech/v1
55-
- path: google/cloud/speech/v1p1beta1
56-
- path: google/cloud/speech/v2
5753
- path: grafeas/v1
5854
- name: %s
5955
output: %s
56+
channels:
57+
- path: google/cloud/texttospeech/v1
6058
`, googleapisDir, lib1, lib1Output, lib2, lib2Output)
6159
if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
6260
t.Fatal(err)
@@ -147,11 +145,12 @@ func TestGenerateSkip(t *testing.T) {
147145
lib2 = "library-two"
148146
lib2Output = "output2"
149147
)
150-
wd, err := os.Getwd()
151-
if err != nil {
152-
t.Fatal(err)
153-
}
154-
googleapisDir := filepath.Join(wd, "testdata", "googleapis")
148+
tempDir := t.TempDir()
149+
t.Chdir(tempDir)
150+
googleapisDir := createGoogleapisServiceConfigs(t, tempDir, map[string]string{
151+
"google/cloud/speech/v1": "speech_v1.yaml",
152+
"google/cloud/texttospeech/v1": "texttospeech_v1.yaml",
153+
})
155154

156155
allLibraries := map[string]string{
157156
lib1: lib1Output,
@@ -184,8 +183,12 @@ libraries:
184183
- name: %s
185184
output: %s
186185
skip_generate: true
186+
channels:
187+
- path: google/cloud/speech/v1
187188
- name: %s
188189
output: %s
190+
channels:
191+
- path: google/cloud/texttospeech/v1
189192
`, googleapisDir, lib1, lib1Output, lib2, lib2Output)
190193
if err := os.WriteFile(filepath.Join(tempDir, librarianConfigPath), []byte(configContent), 0644); err != nil {
191194
t.Fatal(err)
@@ -218,54 +221,59 @@ libraries:
218221

219222
func TestPrepareLibrary(t *testing.T) {
220223
for _, test := range []struct {
221-
name string
222-
language string
223-
output string
224-
veneer bool
225-
channels []*config.Channel
226-
want string
227-
wantErr bool
224+
name string
225+
language string
226+
output string
227+
veneer bool
228+
channels []*config.Channel
229+
wantOutput string
230+
wantErr bool
231+
wantChannelPath string
232+
wantServiceConfig string
228233
}{
229234
{
230-
name: "empty output derives path from channel",
231-
language: "rust",
232-
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
233-
want: "src/generated/cloud/secretmanager/v1",
235+
name: "empty output derives path from channel",
236+
language: "rust",
237+
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
238+
wantOutput: "src/generated/cloud/secretmanager/v1",
234239
},
235240
{
236-
name: "explicit output keeps explicit path",
237-
language: "rust",
238-
output: "custom/output",
239-
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
240-
want: "custom/output",
241+
name: "explicit output keeps explicit path",
242+
language: "rust",
243+
output: "custom/output",
244+
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
245+
wantOutput: "custom/output",
241246
},
242247
{
243-
name: "empty output uses default for non-rust",
244-
language: "go",
245-
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
246-
want: "src/generated",
248+
name: "empty output uses default for non-rust",
249+
language: "go",
250+
channels: []*config.Channel{{Path: "google/cloud/secretmanager/v1"}},
251+
wantOutput: "src/generated",
247252
},
248253
{
249-
name: "rust with no channels returns error",
250-
language: "rust",
251-
channels: nil,
252-
wantErr: true,
254+
name: "rust with no channels creates default and derives path",
255+
language: "rust",
256+
channels: nil,
257+
wantErr: false,
258+
wantOutput: "src/generated/cloud/test/v1",
259+
wantChannelPath: "google/cloud/test/v1",
260+
wantServiceConfig: "google/cloud/test/v1/test_v1.yaml",
253261
},
254262
{
255263
name: "veneer without output returns error",
256264
veneer: true,
257265
wantErr: true,
258266
},
259267
{
260-
name: "veneer with explicit output succeeds",
261-
veneer: true,
262-
output: "src/storage",
263-
want: "src/storage",
268+
name: "veneer with explicit output succeeds",
269+
veneer: true,
270+
output: "src/storage",
271+
wantOutput: "src/storage",
264272
},
265273
} {
266274
t.Run(test.name, func(t *testing.T) {
267275
lib := &config.Library{
268-
Name: "test-lib",
276+
Name: "google-cloud-test-v1",
269277
Output: test.output,
270278
Veneer: test.veneer,
271279
Channels: test.channels,
@@ -283,13 +291,45 @@ func TestPrepareLibrary(t *testing.T) {
283291
if err != nil {
284292
t.Fatal(err)
285293
}
286-
if got.Output != test.want {
287-
t.Errorf("got output %q, want %q", got.Output, test.want)
294+
if got.Output != test.wantOutput {
295+
t.Errorf("got output %q, want %q", got.Output, test.wantOutput)
296+
}
297+
if test.wantChannelPath != "" || test.wantServiceConfig != "" {
298+
if len(got.Channels) == 0 {
299+
t.Fatalf("expected a channel, got none")
300+
}
301+
ch := got.Channels[0]
302+
if test.wantChannelPath != "" && ch.Path != test.wantChannelPath {
303+
t.Errorf("got channel path %q, want %q", ch.Path, test.wantChannelPath)
304+
}
305+
if test.wantServiceConfig != "" && ch.ServiceConfig != test.wantServiceConfig {
306+
t.Errorf("got service config %q, want %q", ch.ServiceConfig, test.wantServiceConfig)
307+
}
288308
}
289309
})
290310
}
291311
}
292312

313+
// createGoogleapisServiceConfigs creates a mock googleapis directory structure
314+
// with service config files for testing purposes.
315+
// The configs map keys are channel paths (e.g., "google/cloud/speech/v1")
316+
// and values are the service config filenames (e.g., "speech_v1.yaml").
317+
func createGoogleapisServiceConfigs(t *testing.T, tempDir string, configs map[string]string) string {
318+
t.Helper()
319+
googleapisDir := filepath.Join(tempDir, "googleapis")
320+
321+
for channelPath, filename := range configs {
322+
dir := filepath.Join(googleapisDir, channelPath)
323+
if err := os.MkdirAll(dir, 0755); err != nil {
324+
t.Fatal(err)
325+
}
326+
if err := os.WriteFile(filepath.Join(dir, filename), []byte(""), 0644); err != nil {
327+
t.Fatal(err)
328+
}
329+
}
330+
return googleapisDir
331+
}
332+
293333
func TestCleanOutput(t *testing.T) {
294334
for _, test := range []struct {
295335
name string

internal/librarian/internal/rust/generate.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ func DefaultLibraryName(channel string) string {
153153
return strings.ReplaceAll(channel, "/", "-")
154154
}
155155

156+
// DeriveChannelPath derives a channel path from a library name.
157+
// For example: google-cloud-secretmanager-v1 -> google/cloud/secretmanager/v1.
158+
func DeriveChannelPath(name string) string {
159+
return strings.ReplaceAll(name, "-", "/")
160+
}
161+
156162
// DefaultOutput derives an output path from a channel path and default output.
157163
// For example: google/cloud/secretmanager/v1 with default src/generated/
158164
// returns src/generated/cloud/secretmanager/v1.

internal/librarian/tidy.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ func RunTidy() error {
5555
if lib.Output != "" && len(lib.Channels) == 1 && isDerivableOutput(cfg, lib) {
5656
lib.Output = ""
5757
}
58+
for _, ch := range lib.Channels {
59+
if isDerivableChannelPath(cfg.Language, lib, ch) {
60+
ch.Path = ""
61+
}
62+
if isDerivableServiceConfig(cfg.Language, lib, ch) {
63+
ch.ServiceConfig = ""
64+
}
65+
}
66+
lib.Channels = slices.DeleteFunc(lib.Channels, func(ch *config.Channel) bool {
67+
return ch.Path == "" && ch.ServiceConfig == ""
68+
})
5869
}
5970
return yaml.Write(librarianConfigPath, formatConfig(cfg))
6071
}
@@ -64,6 +75,18 @@ func isDerivableOutput(cfg *config.Config, lib *config.Library) bool {
6475
return lib.Output == derivedOutput
6576
}
6677

78+
func isDerivableChannelPath(language string, lib *config.Library, ch *config.Channel) bool {
79+
return ch.Path == deriveChannelPath(language, lib)
80+
}
81+
82+
func isDerivableServiceConfig(language string, lib *config.Library, ch *config.Channel) bool {
83+
path := ch.Path
84+
if path == "" {
85+
path = deriveChannelPath(language, lib)
86+
}
87+
return ch.ServiceConfig != "" && ch.ServiceConfig == deriveServiceConfig(path)
88+
}
89+
6790
func validateLibraries(cfg *config.Config) error {
6891
var (
6992
errs []error

0 commit comments

Comments
 (0)