Re: [PATCH 2/2] cvsserver: Fix for histories with multiple roots
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 27, 2008, 01:56 UTC
- Message-ID
- <7v8x2cyqsg.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <12013412462758-git-send-email-prohaska@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
Show 29 quoted lines
> Git histories may have multiple roots, which can cause
> git merge-base to fail and this caused git cvsserver to die.
>
> This commit teaches git cvsserver to handle a failing git
> merge-base gracefully, and modifies the test case to verify this.
> All the test cases now use a history with two roots.
>
> Signed-off-by: Steffen Prohaska <prohaska@zib.de>
> ---
> git-cvsserver.perl | 5 +++++
> t/t9400-git-cvsserver-server.sh | 10 +++++++++-
> 2 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index ecded3b..534b41e 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -2543,6 +2543,11 @@ sub update
> if ($parent eq $lastpicked) {
> next;
> }
> + # or it may fail to find a merge base. In this
> + # case we just ignore this merge.
> + if (system("git merge-base $lastpicked $parent >/dev/null 2>/dev/null")) {
> + next;
> + }
> my $base = safe_pipe_capture('git-merge-base',
> $lastpicked, $parent);
> chomp $base;That is a "Yes, but..." patch.
Running merge-base always twice, due to fear of uncommon case of failure, feels quite backwards.
Doesn't this work equally well without running a rather expensive merge-base twice?
git-cvsserver.perl | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl index ecded3b..afe3d0b 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -2543,8 +2543,15 @@ sub update if ($parent eq $lastpicked) { next; } - my $base = safe_pipe_capture('git-merge-base', + my $base = eval { + safe_pipe_capture('git-merge-base', $lastpicked, $parent); + }; + # The two branches may not be related at all, + # in which case merge base simply fails to find + # any, but that's Ok. + next if ($@); + chomp $base; if ($base) { my @merged;