Skip to content

remove deprecated settings #33872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

remove deprecated settings #33872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b74f666
move deprecated settings into a single function
TheFox0x7 Mar 5, 2025
540b192
align tests
TheFox0x7 Mar 14, 2025
3cdf752
add copyright header
TheFox0x7 Mar 14, 2025
4c18f5c
unify error syntax
TheFox0x7 Mar 15, 2025
8428403
add basic comments about the removal
TheFox0x7 Mar 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions modules/setting/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,13 @@ func loadGitFrom(rootCfg ConfigProvider) {
log.Fatal("Failed to map Git settings: %v", err)
}

// legacy reflog config options have been moved to removed.go
secGitConfig := rootCfg.Section("git.config")
GitConfig.Options = make(map[string]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 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())
}
Expand Down
12 changes: 6 additions & 6 deletions modules/setting/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGitConfig(t *testing.T) {
Expand Down Expand Up @@ -50,16 +51,15 @@ func TestGitReflog(t *testing.T) {

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(`
func TestLegacyRefLog(t *testing.T) {
cfg, err := NewConfigProviderFromData(`
[git.reflog]
ENABLED = false
EXPIRATION = 123
`)
assert.NoError(t, err)
loadGitFrom(cfg)
require.NoError(t, err)

assert.EqualValues(t, "false", GitConfig.GetOption("core.logAllRefUpdates"))
assert.EqualValues(t, "123", GitConfig.GetOption("gc.reflogExpire"))
require.Len(t, checkForRemovedSettings(cfg), 2)
}
13 changes: 1 addition & 12 deletions modules/setting/lfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,9 @@ func loadLFSFrom(rootCfg ConfigProvider) error {
mustMapSetting(rootCfg, "server", &LFS)
sec := rootCfg.Section("server")

// legacy LFS_CONTENT_PATH option has been moved to removed.go
lfsSec, _ := rootCfg.GetSection("lfs")

// Specifically default PATH to LFS_CONTENT_PATH
// DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version
// if these are removed, the warning will not be shown
deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we still keep them at the original places? I think that will help code readers to find the history of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First version added errors to every setting loader function which would allow to keep the history, but as it has been explained - settings might do changes based on configuration options so early failure is better.

See #33761 (comment) for explanation and basis for this approach.

I do agree that it's less than ideal though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can put some comments in the original places as a reference?


if val := sec.Key("LFS_CONTENT_PATH").String(); val != "" {
if lfsSec == nil {
lfsSec = rootCfg.Section("lfs")
}
lfsSec.Key("PATH").MustString(val)
}

var err error
LFS.Storage, err = getStorage(rootCfg, "lfs", "", lfsSec)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions modules/setting/lfs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ LFS_CONTENT_PATH = deprecatedpath
`
cfg, err = NewConfigProviderFromData(iniStr)
assert.NoError(t, err)
assert.NoError(t, loadLFSFrom(cfg))

assert.EqualValues(t, "local", LFS.Storage.Type)
assert.Contains(t, LFS.Storage.Path, "deprecatedpath")
assert.Len(t, checkForRemovedSettings(cfg), 1)

iniStr = `
[storage.lfs]
Expand Down
26 changes: 1 addition & 25 deletions modules/setting/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,44 +54,20 @@ func loadLogGlobalFrom(rootCfg ConfigProvider) {
}

func prepareLoggerConfig(rootCfg ConfigProvider) {
// legacy log config options have been moved to removed.go
sec := rootCfg.Section("log")

if !sec.HasKey("logger.default.MODE") {
sec.Key("logger.default.MODE").MustString(",")
}

deprecatedSetting(rootCfg, "log", "ACCESS", "log", "logger.access.MODE", "1.21")
deprecatedSetting(rootCfg, "log", "ENABLE_ACCESS_LOG", "log", "logger.access.MODE", "1.21")
if val := sec.Key("ACCESS").String(); val != "" {
sec.Key("logger.access.MODE").MustString(val)
}
if sec.HasKey("ENABLE_ACCESS_LOG") && !sec.Key("ENABLE_ACCESS_LOG").MustBool() {
sec.Key("logger.access.MODE").SetValue("")
}

deprecatedSetting(rootCfg, "log", "ROUTER", "log", "logger.router.MODE", "1.21")
deprecatedSetting(rootCfg, "log", "DISABLE_ROUTER_LOG", "log", "logger.router.MODE", "1.21")
if val := sec.Key("ROUTER").String(); val != "" {
sec.Key("logger.router.MODE").MustString(val)
}
if !sec.HasKey("logger.router.MODE") {
sec.Key("logger.router.MODE").MustString(",") // use default logger
}
if sec.HasKey("DISABLE_ROUTER_LOG") && sec.Key("DISABLE_ROUTER_LOG").MustBool() {
sec.Key("logger.router.MODE").SetValue("")
}

deprecatedSetting(rootCfg, "log", "XORM", "log", "logger.xorm.MODE", "1.21")
deprecatedSetting(rootCfg, "log", "ENABLE_XORM_LOG", "log", "logger.xorm.MODE", "1.21")
if val := sec.Key("XORM").String(); val != "" {
sec.Key("logger.xorm.MODE").MustString(val)
}
if !sec.HasKey("logger.xorm.MODE") {
sec.Key("logger.xorm.MODE").MustString(",") // use default logger
}
if sec.HasKey("ENABLE_XORM_LOG") && !sec.Key("ENABLE_XORM_LOG").MustBool() {
sec.Key("logger.xorm.MODE").SetValue("")
}
}

func LogPrepareFilenameForWriter(fileName, defaultFileName string) string {
Expand Down
75 changes: 7 additions & 68 deletions modules/setting/log_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,90 +150,29 @@ MODE = console
func TestLogConfigLegacyMode(t *testing.T) {
tempDir := t.TempDir()

tempPath := func(file string) string {
return filepath.Join(tempDir, file)
}

manager, managerClose := initLoggersByConfig(t, `
cfg, err := NewConfigProviderFromData(`
[log]
ROOT_PATH = `+tempDir+`
ROOT_PATH = ` + tempDir + `
MODE = file
ROUTER = file
ACCESS = file
`)
defer managerClose()
require.NoError(t, err)

writerDump := `
{
"file": {
"BufferLen": 10000,
"Colorize": false,
"Expression": "",
"Flags": "stdflags",
"Level": "info",
"Prefix": "",
"StacktraceLevel": "none",
"WriterOption": {
"Compress": true,
"CompressionLevel": -1,
"DailyRotate": true,
"FileName": "$FILENAME",
"LogRotate": true,
"MaxDays": 7,
"MaxSize": 268435456
},
"WriterType": "file"
}
}
`
writerDumpAccess := `
{
"file.access": {
"BufferLen": 10000,
"Colorize": false,
"Expression": "",
"Flags": "none",
"Level": "info",
"Prefix": "",
"StacktraceLevel": "none",
"WriterOption": {
"Compress": true,
"CompressionLevel": -1,
"DailyRotate": true,
"FileName": "$FILENAME",
"LogRotate": true,
"MaxDays": 7,
"MaxSize": 268435456
},
"WriterType": "file"
}
}
`
dump := manager.GetLogger(log.DEFAULT).DumpWriters()
require.JSONEq(t, strings.ReplaceAll(writerDump, "$FILENAME", tempPath("gitea.log")), toJSON(dump))

dump = manager.GetLogger("access").DumpWriters()
require.JSONEq(t, strings.ReplaceAll(writerDumpAccess, "$FILENAME", tempPath("access.log")), toJSON(dump))

dump = manager.GetLogger("router").DumpWriters()
require.JSONEq(t, strings.ReplaceAll(writerDump, "$FILENAME", tempPath("gitea.log")), toJSON(dump))
require.Len(t, checkForRemovedSettings(cfg), 2)
}

func TestLogConfigLegacyModeDisable(t *testing.T) {
manager, managerClose := initLoggersByConfig(t, `
cfg, err := NewConfigProviderFromData(`
[log]
ROUTER = file
ACCESS = file
DISABLE_ROUTER_LOG = true
ENABLE_ACCESS_LOG = false
`)
defer managerClose()
require.NoError(t, err)

dump := manager.GetLogger("access").DumpWriters()
require.JSONEq(t, "{}", toJSON(dump))

dump = manager.GetLogger("router").DumpWriters()
require.JSONEq(t, "{}", toJSON(dump))
require.Len(t, checkForRemovedSettings(cfg), 4)
}

func TestLogConfigNewConfig(t *testing.T) {
Expand Down
71 changes: 1 addition & 70 deletions modules/setting/mailer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,76 +72,7 @@ func loadMailerFrom(rootCfg ConfigProvider) {
if !sec.Key("ENABLED").MustBool() {
return
}

// Handle Deprecations and map on to new configuration
// DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version
// if these are removed, the warning will not be shown
deprecatedSetting(rootCfg, "mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0")
if sec.HasKey("MAILER_TYPE") && !sec.HasKey("PROTOCOL") {
if sec.Key("MAILER_TYPE").String() == "sendmail" {
sec.Key("PROTOCOL").MustString("sendmail")
}
}

deprecatedSetting(rootCfg, "mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0")
if sec.HasKey("HOST") && !sec.HasKey("SMTP_ADDR") {
givenHost := sec.Key("HOST").String()
addr, port, err := net.SplitHostPort(givenHost)
if err != nil && strings.Contains(err.Error(), "missing port in address") {
addr = givenHost
} else if err != nil {
log.Fatal("Invalid mailer.HOST (%s): %v", givenHost, err)
}
if addr == "" {
addr = "127.0.0.1"
}
sec.Key("SMTP_ADDR").MustString(addr)
sec.Key("SMTP_PORT").MustString(port)
}

deprecatedSetting(rootCfg, "mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0")
if sec.HasKey("IS_TLS_ENABLED") && !sec.HasKey("PROTOCOL") {
if sec.Key("IS_TLS_ENABLED").MustBool() {
sec.Key("PROTOCOL").MustString("smtps")
} else {
sec.Key("PROTOCOL").MustString("smtp+starttls")
}
}

deprecatedSetting(rootCfg, "mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0")
if sec.HasKey("DISABLE_HELO") && !sec.HasKey("ENABLE_HELO") {
sec.Key("ENABLE_HELO").MustBool(!sec.Key("DISABLE_HELO").MustBool())
}

deprecatedSetting(rootCfg, "mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0")
if sec.HasKey("SKIP_VERIFY") && !sec.HasKey("FORCE_TRUST_SERVER_CERT") {
sec.Key("FORCE_TRUST_SERVER_CERT").MustBool(sec.Key("SKIP_VERIFY").MustBool())
}

deprecatedSetting(rootCfg, "mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0")
if sec.HasKey("USE_CERTIFICATE") && !sec.HasKey("USE_CLIENT_CERT") {
sec.Key("USE_CLIENT_CERT").MustBool(sec.Key("USE_CERTIFICATE").MustBool())
}

deprecatedSetting(rootCfg, "mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0")
if sec.HasKey("CERT_FILE") && !sec.HasKey("CLIENT_CERT_FILE") {
sec.Key("CERT_FILE").MustString(sec.Key("CERT_FILE").String())
}

deprecatedSetting(rootCfg, "mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0")
if sec.HasKey("KEY_FILE") && !sec.HasKey("CLIENT_KEY_FILE") {
sec.Key("KEY_FILE").MustString(sec.Key("KEY_FILE").String())
}

deprecatedSetting(rootCfg, "mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT", "v1.19.0")
if sec.HasKey("ENABLE_HTML_ALTERNATIVE") && !sec.HasKey("SEND_AS_PLAIN_TEXT") {
sec.Key("SEND_AS_PLAIN_TEXT").MustBool(!sec.Key("ENABLE_HTML_ALTERNATIVE").MustBool(false))
}

if sec.HasKey("PROTOCOL") && sec.Key("PROTOCOL").String() == "smtp+startls" {
log.Error("Deprecated fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead. This fallback will be removed in v1.19.0")
sec.Key("PROTOCOL").SetValue("smtp+starttls")
}
// legacy mailer config options have been moved to removed.go

// Set default values & validate
sec.Key("NAME").MustString(AppName)
Expand Down
11 changes: 4 additions & 7 deletions modules/setting/mailer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func Test_loadMailerFrom(t *testing.T) {
kases := map[string]*Mailer{
cases := map[string]*Mailer{
"smtp.mydomain.com": {
SMTPAddr: "smtp.mydomain.com",
SMTPPort: "465",
Expand All @@ -24,18 +24,15 @@ func Test_loadMailerFrom(t *testing.T) {
SMTPPort: "123",
},
}
for host, kase := range kases {
for host := range cases {
t.Run(host, func(t *testing.T) {
cfg, _ := NewConfigProviderFromData("")
sec := cfg.Section("mailer")
sec.NewKey("ENABLED", "true")
sec.NewKey("HOST", host)

// Check mailer setting
loadMailerFrom(cfg)

assert.EqualValues(t, kase.SMTPAddr, MailService.SMTPAddr)
assert.EqualValues(t, kase.SMTPPort, MailService.SMTPPort)
// Ensure removed settings catches the removed config setting
assert.Len(t, checkForRemovedSettings(cfg), 1)
})
}
}
10 changes: 1 addition & 9 deletions modules/setting/mirror.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,7 @@ var Mirror = struct {
}

func loadMirrorFrom(rootCfg ConfigProvider) {
// Handle old configuration through `[repository]` `DISABLE_MIRRORS`
// - please note this was badly named and only disabled the creation of new pull mirrors
// DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version
// if these are removed, the warning will not be shown
deprecatedSetting(rootCfg, "repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0")
if ConfigSectionKeyBool(rootCfg.Section("repository"), "DISABLE_MIRRORS") {
Mirror.DisableNewPull = true
}

// legacy mirror config options have been moved to removed.go
if err := rootCfg.Section("mirror").MapTo(&Mirror); err != nil {
log.Fatal("Failed to map Mirror settings: %v", err)
}
Expand Down
Loading