Skip to content

Commit 6eb18c4

Browse files
JoeWang1127JoeWang1127
authored
feat: restore files after build failure (#2273)
Fixes #1682
1 parent 4ad6c9a commit 6eb18c4

File tree

3 files changed

+112
-43
lines changed

3 files changed

+112
-43
lines changed

‎internal/librarian/generate.go‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,14 +294,22 @@ func (r *generateRunner) runBuildCommand(ctx context.Context, libraryID string)
294294
State: r.state,
295295
}
296296
slog.Info("Performing build for library", "id", libraryID)
297-
if err := r.containerClient.Build(ctx, buildRequest); err != nil {
298-
return err
297+
if containerErr := r.containerClient.Build(ctx, buildRequest); containerErr != nil {
298+
if restoreErr := r.restoreLibrary(libraryID); restoreErr != nil {
299+
return errors.Join(containerErr, restoreErr)
300+
}
301+
302+
return errors.Join(containerErr)
299303
}
300304

301305
// Read the library state from the response.
302-
if _, err := readLibraryState(
303-
filepath.Join(buildRequest.RepoDir, config.LibrarianDir, config.BuildResponse)); err != nil {
304-
return err
306+
if _, responseErr := readLibraryState(
307+
filepath.Join(buildRequest.RepoDir, config.LibrarianDir, config.BuildResponse)); responseErr != nil {
308+
if restoreErr := r.restoreLibrary(libraryID); restoreErr != nil {
309+
return errors.Join(responseErr, restoreErr)
310+
}
311+
312+
return responseErr
305313
}
306314

307315
slog.Info("Build succeeds", "id", libraryID)
@@ -388,6 +396,16 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
388396
return libraryState.ID, nil
389397
}
390398

399+
func (r *generateRunner) restoreLibrary(libraryID string) error {
400+
// At this point, we should have a library in the state.
401+
library := findLibraryByID(r.state, libraryID)
402+
if err := r.repo.Restore(library.SourceRoots); err != nil {
403+
return err
404+
}
405+
406+
return r.repo.CleanUntracked(library.SourceRoots)
407+
}
408+
391409
func setAllAPIStatus(state *config.LibrarianState, status string) {
392410
for _, library := range state.Libraries {
393411
for _, api := range library.APIs {

‎internal/librarian/generate_test.go‎

Lines changed: 88 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -110,67 +110,51 @@ func TestRunBuildCommand(t *testing.T) {
110110
name string
111111
build bool
112112
libraryID string
113-
repo gitrepo.Repository
114-
state *config.LibrarianState
115113
container *mockContainerClient
116114
wantBuildCalls int
117115
wantErr bool
118116
}{
119117
{
120-
name: "build flag not specified",
118+
name: "build_flag_not_specified",
121119
build: false,
122120
container: &mockContainerClient{},
123121
wantBuildCalls: 0,
124122
},
125123
{
126-
name: "build with library id",
127-
build: true,
128-
libraryID: "some-library",
129-
repo: newTestGitRepo(t),
130-
state: &config.LibrarianState{
131-
Libraries: []*config.LibraryState{
132-
{
133-
ID: "some-library",
134-
},
135-
},
136-
},
124+
name: "build_with_library_id",
125+
build: true,
126+
libraryID: "some-library",
137127
container: &mockContainerClient{},
138128
wantBuildCalls: 1,
139129
},
140130
{
141-
name: "build with no library id",
142-
build: true,
143-
container: &mockContainerClient{},
131+
name: "build_with_no_library_id",
132+
build: true,
133+
container: &mockContainerClient{},
134+
wantBuildCalls: 0,
144135
},
145136
{
146-
name: "build with no response",
137+
name: "build_with_no_response",
147138
build: true,
148139
libraryID: "some-library",
149-
repo: newTestGitRepo(t),
150-
state: &config.LibrarianState{
151-
Libraries: []*config.LibraryState{
152-
{
153-
ID: "some-library",
154-
},
155-
},
156-
},
157140
container: &mockContainerClient{
158141
noBuildResponse: true,
159142
},
160143
wantBuildCalls: 1,
161144
},
162145
{
163-
name: "build with error response in response",
146+
name: "build_with_docker_command_error_files_restored",
164147
build: true,
165148
libraryID: "some-library",
166-
repo: newTestGitRepo(t),
167-
state: &config.LibrarianState{
168-
Libraries: []*config.LibraryState{
169-
{
170-
ID: "some-library",
171-
},
172-
},
149+
container: &mockContainerClient{
150+
buildErr: errors.New("simulate build error"),
173151
},
152+
wantErr: true,
153+
},
154+
{
155+
name: "build_with_error_response_in_response",
156+
build: true,
157+
libraryID: "some-library",
174158
container: &mockContainerClient{
175159
wantErrorMsg: true,
176160
},
@@ -179,20 +163,87 @@ func TestRunBuildCommand(t *testing.T) {
179163
} {
180164
t.Run(test.name, func(t *testing.T) {
181165
t.Parallel()
166+
repo := newTestGitRepo(t)
167+
state := &config.LibrarianState{
168+
Libraries: []*config.LibraryState{
169+
{
170+
ID: "some-library",
171+
SourceRoots: []string{
172+
"a/path",
173+
"another/path",
174+
},
175+
},
176+
},
177+
}
182178
r := &generateRunner{
183179
build: test.build,
184-
repo: test.repo,
185-
state: test.state,
180+
repo: repo,
181+
state: state,
186182
containerClient: test.container,
187183
}
188184

185+
// Create library files and commit the change.
186+
repoDir := r.repo.GetDir()
187+
for _, library := range r.state.Libraries {
188+
for _, srcPath := range library.SourceRoots {
189+
relPath := filepath.Join(repoDir, srcPath)
190+
if err := os.MkdirAll(relPath, 0755); err != nil {
191+
t.Fatal(err)
192+
}
193+
file := filepath.Join(relPath, "example.txt")
194+
if err := os.WriteFile(file, []byte("old content"), 0755); err != nil {
195+
t.Fatal(err)
196+
}
197+
}
198+
}
199+
if _, err := r.repo.AddAll(); err != nil {
200+
t.Fatal(err)
201+
}
202+
if err := r.repo.Commit("test commit"); err != nil {
203+
t.Fatal(err)
204+
}
205+
// Modify library files and add untacked files.
206+
for _, library := range r.state.Libraries {
207+
for _, srcPath := range library.SourceRoots {
208+
file := filepath.Join(repoDir, srcPath, "example.txt")
209+
if err := os.WriteFile(file, []byte("new content"), 0755); err != nil {
210+
t.Fatal(err)
211+
}
212+
213+
newFile := filepath.Join(repoDir, srcPath, "another_example.txt")
214+
if err := os.WriteFile(newFile, []byte("new content"), 0755); err != nil {
215+
t.Fatal(err)
216+
}
217+
}
218+
}
219+
189220
err := r.runBuildCommand(context.Background(), test.libraryID)
190221
if test.wantErr {
191222
if err == nil {
192-
t.Fatalf("%s should return error", test.name)
223+
t.Fatal(err)
193224
}
225+
// Verify the library files are restore.
226+
for _, library := range r.state.Libraries {
227+
for _, srcPath := range library.SourceRoots {
228+
file := filepath.Join(repoDir, srcPath, "example.txt")
229+
readFile, err := os.ReadFile(file)
230+
if err != nil {
231+
t.Fatal(err)
232+
}
233+
if diff := cmp.Diff("old content", string(readFile)); diff != "" {
234+
t.Errorf("file content mismatch (-want +got):%s", diff)
235+
}
236+
237+
newFile := filepath.Join(repoDir, srcPath, "another_example.txt")
238+
if _, err := os.Stat(newFile); !os.IsNotExist(err) {
239+
t.Fatal(err)
240+
}
241+
}
242+
}
243+
194244
return
195245
}
246+
196247
if err != nil {
197248
t.Fatal(err)
198249
}

‎internal/librarian/state.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func saveLibrarianState(repoDir string, state *config.LibrarianState) error {
184184
// readLibraryState reads the library state from a container response, if it exists.
185185
// If the response file does not exist, readLibraryState succeeds but returns a nil pointer.
186186
//
187-
// The response file is removed afterwards.
187+
// The response file is removed afterward.
188188
func readLibraryState(jsonFilePath string) (*config.LibraryState, error) {
189189
data, err := os.ReadFile(jsonFilePath)
190190
defer func() {

0 commit comments

Comments
 (0)