Re: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.

Eric Wong
Avery Pennarun <[hidden email]> wrote:

> Previously, if a config entry looked like this:
>
>          svn-remote.svn.fetch=:refs/heads/whatever
>
> git-svn would silently do nothing if you asked it to "git svn fetch", and
> give a strange error if asked to "git svn dcommit".  What it really wants is
> a line that looks like this:
>
> svn-remote.svn.fetch=:refs/remotes/whatever
>
> So we should simply abort if we get the wrong thing.
>
> On the other hand, there's actually no good reason for git-svn to enforce
> using the refs/remotes namespace, but the code seems to have hardcoded this
> in several places and I'm not brave enough to try to fix it all right now.

Fully agreed (as I've stated in the past, too).  I just haven't had
time to fix it.

> Signed-off-by: Avery Pennarun <[hidden email]>

Thanks Avery,
Acked-by: Eric Wong <[hidden email]>

> ---
>
> I just spent altogether too much time tracking down this problem when
> migrating my git-svn settings from one repo to another.
>
>  git-svn.perl |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index cf6dbbc..cc35f50 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1420,8 +1420,12 @@ sub read_all_remotes {
>      svn.useSvmProps/) };
>   $use_svm_props = $use_svm_props eq 'true' if $use_svm_props;
>   foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
> - if (m!^(.+)\.fetch=\s*(.*)\s*:\s*refs/remotes/(.+)\s*$!) {
> - my ($remote, $local_ref, $remote_ref) = ($1, $2, $3);
> + if (m!^(.+)\.fetch=\s*(.*)\s*:\s*(.+)\s*$!) {
> + my ($remote, $local_ref, $_remote_ref) = ($1, $2, $3);
> + die("svn-remote.$remote: remote ref '$_remote_ref' "
> +    . "must start with 'refs/remotes/'\n")
> + unless $_remote_ref =~ m{^refs/remotes/(.+)};
> + my $remote_ref = $1;
>   $local_ref =~ s{^/}{};
>   $r->{$remote}->{fetch}->{$local_ref} = $remote_ref;
>   $r->{$remote}->{svm} = {} if $use_svm_props;
> --
> 1.6.0.rc0.42.g186458.dirty
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.

Junio C Hamano
Eric Wong <[hidden email]> writes:

>> Signed-off-by: Avery Pennarun <[hidden email]>
>
> Thanks Avery,
> Acked-by: Eric Wong <[hidden email]>

I do not seem to find the original message in mbox nor list archive, by
the way...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.

Eric Wong
Junio C Hamano <[hidden email]> wrote:
> Eric Wong <[hidden email]> writes:
>
> >> Signed-off-by: Avery Pennarun <[hidden email]>
> >
> > Thanks Avery,
> > Acked-by: Eric Wong <[hidden email]>
>
> I do not seem to find the original message in mbox nor list archive, by
> the way...

Could've been Avery's original From: line not being spam filter
friendly.

Here you go (with From: line fixed)

From 574c237f4561cf0293e7f44ab604bb701d1931f3 Mon Sep 17 00:00:00 2001
From: Avery Pennarun <[hidden email]>
Date: Wed, 30 Jul 2008 16:53:55 -0400
Subject: [PATCH] git-svn: Abort with an error if 'fetch' parameter is invalid.

Previously, if a config entry looked like this:

         svn-remote.svn.fetch=:refs/heads/whatever

git-svn would silently do nothing if you asked it to "git svn fetch", and
give a strange error if asked to "git svn dcommit".  What it really wants is
a line that looks like this:

        svn-remote.svn.fetch=:refs/remotes/whatever

So we should simply abort if we get the wrong thing.

On the other hand, there's actually no good reason for git-svn to enforce
using the refs/remotes namespace, but the code seems to have hardcoded this
in several places and I'm not brave enough to try to fix it all right now.

Signed-off-by: Avery Pennarun <[hidden email]>
---
 git-svn.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 0a346f8..1c39f45 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1423,8 +1423,12 @@ sub read_all_remotes {
     svn.useSvmProps/) };
  $use_svm_props = $use_svm_props eq 'true' if $use_svm_props;
  foreach (grep { s/^svn-remote\.// } command(qw/config -l/)) {
- if (m!^(.+)\.fetch=\s*(.*)\s*:\s*refs/remotes/(.+)\s*$!) {
- my ($remote, $local_ref, $remote_ref) = ($1, $2, $3);
+ if (m!^(.+)\.fetch=\s*(.*)\s*:\s*(.+)\s*$!) {
+ my ($remote, $local_ref, $_remote_ref) = ($1, $2, $3);
+ die("svn-remote.$remote: remote ref '$_remote_ref' "
+    . "must start with 'refs/remotes/'\n")
+ unless $_remote_ref =~ m{^refs/remotes/(.+)};
+ my $remote_ref = $1;
  $local_ref =~ s{^/}{};
  $r->{$remote}->{fetch}->{$local_ref} = $remote_ref;
  $r->{$remote}->{svm} = {} if $use_svm_props;
--
Eric Wong
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html