I am working on it, and have narrowed down to the get_mergeinfo_walk_cb
function in libsvn_client/merge.c.
There's a call to svn_client__ensure_ra_session_url which generates the
error when called with a path which
includes spaces. This should be modified so that the "mergeinfo_url"
variable is constructed using a call to
the "svn_path_uri_encode" function:
at line 3364 of merge.c
---------------------------------------
const char *mergeinfo_url =
svn_path_join(web->source_root_url,
/* Skip leading '/' or join won't work. */
svn_path_uri_encode(++(merge_src_child_path->data), pool),
pool);
If you want an actual patch file, I can get the code out of the repository,
but it's a pretty small change anyway.
On Tue, Sep 2, 2008 at 4:29 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> "Matthew Inger" <mattinger_at_gmail.com> writes:
> > After trying this, it does not solve the issue I'm having, though I
> > believe it's definately related, and perhaps the suggested fix needs
> > to be applied at other locations in the code.
>
> Hmmm... do you think you can find those other locations? It would be
> great to nail this down, now that we (maybe) are on the right track.
>
> -K
>
> > On Tue, Sep 2, 2008 at 3:26 PM, Matthew Inger <mattinger_at_gmail.com>
> wrote:
> >
> > It looks like it could be related. I had concentrated on the
> interaction
> > with neon, and not what was building
> > the actualy URLs being passed, which it looks like this other thread
> was
> > looking it.
> >
> > Looking at the code changes, then this looks absolutely correct, and
> is
> > much preferrable to what i had done.
> >
> >
> >
> > On Tue, Sep 2, 2008 at 2:30 PM, Karl Fogel <kfogel_at_red-bean.com>
> wrote:
> >
> > "C. Michael Pilato" <cmpilato_at_collab.net> writes:
> > > Matthew Inger wrote:
> > >> I'm running into an issue when doing branch merges. It seems
> that
> > file
> > >> names with spaces in them
> > >> cannot properly be merged, resulting in the merging error:
> > >>
> > >> svn: URL 'http://host/svn/repos/trunk/dir with
> spaces/file.txt' is
> > >> malformed or the scheme or host or path is missing.
> > >>
> > >> I've isolated down the cause of the problem to the following
> > function:
> > >>
> > >> int ne_uri_parse(const char *uri, ne_uri *parsed);
> > >>
> > >> It seems that when trying to parse the path component, it is
> doing
> > the
> > >> following:
> > >>
> > >> while (uri_lookup(*p) & URI_SEGCHAR)
> > >> p++;
> > >>
> > >>
> > >> The problem here is that definition of URI_SEGCHAR includes
> only the
> > >> following:
> > >>
> > >> FS -- /
> > >> PC -- %
> > >> PS -- +
> > >> SD -- ! $ & ' ( ) * + , ; =
> > >> CL -- :
> > >> AL -- Alpabet
> > >> DG -- Digit
> > >> DS -- dash
> > >> DT -- .
> > >> US -- _
> > >> TD -- ~
> > >>
> > >> Notice that spaces are not included in this definition.
> > >>
> > >> So the solution is either to change how neon parses the URI,
> or to
> > >> properly escape the spaces with a "+" symbol
> > >> in the "session.c" file which is calling into the neon
> library.
> > >>
> > >> Any thoughts?
> > >
> > > Subversion should be properly URI-encoding the paths it sends
> through
> > Neon,
> > > and apparently it is not. That's a bug. (Though, the correct
> URI
> > encoded
> > > form of a space character is "%20" -- the plus sign is, I
> believe,
> > only for
> > > when the space occurs in the query parameters portion of a URL,
> not
> > the path
> > > part.)
> >
> > This is a bit of a drive-by reply, but I just responded to a
> tentative
> > patch by Fyodor Sheremetyev that might be related. Below is my
> > response, which quotes his patch, and here is the thread his post
> was
> > part of:
> >
> >
> http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&
> > from=645165
> >
> > Is this the same problem you noticed, Matthew?
> >
> > -Karl
> >
> > > "Fyodor Sheremetyev" <fyodor_at_visualsvn.com> writes:
> > > > On Fri, Aug 29, 2008 at 12:44 AM, David Huang <
> > david.huang_at_alterity.com> wrote:
> > > >> It looks like Fyodor Sheremetyev was able to create a repro
> > script... here's
> > > >> the call stack:
> > > >
> > > > I'm trying to fix this problem. Attached is a short and dirty
> patch
> > > > that makes my reproduction script work. But I'm absolutely
> unsure
> > that
> > > > it fixes the problem correctly.
> > > >
> > > > I see no consistency in using svn_path_uri_encode and
> > > > svn_path_uri_decode in the code and couldn't find any
> guidelines
> > > > regarding uri encoding. Which level should do that by design?
> > Please
> > > > could somebody point at the right direction.
> > >
> > > I think your patch is correct. I've quoted it below so others
> can
> > see
> > > it. A couple of comments:
> > >
> > > 1. A log message always helps :-). See
> > > http://subversion.tigris.org/hacking.html#log-messages
> > >
> > > 2. You attached it with MIME-type
> "application/octet-stream",
> > which
> > > means that mailreaders can't display it inline. If you
> use
> > > "text/plain" instead, that makes things a bit easier.
> See
> > > http://subversion.tigris.org/hacking.html#patches
> > >
> > > The guideline for URI encoding and decoding is just to follow
> the doc
> > > strings of whatever functions the data will be passed to. If
> the
> > > function says it needs URI-encoded data, then make sure it
> receives
> > > that. If it does not, then usually it takes un-encoded data.
> > >
> > > -Karl
> > >
> > > > Index: subversion/libsvn_client/merge.c
> > > >
> ===================================================================
> > > > --- subversion/libsvn_client/merge.c (revision 32799)
> > > > +++ subversion/libsvn_client/merge.c (working copy)
> > > > @@ -6291,9 +6291,9 @@
> > > > over and over. */
> > > > /* We never merged to the source. Just return the
> branch
> > point. */
> > > > const char *yc_ancestor_path,
> > > > - *source_url = svn_path_join(source_repos_root,
> > source_repos_rel_path,
> > > > + *source_url = svn_path_join(source_repos_root,
> > svn_path_uri_encode(source_repos_rel_path, subpool),
> > > > subpool),
> > > > - *target_url = svn_path_join(source_repos_root,
> > target_repos_rel_path,
> > > > + *target_url = svn_path_join(source_repos_root,
> > svn_path_uri_encode(target_repos_rel_path, subpool),
> > > > subpool);
> > > >
> > > > SVN_ERR(svn_client__get_youngest_common_ancestor(&
> > yc_ancestor_path,
> > > > @@ -6306,7 +6306,7 @@
> > > > _("'%s@%ld' must be
> ancestrally
> > related to "
> > > > "'%s@%ld'"), source_url,
> > source_rev,
> > > > target_url, target_rev);
> > > > - *url_left = svn_path_join(source_repos_root,
> > yc_ancestor_path, pool);
> > > > + *url_left = svn_path_join(source_repos_root,
> > svn_path_uri_encode(yc_ancestor_path, pool), pool);
> > > > *source_mergeinfo_p = apr_hash_make(pool);
> > > >
> > > > svn_pool_destroy(subpool);
>
Received on 2008-09-02 23:19:33 CEST