* [PATCH 1/2] t9400-git-cvsserver-server: Wrap setup into test case @ 2008-01-26 9:54 Steffen Prohaska 2008-01-26 9:54 ` [PATCH 2/2] cvsserver: Fix for histories with multiple roots Steffen Prohaska 0 siblings, 1 reply; 5+ messages in thread From: Steffen Prohaska @ 2008-01-26 9:54 UTC (permalink / raw) To: git; +Cc: Steffen Prohaska The prefer to have the test setup in a test case. The setup itself may fail and having it as a test case handles this situation gracefully. Signed-off-by: Steffen Prohaska <prohaska@zib.de> --- t/t9400-git-cvsserver-server.sh | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index 641303e..1f2749e 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -33,13 +33,14 @@ CVS_SERVER=git-cvsserver export CVSROOT CVS_SERVER rm -rf "$CVSWORK" "$SERVERDIR" -echo >empty && +test_expect_success 'setup' ' + echo >empty && git add empty && git commit -q -m "First Commit" && git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 && GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true && - GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" || - exit 1 + GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" +' # note that cvs doesn't accept absolute pathnames # as argument to co -d -- 1.5.4.rc4.42.gacc73 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] cvsserver: Fix for histories with multiple roots 2008-01-26 9:54 [PATCH 1/2] t9400-git-cvsserver-server: Wrap setup into test case Steffen Prohaska @ 2008-01-26 9:54 ` Steffen Prohaska 2008-01-27 1:56 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Steffen Prohaska @ 2008-01-26 9:54 UTC (permalink / raw) To: git; +Cc: Steffen Prohaska 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; diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index 1f2749e..75d1ce4 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -37,6 +37,13 @@ test_expect_success 'setup' ' echo >empty && git add empty && git commit -q -m "First Commit" && + mkdir secondroot && + ( cd secondroot && + git init && + touch secondrootfile && + git add secondrootfile && + git commit -m "second root") && + git pull secondroot master && git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 && GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true && GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" @@ -46,7 +53,8 @@ test_expect_success 'setup' ' # as argument to co -d test_expect_success 'basic checkout' \ 'GIT_CONFIG="$git_config" cvs -Q co -d cvswork master && - test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"' + test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | head -n 1))" = "empty/1.1/" + test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | tail -n 1))" = "secondrootfile/1.1/"' #------------------------ # PSERVER AUTHENTICATION -- 1.5.4.rc4.42.gacc73 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] cvsserver: Fix for histories with multiple roots 2008-01-26 9:54 ` [PATCH 2/2] cvsserver: Fix for histories with multiple roots Steffen Prohaska @ 2008-01-27 1:56 ` Junio C Hamano 2008-01-27 9:37 ` [PATCH v2] " Steffen Prohaska 0 siblings, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2008-01-27 1:56 UTC (permalink / raw) To: Steffen Prohaska; +Cc: git Steffen Prohaska <prohaska@zib.de> writes: > 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; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] cvsserver: Fix for histories with multiple roots 2008-01-27 1:56 ` Junio C Hamano @ 2008-01-27 9:37 ` Steffen Prohaska 2008-01-27 9:47 ` Steffen Prohaska 0 siblings, 1 reply; 5+ messages in thread From: Steffen Prohaska @ 2008-01-27 9:37 UTC (permalink / raw) To: gitster; +Cc: git, Steffen Prohaska Junio, here is a replacement for the second patch. Different from your diff, I replaced tabs with 8 spaces because all of the surrounding code uses spaces for indentation. Steffen ---- snip --- 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. Thanks to Junio C Hamano for the implementation that avoids calling git-merge-base twice. Signed-off-by: Steffen Prohaska <prohaska@zib.de> --- git-cvsserver.perl | 11 +++++++++-- t/t9400-git-cvsserver-server.sh | 10 +++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/git-cvsserver.perl b/git-cvsserver.perl index ecded3b..920e7de 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', - $lastpicked, $parent); + 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; diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index 1f2749e..75d1ce4 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -37,6 +37,13 @@ test_expect_success 'setup' ' echo >empty && git add empty && git commit -q -m "First Commit" && + mkdir secondroot && + ( cd secondroot && + git init && + touch secondrootfile && + git add secondrootfile && + git commit -m "second root") && + git pull secondroot master && git clone -q --local --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 && GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true && GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" @@ -46,7 +53,8 @@ test_expect_success 'setup' ' # as argument to co -d test_expect_success 'basic checkout' \ 'GIT_CONFIG="$git_config" cvs -Q co -d cvswork master && - test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"' + test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | head -n 1))" = "empty/1.1/" + test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5 | tail -n 1))" = "secondrootfile/1.1/"' #------------------------ # PSERVER AUTHENTICATION -- 1.5.4.rc4.42.gacc73 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] cvsserver: Fix for histories with multiple roots 2008-01-27 9:37 ` [PATCH v2] " Steffen Prohaska @ 2008-01-27 9:47 ` Steffen Prohaska 0 siblings, 0 replies; 5+ messages in thread From: Steffen Prohaska @ 2008-01-27 9:47 UTC (permalink / raw) To: gitster; +Cc: git On Jan 27, 2008, at 10:37 AM, Steffen Prohaska wrote: > Junio, > here is a replacement for the second patch. Different from your diff, > I replaced tabs with 8 spaces because all of the surrounding code > uses spaces for indentation. Ah... too late... I should have pulled first ;) Anyway; thanks for improving the first version. Steffen ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-01-27 9:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-01-26 9:54 [PATCH 1/2] t9400-git-cvsserver-server: Wrap setup into test case Steffen Prohaska 2008-01-26 9:54 ` [PATCH 2/2] cvsserver: Fix for histories with multiple roots Steffen Prohaska 2008-01-27 1:56 ` Junio C Hamano 2008-01-27 9:37 ` [PATCH v2] " Steffen Prohaska 2008-01-27 9:47 ` Steffen Prohaska
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).