Skip to content

Revert "Disallow duplicate storage paths (#26484)" #29171

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

Closed
wants to merge 3 commits into from
Closed

Revert "Disallow duplicate storage paths (#26484)" #29171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98925a3
Revert "Disallow duplicate storage paths (#26484)"
wxiaoguang Feb 14, 2024
419b5f6
keep filepath
wxiaoguang Feb 19, 2024
1de8353
Merge branch 'main' into revert-path-check
lunny Mar 29, 2024
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
31 changes: 14 additions & 17 deletions modules/setting/indexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,21 @@ var Indexer = struct {
func loadIndexerFrom(rootCfg ConfigProvider) {
sec := rootCfg.Section("indexer")
Indexer.IssueType = sec.Key("ISSUE_INDEXER_TYPE").MustString("bleve")
if Indexer.IssueType == "bleve" {
Indexer.IssuePath = filepath.ToSlash(sec.Key("ISSUE_INDEXER_PATH").MustString(filepath.ToSlash(filepath.Join(AppDataPath, "indexers/issues.bleve"))))
if !filepath.IsAbs(Indexer.IssuePath) {
Indexer.IssuePath = filepath.ToSlash(filepath.Join(AppWorkPath, Indexer.IssuePath))
}
fatalDuplicatedPath("issue_indexer", Indexer.IssuePath)
} else {
Indexer.IssueConnStr = sec.Key("ISSUE_INDEXER_CONN_STR").MustString(Indexer.IssueConnStr)
if Indexer.IssueType == "meilisearch" {
u, err := url.Parse(Indexer.IssueConnStr)
if err != nil {
log.Warn("Failed to parse ISSUE_INDEXER_CONN_STR: %v", err)
u = &url.URL{}
}
Indexer.IssueConnAuth, _ = u.User.Password()
u.User = nil
Indexer.IssueConnStr = u.String()
Indexer.IssuePath = filepath.ToSlash(sec.Key("ISSUE_INDEXER_PATH").MustString(filepath.ToSlash(filepath.Join(AppDataPath, "indexers/issues.bleve"))))
if !filepath.IsAbs(Indexer.IssuePath) {
Indexer.IssuePath = filepath.ToSlash(filepath.Join(AppWorkPath, Indexer.IssuePath))
}
Indexer.IssueConnStr = sec.Key("ISSUE_INDEXER_CONN_STR").MustString(Indexer.IssueConnStr)

if Indexer.IssueType == "meilisearch" {
u, err := url.Parse(Indexer.IssueConnStr)
if err != nil {
log.Warn("Failed to parse ISSUE_INDEXER_CONN_STR: %v", err)
u = &url.URL{}
}
Indexer.IssueConnAuth, _ = u.User.Password()
u.User = nil
Indexer.IssueConnStr = u.String()
}

Indexer.IssueIndexerName = sec.Key("ISSUE_INDEXER_NAME").MustString(Indexer.IssueIndexerName)
Expand Down
4 changes: 0 additions & 4 deletions modules/setting/path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,8 @@ func init() {
AppWorkPath = filepath.Dir(AppPath)
}

fatalDuplicatedPath("app_work_path", AppWorkPath)

appWorkPathBuiltin = AppWorkPath
customPathBuiltin = CustomPath

fatalDuplicatedPath("custom_path", CustomPath)
customConfBuiltin = CustomConf
}

Expand Down
3 changes: 0 additions & 3 deletions modules/setting/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,6 @@ func loadRepositoryFrom(rootCfg ConfigProvider) {
} else {
RepoRootPath = filepath.Clean(RepoRootPath)
}

fatalDuplicatedPath("repository.ROOT", RepoRootPath)

defaultDetectedCharsetsOrder := make([]string, 0, len(Repository.DetectedCharsetsOrder))
for _, charset := range Repository.DetectedCharsetsOrder {
defaultDetectedCharsetsOrder = append(defaultDetectedCharsetsOrder, strings.ToLower(strings.TrimSpace(charset)))
Expand Down
2 changes: 0 additions & 2 deletions modules/setting/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,15 +324,13 @@ 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)

landingPage := sec.Key("LANDING_PAGE").MustString("home")
switch landingPage {
Expand Down
6 changes: 3 additions & 3 deletions modules/setting/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package setting

import (
"net/http"
"path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -43,10 +44,9 @@ func loadSessionFrom(rootCfg ConfigProvider) {
sec := rootCfg.Section("session")
SessionConfig.Provider = sec.Key("PROVIDER").In("memory",
[]string{"memory", "file", "redis", "mysql", "postgres", "couchbase", "memcache", "db"})
SessionConfig.ProviderConfig = strings.Trim(sec.Key("PROVIDER_CONFIG").MustString(filepath.Join(AppDataPath, "sessions")), "\" ")
SessionConfig.ProviderConfig = strings.Trim(sec.Key("PROVIDER_CONFIG").MustString(path.Join(AppDataPath, "sessions")), "\" ")
if SessionConfig.Provider == "file" && !filepath.IsAbs(SessionConfig.ProviderConfig) {
SessionConfig.ProviderConfig = filepath.Join(AppWorkPath, SessionConfig.ProviderConfig)
fatalDuplicatedPath("session", SessionConfig.ProviderConfig)
SessionConfig.ProviderConfig = path.Join(AppWorkPath, SessionConfig.ProviderConfig)
}
SessionConfig.CookieName = sec.Key("COOKIE_NAME").MustString("i_like_gitea")
SessionConfig.CookiePath = AppSubURL
Expand Down
9 changes: 0 additions & 9 deletions modules/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,3 @@ func LoadSettingsForInstall() {
loadServiceFrom(CfgProvider)
loadMailerFrom(CfgProvider)
}

var uniquePaths = 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)
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it suffice to convert the Fatal to an Error for now?
I know, it doesn't fix the problem completely, but it would be an improvement already…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it comes to the same question:

  • How many users really care about the startup logs .... I have answered many questions about "why something doesn't work" but the startup log already told users.
  • What an user could do if they see such message. Any document or guideline?

Copy link
Member

@delvh delvh Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Let me throw the ball right back at you:
Yes, it is in no way a complete solution, I've understood that already.

However, what is better?

  • Keeping a fix that while not perfect does work at scaring (some) people from shooting themselves in the foot
  • Disabling this check completely because we don't have any idea what recovery measures are available

That's exactly the problem: Everyone who has this problem has to decide how to proceed best themselves.
Gitea does not know which file belongs where. That's the original problem we want to fix.
I don't see any other way than letting each affected admin decide what should be moved where.

I'm in favor of at least keeping the log.Error.
Originally, before you notified me that all Docker users have this problem by default, I would have even argued for Fatal, but that's a valid problem that we have caused ourselves.
As such, we cannot Fatal anymore, but we can at least make every user (that reads the logs) aware of the problem.

And in the meantime, we can work on a better fix.

}
uniquePaths[p] = name
}
2 changes: 0 additions & 2 deletions modules/setting/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ func getStorageForLocal(targetSec, overrideSec ConfigSection, tp targetSecType,
}
}

fatalDuplicatedPath("storage."+name, storage.Path)

return &storage, nil
}

Expand Down