Continue GCing other repos on error in one repo (go-gitea#22422)

Backport go-gitea#22422 The current code propagates all errors up to the iteration step meaning that a single malformed repo will prevent GC of other repos. This PR simply stops that propagation. Fix go-gitea#21605 Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath · Jan 13, 2023 · a3826a2 · a3826a2
1 parent 426c0ad
commit a3826a2
Showing 1 changed file with 34 additions and 27 deletions.
diff --git a/services/repository/check.go b/services/repository/check.go
@@ -73,32 +73,8 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg)
 				return db.ErrCancelledf("before GC of %s", repo.FullName())
 			default:
 			}
-			log.Trace("Running git gc on %v", repo)
-			command := git.NewCommand(ctx, args...).
-				SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName()))
-			var stdout string
-			var err error
-			stdout, _, err = command.RunStdString(&git.RunOpts{Timeout: timeout, Dir: repo.RepoPath()})
-
-			if err != nil {
-				log.Error("Repository garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err)
-				desc := fmt.Sprintf("Repository garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err)
-				if err = system_model.CreateRepositoryNotice(desc); err != nil {
-					log.Error("CreateRepositoryNotice: %v", err)
-				}
-				return fmt.Errorf("Repository garbage collection failed in repo: %s: Error: %w", repo.FullName(), err)
-			}
-
-			// Now update the size of the repository
-			if err := repo_module.UpdateRepoSize(ctx, repo); err != nil {
-				log.Error("Updating size as part of garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err)
-				desc := fmt.Sprintf("Updating size as part of garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err)
-				if err = system_model.CreateRepositoryNotice(desc); err != nil {
-					log.Error("CreateRepositoryNotice: %v", err)
-				}
-				return fmt.Errorf("Updating size as part of garbage collection failed in repo: %s: Error: %w", repo.FullName(), err)
-			}
-
+			// we can ignore the error here because it will be logged in GitGCRepo
+			_ = GitGcRepo(ctx, repo, timeout, args)
 			return nil
 		},
 	); err != nil {
@@ -109,6 +85,37 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg)
 	return nil
 }
 
+// GitGcRepo calls 'git gc' to remove unnecessary files and optimize the local repository
+func GitGcRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args []git.CmdArg) error {
+	log.Trace("Running git gc on %-v", repo)
+	command := git.NewCommand(ctx, args...).
+		SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName()))
+	var stdout string
+	var err error
+	stdout, _, err = command.RunStdString(&git.RunOpts{Timeout: timeout, Dir: repo.RepoPath()})
+
+	if err != nil {
+		log.Error("Repository garbage collection failed for %-v. Stdout: %s\nError: %v", repo, stdout, err)
+		desc := fmt.Sprintf("Repository garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err)
+		if err := system_model.CreateRepositoryNotice(desc); err != nil {
+			log.Error("CreateRepositoryNotice: %v", err)
+		}
+		return fmt.Errorf("Repository garbage collection failed in repo: %s: Error: %w", repo.FullName(), err)
+	}
+
+	// Now update the size of the repository
+	if err := repo_module.UpdateRepoSize(ctx, repo); err != nil {
+		log.Error("Updating size as part of garbage collection failed for %-v. Stdout: %s\nError: %v", repo, stdout, err)
+		desc := fmt.Sprintf("Updating size as part of garbage collection failed for %s. Stdout: %s\nError: %v", repo.RepoPath(), stdout, err)
+		if err := system_model.CreateRepositoryNotice(desc); err != nil {
+			log.Error("CreateRepositoryNotice: %v", err)
+		}
+		return fmt.Errorf("Updating size as part of garbage collection failed in repo: %s: Error: %w", repo.FullName(), err)
+	}
+
+	return nil
+}
+
 func gatherMissingRepoRecords(ctx context.Context) ([]*repo_model.Repository, error) {
 	repos := make([]*repo_model.Repository, 0, 10)
 	if err := db.Iterate(
@@ -162,7 +169,7 @@ func DeleteMissingRepositories(ctx context.Context, doer *user_model.User) error
 		}
 		log.Trace("Deleting %d/%d...", repo.OwnerID, repo.ID)
 		if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil {
-			log.Error("Failed to DeleteRepository %s [%d]: Error: %v", repo.FullName(), repo.ID, err)
+			log.Error("Failed to DeleteRepository %-v: Error: %v", repo, err)
 			if err2 := system_model.CreateRepositoryNotice("Failed to DeleteRepository %s [%d]: Error: %v", repo.FullName(), repo.ID, err); err2 != nil {
 				log.Error("CreateRepositoryNotice: %v", err)
 			}