Re: [PATCH] t9117: test specifying full url to git svn init -T

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Adam Dinwoodie <adam@xxxxxxxxxxxxx> wrote:
> According to the documentation, full URLs can be specified in the `-T`
> argument to `git svn init`.  However, the canonicalization of such
> arguments squashes together consecutive "/"s, which unsurprisingly
> breaks http://, svn://, etc URLs.  Add a failing test case to provide
> evidence of that.
> 
> On systems where Subversion provides svn_path_canonicalize but not
> svn_dirent_canonicalize (Subversion 1.6 and earlier?), this test passes,
> as svn_path_canonicalize doesn't mangle the consecutive "/"s.
> 
> Signed-off-by: Adam Dinwoodie <adam@xxxxxxxxxxxxx>

Thanks, I was just working on a patch to fix this problem
when I got this patch :)

Signed-off-by: Eric Wong <normalperson@xxxxxxxx>

> ---
> 
> I think the bug here is in using perl/Git/SVN/Utils.pm's
> `canonicalize_path` on the `-T` argument.  If it's available, that
> function calls Subversion's `svn_dirent_canonicalize`.  The Subversion
> code[0] makes it clear that this function is fine for relative and
> absolute local paths, and for UNC paths on Windows, but it isn't
> suitable for use on URLs.
> 
> [0]: https://svn.apache.org/repos/asf/subversion/trunk/subversion/include/svn_dirent_uri.h

Yep, we should be using canonicalize_url for URLs.  I was able
to reproduce this on Debian jessie (GNU/Linux), too.

> It occurs to me that the correct "fix" here may simply be to stop
> claiming support for specifying URLs as arguments to -T, and mandate
> users use the `git svn init $url -T $dirent` syntax instead,

Nope, we should never stop supporting existing behavior without
very good reason and adequate deprecation warnings.

> --- a/t/t9117-git-svn-init-clone.sh
> +++ b/t/t9117-git-svn-init-clone.sh
> @@ -119,4 +119,10 @@ test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
>         rm -f warning
>         '

For future reference, your mail editor is expanding tabs to
spaces and munging your patches.  mutt won't do that itself
(at least not with my config), so I guess it's your editor.

Manually fixing up the whitespaces damage on my end.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]