Only write to global gitconfig if necessary (#11876)

* Only write to global gitconfig if necessary

Fix #11855

Signed-off-by: Andrew Thornton <art27@cantab.net>

* placate lint

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>

2 files changed, 65 insertions, 25 deletions

diff --git a/modules/git/git.go b/modules/git/git.go
index 7f718511f7..6f231cee7a 100644
--- a/modules/git/git.go
+++ b/modules/git/git.go
@@ -108,44 +108,58 @@ func SetExecutablePath(path string) error {
 // Init initializes git module
 func Init(ctx context.Context) error {
 	DefaultContext = ctx
-	// Git requires setting user.name and user.email in order to commit changes.
+	// Git requires setting user.name and user.email in order to commit changes - if they're not set just add some defaults
 	for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "gitea@fake.local"} {
-		if stdout, stderr, err := process.GetManager().Exec("git.Init(get setting)", GitExecutable, "config", "--get", configKey); err != nil || strings.TrimSpace(stdout) == "" {
-			// ExitError indicates this config is not set
-			if _, ok := err.(*exec.ExitError); ok || strings.TrimSpace(stdout) == "" {
-				if _, stderr, gerr := process.GetManager().Exec("git.Init(set "+configKey+")", "git", "config", "--global", configKey, defaultValue); gerr != nil {
-					return fmt.Errorf("Failed to set git %s(%s): %s", configKey, gerr, stderr)
-				}
-			} else {
-				return fmt.Errorf("Failed to get git %s(%s): %s", configKey, err, stderr)
-			}
+		if err := checkAndSetConfig(configKey, defaultValue, false); err != nil {
+			return err
 		}
 	}
 
-	// Set git some configurations.
-	if _, stderr, err := process.GetManager().Exec("git.Init(git config --global core.quotepath false)",
-		GitExecutable, "config", "--global", "core.quotepath", "false"); err != nil {
-		return fmt.Errorf("Failed to execute 'git config --global core.quotepath false': %s", stderr)
+	// Set git some configurations - these must be set to these values for gitea to work correctly
+	if err := checkAndSetConfig("core.quotePath", "false", true); err != nil {
+		return err
 	}
 
 	if version.Compare(gitVersion, "2.18", ">=") {
-		if _, stderr, err := process.GetManager().Exec("git.Init(git config --global core.commitGraph true)",
-			GitExecutable, "config", "--global", "core.commitGraph", "true"); err != nil {
-			return fmt.Errorf("Failed to execute 'git config --global core.commitGraph true': %s", stderr)
+		if err := checkAndSetConfig("core.commitGraph", "true", true); err != nil {
+			return err
 		}
-
-		if _, stderr, err := process.GetManager().Exec("git.Init(git config --global gc.writeCommitGraph true)",
-			GitExecutable, "config", "--global", "gc.writeCommitGraph", "true"); err != nil {
-			return fmt.Errorf("Failed to execute 'git config --global gc.writeCommitGraph true': %s", stderr)
+		if err := checkAndSetConfig("gc.writeCommitGraph", "true", true); err != nil {
+			return err
 		}
 	}
 
 	if runtime.GOOS == "windows" {
-		if _, stderr, err := process.GetManager().Exec("git.Init(git config --global core.longpaths true)",
-			GitExecutable, "config", "--global", "core.longpaths", "true"); err != nil {
-			return fmt.Errorf("Failed to execute 'git config --global core.longpaths true': %s", stderr)
+		if err := checkAndSetConfig("core.longpaths", "true", true); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error {
+	stdout, stderr, err := process.GetManager().Exec("git.Init(get setting)", GitExecutable, "config", "--get", key)
+	if err != nil {
+		perr, ok := err.(*process.Error)
+		if !ok {
+			return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr)
 		}
+		eerr, ok := perr.Err.(*exec.ExitError)
+		if !ok || eerr.ExitCode() != 1 {
+			return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr)
+		}
+	}
+
+	currValue := strings.TrimSpace(stdout)
+
+	if currValue == defaultValue || (!forceToDefault && len(currValue) > 0) {
+		return nil
 	}
+
+	if _, stderr, err = process.GetManager().Exec(fmt.Sprintf("git.Init(set %s)", key), "git", "config", "--global", key, defaultValue); err != nil {
+		return fmt.Errorf("Failed to set git %s(%s): %s", key, err, stderr)
+	}
+
 	return nil
 }
 
diff --git a/modules/process/manager.go b/modules/process/manager.go
index af6ee9b81d..27ed1d4964 100644
--- a/modules/process/manager.go
+++ b/modules/process/manager.go
@@ -157,7 +157,14 @@ func (pm *Manager) ExecDirEnvStdIn(timeout time.Duration, dir, desc string, env
 	pm.Remove(pid)
 
 	if err != nil {
-		err = fmt.Errorf("exec(%d:%s) failed: %v(%v) stdout: %v stderr: %v", pid, desc, err, ctx.Err(), stdOut, stdErr)
+		err = &Error{
+			PID:         pid,
+			Description: desc,
+			Err:         err,
+			CtxErr:      ctx.Err(),
+			Stdout:      stdOut.String(),
+			Stderr:      stdErr.String(),
+		}
 	}
 
 	return stdOut.String(), stdErr.String(), err
@@ -176,3 +183,22 @@ func (l processList) Less(i, j int) bool {
 func (l processList) Swap(i, j int) {
 	l[i], l[j] = l[j], l[i]
 }
+
+// Error is a wrapped error describing the error results of Process Execution
+type Error struct {
+	PID         int64
+	Description string
+	Err         error
+	CtxErr      error
+	Stdout      string
+	Stderr      string
+}
+
+func (err *Error) Error() string {
+	return fmt.Sprintf("exec(%d:%s) failed: %v(%v) stdout: %s stderr: %s", err.PID, err.Description, err.Err, err.CtxErr, err.Stdout, err.Stderr)
+}
+
+// Unwrap implements the unwrappable implicit interface for go1.13 Unwrap()
+func (err *Error) Unwrap() error {
+	return err.Err
+}