-
Notifications
You must be signed in to change notification settings - Fork 180
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
mod_dav_svn: Use mod_dav's DAVBasePath setting #28
base: trunk
Are you sure you want to change the base?
Conversation
The arm64 test failure here is interesting, it is likely to be unrelated to the code change. The arm64 support in GHA is new/experimental so it might be something random but the tests have failed twice.
|
repository root path, allowing SVN to be configured via LocationMatch like: <LocationMatch ^/repos/svn(.*)> DAV svn SVNParentPath /srv/repos DAVBasePath /repos </LocationMatch> by mapping URL path "/repos/svnfoo" to the filesystem path of "/srv/repos/svnfoo". This partially addresses issues like SVN-4790 (with minimal added complexity), though does not provide as much flexibility as discussed in SVN-2679. * subversion/mod_dav_svn/mod_dav_svn.c (dav_svn__get_root_dir): Retrieve and return the setting from DavBasePath via dav_get_base_path() where that API is available and DavBasePath is configured. (dav_svn__translate_name): Use dav_svn__get_root_dir() to determine the repos root for the same reason.
Try to capture tests.log if the tests fail.
e53e4f6
to
3fbe60a
Compare
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.
Builds and tests complete successfully with HTTPD 2.4.58.
Have not tested with HTTPD trunk and I don't think we have a test case for LocationMatch anyway.
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 already added something very similar in r1923964/r1923965, didn't notice this was already included in this PR.
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.
Ah great, looks perfect.
@@ -781,6 +781,15 @@ const char * | |||
dav_svn__get_root_dir(request_rec *r) | |||
{ | |||
dir_conf_t *conf; | |||
const char *base; | |||
|
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.
Any particular reason why base isn't declared within the #if block?
If compiled with < 20211221.28, there is a compiler warning about an unused variable.
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.
Thanks good catch!
您的来信已收到
|
Yeah, I will add a CI workflow against an HTTPD trunk build to get this tested, it'd be a useful thing to have anyway. |
@notroj Do you want to add the CI workflow before you commit this or can we go ahead and commit and close the PR? |
您的来信已收到
|
Note this mod_dav API is only present in httpd trunk (currently).