Re: [PATCH v2 0/25] prune-safety
- From
Jeff King <peff@peff.net>
- Date
- Oct 17, 2014, 00:03 UTC
- Message-ID
- <20141017000341.GA7848@peff.net>
- In-Reply-To
- <xmqq8ukfslol.fsf@gitster.dls.corp.google.com>
On Thu, Oct 16, 2014 at 03:18:34PM -0700, Junio C Hamano wrote:
> > We should probably add a test for cloning a tag of an otherwise > > unreferenced object, too. > > Yeah, that too ;-)
Here's that test (after the scissors below). It can be applied totally separately, though I stuck it in as patch "18.5/25" in the existing series to confirm that my original series does cause the test to fail, and that it passes with the patch I posted earlier.
Do you want to just squash the fix I posted earlier (into patch 19, the "traverse_commit_list: ..." one)? I'm happy to repost the revised patch, but my impression of your workflow is that squashing is just as easy than replacing a patch (i.e., you're running "rebase -i" either way).
Or I'm happy to re-post the whole series, but it's rather long. :)
> Thanks for a quick diag.
I'm impressed that you found the bug so quickly. :) My biggest fear with the whole series is that it's disrupting and refactoring some fundamental parts of the code that might cause regressions. I put a lot of my faith in the test suite, but that did not work out here (I do still feel pretty good about the series overall, though I think I'd cook it longer than usual given the complexity and the areas it touches).
-- >8 -- Subject: t5516: test pushing a tag of an otherwise unreferenced blob
It's not unreasonable to have a tag that points to a blob that is not part of the normal history. We do this in git.git to distribute gpg keys. However, we never explicitly checked in our test suite that this actually works (i.e., that pack-objects actually sends the blob because of the tag mentioning it).
It does in fact work fine, but a recent patch under discussion broke this, and the test suite didn't notice. Let's make the test suite more complete.
Signed-off-by: Jeff King <peff@peff.net> --- The "should have" below is belt-and-suspenders. The test actually fails with my series without the cat-file check, but I'm concerned a bug that affects pack-objects could also affect the connectivity check on the receiving side.
t/t5516-fetch-push.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 67e0ab3..7c8a769 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1277,4 +1277,17 @@ EOF git push --no-thin --receive-pack="$rcvpck" no-thin/.git refs/heads/master:refs/heads/foo ' +test_expect_success 'pushing a tag pushes the tagged object' ' + rm -rf dst.git && + blob=$(echo unreferenced | git hash-object -w --stdin) && + git tag -m foo tag-of-blob $blob && + git init --bare dst.git && + git push dst.git tag-of-blob && + # the receiving index-pack should have noticed + # any problems, but we double check + echo unreferenced >expect && + git --git-dir=dst.git cat-file blob tag-of-blob >actual && + test_cmp expect actual +' + test_done
-- 2.1.2.596.g7379948