public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Eric Wong <normalperson@yhbt•net>
To: "João Abecasis" <joao@abecasis•name>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] git-svn: find-rev and rebase for SVN::Mirror repositories
Date: Mon, 14 Jul 2008 02:12:43 -0700	[thread overview]
Message-ID: <20080714091243.GA27794@untitled> (raw)
In-Reply-To: <7bf6f1d20807081908kdf9f615taa532ae579b457d7@mail.gmail.com>

João Abecasis <joao@abecasis•name> wrote:
> find-rev and rebase error out on svm because git-svn doesn't trace the original
> svn revision numbers back to git commits. The updated test case, included in the
> patch, shows the issue and passes with the rest of the patch applied.
> 
> This fixes Git::SVN::find_by_url to find branches based on the svm:source URL,
> where useSvmProps is set. Also makes sure cmd_find_rev and working_head_info use
> the information they have to correctly track the source repository. This is
> enough to get find-rev and rebase working.
> 
> Signed-off-by: João Abecasis <joao@abecasis•name>
> ---
> Incidentally, I've tried submitting these fixes before, but failed to
> get much attention, so I could be doing something wrong... Any input
> on the patch or my approach to this list is appreciated. This time I
> reworded the commit message and added Eric Wong to the CC list, since
> he seems to be Ack'ing git-svn patches.

Hi João,

Sorry, I haven't been following the mailing list much lately, and
even keeping up with my inbox is getting to be a chore :(

The commit message should be wrapped at 72 characters or less.
Some further notes below (mostly whitespace issues, but one
regression).

>  sub read_all_remotes {
>  	my $r = {};
> +        my $usesvmprops = eval { command_oneline(qw/config --bool
> +            svn.useSvmProps/) };
> +        $usesvmprops = $usesvmprops eq 'true' if $usesvmprops;

Always use tabs for indentation (but spaces for alignment is alright).

I'd also rather not propagate the crazy alllowercasewithoutunderscores
convention of git-config into code, either.  $uses_svm_props is much
easier to parse when I'm half-awake :)

>  	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);
>  			$local_ref =~ s{^/}{};
>  			$r->{$remote}->{fetch}->{$local_ref} = $remote_ref;
> +			$r->{$remote}->{svm} = {} if $usesvmprops;
> +		} elsif (m!^(.+)\.usesvmprops=\s*(.*)\s*$!) {
> +			$r->{$1}->{svm} = {};
>  		} elsif (m!^(.+)\.url=\s*(.*)\s*$!) {
>  			$r->{$1}->{url} = $2;
>  		} elsif (m!^(.+)\.(branches|tags)=
> @@ -1437,6 +1443,21 @@ sub read_all_remotes {
>  			}
>  		}
>  	}
> +
> +	map {
> +		if (defined $r->{$_}->{svm}) {
> +			my $svm;
> +			eval {
> +				my $section = "svn-remote.$_";
> +				$svm = {
> +					source => tmp_config('--get', "$section.svm-source"),
> +					replace => tmp_config('--get', "$section.svm-replace"),
> +				}
> +			};
> +			$r->{$_}->{svm} = $svm;
> +		}
> +	} keys %$r;
> +

Please wrap all code at 80-columns.

> @@ -1563,13 +1584,21 @@ sub find_by_url { # repos_root and, path are optional
>  		}
>  		my $p = $path;
>  		my $rwr = rewrite_root({repo_id => $repo_id});
> +		my $svm = $remotes->{$repo_id}->{svm}
> +			if defined $remotes->{$repo_id}->{svm};
>  		unless (defined $p) {
>  			$p = $full_url;
>  			my $z = $u;
> +			my $prefix = '';
>  			if ($rwr) {
>  				$z = $rwr;
> +			} elsif (defined $svm) {
> +				$z = $svm->{source};
> +				$prefix = $svm->{replace};
> +				$prefix =~ s#^\Q$u\E(?:/|$)##;
> +				$prefix =~ s#/$##;
>  			}
> -			$p =~ s#^\Q$z\E(?:/|$)## or next;
> +			$p =~ s#^\Q$z\E(?=/|$)#$prefix# or next;

This broke t9100 for me at: 'able to dcommit to a subdirectory'
Changing the ?= back to ?: works.

Thanks for the patch, please let us know if that change is OK.

-- 
Eric Wong

next prev parent reply	other threads:[~2008-07-14  9:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-09  2:08 [PATCH] git-svn: find-rev and rebase for SVN::Mirror repositories João Abecasis
2008-07-14  9:12 ` Eric Wong [this message]
2008-07-14 15:27   ` João Abecasis
2008-07-14 15:28     ` João Abecasis
2008-07-15  4:25       ` Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080714091243.GA27794@untitled \
    --to=normalperson@yhbt$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=joao@abecasis$(echo .)name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox