Re: [PATCH] Please review : repositories recursive finding with SVNParentPath

On Dec 9, 2007 6:03 AM, Stephane Crivisier
<stephane.crivisier@epitech.net> wrote:
> Hi
>
> I'm attaching a patch which enable recursive finding of repositories
> with SVNParentPath.
> I hope some of you can have a look a it and tell me what's good and
> what's wrong.
> This feature seems to be asked by someone regulary, it may be
> something you can review and add. I hope to see it in subversion one
> day.
> I worked on 1.4.x branche if this might help.

You should really develop new patches against trunk, since that's what
we actively change.

Adding this feature means that links will suddenly start appearing on
people's servers that were not formerly there, which could
hypothetically be a security issue; should this feature require
specific activation?

> Thanks a lot to each of you.
>
> --
> Stephane
>
> [[[
> recursively search for repository when accessing mod_dav_svn with
> SVNParentPath enabled and when SVNListParentPath is enabled, allow
> user to "navigate" through the directory tree

Try to use full sentences with capitalization and periods.

> This means that you can have your SVNParentPath to /svn and create a
> directories tree before creating your repositories. When accessing
> with a browser, you'll get the current listing plus navigation through
> the tree before looking to a specific repository.
>
> * subversion/libsvn_repos/repos.c
> make check_repos_path() a non static function to be able to use it in
> mod_dav_svn
>
> * subversion/include/svn_repos.h
> add check_repos_path() prototype

Non-static functions in Subversion need to follow our naming
conventions. If it should be truly public and declared in svn_repos.h
then it should be named svn_repos_check_repos_path. (If nothing
outside of Subversion should call it, then it should be
svn_repos__check_repos_path and declared in include/private/.)
However, what's wrong with using svn_repos_find_root_path?

> * subversion/mod_dav_svn/repos.c
> add a test to the repository before telling the name of this repo. If
> this isn't a valid repository (by the way check_repos_path() check
> it), it will try to get a new chunk of the uri and test it again and
> again unless there is no more uri chunk.
> also add the root_path to the repos structure when building the dav
> resource and use it when it builds the directory content list
> ]]]

Thanks for including a log message; it contained all the useful
information. Do try to follow the formatting guidelines (like
indentation and symbol names in parentheses) at
http://subversion.tigris.org/hacking.html#log-messages though.

--dave

-- 
David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org