mirror of
https://github.com/git/git
synced 2024-08-27 11:39:22 +00:00
![Jeff King](/assets/img/avatar_default.png)
When pack-objects is given --include-tag, it peels each tag ref down to a non-tag object, and if that non-tag object is going to be packed, we include the tag, too. But what happens if we have a chain of tags (e.g., tag "A" points to tag "B", which points to commit "C")? We'll peel down to "C" and realize that we want to include tag "A", but we do not ever consider tag "B", leading to a broken pack (assuming "B" was not otherwise selected). Instead, we have to walk the whole chain, adding any tags we find to the pack. Interestingly, it doesn't seem possible to trigger this problem with "git fetch", but you can with "git clone --single-branch". The reason is that we generate the correct pack when the client explicitly asks for "A" (because we do a real reachability analysis there), and "fetch" is more willing to do so. There are basically two cases: 1. If "C" is already a ref tip, then the client can deduce that it needs "A" itself (via find_non_local_tags), and will ask for it explicitly rather than relying on the include-tag capability. Everything works. 2. If "C" is not already a ref tip, then we hope for include-tag to send us the correct tag. But it doesn't; it generates a broken pack. However, the next step is to do a follow-up run of find_non_local_tags(), followed by fetch_refs() to backfill any tags we learned about. In the normal case, fetch_refs() calls quickfetch(), which does a connectivity check and sees we have no new objects to fetch. We just write the refs. But for the broken-pack case, the connectivity check fails, and quickfetch will follow-up with the remote, asking explicitly for each of the ref tips. This picks up the missing object in a new pack. For a regular "git clone", we are similarly OK, because we explicitly request all of the tag refs, and get a correct pack. But with "--single-branch", we kick in tag auto-following via "include-tag", but do _not_ do a follow-up backfill. We just take whatever the server sent us via include-tag and write out tag refs for any tag objects we were sent. So prior toc6807a4
(clone: open a shortcut for connectivity check, 2013-05-26), we actually claimed the clone was a success, but the result was silently corrupted! Sincec6807a4
, index-pack's connectivity check catches this case, and we correctly complain. The included test directly checks that pack-objects does not generate a broken pack, but also confirms that "clone --single-branch" does not hit the bug. Note that tag chains introduce another interesting question: if we are packing the tag "B" but not the commit "C", should "A" be included? Both before and after this patch, we do not include "A", because the initial peel_ref() check only knows about the bottom-most level, "C". To realize that "B" is involved at all, we would have to switch to an incremental peel, in which we examine each tagged object, asking if it is being packed (and including the outer tag if so). But that runs contrary to the optimizations in peel_ref(), which avoid accessing the objects at all, in favor of using the value we pull from packed-refs. It's OK to walk the whole chain once we know we're going to include the tag (we have to access it anyway, so the effort is proportional to the pack we're generating). But for the initial selection, we have to look at every ref. If we're only packing a few objects, we'd still have to parse every single referenced tag object just to confirm that it isn't part of a tag chain. This could be addressed if packed-refs stored the complete tag chain for each peeled ref (in most cases, this would be the same cost as now, as each "chain" is only a single link). But given the size of that project, it's out of scope for this fix (and probably nobody cares enough anyway, as it's such an obscure situation). This commit limits itself to just avoiding the creation of a broken pack. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
119 lines
2.9 KiB
Bash
Executable file
119 lines
2.9 KiB
Bash
Executable file
#!/bin/sh
|
|
|
|
test_description='git pack-object --include-tag'
|
|
. ./test-lib.sh
|
|
|
|
TRASH=$(pwd)
|
|
|
|
test_expect_success setup '
|
|
echo c >d &&
|
|
git update-index --add d &&
|
|
tree=$(git write-tree) &&
|
|
commit=$(git commit-tree $tree </dev/null) &&
|
|
echo "object $commit" >sig &&
|
|
echo "type commit" >>sig &&
|
|
echo "tag mytag" >>sig &&
|
|
echo "tagger $(git var GIT_COMMITTER_IDENT)" >>sig &&
|
|
echo >>sig &&
|
|
echo "our test tag" >>sig &&
|
|
tag=$(git mktag <sig) &&
|
|
rm d sig &&
|
|
git update-ref refs/tags/mytag $tag && {
|
|
echo $tree &&
|
|
echo $commit &&
|
|
git ls-tree $tree | sed -e "s/.* \\([0-9a-f]*\\) .*/\\1/"
|
|
} >obj-list
|
|
'
|
|
|
|
test_expect_success 'pack without --include-tag' '
|
|
packname=$(git pack-objects \
|
|
--window=0 \
|
|
test-no-include <obj-list)
|
|
'
|
|
|
|
test_expect_success 'unpack objects' '
|
|
rm -rf clone.git &&
|
|
git init clone.git &&
|
|
git -C clone.git unpack-objects <test-no-include-${packname}.pack
|
|
'
|
|
|
|
test_expect_success 'check unpacked result (have commit, no tag)' '
|
|
git rev-list --objects $commit >list.expect &&
|
|
test_must_fail git -C clone.git cat-file -e $tag &&
|
|
git -C clone.git rev-list --objects $commit >list.actual &&
|
|
test_cmp list.expect list.actual
|
|
'
|
|
|
|
test_expect_success 'pack with --include-tag' '
|
|
packname=$(git pack-objects \
|
|
--window=0 \
|
|
--include-tag \
|
|
test-include <obj-list)
|
|
'
|
|
|
|
test_expect_success 'unpack objects' '
|
|
rm -rf clone.git &&
|
|
git init clone.git &&
|
|
git -C clone.git unpack-objects <test-include-${packname}.pack
|
|
'
|
|
|
|
test_expect_success 'check unpacked result (have commit, have tag)' '
|
|
git rev-list --objects mytag >list.expect &&
|
|
git -C clone.git rev-list --objects $tag >list.actual &&
|
|
test_cmp list.expect list.actual
|
|
'
|
|
|
|
# A tag of a tag, where the "inner" tag is not otherwise
|
|
# reachable, and a full peel points to a commit reachable from HEAD.
|
|
test_expect_success 'create hidden inner tag' '
|
|
test_commit commit &&
|
|
git tag -m inner inner HEAD &&
|
|
git tag -m outer outer inner &&
|
|
git tag -d inner
|
|
'
|
|
|
|
test_expect_success 'pack explicit outer tag' '
|
|
packname=$(
|
|
{
|
|
echo HEAD &&
|
|
echo outer
|
|
} |
|
|
git pack-objects --revs test-hidden-explicit
|
|
)
|
|
'
|
|
|
|
test_expect_success 'unpack objects' '
|
|
rm -rf clone.git &&
|
|
git init clone.git &&
|
|
git -C clone.git unpack-objects <test-hidden-explicit-${packname}.pack
|
|
'
|
|
|
|
test_expect_success 'check unpacked result (have all objects)' '
|
|
git -C clone.git rev-list --objects $(git rev-parse outer HEAD)
|
|
'
|
|
|
|
test_expect_success 'pack implied outer tag' '
|
|
packname=$(
|
|
echo HEAD |
|
|
git pack-objects --revs --include-tag test-hidden-implied
|
|
)
|
|
'
|
|
|
|
test_expect_success 'unpack objects' '
|
|
rm -rf clone.git &&
|
|
git init clone.git &&
|
|
git -C clone.git unpack-objects <test-hidden-implied-${packname}.pack
|
|
'
|
|
|
|
test_expect_success 'check unpacked result (have all objects)' '
|
|
git -C clone.git rev-list --objects $(git rev-parse outer HEAD)
|
|
'
|
|
|
|
test_expect_success 'single-branch clone can transfer tag' '
|
|
rm -rf clone.git &&
|
|
git clone --no-local --single-branch -b master . clone.git &&
|
|
git -C clone.git fsck
|
|
'
|
|
|
|
test_done
|