fix: enforce removal before copying library files (#2765)

Fixes #2593

Author: JoeWang1127

internal/librarian/command.go

@@ -299,15 +299,21 @@ func cleanAndCopyLibrary(state *config.LibrarianState, repoDir, libraryID, outpu
 		return fmt.Errorf("failed to clean library, %s: %w", library.ID, err)
 	}
 
-	return copyLibraryFiles(state, repoDir, libraryID, outputDir)
+	return copyLibraryFiles(state, repoDir, libraryID, outputDir, true)
 }
 
 // copyLibraryFiles copies the files in state.SourceRoots relative to the src folder to the dest
-// folder. It overwrites any existing files.
+// folder.
+//
+// If `failOnExistingFile` is true, the function will check if a file already
+// exists at the destination. If it does, an error is returned immediately without copying.
+// If `failOnExistingFile` is false, it will overwrite any existing files.
+//
 // If there's no files in the library's SourceRoots under the src directory, no copy will happen.
+//
 // If a file is being copied to the library's SourceRoots in the dest folder but the folder does
 // not exist, the copy fails.
-func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string) error {
+func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string, failOnExistingFile bool) error {
 	library := state.LibraryByID(libraryID)
 	if library == nil {
 		return fmt.Errorf("library %q not found", libraryID)
@@ -324,6 +330,9 @@ func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string)
 			slog.Debug("copying file", "file", file)
 			srcFile := filepath.Join(srcPath, file)
 			dstFile := filepath.Join(dstPath, file)
+			if _, err := os.Stat(dstFile); failOnExistingFile && err == nil {
+				return fmt.Errorf("file existed in destination: %s", dstFile)
+			}
 			if err := copyFile(dstFile, srcFile); err != nil {
 				return fmt.Errorf("failed to copy file %q for library %s: %w", srcFile, library.ID, err)
 			}

internal/librarian/command_test.go

@@ -1782,19 +1782,20 @@ func TestCopyLibraryFiles(t *testing.T) {
 		}
 	}
 	for _, test := range []struct {
-		name          string
-		repoDir       string
-		outputDir     string
-		libraryID     string
-		state         *config.LibrarianState
-		existingFiles []string
-		filesToCreate []string
-		setup         func(t *testing.T, outputDir string)
-		verify        func(t *testing.T, repoDir string)
-		wantFiles     []string
-		skipFiles     []string
-		wantErr       bool
-		wantErrMsg    string
+		name               string
+		repoDir            string
+		outputDir          string
+		libraryID          string
+		state              *config.LibrarianState
+		existingFiles      []string
+		filesToCreate      []string
+		setup              func(t *testing.T, outputDir string)
+		verify             func(t *testing.T, repoDir string)
+		wantFiles          []string
+		skipFiles          []string
+		failOnExistingFile bool
+		wantErr            bool
+		wantErrMsg         string
 	}{
 		{
 			repoDir:   "/invalid-dst-path",
@@ -1960,6 +1961,33 @@ func TestCopyLibraryFiles(t *testing.T) {
 				"another/path/example.txt",
 			},
 		},
+		{
+			name:      "file_existed_in_dst",
+			repoDir:   filepath.Join(t.TempDir(), "dst"),
+			outputDir: filepath.Join(t.TempDir(), "foo"),
+			libraryID: "example-library",
+			state: &config.LibrarianState{
+				Libraries: []*config.LibraryState{
+					{
+						ID: "example-library",
+						SourceRoots: []string{
+							"a/path",
+							"another/path",
+						},
+					},
+				},
+			},
+			existingFiles: []string{
+				"a/path/example.txt",
+			},
+			filesToCreate: []string{
+				"a/path/example.txt",
+				"another/path/example.txt",
+			},
+			failOnExistingFile: true,
+			wantErr:            true,
+			wantErrMsg:         "file existed in destination",
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			if len(test.existingFiles) > 0 {
@@ -1971,10 +1999,10 @@ func TestCopyLibraryFiles(t *testing.T) {
 			if test.setup != nil {
 				test.setup(t, test.outputDir)
 			}
-			err := copyLibraryFiles(test.state, test.repoDir, test.libraryID, test.outputDir)
+			err := copyLibraryFiles(test.state, test.repoDir, test.libraryID, test.outputDir, test.failOnExistingFile)
 			if test.wantErr {
 				if err == nil {
-					t.Fatal("copyLibraryFiles() shoud fail")
+					t.Fatal("copyLibraryFiles() should fail")
 				}
 				if !strings.Contains(err.Error(), test.wantErrMsg) {
 					t.Errorf("want error message: %s, got: %s", test.wantErrMsg, err.Error())

internal/librarian/generate_command.go

@@ -375,7 +375,7 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context, outputDir stri
 		r.state.Libraries[i] = libraryState
 	}
 
-	if err := copyLibraryFiles(r.state, r.repo.GetDir(), libraryState.ID, outputDir); err != nil {
+	if err := copyLibraryFiles(r.state, r.repo.GetDir(), libraryState.ID, outputDir, false); err != nil {
 		return "", err
 	}
 

internal/librarian/release_stage.go

@@ -190,7 +190,7 @@ func (r *stageRunner) runStageCommand(ctx context.Context, outputDir string) err
 	for _, library := range librariesToRelease {
 		// Copy the library files back if a release is needed
 		if library.ReleaseTriggered {
-			if err := copyLibraryFiles(r.state, r.repo.GetDir(), library.ID, outputDir); err != nil {
+			if err := copyLibraryFiles(r.state, r.repo.GetDir(), library.ID, outputDir, false); err != nil {
 				return err
 			}
 		}