From bbe5cd7c92ccc3793473ae0163398cdbccdd4246 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 7 Apr 2024 09:11:25 +0800 Subject: [PATCH] Refactor startup deprecation messages (#30305) It doesn't change logic, it only does: 1. Rename the variable and function names 2. Use more consistent format when mentioning config section&key 3. Improve some messages --- modules/setting/config_provider.go | 16 ++++++++++------ modules/setting/indexer.go | 2 +- modules/setting/oauth2.go | 2 +- modules/setting/repository.go | 2 +- modules/setting/server.go | 2 +- modules/setting/session.go | 2 +- modules/setting/setting.go | 4 +--- modules/setting/storage.go | 2 +- routers/web/admin/admin.go | 18 +++++++++--------- routers/web/admin/config.go | 2 +- templates/admin/self_check.tmpl | 6 +++--- 11 files changed, 30 insertions(+), 28 deletions(-) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 3fa3f3b50b..03f27ba203 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -315,21 +315,25 @@ func mustMapSetting(rootCfg ConfigProvider, sectionName string, setting any) { } } -// DeprecatedWarnings contains the warning message for various deprecations, including: setting option, file/folder, etc -var DeprecatedWarnings []string +// StartupProblems contains the messages for various startup problems, including: setting option, file/folder, etc +var StartupProblems []string + +func logStartupProblem(skip int, level log.Level, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + log.Log(skip+1, level, "%s", msg) + StartupProblems = append(StartupProblems, msg) +} func deprecatedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) { if rootCfg.Section(oldSection).HasKey(oldKey) { - msg := fmt.Sprintf("Deprecated config option `[%s]` `%s` present. Use `[%s]` `%s` instead. This fallback will be/has been removed in %s", oldSection, oldKey, newSection, newKey, version) - log.Error("%v", msg) - DeprecatedWarnings = append(DeprecatedWarnings, msg) + logStartupProblem(1, log.ERROR, "Deprecation: config option `[%s].%s` presents, please use `[%s].%s` instead because this fallback will be/has been removed in %s", oldSection, oldKey, newSection, newKey, version) } } // deprecatedSettingDB add a hint that the configuration has been moved to database but still kept in app.ini func deprecatedSettingDB(rootCfg ConfigProvider, oldSection, oldKey string) { if rootCfg.Section(oldSection).HasKey(oldKey) { - log.Error("Deprecated `[%s]` `%s` present which has been copied to database table sys_setting", oldSection, oldKey) + logStartupProblem(1, log.ERROR, "Deprecation: config option `[%s].%s` presents but it won't take effect because it has been moved to admin panel -> config setting", oldSection, oldKey) } } diff --git a/modules/setting/indexer.go b/modules/setting/indexer.go index cec364d370..6877d70e3c 100644 --- a/modules/setting/indexer.go +++ b/modules/setting/indexer.go @@ -58,7 +58,7 @@ func loadIndexerFrom(rootCfg ConfigProvider) { if !filepath.IsAbs(Indexer.IssuePath) { Indexer.IssuePath = filepath.ToSlash(filepath.Join(AppWorkPath, Indexer.IssuePath)) } - checkOverlappedPath("indexer.ISSUE_INDEXER_PATH", Indexer.IssuePath) + checkOverlappedPath("[indexer].ISSUE_INDEXER_PATH", Indexer.IssuePath) } else { Indexer.IssueConnStr = sec.Key("ISSUE_INDEXER_CONN_STR").MustString(Indexer.IssueConnStr) if Indexer.IssueType == "meilisearch" { diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index 4d3bfd3eb6..1429a7585c 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -168,7 +168,7 @@ func GetGeneralTokenSigningSecret() []byte { } if generalSigningSecret.CompareAndSwap(old, &jwtSecret) { // FIXME: in main branch, the signing token should be refactored (eg: one unique for LFS/OAuth2/etc ...) - log.Warn("OAuth2 is not enabled, unable to use a persistent signing secret, a new one is generated, which is not persistent between restarts and cluster nodes") + logStartupProblem(1, log.WARN, "OAuth2 is not enabled, unable to use a persistent signing secret, a new one is generated, which is not persistent between restarts and cluster nodes") return jwtSecret } return *generalSigningSecret.Load() diff --git a/modules/setting/repository.go b/modules/setting/repository.go index a332d6adb3..8656ebc7ec 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -286,7 +286,7 @@ func loadRepositoryFrom(rootCfg ConfigProvider) { RepoRootPath = filepath.Clean(RepoRootPath) } - checkOverlappedPath("repository.ROOT", RepoRootPath) + checkOverlappedPath("[repository].ROOT", RepoRootPath) defaultDetectedCharsetsOrder := make([]string, 0, len(Repository.DetectedCharsetsOrder)) for _, charset := range Repository.DetectedCharsetsOrder { diff --git a/modules/setting/server.go b/modules/setting/server.go index 315faaeb21..7d6ece2727 100644 --- a/modules/setting/server.go +++ b/modules/setting/server.go @@ -331,7 +331,7 @@ func loadServerFrom(rootCfg ConfigProvider) { if !filepath.IsAbs(PprofDataPath) { PprofDataPath = filepath.Join(AppWorkPath, PprofDataPath) } - checkOverlappedPath("server.PPROF_DATA_PATH", PprofDataPath) + checkOverlappedPath("[server].PPROF_DATA_PATH", PprofDataPath) landingPage := sec.Key("LANDING_PAGE").MustString("home") switch landingPage { diff --git a/modules/setting/session.go b/modules/setting/session.go index 3cb1bfe7b5..afe63bfdb7 100644 --- a/modules/setting/session.go +++ b/modules/setting/session.go @@ -46,7 +46,7 @@ func loadSessionFrom(rootCfg ConfigProvider) { SessionConfig.ProviderConfig = strings.Trim(sec.Key("PROVIDER_CONFIG").MustString(filepath.Join(AppDataPath, "sessions")), "\" ") if SessionConfig.Provider == "file" && !filepath.IsAbs(SessionConfig.ProviderConfig) { SessionConfig.ProviderConfig = filepath.Join(AppWorkPath, SessionConfig.ProviderConfig) - checkOverlappedPath("session.PROVIDER_CONFIG", SessionConfig.ProviderConfig) + checkOverlappedPath("[session].PROVIDER_CONFIG", SessionConfig.ProviderConfig) } SessionConfig.CookieName = sec.Key("COOKIE_NAME").MustString("i_like_gitea") SessionConfig.CookiePath = AppSubURL diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 6aca9ec6cf..92bb0b6541 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -235,9 +235,7 @@ var configuredPaths = make(map[string]string) func checkOverlappedPath(name, path string) { // TODO: some paths shouldn't overlap (storage.xxx.path), while some could (data path is the base path for storage path) if targetName, ok := configuredPaths[path]; ok && targetName != name { - msg := fmt.Sprintf("Configured path %q is used by %q and %q at the same time. The paths must be unique to prevent data loss.", path, targetName, name) - log.Error("%s", msg) - DeprecatedWarnings = append(DeprecatedWarnings, msg) + logStartupProblem(1, log.ERROR, "Configured path %q is used by %q and %q at the same time. The paths must be unique to prevent data loss.", path, targetName, name) } configuredPaths[path] = name } diff --git a/modules/setting/storage.go b/modules/setting/storage.go index f4e33a53af..aeb61ac513 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -240,7 +240,7 @@ func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType, } } - checkOverlappedPath("storage."+name+".PATH", storage.Path) + checkOverlappedPath("[storage."+name+"].PATH", storage.Path) return &storage, nil } diff --git a/routers/web/admin/admin.go b/routers/web/admin/admin.go index 4dc0dfdef8..e6585d8833 100644 --- a/routers/web/admin/admin.go +++ b/routers/web/admin/admin.go @@ -117,11 +117,11 @@ func updateSystemStatus() { sysStatus.NumGC = m.NumGC } -func prepareDeprecatedWarningsAlert(ctx *context.Context) { - if len(setting.DeprecatedWarnings) > 0 { - content := setting.DeprecatedWarnings[0] - if len(setting.DeprecatedWarnings) > 1 { - content += fmt.Sprintf(" (and %d more)", len(setting.DeprecatedWarnings)-1) +func prepareStartupProblemsAlert(ctx *context.Context) { + if len(setting.StartupProblems) > 0 { + content := setting.StartupProblems[0] + if len(setting.StartupProblems) > 1 { + content += fmt.Sprintf(" (and %d more)", len(setting.StartupProblems)-1) } ctx.Flash.Error(content, true) } @@ -136,7 +136,7 @@ func Dashboard(ctx *context.Context) { updateSystemStatus() ctx.Data["SysStatus"] = sysStatus ctx.Data["SSH"] = setting.SSH - prepareDeprecatedWarningsAlert(ctx) + prepareStartupProblemsAlert(ctx) ctx.HTML(http.StatusOK, tplDashboard) } @@ -191,10 +191,10 @@ func DashboardPost(ctx *context.Context) { func SelfCheck(ctx *context.Context) { ctx.Data["PageIsAdminSelfCheck"] = true - ctx.Data["DeprecatedWarnings"] = setting.DeprecatedWarnings - if len(setting.DeprecatedWarnings) == 0 && !setting.IsProd { + ctx.Data["StartupProblems"] = setting.StartupProblems + if len(setting.StartupProblems) == 0 && !setting.IsProd { if time.Now().Unix()%2 == 0 { - ctx.Data["DeprecatedWarnings"] = []string{"This is a test warning message in dev mode"} + ctx.Data["StartupProblems"] = []string{"This is a test warning message in dev mode"} } } diff --git a/routers/web/admin/config.go b/routers/web/admin/config.go index 2f5f17e201..48f80dbbf1 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -165,7 +165,7 @@ func Config(ctx *context.Context) { ctx.Data["Loggers"] = log.GetManager().DumpLoggers() config.GetDynGetter().InvalidateCache() - prepareDeprecatedWarningsAlert(ctx) + prepareStartupProblemsAlert(ctx) ctx.HTML(http.StatusOK, tplConfig) } diff --git a/templates/admin/self_check.tmpl b/templates/admin/self_check.tmpl index c100ffd504..a6c2ac1ac9 100644 --- a/templates/admin/self_check.tmpl +++ b/templates/admin/self_check.tmpl @@ -5,11 +5,11 @@ {{ctx.Locale.Tr "admin.self_check"}} - {{if .DeprecatedWarnings}} + {{if .StartupProblems}}
{{ctx.Locale.Tr "admin.self_check.startup_warnings"}}
-
    {{range .DeprecatedWarnings}}
  • {{.}}
    • {{end}}
+
    {{range .StartupProblems}}
  • {{.}}
    • {{end}}
{{end}} @@ -40,7 +40,7 @@ {{end}} - {{if and (not .DeprecatedWarnings) (not .DatabaseCheckHasProblems)}} + {{if and (not .StartupProblems) (not .DatabaseCheckHasProblems)}}
{{ctx.Locale.Tr "admin.self_check.no_problem_found"}}