You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Gavin Beau Baumanis <ga...@thespidernet.com> on 2011/02/06 11:48:37 UTC
Re: [PATCH] Please review : repositories recursive finding with SVNParentPath
Ping. This patch is yet to receive any new comments.
It seems that David has already attached the patches to issue #3588
Gavin "Beau" Baumanis
On 09/12/2010, at 11:23 PM, David Burley wrote:
> They got mangled on the way to my mail client -- I tested them again locally before attaching them this time -- and they are now unmangled.
>
> Thank you,
>
> David
>
> On Thu, Dec 9, 2010 at 2:38 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> David Burley wrote on Wed, Dec 08, 2010 at 19:37:51 -0500:
> > Patches attached again but can also be found here:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=3588
> >
> > On Wed, Dec 8, 2010 at 5:24 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> >
> > > David Burley wrote on Wed, Dec 08, 2010 at 17:03:06 -0500:
> > > > So I have forward-ported Stephane's patch to add recursion to
> > > SVNParentPath
> > > > for 1.6.15 (see attached patch).
> > >
> > > The patch didn't get to the list. Please re-send it with *.txt extension.
> > >
> > > Thanks.
> > >
> > > Daniel
> > >
>
> The 1.7.x patch doesn't apply to HEAD of trunk.
>
> > Index: subversion/mod_dav_svn/repos.c
> > ===================================================================
> > --- subversion/mod_dav_svn/repos.c (revision 1043620)
> > +++ subversion/mod_dav_svn/repos.c (working copy)
> > @@ -1235,34 +1235,63 @@
> > {
> > /* SVNParentPath was used instead: assume the first component of
> > 'relative' is the name of a repository. */
> > - const char *magic_component, *magic_end;
> > + extern svn_boolean_t check_repos_path(const char *path, apr_pool_t *pool);
> > + const char *magic_component = NULL, *magic_end;
> > + char *repos_path;
> >
> > - /* A repository name is required here.
> > - Remember that 'relative' always starts with a "/". */
> > - if (relative[1] == '\0')
> > - {
> > - /* ### are SVN_ERR_APMOD codes within the right numeric space? */
> > - return dav_svn__new_error(r->pool, HTTP_FORBIDDEN,
> > - SVN_ERR_APMOD_MALFORMED_URI,
> > - "The URI does not contain the name "
> > - "of a repository.");
> > - }
> > + do
> > + {
> > + /* A repository name is required here.
> > + Remember that 'relative' always starts with a "/". */
> > + if (relative[1] == '\0')
> > + {
> > + /* ### are SVN_ERR_APMOD codes within the right numeric space? */
> > + return dav_new_error(r->pool, HTTP_FORBIDDEN,
> > + SVN_ERR_APMOD_MALFORMED_URI,
> > + "The URI does not contain the name "
> > + "of a repository.");
> > + }
> > +
> > + magic_end = ap_strchr_c(relative + 1, '/');
> > + if (!magic_end)
> > + {
> > + /* ### Request was for parent directory with no trailing
> > + slash; we probably ought to just redirect to same with
> > + trailing slash appended. */
> > + if (!magic_component)
> > + {
> > + magic_component = relative + 1;
> > + }
> > + else
> > + {
> > + magic_component = apr_pstrcat(r->pool, magic_component,
> > + relative, NULL);
> > + }
> > + relative = "/";
> > + }
> > + else
> > + {
> > + if (!magic_component)
> > + {
> > + magic_component = apr_pstrndup(r->pool, relative + 1,
> > + magic_end - relative - 1);
> > + }
> > + else
> > + {
> > + char *tmp_magic_component;
> >
> > - magic_end = ap_strchr_c(relative + 1, '/');
> > - if (!magic_end)
> > - {
> > - /* ### Request was for parent directory with no trailing
> > - slash; we probably ought to just redirect to same with
> > - trailing slash appended. */
> > - magic_component = relative + 1;
> > - relative = "/";
> > - }
> > - else
> > - {
> > - magic_component = apr_pstrndup(r->pool, relative + 1,
> > - magic_end - relative - 1);
> > - relative = magic_end;
> > - }
> > + tmp_magic_component = apr_pstrndup(r->pool, relative,
> > + magic_end - relative);
> > + magic_component = apr_pstrcat(r->pool, magic_component,
> > + tmp_magic_component, NULL);
> > + }
> > + relative = magic_end;
> > + }
> > +
> > + repos_path = svn_path_join(fs_parent_path, magic_component,
> > + r->pool);
> > + }
> > + while (check_repos_path(repos_path, r->pool) != TRUE);
> >
> > /* return answer */
> > *repos_basename = magic_component;
> > @@ -1881,7 +1910,7 @@
> > dav_resource_combined *comb;
> > dav_svn_repos *repos;
> > const char *cleaned_uri;
> > - const char *repo_basename;
> > + const char *repo_basename = NULL;
> > const char *relative;
> > const char *repos_path;
> > const char *repos_key;
> > @@ -1897,9 +1926,15 @@
> > xslt_uri = dav_svn__get_xslt_uri(r);
> > fs_parent_path = dav_svn__get_fs_parent_path(r);
> >
> > + /* This does all the work of interpreting/splitting the request uri. */
> > + err = dav_svn_split_uri(r, r->uri, root_path,
> > + &cleaned_uri, &had_slash,
> > + &repo_basename, &relative, &repos_path);
> > +
> > +
> > /* Special case: detect and build the SVNParentPath as a unique type
> > of private resource, iff the SVNListParentPath directive is 'on'. */
> > - if (fs_parent_path && dav_svn__get_list_parentpath_flag(r))
> > + if (fs_parent_path && dav_svn__get_list_parentpath_flag(r) && !repo_basename)
> > {
> > char *uri = apr_pstrdup(r->pool, r->uri);
> > char *parentpath = apr_pstrdup(r->pool, root_path);
> > @@ -1912,13 +1947,10 @@
> > if (parentpath[parentpath_len-1] == '/')
> > parentpath[parentpath_len-1] = '\0';
> >
> > - if (strcmp(parentpath, uri) == 0)
> > - {
> > - err = get_parentpath_resource(r, resource);
> > - if (err)
> > - return err;
> > - return NULL;
> > - }
> > + err = get_parentpath_resource(r, resource);
> > + if (err)
> > + return err;
> > + return NULL;
> > }
> >
> > /* This does all the work of interpreting/splitting the request uri. */
> > @@ -3161,8 +3193,13 @@
> > {
> > apr_hash_index_t *hi;
> > apr_hash_t *dirents;
> > + int root_path_len = strlen(resource->info->repos->root_path);
> > const char *fs_parent_path =
> > - dav_svn__get_fs_parent_path(resource->info->r);
> > + apr_pstrcat(resource->info->r->pool,
> > + dav_svn__get_fs_parent_path(resource->info->r),
> > + resource->info->r->uri
> > + + ((root_path_len > 1) ? root_path_len : 0),
> > + NULL);
> >
> > serr = svn_io_get_dirents3(&dirents, fs_parent_path, TRUE,
> > resource->pool, resource->pool);
> > Index: subversion/libsvn_repos/repos.c
> > ===================================================================
> > --- subversion/libsvn_repos/repos.c (revision 1043620)
> > +++ subversion/libsvn_repos/repos.c (working copy)
> > @@ -1428,7 +1428,7 @@
> > * on errors (which would be permission errors, probably) so that
> > * we the user will see them after we try to open the repository
> > * for real. */
> > -static svn_boolean_t
> > +svn_boolean_t
> > check_repos_path(const char *path,
> > apr_pool_t *pool)
>
> As earlier reviewers told you, we do not define functions in one library
> and use them in another unless they have a svn_ svn_foo__ name prefix.
>
> > {
> >
>
>
> <svnparentpath-recursion-1.6.15.patch.txt><svnparentpath-recursion-1.7.x.patch.txt>