> -----Original Message----- > From: Bert Huijben [mailto:b...@qqmail.nl] > Sent: donderdag 5 september 2013 22:59 > To: 'Philip Martin'; 'Daniel Shahaf' > Cc: 'James McCoy'; dev@subversion.apache.org > Subject: RE: upgrade_tests 7 FAIL when working directory has a "+" character > > > > > -----Original Message----- > > From: Philip Martin [mailto:philip.mar...@wandisco.com] > > Sent: donderdag 5 september 2013 21:20 > > To: Daniel Shahaf > > Cc: James McCoy; dev@subversion.apache.org > > Subject: Re: upgrade_tests 7 FAIL when working directory has a "+" > character > > > > Philip Martin <philip.mar...@wandisco.com> writes: > > > > > Daniel Shahaf <danie...@apache.org> writes: > > > > > >> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote: > > >>> This was with svn 1.7.9. I haven't had a chance to test newer > > >>> versions, but looking through the code in trunk, it looks like it > > >>> would still be a problem. > > >> > > >> Confirmed, it reproduces in trunk: > > >> > > >> Index: subversion/tests/cmdline/upgrade_tests.py > > >> > > > ========================================================== > > ========= > > >> --- subversion/tests/cmdline/upgrade_tests.py (revision 1520363) > > >> +++ subversion/tests/cmdline/upgrade_tests.py (working copy) > > >> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url, > > to_url) > > >> def basic_upgrade_1_0(sbox): > > >> "test upgrading a working copy created with 1.0.0" > > >> > > >> - sbox.build(create_wc = False) > > >> + sbox.build(name='foo+', create_wc = False) > > >> replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2') > > >> > > >> url = sbox.repo_url > > >> > > >> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head > > >> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal: > > Assertion `svn_path_is_canonical_internal(base, pool)' failed. > > >> > > >> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn- > > test-work/working_copies/foo+' --config-dir > > /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test- > > work/local_tmp/config --password rayjandom --no-auth-cache --username > > jrandom terminated by signal 6 > > > > > > It looks like the %2B is failing svn_uri_is_canonical. Looking at > > > svn_uri__char_validity in libsvn_subr/path.c the table says that '+' > > > should not be % encoded. The problem is that the Python testsuite uses > > > urllib.pathname2url(self.repo_dir) and that has different ideas, it > > > does % encode '+'. > > > > Index: subversion/tests/cmdline/svntest/sandbox.py > > > ========================================================== > > ========= > > --- subversion/tests/cmdline/svntest/sandbox.py (revision 1520259) > > +++ subversion/tests/cmdline/svntest/sandbox.py (working copy) > > @@ -57,7 +57,7 @@ > > if not read_only: > > self.repo_dir = os.path.join(svntest.main.general_repo_dir, > self.name) > > self.repo_url = (svntest.main.options.test_area_url + '/' > > - + urllib.pathname2url(self.repo_dir)) > > + + urllib.quote(self.repo_dir, '/+')) > > self.add_test_path(self.repo_dir) > > else: > > self.repo_dir = svntest.main.pristine_greek_repos_dir > > > > That works for '+'. A real patch would need to ensure that the safe set > > passed to urllib.quote matches the safe set required by Subversion. > > How does this path enter 'svn'? > > Shouldn't this just be handled by the 'svn_uri_canonicalize()' that handles > all url commandline arguments? > > (Or does this code introduce it in the raw 'entries' files before the > conversion?)
Yes, basic_upgrade_1_0 (=upgrade tests 7) does indeed do a raw search and replace on the entries file via xml_entries_relocate(). So the fix is probably to just add the necessary svn_uri_canonicalize() call in the right place of the upgrade (=read from 'entries') code. [Note the other mail: Changing the canonicalization rules requires a format bump, as that can make urls in wc.db invalid] > Bert
