Skip to content

Commit 475a62d

Browse files
authored
feat: include source root in configure request (#2431)
Updates #1922
1 parent a184f0d commit 475a62d

File tree

4 files changed

+161
-6
lines changed

4 files changed

+161
-6
lines changed

internal/docker/docker.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ type ConfigureRequest struct {
9797
// RepoDir is the local root directory of the language repository.
9898
RepoDir string
9999

100+
// ExistingSourceRoots are existing source roots in the language repository.
101+
ExistingSourceRoots []string
102+
100103
// GlobalFiles are global files of the language repository.
101104
GlobalFiles []string
102105

@@ -284,6 +287,10 @@ func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (stri
284287
fmt.Sprintf("%s:/input", generatorInput),
285288
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume
286289
}
290+
// Mount existing source roots as a readonly volume.
291+
for _, sourceRoot := range request.ExistingSourceRoots {
292+
mounts = append(mounts, fmt.Sprintf("%s/%s:/repo/%s:ro", request.RepoDir, sourceRoot, sourceRoot))
293+
}
287294
// Mount global files as a readonly volume.
288295
for _, globalFile := range request.GlobalFiles {
289296
mounts = append(mounts, fmt.Sprintf("%s/%s:/repo/%s:ro", request.RepoDir, globalFile, globalFile))

internal/docker/docker_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,73 @@ func TestDockerRun(t *testing.T) {
299299
"--source=/source",
300300
},
301301
},
302+
{
303+
name: "configure_with_source_roots",
304+
docker: &Docker{
305+
Image: testImage,
306+
},
307+
runCommand: func(ctx context.Context, d *Docker) error {
308+
configureRequest := &ConfigureRequest{
309+
State: state,
310+
LibraryID: testLibraryID,
311+
RepoDir: repoDir,
312+
ApiRoot: testAPIRoot,
313+
ExistingSourceRoots: []string{
314+
"a/path",
315+
"b/path",
316+
},
317+
}
318+
319+
_, err := d.Configure(ctx, configureRequest)
320+
321+
return err
322+
},
323+
want: []string{
324+
"run", "--rm",
325+
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
326+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
327+
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
328+
"-v", fmt.Sprintf("%s/a/path:/repo/a/path:ro", repoDir),
329+
"-v", fmt.Sprintf("%s/b/path:/repo/b/path:ro", repoDir),
330+
testImage,
331+
string(CommandConfigure),
332+
"--librarian=/librarian",
333+
"--input=/input",
334+
"--repo=/repo",
335+
"--source=/source",
336+
},
337+
},
338+
{
339+
name: "configure_with_nil_source_roots",
340+
docker: &Docker{
341+
Image: testImage,
342+
},
343+
runCommand: func(ctx context.Context, d *Docker) error {
344+
configureRequest := &ConfigureRequest{
345+
State: state,
346+
LibraryID: testLibraryID,
347+
RepoDir: repoDir,
348+
ApiRoot: testAPIRoot,
349+
ExistingSourceRoots: nil,
350+
}
351+
352+
_, err := d.Configure(ctx, configureRequest)
353+
354+
return err
355+
},
356+
want: []string{
357+
"run", "--rm",
358+
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
359+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
360+
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
361+
testImage,
362+
string(CommandConfigure),
363+
"--librarian=/librarian",
364+
"--input=/input",
365+
"--repo=/repo",
366+
"--source=/source",
367+
},
368+
},
302369
{
303370
name: "configure_with_multiple_libraries_in_librarian_state",
304371
docker: &Docker{

internal/librarian/generate.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,13 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
365365
}
366366

367367
configureRequest := &docker.ConfigureRequest{
368-
ApiRoot: apiRoot,
369-
HostMount: r.hostMount,
370-
LibraryID: r.library,
371-
RepoDir: r.repo.GetDir(),
372-
GlobalFiles: globalFiles,
373-
State: r.state,
368+
ApiRoot: apiRoot,
369+
HostMount: r.hostMount,
370+
LibraryID: r.library,
371+
RepoDir: r.repo.GetDir(),
372+
GlobalFiles: globalFiles,
373+
ExistingSourceRoots: r.getExistingSrc(r.library),
374+
State: r.state,
374375
}
375376
slog.Info("Performing configuration for library", "id", r.library)
376377
if _, err := r.containerClient.Configure(ctx, configureRequest); err != nil {
@@ -414,6 +415,20 @@ func (r *generateRunner) restoreLibrary(libraryID string) error {
414415
return r.repo.CleanUntracked(library.SourceRoots)
415416
}
416417

418+
// getExistingSrc returns source roots as-is of a given library ID, if the source roots exist in the language repo.
419+
func (r *generateRunner) getExistingSrc(libraryID string) []string {
420+
library := findLibraryByID(r.state, libraryID)
421+
var existingSrc []string
422+
for _, src := range library.SourceRoots {
423+
relPath := filepath.Join(r.repo.GetDir(), src)
424+
if _, err := os.Stat(relPath); err == nil {
425+
existingSrc = append(existingSrc, src)
426+
}
427+
}
428+
429+
return existingSrc
430+
}
431+
417432
func setAllAPIStatus(state *config.LibrarianState, status string) {
418433
for _, library := range state.Libraries {
419434
for _, api := range library.APIs {

internal/librarian/generate_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,3 +1124,69 @@ func TestUpdateLastGeneratedCommitState(t *testing.T) {
11241124
t.Errorf("updateState() got = %v, want %v", r.state.Libraries[0].LastGeneratedCommit, hash)
11251125
}
11261126
}
1127+
1128+
func TestGetExistingSrc(t *testing.T) {
1129+
t.Parallel()
1130+
for _, test := range []struct {
1131+
name string
1132+
paths []string
1133+
want []string
1134+
}{
1135+
{
1136+
name: "all_source_paths_existed",
1137+
paths: []string{
1138+
"a/path",
1139+
"another/path",
1140+
},
1141+
want: []string{
1142+
"a/path",
1143+
"another/path",
1144+
},
1145+
},
1146+
{
1147+
name: "one_source_paths_existed",
1148+
paths: []string{
1149+
"a/path",
1150+
},
1151+
want: []string{
1152+
"a/path",
1153+
},
1154+
},
1155+
{
1156+
name: "no_source_paths_existed",
1157+
want: nil,
1158+
},
1159+
} {
1160+
t.Run(test.name, func(t *testing.T) {
1161+
t.Parallel()
1162+
repo := newTestGitRepo(t)
1163+
state := &config.LibrarianState{
1164+
Libraries: []*config.LibraryState{
1165+
{
1166+
ID: "some-library",
1167+
SourceRoots: []string{
1168+
"a/path",
1169+
"another/path",
1170+
},
1171+
},
1172+
},
1173+
}
1174+
for _, path := range test.paths {
1175+
relPath := filepath.Join(repo.GetDir(), path)
1176+
if err := os.MkdirAll(relPath, 0755); err != nil {
1177+
t.Fatal(err)
1178+
}
1179+
}
1180+
1181+
r := &generateRunner{
1182+
repo: repo,
1183+
state: state,
1184+
}
1185+
1186+
got := r.getExistingSrc("some-library")
1187+
if diff := cmp.Diff(test.want, got); diff != "" {
1188+
t.Errorf("getExistingSrc() mismatch (-want +got):%s", diff)
1189+
}
1190+
})
1191+
}
1192+
}

0 commit comments

Comments
 (0)