-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Clean repository ROOT directory name with filepath.Clean #2846
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2846 +/- ##
=======================================
Coverage 26.85% 26.85%
=======================================
Files 89 89
Lines 17607 17607
=======================================
Hits 4728 4728
Misses 12193 12193
Partials 686 686 Continue to review full report at Codecov.
|
FWIW I'm already running this patch in a windows production environment. Although I applied it to the release/V1.2 branch in that case. I'm happy to provide a backport to that branch if/when this PR is accepted. |
modules/setting/setting.go
Outdated
@@ -973,7 +973,7 @@ func NewContext() { | |||
if !filepath.IsAbs(RepoRootPath) { | |||
RepoRootPath = path.Join(AppWorkPath, RepoRootPath) |
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.
Maybe change also here to filepath.Join
so that behaviour is the same in both cases
For better compatibility with Windows.
@lafriks I've made the change you requested. I think it is important to clarify that only filepath.Clean is really important in the context of paths based on settings that come out of app.ini. As mentioned in the related issue #2845, app.ini only allows front slashes in the repository ROOT setting. This is enforced by $ grep forcePathSeparator modules/setting/setting.go
func forcePathSeparator(path string) {
forcePathSeparator(LogRootPath)
forcePathSeparator(RepoRootPath)
forcePathSeparator(AvatarUploadPath) As a result the calls to path.Join vs. filepath.Join don't matter so much in this context. For example:
On Windows with the a GITEA work dir of filepath.Join Fortunately either variant works with go's file and directory functions. Go seems very forgiving when it comes to this sort of problem and seems to accept either slash in path names on Windows. TL;DR I don't think we have to worry about path.Join, only path.Clean when it comes to UNC file system paths (or any other file paths) on Windows. |
LGTM |
LGTM |
For better compatibility with Windows.
Allows UNC paths for the repository ROOT. Fixes: #2845