feat(internal/librarian): rework build container contract

internal/config/config.go

@@ -24,6 +24,8 @@ import (
 )
 
 const (
+	// BuildRequest is a JSON file that describes which library to build/test.
+	BuildRequest      string = "build-request.json"
 	DefaultPushConfig string = "noreply-cloudsdk@google.com,Google Cloud SDK"
 	// GeneratorInputDir is the default directory to store files that generator
 	// needs to regenerate libraries from an empty directory.

internal/docker/docker.go

@@ -84,6 +84,21 @@ type GenerateRequest struct {
 	RepoDir string
 }
 
+// BuildRequest contains all the information required for a language
+// container to run the build command.
+type BuildRequest struct {
+	// cfg is a pointer to the [config.Config] struct, holding general configuration
+	// values parsed from flags or environment variables.
+	Cfg *config.Config
+	// state is a pointer to the [config.LibrarianState] struct, representing
+	// the overall state of the generation and release pipeline.
+	State *config.LibrarianState
+	// libraryID specifies the ID of the library to build
+	LibraryID string
+	// RepoDir is the local root directory of the language repository.
+	RepoDir string
+}
+
 // New constructs a Docker instance which will invoke the specified
 // Docker image as required to implement language-specific commands,
 // providing the container with required environment variables.
@@ -105,7 +120,7 @@ func New(workRoot, image, secretsProject, uid, gid string, pipelineConfig *confi
 // library.
 func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
 	jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.GenerateRequest)
-	if err := writeGenerateRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
+	if err := writeRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
 		return err
 	}
 	commandArgs := []string{
@@ -129,17 +144,22 @@ func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
 
 // Build builds the library with an ID of libraryID, as configured in
 // the Librarian state file for the repository with a root of repoRoot.
-func (c *Docker) Build(ctx context.Context, cfg *config.Config, repoRoot, libraryID string) error {
+func (c *Docker) Build(ctx context.Context, request *BuildRequest) error {
+	jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.BuildRequest)
+	if err := writeRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
+		return err
+	}
 	mounts := []string{
-		fmt.Sprintf("%s:/repo", repoRoot),
+		fmt.Sprintf("%s:/librarian:ro", config.LibrarianDir),
+		fmt.Sprintf("%s:/repo", request.RepoDir),
 	}
 	commandArgs := []string{
 		"--repo-root=/repo",
 		"--test=true",
-		fmt.Sprintf("--library-id=%s", libraryID),
+		fmt.Sprintf("--library-id=%s", request.LibraryID),
 	}
 
-	return c.runDocker(ctx, cfg, CommandBuild, mounts, commandArgs)
+	return c.runDocker(ctx, request.Cfg, CommandBuild, mounts, commandArgs)
 }
 
 // Configure configures an API within a repository, either adding it to an
@@ -228,7 +248,7 @@ func (c *Docker) runCommand(cmdName string, args ...string) error {
 	return err
 }
 
