You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Reser <be...@reser.org> on 2012/12/19 04:41:11 UTC
In-Repo-Authz
I've finished the work needed to implement in-repo-authz on the branch.
I'd like to merge this code back to trunk. If there are no objections
I'll merge it into trunk next week.
Please review my changes. You can view the overall set of changes
that would be merged with:
svn diff -x-p http://svn.apache.org/repos/asf/subversion/trunk@1423749
http://svn.apache.org/repos/asf/subversion/branches/in-repo-authz@1423749
The high level details of the changes and some performance testing of
the changes are on the wiki here:
http://wiki.apache.org/subversion/InRepoAuthz
Perf testing has showing very little in the way of performance impact
of this feature.
Details of the non-obvious changes are as follows:
svn_config_parse() added which allows parsing a configuration file
from a stream rather than a file.
Errors while parsing config files will now have the file and the line
the error occurs in separate errors since parsing the stream is now
abstracted from reading it in.
svn_path_is_repos_relative_url() and
svn_path_resolve_repos_relative_url() moved from private functions in
libsvn_client/cmdline.c to public functions.
svn_repos_authz_read2() added, which knows how to read authz files
from within the repo.
svnserve --config-file no longer caches the password and authz db
files specified in the config file passed to the option. They are
loaded on each connection, just like every other mode of operation for
svnserve.
mod_authz_svn has had the logging of svn_error_t structs generalized
and now logs the full chain.
Re: In-Repo-Authz
Posted by Julian Foad <ju...@btopenworld.com>.
Ben Reser wrote:
> Julian Foad wrote:
>> Only the standard general benefits of committing separate changes
>> separately, such as being able to revert one without another, or
>> backport one without another, or review one at a time.
>
> The changes in the branch are clearly split. I could probably remove
> the --config-file behavior change pretty easily by producing a reverse
> patch from the changes that added it. None of this is backportable in
> my opinion. Probably the best way to review the changes is to read
> all the changes on the branch, wants to review specific pieces let me
> know and I'll direct you to the specific changes on the branch you
> want to look at.
Thanks for the additional info. That's fine.
- Julian
Re: In-Repo-Authz
Posted by Ben Reser <be...@reser.org>.
On Wed, Dec 19, 2012 at 8:30 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Only the standard general benefits of committing separate changes separately, such as being able to revert one without another, or backport one without another, or review one at a time.
The changes in the branch are clearly split. I could probably remove
the --config-file behavior change pretty easily by producing a reverse
patch from the changes that added it. None of this is backportable in
my opinion. Probably the best way to review the changes is to read
all the changes on the branch, wants to review specific pieces let me
know and I'll direct you to the specific changes on the branch you
want to look at.
Re: In-Repo-Authz
Posted by Julian Foad <ju...@btopenworld.com>.
Ben Reser wrote:
> Julian Foad wrote:
>> Just a thought. Some of these changes are significant (in both
>> lines touched and functionality) on their own. One that stands
>> out is "svnserve --config-file no longer caches the password and
>> authz db ...". Would it be a good idea (and not too much work)
>> to merge that first, as a separate change?
>
> I think it would be moderately painful to do that first since the code
> I changed to effect that was the same code I'd already touched to add
> in-repo-authz for svnserve before I realized I should just remove the
> whole caching of password and authz db on startup entirely.
OK, probably not worth it then.
> What value do you think merging the --config-file change first would have
Only the standard general benefits of committing separate changes separately, such as being able to revert one without another, or backport one without another, or review one at a time.
- Julian
Re: In-Repo-Authz
Posted by Ben Reser <be...@reser.org>.
On Wed, Dec 19, 2012 at 6:57 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Just a thought. Some of these changes are significant (in both lines touched and functionality) on their own. One that stands out is "svnserve --config-file no longer caches the password and authz db ...". Would it be a good idea (and not too much work) to merge that first, as a separate change?
I think it would be moderately painful to do that first since the code
I changed to effect that was the same code I'd already touched to add
in-repo-authz for svnserve before I realized I should just remove the
whole caching of password and authz db on startup entirely.
What value do you think merging the --config-file change first would have?
Re: In-Repo-Authz
Posted by Julian Foad <ju...@btopenworld.com>.
Ben Reser wrote:
> I've finished the work needed to implement in-repo-authz on the branch.
>
> I'd like to merge this code back to trunk. If there are no objections
> I'll merge it into trunk next week.
>
> Please review my changes. You can view the overall set of changes
> that would be merged with:
> svn diff -x-p http://svn.apache.org/repos/asf/subversion/trunk@1423749
> http://svn.apache.org/repos/asf/subversion/branches/in-repo-authz@1423749
>
> The high level details of the changes and some performance testing of
> the changes are on the wiki here:
> http://wiki.apache.org/subversion/InRepoAuthz
>
> Perf testing has showing very little in the way of performance impact
> of this feature.
>
> Details of the non-obvious changes are as follows:
> svn_config_parse() added which allows parsing a configuration file
> from a stream rather than a file.
> Errors while parsing config files will now have the file and the line
> the error occurs in separate errors since parsing the stream is now
> abstracted from reading it in.
> svn_path_is_repos_relative_url() and
> svn_path_resolve_repos_relative_url() moved from private functions in
> libsvn_client/cmdline.c to public functions.
> svn_repos_authz_read2() added, which knows how to read authz files
> from within the repo.
> svnserve --config-file no longer caches the password and authz db
> files specified in the config file passed to the option. They are
> loaded on each connection, just like every other mode of operation for
> svnserve.
> mod_authz_svn has had the logging of svn_error_t structs generalized
> and now logs the full chain.
Just a thought. Some of these changes are significant (in both lines touched and functionality) on their own. One that stands out is "svnserve --config-file no longer caches the password and authz db ...". Would it be a good idea (and not too much work) to merge that first, as a separate change?
- Julian
Re: In-Repo-Authz
Posted by Stefan Sperling <st...@elego.de>.
Sounds great!
Just recently I was talking to people who wrote their own web frontend
wrapper to manage authz. I believe this new feature will make things easier
for them since it allows them to skip the authz file deploy step.
On Tue, Dec 18, 2012 at 07:41:11PM -0800, Ben Reser wrote:
> I've finished the work needed to implement in-repo-authz on the branch.
>
> I'd like to merge this code back to trunk. If there are no objections
> I'll merge it into trunk next week.
>
> Please review my changes. You can view the overall set of changes
> that would be merged with:
> svn diff -x-p http://svn.apache.org/repos/asf/subversion/trunk@1423749
> http://svn.apache.org/repos/asf/subversion/branches/in-repo-authz@1423749
>
> The high level details of the changes and some performance testing of
> the changes are on the wiki here:
> http://wiki.apache.org/subversion/InRepoAuthz
>
> Perf testing has showing very little in the way of performance impact
> of this feature.
>
> Details of the non-obvious changes are as follows:
> svn_config_parse() added which allows parsing a configuration file
> from a stream rather than a file.
> Errors while parsing config files will now have the file and the line
> the error occurs in separate errors since parsing the stream is now
> abstracted from reading it in.
> svn_path_is_repos_relative_url() and
> svn_path_resolve_repos_relative_url() moved from private functions in
> libsvn_client/cmdline.c to public functions.
> svn_repos_authz_read2() added, which knows how to read authz files
> from within the repo.
> svnserve --config-file no longer caches the password and authz db
> files specified in the config file passed to the option. They are
> loaded on each connection, just like every other mode of operation for
> svnserve.
> mod_authz_svn has had the logging of svn_error_t structs generalized
> and now logs the full chain.
Re: In-Repo-Authz
Posted by Ben Reser <be...@reser.org>.
On Wed, Dec 19, 2012 at 8:21 AM, Ben Reser <be...@reser.org> wrote:
> My intention was to wait 72 hours and merge giving people a chance to
> object to the feature or implementation. However, I said next week
> because 72 hours would have had me merging this weekend, which I
> didn't figure was realistic. But your point is taken, given the
> feedback I've gotten so far I may do the merge today.
Merged in r1424780
Re: In-Repo-Authz
Posted by Ben Reser <be...@reser.org>.
On Wed, Dec 19, 2012 at 6:41 AM, C. Michael Pilato <cm...@collab.net> wrote:
> +1, but why wait until next week? If you are confident in your changes,
> then get 'em into the practically visible/reviewable space ASAP! At this
> point in the merry month of December, "next week" for many folks translates
> effectively to "next year".
My intention was to wait 72 hours and merge giving people a chance to
object to the feature or implementation. However, I said next week
because 72 hours would have had me merging this weekend, which I
didn't figure was realistic. But your point is taken, given the
feedback I've gotten so far I may do the merge today.
Re: In-Repo-Authz
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/18/2012 10:41 PM, Ben Reser wrote:
> I've finished the work needed to implement in-repo-authz on the branch.
>
> I'd like to merge this code back to trunk. If there are no objections
> I'll merge it into trunk next week.
+1, but why wait until next week? If you are confident in your changes,
then get 'em into the practically visible/reviewable space ASAP! At this
point in the merry month of December, "next week" for many folks translates
effectively to "next year".
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development