-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Do not allow different storage configurations to point to the same directory #30169
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.
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
Changes from all commits
ce64832
bbd966d
4729dac
90322c5
0975f2c
00a656c
3e5e2f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -286,7 +286,7 @@ func loadRepositoryFrom(rootCfg ConfigProvider) { | |||||
RepoRootPath = filepath.Clean(RepoRootPath) | ||||||
} | ||||||
|
||||||
fatalDuplicatedPath("repository.ROOT", RepoRootPath) | ||||||
checkOverlappedPath("repository.ROOT", RepoRootPath) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
defaultDetectedCharsetsOrder := make([]string, 0, len(Repository.DetectedCharsetsOrder)) | ||||||
for _, charset := range Repository.DetectedCharsetsOrder { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -324,15 +324,14 @@ func loadServerFrom(rootCfg ConfigProvider) { | |||||
if !filepath.IsAbs(AppDataPath) { | ||||||
AppDataPath = filepath.ToSlash(filepath.Join(AppWorkPath, AppDataPath)) | ||||||
} | ||||||
fatalDuplicatedPath("app_data_path", AppDataPath) | ||||||
|
||||||
EnableGzip = sec.Key("ENABLE_GZIP").MustBool() | ||||||
EnablePprof = sec.Key("ENABLE_PPROF").MustBool(false) | ||||||
PprofDataPath = sec.Key("PPROF_DATA_PATH").MustString(filepath.Join(AppWorkPath, "data/tmp/pprof")) | ||||||
if !filepath.IsAbs(PprofDataPath) { | ||||||
PprofDataPath = filepath.Join(AppWorkPath, PprofDataPath) | ||||||
} | ||||||
fatalDuplicatedPath("pprof_data_path", PprofDataPath) | ||||||
checkOverlappedPath("server.PPROF_DATA_PATH", PprofDataPath) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
landingPage := sec.Key("LANDING_PAGE").MustString("home") | ||||||
switch landingPage { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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) | ||||||
fatalDuplicatedPath("session", SessionConfig.ProviderConfig) | ||||||
checkOverlappedPath("session.PROVIDER_CONFIG", SessionConfig.ProviderConfig) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
SessionConfig.CookieName = sec.Key("COOKIE_NAME").MustString("i_like_gitea") | ||||||
SessionConfig.CookiePath = AppSubURL | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -230,11 +230,14 @@ func LoadSettingsForInstall() { | |||||||||||||||||||||||||||||||||||||||
loadMailerFrom(CfgProvider) | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
var uniquePaths = make(map[string]string) | ||||||||||||||||||||||||||||||||||||||||
var configuredPaths = make(map[string]string) | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
func fatalDuplicatedPath(name, p string) { | ||||||||||||||||||||||||||||||||||||||||
if targetName, ok := uniquePaths[p]; ok && targetName != name { | ||||||||||||||||||||||||||||||||||||||||
log.Fatal("storage path %q is being used by %q and %q and all storage paths must be unique to prevent data loss.", p, targetName, name) | ||||||||||||||||||||||||||||||||||||||||
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) | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+236
to
241
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we fix this completely using
Suggested change
(assuming we have an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not right (incomplete). If you would like to "complete" it, it also needs enough tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, fixed my comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But your "msg" is still not right, and it doesn't have tests. As I said: "I won't do any further change". So feel free to take it, or improve it, or discard it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And one more thing, |
||||||||||||||||||||||||||||||||||||||||
uniquePaths[p] = name | ||||||||||||||||||||||||||||||||||||||||
configuredPaths[path] = name | ||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -240,7 +240,7 @@ func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType, | |||||
} | ||||||
} | ||||||
|
||||||
fatalDuplicatedPath("storage."+name, storage.Path) | ||||||
checkOverlappedPath("storage."+name+".PATH", storage.Path) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not right by your approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, fixed my comment |
||||||
|
||||||
return &storage, nil | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better if we notify the users explicitly what the section is: