Some code cleanups

Author: bsofiato

modules/git/command.go

@@ -276,23 +276,37 @@ func (c *Command) Run(opts *RunOpts) error {
 		return err
 	}
 	// Execute the git command
-	if err := c.run(opts); err != nil {
-		if exitError, ok := err.(*exec.ExitError); ok {
-			if exitError.ExitCode() > 128 {
-				// Errors greater than 128 means that the process was killed by an OS signal (see https://unix.stackexchange.com/questions/99112/default-exit-code-when-process-is-terminated)
-				log.Warn("It appears that the git process %s has crashed. Attempting to forcbily unlock it [repo: %s]", exitError.Pid(), opts.Dir)
-				ForciblyUnlockRepository(c.parentContext, opts.Dir)
-				return err
-			}
-			return err
-		}
+	if err := c.doRun(opts); err != nil {
+		unlockUponCrashing(c.parentContext, err, opts.Dir)
 		return err
 	}
 	return nil
 }
 
+func unlockUponCrashing(ctx context.Context, originalError error, repoDir string) {
+	if hasGitProcessCrashed(originalError) {
+		log.Warn("The git process has crashed. Attempting to forcbily unlock the underlying repo at %s", repoDir)
+		if err := ForciblyUnlockRepository(ctx, repoDir); err != nil {
+			log.Error("Error while trying to unlock repository at %v", err)
+		}
+	}
+}
+
+func hasGitProcessCrashed(err error) bool {
+	if exitError, ok := err.(*exec.ExitError); ok {
+		if runtime.GOOS == "windows" {
+			log.Warn("Cannot realiably detected if the git process has crashed in windows. Assuming it hasn't [exitCode: %s, pid: %s]", exitError.ExitCode(), exitError.Pid())
+			return false
+		}
+		return exitError.ExitCode() > 128
+	}
+	// This function should only be called with an ExitError
+	log.Error("hasGitProcessCrashed should only be called with an ExitError [err: %v]. Assuming it the git process hasn't crashed", err)
+	return false
+}
+
 // Run runs the command with the RunOpts
-func (c *Command) run(opts *RunOpts) error {
+func (c *Command) doRun(opts *RunOpts) error {
 	if len(c.brokenArgs) != 0 {
 		log.Error("git command is broken: %s, broken args: %s", c.String(), strings.Join(c.brokenArgs, " "))
 		return ErrBrokenCommand

modules/git/repo_cleanup.go

@@ -27,19 +27,18 @@ func ForciblyUnlockRepositoryIfNeeded(ctx context.Context, repoPath string) erro
 func cleanLocksIfNeeded(repoPath string, threshold time.Time) error {
 	if repoPath == "" {
 		return nil
-	} else {
-		log.Trace("Checking if repository %s is locked [lock threshold is %s]", repoPath, threshold)
-		return filepath.Walk(repoPath, func(filePath string, fileInfo os.FileInfo, err error) error {
-			if err != nil {
-				return err
-			}
-			if err := cleanLockIfNeeded(filePath, fileInfo, threshold); err != nil {
-				log.Error("Failed to remove lock file %s: %v", filePath, err)
-				return err
-			}
-			return nil
-		})
 	}
+	log.Trace("Checking if repository %s is locked [lock threshold is %s]", repoPath, threshold)
+	return filepath.Walk(repoPath, func(filePath string, fileInfo os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if err := cleanLockIfNeeded(filePath, fileInfo, threshold); err != nil {
+			log.Error("Failed to remove lock file %s: %v", filePath, err)
+			return err
+		}
+		return nil
+	})
 }
 
 func cleanLockIfNeeded(filePath string, fileInfo os.FileInfo, threshold time.Time) error {