Refactor git version functions and check compatibility (#29155)

modules/git/git.go

@@ -39,36 +39,37 @@ var (
 	gitVersion *version.Version
 )
 
-// loadGitVersion returns current Git version from shell. Internal usage only.
-func loadGitVersion() (*version.Version, error) {
+// loadGitVersion tries to get the current git version and stores it into a global variable
+func loadGitVersion() error {
 	// doesn't need RWMutex because it's executed by Init()
 	if gitVersion != nil {
-		return gitVersion, nil
+		return nil
 	}
 
 	stdout, _, runErr := NewCommand(DefaultContext, "version").RunStdString(nil)
 	if runErr != nil {
-		return nil, runErr
+		return runErr
 	}
 
-	fields := strings.Fields(stdout)
-	if len(fields) < 3 {
-		return nil, fmt.Errorf("invalid git version output: %s", stdout)
+	ver, err := parseGitVersionLine(strings.TrimSpace(stdout))
+	if err == nil {
+		gitVersion = ver
 	}
+	return err
+}
 
-	var versionString string
-
-	// Handle special case on Windows.
-	i := strings.Index(fields[2], "windows")
-	if i >= 1 {
-		versionString = fields[2][:i-1]
-	} else {
-		versionString = fields[2]
+func parseGitVersionLine(s string) (*version.Version, error) {
+	fields := strings.Fields(s)
+	if len(fields) < 3 {
+		return nil, fmt.Errorf("invalid git version: %q", s)
 	}
 
-	var err error
-	gitVersion, err = version.NewVersion(versionString)
-	return gitVersion, err
+	// version string is like: "git version 2.29.3" or "git version 2.29.3.windows.1"
+	versionString := fields[2]
+	if pos := strings.Index(versionString, "windows"); pos >= 1 {
+		versionString = versionString[:pos-1]
+	}
+	return version.NewVersion(versionString)
 }
 
 // SetExecutablePath changes the path of git executable and checks the file permission and version.
@@ -83,8 +84,7 @@ func SetExecutablePath(path string) error {
 	}
 	GitExecutable = absPath
 
-	_, err = loadGitVersion()
-	if err != nil {
+	if err = loadGitVersion(); err != nil {
 		return fmt.Errorf("unable to load git version: %w", err)
 	}
 
@@ -105,6 +105,9 @@ func SetExecutablePath(path string) error {
 		return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", gitVersion.Original(), RequiredVersion, moreHint)
 	}
 
+	if err = checkGitVersionCompatibility(gitVersion); err != nil {
+		log.Error("installed git version %s has a known compatibility issue with Gitea: %s, please downgrade (or upgrade) your git", gitVersion.String(), err.Error())
+	}
 	return nil
 }
 
@@ -256,19 +259,18 @@ func syncGitConfig() (err error) {
 		}
 	}
 
-	// Due to CVE-2022-24765, git now denies access to git directories which are not owned by current user
-	// however, some docker users and samba users find it difficult to configure their systems so that Gitea's git repositories are owned by the Gitea user. (Possibly Windows Service users - but ownership in this case should really be set correctly on the filesystem.)
-	// see issue: https://github.com/go-gitea/gitea/issues/19455
-	// Fundamentally the problem lies with the uid-gid-mapping mechanism for filesystems in docker on windows (and to a lesser extent samba).
-	// Docker's configuration mechanism for local filesystems provides no way of setting this mapping and although there is a mechanism for setting this uid through using cifs mounting it is complicated and essentially undocumented
-	// Thus the owner uid/gid for files on these filesystems will be marked as root.
+	// Due to CVE-2022-24765, git now denies access to git directories which are not owned by current user.
+	// However, some docker users and samba users find it difficult to configure their systems correctly,
+	// so that Gitea's git repositories are owned by the Gitea user.
+	// (Possibly Windows Service users - but ownership in this case should really be set correctly on the filesystem.)
+	// See issue: https://github.com/go-gitea/gitea/issues/19455
 	// As Gitea now always use its internal git config file, and access to the git repositories is managed through Gitea,
 	// it is now safe to set "safe.directory=*" for internal usage only.
-	// Please note: the wildcard "*" is only supported by Git 2.30.4/2.31.3/2.32.2/2.33.3/2.34.3/2.35.3/2.36 and later
-	// Although only supported by Git 2.30.4/2.31.3/2.32.2/2.33.3/2.34.3/2.35.3/2.36 and later - this setting is tolerated by earlier versions
+	// Although this setting is only supported by some new git versions, it is also tolerated by earlier versions
 	if err := configAddNonExist("safe.directory", "*"); err != nil {
 		return err
 	}
+
 	if runtime.GOOS == "windows" {
 		if err := configSet("core.longpaths", "true"); err != nil {
 			return err
@@ -301,8 +303,8 @@ func syncGitConfig() (err error) {
 
 // CheckGitVersionAtLeast check git version is at least the constraint version
 func CheckGitVersionAtLeast(atLeast string) error {
-	if _, err := loadGitVersion(); err != nil {
-		return err
+	if gitVersion == nil {
+		panic("git module is not initialized") // it shouldn't happen
 	}
 	atLeastVersion, err := version.NewVersion(atLeast)
 	if err != nil {
@@ -314,6 +316,21 @@ func CheckGitVersionAtLeast(atLeast string) error {
 	return nil
 }
 
+func checkGitVersionCompatibility(gitVer *version.Version) error {
+	badVersions := []struct {
+		Version *version.Version
+		Reason  string
+	}{
+		{version.Must(version.NewVersion("2.43.1")), "regression bug of GIT_FLUSH"},
+	}
+	for _, bad := range badVersions {
+		if gitVer.Equal(bad.Version) {
+			return errors.New(bad.Reason)
+		}
+	}
+	return nil
+}
+
 func configSet(key, value string) error {
 	stdout, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil)
 	if err != nil && !err.IsExitCode(1) {

modules/git/git_test.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
+	"github.com/hashicorp/go-version"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -93,3 +94,25 @@ func TestSyncConfig(t *testing.T) {
 	assert.True(t, gitConfigContains("[sync-test]"))
 	assert.True(t, gitConfigContains("cfg-key-a = CfgValA"))
 }
+
+func TestParseGitVersion(t *testing.T) {
+	v, err := parseGitVersionLine("git version 2.29.3")
+	assert.NoError(t, err)
+	assert.Equal(t, "2.29.3", v.String())
+
+	v, err = parseGitVersionLine("git version 2.29.3.windows.1")
+	assert.NoError(t, err)
+	assert.Equal(t, "2.29.3", v.String())
+
+	_, err = parseGitVersionLine("git version")
+	assert.Error(t, err)
+
+	_, err = parseGitVersionLine("git version windows")
+	assert.Error(t, err)
+}
+
+func TestCheckGitVersionCompatibility(t *testing.T) {
+	assert.NoError(t, checkGitVersionCompatibility(version.Must(version.NewVersion("2.43.0"))))
+	assert.ErrorContains(t, checkGitVersionCompatibility(version.Must(version.NewVersion("2.43.1"))), "regression bug of GIT_FLUSH")
+	assert.NoError(t, checkGitVersionCompatibility(version.Must(version.NewVersion("2.43.2"))))
+}

modules/git/repo_attribute.go

@@ -11,6 +11,8 @@ import (
 	"os"
 
 	"code.gitea.io/gitea/modules/log"
+
+	"github.com/hashicorp/go-version"
 )
 
 // CheckAttributeOpts represents the possible options to CheckAttribute
@@ -133,7 +135,14 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
 		c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree)
 	}
 
-	c.env = append(c.env, "GIT_FLUSH=1")
+	if gitVersion.Equal(version.Must(version.NewVersion("2.43.1"))) {
+		// https://github.com/go-gitea/gitea/issues/29141  gitea hanging with git 2.43.1 #29141
+		// https://lore.kernel.org/git/CABn0oJvg3M_kBW-u=j3QhKnO=6QOzk-YFTgonYw_UvFS1NTX4g@mail.gmail.com/
+		// git 2.43.1 has a bug: the GIT_FLUSH polarity is flipped
+		c.env = append(c.env, "GIT_FLUSH=0")
+	} else {
+		c.env = append(c.env, "GIT_FLUSH=1")
+	}
 
 	c.cmd.AddDynamicArguments(c.Attributes...)