-func writeGenerateRequest(state *config.LibrarianState, libraryID, jsonFilePath string) error {
+func writeRequest(state *config.LibrarianState, libraryID, jsonFilePath string) error {
 	if err := os.MkdirAll(filepath.Dir(jsonFilePath), 0755); err != nil {
 		return fmt.Errorf("failed to make directory: %w", err)
 	}

internal/docker/docker_test.go

@@ -66,7 +66,6 @@ func TestDockerRun(t *testing.T) {
 		testImage           = "testImage"
 		testLibraryID       = "testLibraryID"
 		testOutput          = "testOutput"
-		testRepoRoot        = "testRepoRoot"
 	)
 
 	state := &config.LibrarianState{}
@@ -79,6 +78,7 @@ func TestDockerRun(t *testing.T) {
 		docker     *Docker
 		runCommand func(ctx context.Context, d *Docker) error
 		want       []string
+		wantErr    bool
 	}{
 		{
 			name: "Generate",
@@ -110,6 +110,7 @@ func TestDockerRun(t *testing.T) {
 				"--source=/source",
 				fmt.Sprintf("--library-id=%s", testLibraryID),
 			},
+			wantErr: false,
 		},
 		{
 			name: "Generate runs in docker",
@@ -141,24 +142,50 @@ func TestDockerRun(t *testing.T) {
 				"--source=/source",
 				fmt.Sprintf("--library-id=%s", testLibraryID),
 			},
+			wantErr: false,
 		},
 		{
 			name: "Build",
 			docker: &Docker{
 				Image: testImage,
 			},
 			runCommand: func(ctx context.Context, d *Docker) error {
-				return d.Build(ctx, cfg, testRepoRoot, testLibraryID)
+				buildRequest := &BuildRequest{
+					Cfg:       cfg,
+					State:     state,
+					LibraryID: testLibraryID,
+					RepoDir:   ".",
+				}
+				return d.Build(ctx, buildRequest)
 			},
 			want: []string{
 				"run", "--rm",
-				"-v", fmt.Sprintf("%s:/repo", testRepoRoot),
+				"-v", ".librarian:/librarian:ro",
+				"-v", ".:/repo",
 				testImage,
 				string(CommandBuild),
 				"--repo-root=/repo",
 				"--test=true",
 				fmt.Sprintf("--library-id=%s", testLibraryID),
 			},
+			wantErr: false,
+		},
+		{
+			name: "Build with invalid repo dir",
+			docker: &Docker{
+				Image: testImage,
+			},
+			runCommand: func(ctx context.Context, d *Docker) error {
+				buildRequest := &BuildRequest{
+					Cfg:       cfg,
+					State:     state,
+					LibraryID: testLibraryID,
+					RepoDir:   "/non-exist-dir",
+				}
+				return d.Build(ctx, buildRequest)
+			},
+			want:    []string{},
+			wantErr: true,
 		},
 		{
 			name: "Configure",
@@ -178,6 +205,7 @@ func TestDockerRun(t *testing.T) {
 				"--.librarian/generator-input=/.librarian/generator-input",
 				fmt.Sprintf("--api=%s", testAPIPath),
 			},
+			wantErr: false,
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -188,9 +216,19 @@ func TestDockerRun(t *testing.T) {
 				return nil
 			}
 			ctx := t.Context()
-			if err := test.runCommand(ctx, test.docker); err != nil {
+			err := test.runCommand(ctx, test.docker)
+
+			if test.wantErr {
+				if err == nil {
+					t.Errorf("%s should return error", test.name)
+				}
+				return
+			}
+
+			if err != nil {
 				t.Fatal(err)
 			}
+
 			os.RemoveAll(".librarian")
 			os.Remove(testGenerateRequest)
 		})
@@ -263,22 +301,22 @@ func TestToGenerateRequestJSON(t *testing.T) {
 			tempDir := t.TempDir()
 			if test.name == "invalid_file_name" {
 				filePath := filepath.Join(tempDir, "my\x00file.json")
-				err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
+				err := writeRequest(test.state, "google-cloud-go", filePath)
 				if err == nil {
 					t.Errorf("writeGenerateRequest() expected an error but got nil")
 				}
 				return
 			} else if test.expectErr {
 				filePath := filepath.Join("/non-exist-dir", "generate-request.json")
-				err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
+				err := writeRequest(test.state, "google-cloud-go", filePath)
 				if err == nil {
 					t.Errorf("writeGenerateRequest() expected an error but got nil")
 				}
 				return
 			}
 
 			filePath := filepath.Join(tempDir, "generate-request.json")
-			err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
+			err := writeRequest(test.state, "google-cloud-go", filePath)
 
 			if err != nil {
 				t.Fatalf("writeGenerateRequest() unexpected error: %v", err)

internal/librarian/generate.go

@@ -177,12 +177,10 @@
 	return nil
 }
 
-// runGenerateCommand checks if the library exists in the remote pipeline state, if so use GenerateLibrary command
-// otherwise use GenerateRaw command.
-// In case of non-fatal error when looking up library, we will fall back to GenerateRaw command
-// and log the error.
-// If refined generation is used, the context's languageRepo field will be populated and the
-// library ID will be returned; otherwise, an empty string will be returned.
+// runGenerateCommand attempts to perform generation for an API.
+//
+// If successful, it returns the ID of the generated library; otherwise, it
+// returns an empty string and an error.
 func (r *generateRunner) runGenerateCommand(ctx context.Context, outputDir string) (string, error) {
 	apiRoot, err := filepath.Abs(r.cfg.Source)
 	if err != nil {
@@ -212,23 +210,35 @@
 	return "", fmt.Errorf("library not found")
 }
 
+// runBuildCommand orchestrates the building of an API library using a containerized
+// environment.
+//
+// The `outputDir` parameter specifies the target directory where the built artifacts
+// should be placed.
 func (r *generateRunner) runBuildCommand(ctx context.Context, outputDir, libraryID string) error {
 	if !r.cfg.Build {
 		slog.Info("Build flag not specified, skipping")
 		return nil
 	}
-	if libraryID != "" {
-		slog.Info("Build requested in the context of refined generation; cleaning and copying code to the local language repo before building.")
-		// TODO(https://github.com/googleapis/librarian/issues/775)
-		if err := os.CopyFS(r.repo.Dir, os.DirFS(outputDir)); err != nil {
-			return err
-		}
-		if err := r.containerClient.Build(ctx, r.cfg, r.repo.Dir, libraryID); err != nil {
-			return err
-		}
+	if libraryID == "" {
+		slog.Warn("Cannot perform build, missing library ID")
+		return nil
 	}
-	slog.Warn("Cannot perform build, missing library ID")
-	return nil
+
+	buildRequest := &docker.BuildRequest{
+		Cfg:       r.cfg,
+		State:     r.state,
+		LibraryID: libraryID,
+		RepoDir:   r.repo.Dir,
+	}
+
+	slog.Info("Build requested in the context of refined generation; cleaning and copying code to the local language repo before building.")
+	// TODO(https://github.com/googleapis/librarian/issues/775)
+	if err := os.CopyFS(r.repo.Dir, os.DirFS(outputDir)); err != nil {
+		return err
+	}
+
+	return r.containerClient.Build(ctx, buildRequest)
 }
 
 // detectIfLibraryConfigured returns whether a library has been configured for

internal/librarian/generate_test.go

@@ -40,7 +40,7 @@ func (m *mockContainerClient) Generate(ctx context.Context, request *docker.Gene
 	return nil
 }
 
-func (m *mockContainerClient) Build(ctx context.Context, cfg *config.Config, buildDir, libraryID string) error {
+func (m *mockContainerClient) Build(ctx context.Context, request *docker.BuildRequest) error {
 	m.buildCalls++
 	return nil
 }

internal/librarian/librarian.go

@@ -82,7 +82,7 @@ type GitHubClient interface {
 // ContainerClient is an abstraction over the Docker client.
 type ContainerClient interface {
 	Generate(ctx context.Context, request *docker.GenerateRequest) error
-	Build(ctx context.Context, cfg *config.Config, repoRoot, libraryID string) error
+	Build(ctx context.Context, request *docker.BuildRequest) error
 	Configure(ctx context.Context, cfg *config.Config, apiRoot, apiPath, generatorInput string) error
 }