Hi Eric,
Thanks for your reply and comments.
Eric Wong <normalperson@xxxxxxxx> wrote:> João Abecasis <joao@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>>> --->> 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 :(
I can relate to that ;-)
> The commit message should be wrapped at 72 characters or less.> Some further notes below (mostly whitespace issues, but one> regression).
Noted.
>> 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 :)
Done!
>> 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.
Okidoki. I just wasn't sure how wide was a tab ;-)
>> @@ -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.
Duh! Missed that. I had to make a couple of tweaks after the testswere working, because I had some another test case that was breakingsomewhere. Anyway, my other test case (which is the one where I needit to work) works without that change after all.
> Thanks for the patch, please let us know if that change is OK.
Let's go with that, since it works. If I do find a test case where itbreaks we'll fix it later.
Updated patch coming up.
Cheers,
João��.n��������+%������w��{.n��������n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�m
