threads / patch / 8358

patchTest for recent rev-parse $abbrev_sha1 regression

Subject: [PATCH] Test for recent rev-parse $abbrev_sha1 regression

## tl;dr

4 messages between May 30, 2007 and May 30, 2007. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Shawn O. Pearce· May 30, 2007, 04:50 UTC · lore

My recent patch "Lazily open pack index files on demand" caused a regression in the case of parsing abbreviated SHA-1 object names. Git was unable to translate the abbreviated name into the full name if the object was packed, as the pack .idx files were not opened before being accessed.

This is a simple test to repack a repository then test for an abbreviated SHA-1 within the packfile.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 t/t6101-rev-parse-parents.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
Show changes to t/t6101-rev-parse-parents.sh +10 −0
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 7d354a1..b0252b9 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -29,5 +29,15 @@ test_expect_success 'final^1^3 not valid' "if git-rev-parse --verify final^1^3;
 test_expect_failure '--verify start2^1' 'git-rev-parse --verify start2^1'
 test_expect_success '--verify start2^0' 'git-rev-parse --verify start2^0'
 
+test_expect_success 'repack for next test' 'git repack -a -d'
+test_expect_success 'short SHA-1 works' '
+	start=`git rev-parse --verify start` &&
+	echo $start &&
+	abbrv=`echo $start | sed s/.\$//` &&
+	echo $abbrv &&
+	abbrv=`git rev-parse --verify $abbrv` &&
+	echo $abbrv &&
+	test $start = $abbrv'
+
 test_done
 

-- 
1.5.2.869.g6b3ba
Junio C Hamano· May 30, 2007, 05:49 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 5 quoted lines
> My recent patch "Lazily open pack index files on demand" caused a
> regression in the case of parsing abbreviated SHA-1 object names.
> Git was unable to translate the abbreviated name into the full name
> if the object was packed, as the pack .idx files were not opened
> before being accessed.

Thanks. As long as we catch it before it goes to 'master', it is not a regression ;-)

Shawn O. Pearce· May 30, 2007, 05:58 UTC · re: Junio C Hamano · lore

Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression

Junio C Hamano <junkio@cox.net> wrote:
Show 10 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > My recent patch "Lazily open pack index files on demand" caused a
> > regression in the case of parsing abbreviated SHA-1 object names.
> > Git was unable to translate the abbreviated name into the full name
> > if the object was packed, as the pack .idx files were not opened
> > before being accessed.
> 
> Thanks.  As long as we catch it before it goes to 'master', it
> is not a regression ;-)
 
Good point.  Next time I won't call it a regression.

But to me, anything that hits 'next' that breaks Git this badly is a regression. Why? Because I run my production repositories off next, that's why. Of course I do this to exercise next more fully... to prevent this sort of stuff from getting to master. ;-)

For what its worth, I'm going through the code again and auditing our use of the index related variables in packed_git that are now lazily loaded. I'm not seeing anything that is critical. I do have two minor patches queued up, but they are just to make things look prettier. Will send soon.

-- 
Shawn.
Junio C Hamano· May 30, 2007, 06:28 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression

"Shawn O. Pearce" <spearce@spearce.org> writes:
> But to me, anything that hits 'next' that breaks Git this badly is
> a regression.  Why?  Because I run my production repositories off
> next, that's why.  Of course I do this to exercise next more fully...
> to prevent this sort of stuff from getting to master.  ;-)

I'm almost always on 'next'. I switch to run 'master' a few days before -rc0 and a few days after the final feature release I switch back to 'next'.

The tip of 'next' may not be perfect, but it should not be so broken that it is beyond a quick repair to inconvenience users that rely on 'next' working. And it helps us catch problems before the topics hit 'master'.

Because the tip of 'master' gets tagged only once every 6 weeks or so, its tip is not an official release at any other times, but we effectively have the usual pre-release QA continuously running on the stuff that hits 'master'. That's what 'next' is all about.

And that's how we can keep our 'master' a lot more robust than the development branches of other open source projects.

← back to recent threads