Use [git.config] for reflog cleaning up (#24958)

Follow
#24860 (comment)

Use `[git.config]` for reflog cleaning up, the new options are more
flexible.

*
https://git-scm.com/docs/git-config#Documentation/git-config.txt-corelogAllRefUpdates
*
https://git-scm.com/docs/git-config#Documentation/git-config.txt-gcreflogExpire

## ⚠️ BREAKING

The section `[git.reflog]` is now obsolete and its keys have been moved
to the following replacements:
- `[git.reflog].ENABLED` → `[git.config].core.logAllRefUpdates`
- `[git.reflog].EXPIRATION` → `[git.config].gc.reflogExpire`

Author: wxiaoguang

custom/conf/app.example.ini

@@ -693,17 +693,13 @@ LEVEL = Info
 ;PULL = 300
 ;GC = 60
 
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Git Reflog timeout in days
-;[git.reflog]
-;ENABLED = true
-;EXPIRATION = 90
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Git config options
 ;; This section only does "set" config, a removed config key from this section won't be removed from git config automatically. The format is `some.configKey = value`.
 ;[git.config]
 ;diff.algorithm = histogram
+;core.logAllRefUpdates = true
+;gc.reflogExpire = 90
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

docs/content/doc/administration/config-cheat-sheet.en-us.md

@@ -1065,17 +1065,14 @@ Default templates for project boards:
 - `PULL`: **300**: Git pull from internal repositories timeout seconds.
 - `GC`: **60**: Git repository GC timeout seconds.
 
-### Git - Reflog settings (`git.reflog`)
-
-- `ENABLED`: **true** Set to true to enable Git to write changes to reflogs in each repo.
-- `EXPIRATION`: **90** Reflog entry lifetime, in days. Entries are removed opportunistically by Git.
-
 ### Git - Config options (`git.config`)
 
 The key/value pairs in this section will be used as git config.
 This section only does "set" config, a removed config key from this section won't be removed from git config automatically. The format is `some.configKey = value`.
 
 - `diff.algorithm`: **histogram**
+- `core.logAllRefUpdates`: **true**
+- `gc.reflogExpire`: **90**
 
 ## Metrics (`metrics`)
 

modules/git/git.go

@@ -201,23 +201,6 @@ func InitFull(ctx context.Context) (err error) {
 	return syncGitConfig()
 }
 
-func enableReflogs() error {
-	if err := configSet("core.logAllRefUpdates", "true"); err != nil {
-		return err
-	}
-	err := configSet("gc.reflogExpire", fmt.Sprintf("%d", setting.Git.Reflog.Expiration))
-	return err
-}
-
-func disableReflogs() error {
-	if err := configUnsetAll("core.logAllRefUpdates", "true"); err != nil {
-		return err
-	} else if err := configUnsetAll("gc.reflogExpire", ""); err != nil {
-		return err
-	}
-	return nil
-}
-
 // syncGitConfig only modifies gitconfig, won't change global variables (otherwise there will be data-race problem)
 func syncGitConfig() (err error) {
 	if err = os.MkdirAll(HomeDir(), os.ModePerm); err != nil {
@@ -249,16 +232,6 @@ func syncGitConfig() (err error) {
 		return err
 	}
 
-	if setting.Git.Reflog.Enabled {
-		if err := enableReflogs(); err != nil {
-			return err
-		}
-	} else {
-		if err := disableReflogs(); err != nil {
-			return err
-		}
-	}
-
 	if CheckGitVersionAtLeast("2.10") == nil {
 		if err := configSet("receive.advertisePushOptions", "true"); err != nil {
 			return err

modules/setting/git.go

@@ -16,10 +16,7 @@ var Git = struct {
 	Path                 string
 	HomePath             string
 	DisableDiffHighlight bool
-	Reflog               struct {
-		Enabled    bool
-		Expiration int
-	} `ini:"git.reflog"`
+
 	MaxGitDiffLines           int
 	MaxGitDiffLineCharacters  int
 	MaxGitDiffFiles           int
@@ -42,13 +39,6 @@ var Git = struct {
 		GC      int `ini:"GC"`
 	} `ini:"git.timeout"`
 }{
-	Reflog: struct {
-		Enabled    bool
-		Expiration int
-	}{
-		Enabled:    true,
-		Expiration: 90,
-	},
 	DisableDiffHighlight:      false,
 	MaxGitDiffLines:           1000,
 	MaxGitDiffLineCharacters:  5000,
@@ -79,9 +69,19 @@ var Git = struct {
 	},
 }
 
-var GitConfig = struct {
-	Options map[string]string
-}{
+type GitConfigType struct {
+	Options map[string]string // git config key is case-insensitive, always use lower-case
+}
+
+func (c *GitConfigType) SetOption(key, val string) {
+	c.Options[strings.ToLower(key)] = val
+}
+
+func (c *GitConfigType) GetOption(key string) string {
+	return c.Options[strings.ToLower(key)]
+}
+
+var GitConfig = GitConfigType{
 	Options: make(map[string]string),
 }
 
@@ -93,12 +93,22 @@ func loadGitFrom(rootCfg ConfigProvider) {
 
 	secGitConfig := rootCfg.Section("git.config")
 	GitConfig.Options = make(map[string]string)
-	for _, key := range secGitConfig.Keys() {
-		// git config key is case-insensitive, so always use lower-case
-		GitConfig.Options[strings.ToLower(key.Name())] = key.String()
+	GitConfig.SetOption("diff.algorithm", "histogram")
+	GitConfig.SetOption("core.logAllRefUpdates", "true")
+	GitConfig.SetOption("gc.reflogExpire", "90")
+
+	secGitReflog := rootCfg.Section("git.reflog")
+	if secGitReflog.HasKey("ENABLED") {
+		deprecatedSetting(rootCfg, "git.reflog", "ENABLED", "git.config", "core.logAllRefUpdates", "1.21")
+		GitConfig.SetOption("core.logAllRefUpdates", secGitReflog.Key("ENABLED").In("true", []string{"true", "false"}))
 	}
-	if _, ok := GitConfig.Options["diff.algorithm"]; !ok {
-		GitConfig.Options["diff.algorithm"] = "histogram"
+	if secGitReflog.HasKey("EXPIRATION") {
+		deprecatedSetting(rootCfg, "git.reflog", "EXPIRATION", "git.config", "core.reflogExpire", "1.21")
+		GitConfig.SetOption("gc.reflogExpire", secGitReflog.Key("EXPIRATION").String())
+	}
+
+	for _, key := range secGitConfig.Keys() {
+		GitConfig.SetOption(key.Name(), key.String())
 	}
 
 	Git.HomePath = sec.Key("HOME_PATH").MustString("home")

modules/setting/git_test.go

@@ -23,8 +23,6 @@ a.b = 1
 `)
 	assert.NoError(t, err)
 	loadGitFrom(cfg)
-
-	assert.Len(t, GitConfig.Options, 2)
 	assert.EqualValues(t, "1", GitConfig.Options["a.b"])
 	assert.EqualValues(t, "histogram", GitConfig.Options["diff.algorithm"])
 
@@ -34,7 +32,34 @@ diff.algorithm = other
 `)
 	assert.NoError(t, err)
 	loadGitFrom(cfg)
-
-	assert.Len(t, GitConfig.Options, 1)
 	assert.EqualValues(t, "other", GitConfig.Options["diff.algorithm"])
 }
+
+func TestGitReflog(t *testing.T) {
+	oldGit := Git
+	oldGitConfig := GitConfig
+	defer func() {
+		Git = oldGit
+		GitConfig = oldGitConfig
+	}()
+
+	// default reflog config without legacy options
+	cfg, err := NewConfigProviderFromData(``)
+	assert.NoError(t, err)
+	loadGitFrom(cfg)
+
+	assert.EqualValues(t, "true", GitConfig.GetOption("core.logAllRefUpdates"))
+	assert.EqualValues(t, "90", GitConfig.GetOption("gc.reflogExpire"))
+
+	// custom reflog config by legacy options
+	cfg, err = NewConfigProviderFromData(`
+[git.reflog]
+ENABLED = false
+EXPIRATION = 123
+`)
+	assert.NoError(t, err)
+	loadGitFrom(cfg)
+
+	assert.EqualValues(t, "false", GitConfig.GetOption("core.logAllRefUpdates"))
+	assert.EqualValues(t, "123", GitConfig.GetOption("gc.reflogExpire"))
+}