Skip to content

Commit f11bb69

Browse files
authored
fix(internal/librarian): join protobuf-src subdir before passing down to sidekick (#3279)
This is a fix to follow-up on revert #3277. Now it combines protobuf root dir and subdir before passing down to sidekick config as "protobuf-src-root". Issue with implementation before revert: pass down both `protobuf-src-root` and `protobuf-src-subdir` to sidekick, combine the path in `protoc` when constructing protoc command. This was fine, but sidekick flow joins path before this point in [overrideSources](https://github.com/googleapis/librarian/blob/b405072f6a4f72adcc3ff299cb13de6bbe3d4327/internal/sidekick/sidekick/refreshall.go#L60), but somehow kept `-subdir` config at a later point (Not totally clear when, likely during a merge, need a bit more digging), then results in `proto/path/subdir/subdir`. Now: `librarian generate` comibines path after downloading, before translating to sidekick config. `sidekick refresh` continue to do the join in `overrideSources`. This code is not shared with librarian, so logic not repeated. Alternative: Keep path join logic within `protoc` function and remove joins in librarian and `overrideSources`. Did not choose because wanted to limit impact for sidekick codepath to avoid unexpected behaviors. Fix #3183
1 parent b405072 commit f11bb69

File tree

2 files changed

+16
-18
lines changed

2 files changed

+16
-18
lines changed

internal/librarian/internal/rust/codec.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package rust
1616

1717
import (
18+
"path"
1819
"strings"
1920

2021
"github.com/googleapis/librarian/internal/config"
@@ -38,22 +39,18 @@ func toSidekickConfig(library *config.Library, channel *config.Channel, googleap
3839
} else {
3940
source["roots"] = strings.Join(library.Roots, ",")
4041
rootMap := map[string]struct {
41-
path string
42-
subdir string
43-
keys []string
42+
path string
43+
key string
4444
}{
45-
"googleapis": {path: googleapisDir, keys: []string{"googleapis-root"}},
46-
"discovery": {path: discoveryDir, keys: []string{"discovery-root"}},
47-
"showcase": {path: showcaseDir, keys: []string{"showcase-root"}},
48-
"protobuf-src": {path: protobufRootDir, subdir: protobufSubDir, keys: []string{"protobuf-src-root", "protobuf-src-subdir"}},
49-
"conformance": {path: conformanceDir, keys: []string{"conformance-root"}},
45+
"googleapis": {path: googleapisDir, key: "googleapis-root"},
46+
"discovery": {path: discoveryDir, key: "discovery-root"},
47+
"showcase": {path: showcaseDir, key: "showcase-root"},
48+
"protobuf-src": {path: path.Join(protobufRootDir, protobufSubDir), key: "protobuf-src-root"},
49+
"conformance": {path: conformanceDir, key: "conformance-root"},
5050
}
5151
for _, root := range library.Roots {
5252
if r, ok := rootMap[root]; ok && r.path != "" {
53-
source[r.keys[0]] = r.path
54-
if len(r.keys) > 1 {
55-
source[r.keys[1]] = r.subdir
56-
}
53+
source[r.key] = r.path
5754
}
5855
}
5956
}

internal/librarian/internal/rust/codec_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestToSidekickConfig(t *testing.T) {
3232
protobufDir string
3333
conformanceDir string
3434
showcaseDir string
35+
protobufSubDir string
3536
want *sidekickconfig.Config
3637
}{
3738
{
@@ -598,6 +599,7 @@ func TestToSidekickConfig(t *testing.T) {
598599
googleapisDir: "/tmp/googleapis",
599600
protobufDir: "/tmp/protobuf",
600601
conformanceDir: "/tmp/conformance",
602+
protobufSubDir: "src",
601603
want: &sidekickconfig.Config{
602604
General: sidekickconfig.GeneralConfig{
603605
Language: "rust",
@@ -606,11 +608,10 @@ func TestToSidekickConfig(t *testing.T) {
606608
SpecificationSource: "google/cloud/vision/v1",
607609
},
608610
Source: map[string]string{
609-
"googleapis-root": "/tmp/googleapis",
610-
"protobuf-src-root": "/tmp/protobuf",
611-
"protobuf-src-subdir": "",
612-
"conformance-root": "/tmp/conformance",
613-
"roots": "googleapis,protobuf-src,conformance",
611+
"googleapis-root": "/tmp/googleapis",
612+
"protobuf-src-root": "/tmp/protobuf/src",
613+
"conformance-root": "/tmp/conformance",
614+
"roots": "googleapis,protobuf-src,conformance",
614615
},
615616
Codec: map[string]string{
616617
"package-name-override": "google-cloud-vision-v1",
@@ -646,7 +647,7 @@ func TestToSidekickConfig(t *testing.T) {
646647
},
647648
} {
648649
t.Run(tt.name, func(t *testing.T) {
649-
got := toSidekickConfig(tt.library, tt.channel, tt.googleapisDir, tt.discoveryDir, tt.protobufDir, "", tt.conformanceDir, tt.showcaseDir)
650+
got := toSidekickConfig(tt.library, tt.channel, tt.googleapisDir, tt.discoveryDir, tt.protobufDir, tt.protobufSubDir, tt.conformanceDir, tt.showcaseDir)
650651
if diff := cmp.Diff(tt.want, got); diff != "" {
651652
t.Errorf("mismatch (-want +got):\n%s", diff)
652653
}

0 commit comments

Comments
 (0)