| Message ID | 20210107135025.2682-2-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] bundle: lost objects when removing duplicate pendings | expand |
On 2021-01-07 08:50:24-0500, Jiang Xin <worldhello.net@gmail.com> wrote: > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > `git rev-list` will list one commit for the following command: > > $ git rev-list 'main^!' > <tip-commit-of-main-branch> > > But providing the same rev-list args to `git bundle`, fail to create > a bundle file. > > $ git bundle create - 'main^!' > # v2 git bundle > -<OID> <one-line-message> > > fatal: Refusing to create empty bundle. > > This is because when removing duplicate objects in function > `object_array_remove_duplicates()`, one unique pending object which has > the same name is deleted by mistake. The revision arg 'main^!' in the > above example is parsed by `handle_revision_arg()`, and at lease two > different objects will be appended to `revs.pending`, one points to the > parent commit of the "main" branch, and the other points to the tip > commit of the "main" branch. These two objects have the same name > "main". Only one object is left with the name "main" after calling the > function `object_array_remove_duplicates()`. > > And what's worse, when adding boundary commits into pending list, we use > one-line commit message as names, and the arbitory names may surprise > git-bundle. > > Only comparing objects themselves (".item") is also not good enough, > because user may want to create a bundle with two identical objects but > with different reference names, such as: "HEAD" and "refs/heads/main". > > Add new function `contains_object()` which compare both the address and > the name of the object. > > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > --- > object.c | 10 +- > t/t6020-bundle-misc.sh | 488 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 494 insertions(+), 4 deletions(-) > create mode 100755 t/t6020-bundle-misc.sh > > diff --git a/object.c b/object.c > index 68f80b0b3d..98017bed8e 100644 > --- a/object.c > +++ b/object.c > @@ -412,15 +412,16 @@ void object_array_clear(struct object_array *array) > } > > /* > - * Return true iff array already contains an entry with name. > + * Return true if array already contains an entry. > */ > -static int contains_name(struct object_array *array, const char *name) > +static int contains_object(struct object_array *array, > + const struct object *item, const char *name) > { > unsigned nr = array->nr, i; > struct object_array_entry *object = array->objects; > > for (i = 0; i < nr; i++, object++) > - if (!strcmp(object->name, name)) > + if (item == object->item && !strcmp(object->name, name)) I think the comparison of `item == object->item` is a bit too fragile. Yes, we all know `item` must be an entry of array. However, it's separated from its usage may lead to misuse in the future. Perhaps using `oidcmp` or adding a comment to indicate that `item` must be an entry of `array`. > return 1; > return 0; > } > @@ -432,7 +433,8 @@ void object_array_remove_duplicates(struct object_array *array) > > array->nr = 0; > for (src = 0; src < nr; src++) { > - if (!contains_name(array, objects[src].name)) { > + if (!contains_object(array, objects[src].item, > + objects[src].name)) { > if (src != array->nr) > objects[array->nr] = objects[src]; > array->nr++; > diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh > new file mode 100755 > index 0000000000..d15e67c8f7 > --- /dev/null > +++ b/t/t6020-bundle-misc.sh > @@ -0,0 +1,488 @@ > +#!/bin/sh > +# > +# Copyright (c) 2021 Jiang Xin > +# > + > +test_description='Test git-bundle' > + > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > + > +. ./test-lib.sh > + > +test_bundle_object_count () { > + bundle=$1 && > + pack=${bundle%.bdl}.pack && > + convert_bundle_to_pack <"$bundle" >"$pack" && > + git index-pack "$pack" && > + git verify-pack -v "$pack" >verify.out && > + count=$(grep "^$OID_REGEX " verify.out | wc -l) && I think we can use 'grep -c' instead of `grep .. | wc -l`. Or grep "^$OID_REGEX " verify.out >verify.filtered && test_line_count = $2 verify.filtered The same comment applied to test_thin_bundle_object_count > + test $2 = $count && return 0 > + echo object count for $bundle is $count, not $2 > + return 1 > +} > + > + > +test_thin_bundle_object_count () { > + bundle=$1 && > + pack=${bundle%.bdl}.pack && > + convert_bundle_to_pack <"$bundle" | > + test_must_fail git index-pack --stdin "$pack" && > + rm -f "$pack" && > + convert_bundle_to_pack <"$bundle" | > + git index-pack --stdin --fix-thin "$pack" && > + git verify-pack -v "$pack" >verify.out && > + count=$(grep "^$OID_REGEX " verify.out | wc -l) && > + test $2 = $count && return 0 > + echo object count for $bundle is $count, not $2 > + return 1 > +} > + > +convert_bundle_to_pack () { > + while read x && test -n "$x" > + do > + :; > + done > + cat I'm not sure what you would expect in this case, but in my experience, I replace this whole block with sed '1,/^$/d' also works. IOW, I would apply this on top of your change: ----8<----- diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh index 1f60fe180e..3a428454d7 100755 --- a/t/t6020-bundle-misc.sh +++ b/t/t6020-bundle-misc.sh @@ -16,10 +16,8 @@ test_bundle_object_count () { convert_bundle_to_pack <"$bundle" >"$pack" && git index-pack "$pack" && git verify-pack -v "$pack" >verify.out && - count=$(grep "^$OID_REGEX " verify.out | wc -l) && - test $2 = $count && return 0 - echo object count for $bundle is $count, not $2 - return 1 + grep "^$OID_REGEX " verify.out >verify.filtered && + test_line_count = $2 verify.filtered } @@ -32,18 +30,12 @@ test_thin_bundle_object_count () { convert_bundle_to_pack <"$bundle" | git index-pack --stdin --fix-thin "$pack" && git verify-pack -v "$pack" >verify.out && - count=$(grep "^$OID_REGEX " verify.out | wc -l) && - test $2 = $count && return 0 - echo object count for $bundle is $count, not $2 - return 1 + grep "^$OID_REGEX " verify.out >verify.filtered && + test_line_count = $2 verify.filtered } convert_bundle_to_pack () { - while read x && test -n "$x" - do - :; - done - cat + sed '1,/^$/d' } # Format the output of git commands to make a user-friendly and stable ----->8----- For the below change, I haven't checked but I think test_commit should work, no? -- Danh > +} > + > +# Format the output of git commands to make a user-friendly and stable > +# text. We can easily prepare the expect text without having to worry > +# about future changes of the commit ID and spaces of the output. > +make_user_friendly_and_stable_output () { > + sed \ > + -e "s/ *\$//" \ > + -e "s/$A/<COMMIT-A>/g" \ > + -e "s/$B/<COMMIT-B>/g" \ > + -e "s/$C/<COMMIT-C>/g" \ > + -e "s/$D/<COMMIT-D>/g" \ > + -e "s/$E/<COMMIT-E>/g" \ > + -e "s/$F/<COMMIT-F>/g" \ > + -e "s/$G/<COMMIT-G>/g" \ > + -e "s/$H/<COMMIT-H>/g" \ > + -e "s/$I/<COMMIT-I>/g" \ > + -e "s/$J/<COMMIT-J>/g" \ > + -e "s/$K/<COMMIT-K>/g" \ > + -e "s/$L/<COMMIT-L>/g" \ > + -e "s/$M/<COMMIT-M>/g" \ > + -e "s/$N/<COMMIT-N>/g" \ > + -e "s/$O/<COMMIT-O>/g" \ > + -e "s/$P/<COMMIT-P>/g" \ > + -e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<OID-A>/g" \ > + -e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g" \ > + -e "s/$(echo $C | cut -c1-7)[0-9a-f]*/<OID-C>/g" \ > + -e "s/$(echo $D | cut -c1-7)[0-9a-f]*/<OID-D>/g" \ > + -e "s/$(echo $E | cut -c1-7)[0-9a-f]*/<OID-E>/g" \ > + -e "s/$(echo $F | cut -c1-7)[0-9a-f]*/<OID-F>/g" \ > + -e "s/$(echo $G | cut -c1-7)[0-9a-f]*/<OID-G>/g" \ > + -e "s/$(echo $H | cut -c1-7)[0-9a-f]*/<OID-H>/g" \ > + -e "s/$(echo $I | cut -c1-7)[0-9a-f]*/<OID-I>/g" \ > + -e "s/$(echo $J | cut -c1-7)[0-9a-f]*/<OID-J>/g" \ > + -e "s/$(echo $K | cut -c1-7)[0-9a-f]*/<OID-K>/g" \ > + -e "s/$(echo $L | cut -c1-7)[0-9a-f]*/<OID-L>/g" \ > + -e "s/$(echo $M | cut -c1-7)[0-9a-f]*/<OID-M>/g" \ > + -e "s/$(echo $N | cut -c1-7)[0-9a-f]*/<OID-N>/g" \ > + -e "s/$(echo $O | cut -c1-7)[0-9a-f]*/<OID-O>/g" \ > + -e "s/$(echo $P | cut -c1-7)[0-9a-f]*/<OID-P>/g" \ > + -e "s/$TAG1/<TAG-1>/g" \ > + -e "s/$TAG2/<TAG-2>/g" \ > + -e "s/$TAG3/<TAG-3>/g" \ > + -e "s/$(echo $TAG1 | cut -c1-7)[0-9a-f]*/<OID-TAG-1>/g" \ > + -e "s/$(echo $TAG2 | cut -c1-7)[0-9a-f]*/<OID-TAG-2>/g" \ > + -e "s/$(echo $TAG3 | cut -c1-7)[0-9a-f]*/<OID-TAG-3>/g" \ > + -e "s/$ZERO_OID/<ZERO-OID>/g" > +} > + > +# (C) (D, pull/1/head, topic/1) > +# o --- o > +# / \ (L) > +# / \ o (H, topic/2) (M, tag:v2) > +# / (F) \ / (N, tag:v3) > +# / o --------- o (G, pull/2/head) o --- o --- o (release) > +# / / \ \ / \ > +# o --- o --- o -------- o -- o ------------------ o ------- o --- o (main) > +# (A) (B) (E, tag:v1) (I) (J) (K) (O) (P) > +# > +test_expect_success 'setup' ' > + # commit A & B > + cat >main.txt <<-EOF && > + Commit A > + EOF > + git add main.txt && > + test_tick && > + git commit -m "Commit A" && > + > + cat >main.txt <<-EOF && > + Commit B > + EOF > + git add main.txt && > + test_tick && > + git commit -m "Commit B" && > + A=$(git rev-parse HEAD~) && > + B=$(git rev-parse HEAD) && > + > + # branch topic/1 > + git checkout -b topic/1 && > + cat >topic-1.txt <<-EOF && > + Commit C > + EOF > + git add topic-1.txt && > + test_tick && > + git commit -m "Commit C" && > + > + cat >topic-1.txt <<-EOF && > + Commit D > + EOF > + git add -u && > + test_tick && > + git commit -m "Commit D" && > + git update-ref refs/pull/1/head HEAD && > + C=$(git rev-parse topic/1~) && > + D=$(git rev-parse topic/1) && > + > + # commit E > + git checkout main && > + cat >main.txt <<-EOF && > + Commit E > + EOF > + git add main.txt && > + test_tick && > + git commit -m "Commit E" && > + E=$(git rev-parse HEAD) && > + test_tick && > + git tag -m "v1" v1 HEAD && > + TAG1=$(git rev-parse refs/tags/v1) && > + > + # branch topic/2 > + git checkout -b topic/2 && > + cat >topic-2.txt <<-EOF && > + Commit F > + EOF > + git add topic-2.txt && > + test_tick && > + git commit -m "Commit F" && > + > + cat >topic-2.txt <<-EOF && > + Commit G > + EOF > + git add -u && > + test_tick && > + git commit -m "Commit G" && > + git update-ref refs/pull/2/head HEAD && > + > + cat >topic-2.txt <<-EOF && > + Commit H > + EOF > + git add -u && > + test_tick && > + git commit -m "Commit H" && > + F=$(git rev-parse topic/2~2) && > + G=$(git rev-parse topic/2~) && > + H=$(git rev-parse topic/2) && > + > + # merge commit I & J > + git checkout main && > + test_tick && > + git merge --no-ff --no-edit topic/1 && > + test_tick && > + git merge --no-ff --no-edit refs/pull/2/head && > + I=$(git rev-parse HEAD~) && > + J=$(git rev-parse HEAD) && > + > + # commit K > + git checkout main && > + cat >main.txt <<-EOF && > + Commit K > + EOF > + git add main.txt && > + test_tick && > + git commit -m "Commit K" && > + K=$(git rev-parse HEAD) && > + > + # branch release > + git checkout -b release && > + cat >release.txt <<-EOF && > + Commit L > + EOF > + git add release.txt && > + test_tick && > + git commit -m "Commit L" && > + > + cat >release.txt <<-EOF && > + Commit M > + EOF > + git add -u && > + test_tick && > + git commit -m "Commit M" && > + test_tick && > + git tag -m "v2" v2 HEAD && > + > + cat >release.txt <<-EOF && > + Commit N > + EOF > + git add -u && > + test_tick && > + git commit -m "Commit N" && > + test_tick && > + git tag -m "v3" v3 HEAD && > + L=$(git rev-parse HEAD~2) && > + M=$(git rev-parse HEAD~) && > + N=$(git rev-parse HEAD) && > + TAG2=$(git rev-parse refs/tags/v2) && > + TAG3=$(git rev-parse refs/tags/v3) && > + > + # merge commit O > + git checkout main && > + test_tick && > + git merge --no-ff --no-edit tags/v2 && > + O=$(git rev-parse HEAD) && > + > + # commit P > + git checkout main && > + cat >main.txt <<-EOF && > + Commit P > + EOF > + git add main.txt && > + test_tick && > + git commit -m "Commit P" && > + P=$(git rev-parse HEAD) > +' > + > +test_expect_success 'create bundle from special rev: main^!' ' > + git bundle create special-rev.bdl "main^!" && > + > + git bundle list-heads special-rev.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-P> refs/heads/main > + EOF > + test_i18ncmp expect actual && > + > + git bundle verify special-rev.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + The bundle contains this ref: > + <COMMIT-P> refs/heads/main > + The bundle requires this ref: > + <COMMIT-O> > + EOF > + test_i18ncmp expect actual && > + > + test_bundle_object_count special-rev.bdl 3 > +' > + > +test_expect_success 'create bundle with --max-count option' ' > + git bundle create max-count.bdl --max-count 1 \ > + main \ > + "^release" \ > + refs/tags/v1 \ > + refs/pull/1/head \ > + refs/pull/2/head && > + > + git bundle list-heads max-count.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-P> refs/heads/main > + <TAG-1> refs/tags/v1 > + EOF > + test_i18ncmp expect actual && > + > + git bundle verify max-count.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + The bundle contains these 2 refs: > + <COMMIT-P> refs/heads/main > + <TAG-1> refs/tags/v1 > + The bundle requires this ref: > + <COMMIT-O> > + EOF > + test_i18ncmp expect actual && > + > + test_bundle_object_count max-count.bdl 4 > +' > + > +test_expect_success 'create bundle with --since option' ' > + git bundle create since.bdl \ > + --since "Thu Apr 7 15:26:13 2005 -0700" \ > + --all && > + > + git bundle list-heads since.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-P> refs/heads/main > + <COMMIT-N> refs/heads/release > + <TAG-2> refs/tags/v2 > + <TAG-3> refs/tags/v3 > + <COMMIT-P> HEAD > + EOF > + test_i18ncmp expect actual && > + > + git bundle verify since.bdl | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + The bundle contains these 5 refs: > + <COMMIT-P> refs/heads/main > + <COMMIT-N> refs/heads/release > + <TAG-2> refs/tags/v2 > + <TAG-3> refs/tags/v3 > + <COMMIT-P> HEAD > + The bundle requires these 2 refs: > + <COMMIT-L> > + <COMMIT-K> > + EOF > + test_i18ncmp expect actual && > + > + test_thin_bundle_object_count since.bdl 16 > +' > + > +test_expect_success 'create bundle 1 - no prerequisites' ' > + git bundle create 1.bdl topic/1 topic/2 && > + > + cat >expect <<-EOF && > + The bundle contains these 2 refs: > + <COMMIT-D> refs/heads/topic/1 > + <COMMIT-H> refs/heads/topic/2 > + The bundle records a complete history. > + EOF > + > + # verify bundle, which has no prerequisites > + git bundle verify 1.bdl | > + make_user_friendly_and_stable_output >actual && > + test_i18ncmp expect actual && > + > + test_bundle_object_count 1.bdl 24 > +' > + > +test_expect_success 'create bundle 2 - has prerequisites' ' > + git bundle create 2.bdl \ > + --ignore-missing \ > + ^topic/deleted \ > + ^$D \ > + ^topic/2 \ > + release && > + > + cat >expect <<-EOF && > + The bundle contains this ref: > + <COMMIT-N> refs/heads/release > + The bundle requires these 3 refs: > + <COMMIT-D> > + <COMMIT-E> > + <COMMIT-G> > + EOF > + > + git bundle verify 2.bdl | > + make_user_friendly_and_stable_output >actual && > + test_i18ncmp expect actual && > + > + test_bundle_object_count 2.bdl 16 > +' > + > +test_expect_success 'fail to verify bundle without prerequisites' ' > + git init --bare test1.git && > + > + cat >expect <<-EOF && > + error: Repository lacks these prerequisite commits: > + error: <COMMIT-D> > + error: <COMMIT-E> > + error: <COMMIT-G> > + EOF > + > + test_must_fail git -C test1.git bundle verify ../2.bdl 2>&1 | > + make_user_friendly_and_stable_output >actual && > + test_i18ncmp expect actual > +' > + > +test_expect_success 'create bundle 3 - two refs, same object' ' > + git bundle create --version=3 3.bdl \ > + ^release \ > + ^topic/1 \ > + ^topic/2 \ > + main \ > + HEAD && > + > + cat >expect <<-EOF && > + The bundle contains these 2 refs: > + <COMMIT-P> refs/heads/main > + <COMMIT-P> HEAD > + The bundle requires these 2 refs: > + <COMMIT-M> > + <COMMIT-K> > + EOF > + > + git bundle verify 3.bdl | > + make_user_friendly_and_stable_output >actual && > + test_i18ncmp expect actual && > + > + test_bundle_object_count 3.bdl 4 > +' > + > +test_expect_success 'create bundle 4 - with tags' ' > + git bundle create 4.bdl \ > + ^main \ > + ^release \ > + ^topic/1 \ > + ^topic/2 \ > + --all && > + > + cat >expect <<-EOF && > + The bundle contains these 3 refs: > + <TAG-1> refs/tags/v1 > + <TAG-2> refs/tags/v2 > + <TAG-3> refs/tags/v3 > + The bundle records a complete history. > + EOF > + > + git bundle verify 4.bdl | > + make_user_friendly_and_stable_output >actual && > + test_i18ncmp expect actual && > + > + test_bundle_object_count 4.bdl 3 > +' > + > +test_expect_success 'clone from bundle' ' > + git clone --mirror 1.bdl mirror.git && > + git -C mirror.git show-ref | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-D> refs/heads/topic/1 > + <COMMIT-H> refs/heads/topic/2 > + EOF > + test_cmp expect actual && > + > + git -C mirror.git fetch ../2.bdl "+refs/*:refs/*" && > + git -C mirror.git show-ref | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-N> refs/heads/release > + <COMMIT-D> refs/heads/topic/1 > + <COMMIT-H> refs/heads/topic/2 > + EOF > + test_cmp expect actual && > + > + git -C mirror.git fetch ../3.bdl "+refs/*:refs/*" && > + git -C mirror.git show-ref | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-P> refs/heads/main > + <COMMIT-N> refs/heads/release > + <COMMIT-D> refs/heads/topic/1 > + <COMMIT-H> refs/heads/topic/2 > + EOF > + test_cmp expect actual && > + > + git -C mirror.git fetch ../4.bdl "+refs/*:refs/*" && > + git -C mirror.git show-ref | > + make_user_friendly_and_stable_output >actual && > + cat >expect <<-EOF && > + <COMMIT-P> refs/heads/main > + <COMMIT-N> refs/heads/release > + <COMMIT-D> refs/heads/topic/1 > + <COMMIT-H> refs/heads/topic/2 > + <TAG-1> refs/tags/v1 > + <TAG-2> refs/tags/v2 > + <TAG-3> refs/tags/v3 > + EOF > + test_cmp expect actual > +' > + > +test_done > -- > 2.30.0.2.g06d2f50715 >
Đoàn Trần Công Danh <congdanhqx@gmail.com> 于2021年1月7日周四 下午11:37写道： > > > -static int contains_name(struct object_array *array, const char *name) > > +static int contains_object(struct object_array *array, > > + const struct object *item, const char *name) > > { > > unsigned nr = array->nr, i; > > struct object_array_entry *object = array->objects; > > > > for (i = 0; i < nr; i++, object++) > > - if (!strcmp(object->name, name)) > > + if (item == object->item && !strcmp(object->name, name)) > > I think the comparison of `item == object->item` is a bit too fragile. > Yes, we all know `item` must be an entry of array. > However, it's separated from its usage may lead to misuse in the > future. Perhaps using `oidcmp` or adding a comment to indicate that > `item` must be an entry of `array`. You can find the same usage on comparing address of objects in other places: + https://github.com/git/git/blob/v2.30.0/bundle.c#L447 + https://github.com/git/git/blob/v2.30.0/commit.c#L954 Both `item` and `object->item` point to address of a shared object in parsed_object_pool of the repository, we can compare two objects by checking address of them safely. > > +test_bundle_object_count () { > > + bundle=$1 && > > + pack=${bundle%.bdl}.pack && > > + convert_bundle_to_pack <"$bundle" >"$pack" && > > + git index-pack "$pack" && > > + git verify-pack -v "$pack" >verify.out && > > + count=$(grep "^$OID_REGEX " verify.out | wc -l) && > > I think we can use 'grep -c' instead of `grep .. | wc -l`. This function is borrowed from `t5510-fetch.sh`, and will change like this. Thanks. > > + > > +convert_bundle_to_pack () { > > + while read x && test -n "$x" > > + do > > + :; > > + done > > + cat > > I'm not sure what you would expect in this case, > but in my experience, I replace this whole block with > > sed '1,/^$/d' This function is used to convert bundle file to pack file by strip the header, which has a signature, prerequisites, references. This function is also borrowed from "t5510-fetch.sh". > For the below change, I haven't checked but I think test_commit should work, no? > > +test_expect_success 'setup' ' > > + # commit A & B > > + cat >main.txt <<-EOF && > > + Commit A > > + EOF > > + git add main.txt && > > + test_tick && > > + git commit -m "Commit A" && > > + > > + cat >main.txt <<-EOF && > > + Commit B > > + EOF > > + git add main.txt && > > + test_tick && > > + git commit -m "Commit B" && > > + A=$(git rev-parse HEAD~) && > > + B=$(git rev-parse HEAD) && I should refactor these code, but I forgot. After examine the `test_commit` function, I have to write a new helper, because it does not meet my needs. 1. It should not create a tag every time. 2. The tag it created is not an annotated tag. 3. I need to store the object id to a variable. So I write a new helper `test_commit_setvar()` in next reroll. And make this testcase much simpler. test_expect_success 'setup' ' # branch main: commit A & B test_commit_setvar A "Commit A" main.txt && test_commit_setvar B "Commit B" main.txt && # branch topic/1: commit C & D, refs/pull/1/head git checkout -b topic/1 && test_commit_setvar C "Commit C" topic-1.txt && test_commit_setvar D "Commit D" topic-1.txt && git update-ref refs/pull/1/head HEAD && # branch topic/1: commit E, tag v1 git checkout main && test_commit_setvar E "Commit E" main.txt && test_commit_setvar TAG1 --tag v1 && # branch topic/2: commit F & G, refs/pull/2/head git checkout -b topic/2 && test_commit_setvar F "Commit F" topic-2.txt && test_commit_setvar G "Commit G" topic-2.txt && git update-ref refs/pull/2/head HEAD && test_commit_setvar H "Commit H" topic-2.txt && # branch main: merge commit I & J git checkout main && test_tick && test_commit_setvar I --merge topic/1 "Merge commit I" && test_commit_setvar J --merge refs/pull/2/head "Merge commit J" && # branch main: commit K git checkout main && test_commit_setvar K "Commit K" main.txt && # branch release: git checkout -b release && test_commit_setvar L "Commit L" release.txt && test_commit_setvar M "Commit M" release.txt && test_commit_setvar TAG2 --tag v2 && test_commit_setvar N "Commit N" release.txt && test_commit_setvar TAG3 --tag v3 && # branch main: merge commit O, commit P git checkout main && test_commit_setvar O --merge tags/v2 "Merge commit O" && test_commit_setvar P "Commit P" main.txt ' -- Jiang Xin
diff --git a/object.c b/object.c index 68f80b0b3d..98017bed8e 100644 --- a/object.c +++ b/object.c @@ -412,15 +412,16 @@ void object_array_clear(struct object_array *array) } /* - * Return true iff array already contains an entry with name. + * Return true if array already contains an entry. */ -static int contains_name(struct object_array *array, const char *name) +static int contains_object(struct object_array *array, + const struct object *item, const char *name) { unsigned nr = array->nr, i; struct object_array_entry *object = array->objects; for (i = 0; i < nr; i++, object++) - if (!strcmp(object->name, name)) + if (item == object->item && !strcmp(object->name, name)) return 1; return 0; } @@ -432,7 +433,8 @@ void object_array_remove_duplicates(struct object_array *array) array->nr = 0; for (src = 0; src < nr; src++) { - if (!contains_name(array, objects[src].name)) { + if (!contains_object(array, objects[src].item, + objects[src].name)) { if (src != array->nr) objects[array->nr] = objects[src]; array->nr++; diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh new file mode 100755 index 0000000000..d15e67c8f7 --- /dev/null +++ b/t/t6020-bundle-misc.sh @@ -0,0 +1,488 @@ +#!/bin/sh +# +# Copyright (c) 2021 Jiang Xin +# + +test_description='Test git-bundle' + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +test_bundle_object_count () { + bundle=$1 && + pack=${bundle%.bdl}.pack && + convert_bundle_to_pack <"$bundle" >"$pack" && + git index-pack "$pack" && + git verify-pack -v "$pack" >verify.out && + count=$(grep "^$OID_REGEX " verify.out | wc -l) && + test $2 = $count && return 0 + echo object count for $bundle is $count, not $2 + return 1 +} + + +test_thin_bundle_object_count () { + bundle=$1 && + pack=${bundle%.bdl}.pack && + convert_bundle_to_pack <"$bundle" | + test_must_fail git index-pack --stdin "$pack" && + rm -f "$pack" && + convert_bundle_to_pack <"$bundle" | + git index-pack --stdin --fix-thin "$pack" && + git verify-pack -v "$pack" >verify.out && + count=$(grep "^$OID_REGEX " verify.out | wc -l) && + test $2 = $count && return 0 + echo object count for $bundle is $count, not $2 + return 1 +} + +convert_bundle_to_pack () { + while read x && test -n "$x" + do + :; + done + cat +} + +# Format the output of git commands to make a user-friendly and stable +# text. We can easily prepare the expect text without having to worry +# about future changes of the commit ID and spaces of the output. +make_user_friendly_and_stable_output () { + sed \ + -e "s/ *\$//" \ + -e "s/$A/<COMMIT-A>/g" \ + -e "s/$B/<COMMIT-B>/g" \ + -e "s/$C/<COMMIT-C>/g" \ + -e "s/$D/<COMMIT-D>/g" \ + -e "s/$E/<COMMIT-E>/g" \ + -e "s/$F/<COMMIT-F>/g" \ + -e "s/$G/<COMMIT-G>/g" \ + -e "s/$H/<COMMIT-H>/g" \ + -e "s/$I/<COMMIT-I>/g" \ + -e "s/$J/<COMMIT-J>/g" \ + -e "s/$K/<COMMIT-K>/g" \ + -e "s/$L/<COMMIT-L>/g" \ + -e "s/$M/<COMMIT-M>/g" \ + -e "s/$N/<COMMIT-N>/g" \ + -e "s/$O/<COMMIT-O>/g" \ + -e "s/$P/<COMMIT-P>/g" \ + -e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<OID-A>/g" \ + -e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g" \ + -e "s/$(echo $C | cut -c1-7)[0-9a-f]*/<OID-C>/g" \ + -e "s/$(echo $D | cut -c1-7)[0-9a-f]*/<OID-D>/g" \ + -e "s/$(echo $E | cut -c1-7)[0-9a-f]*/<OID-E>/g" \ + -e "s/$(echo $F | cut -c1-7)[0-9a-f]*/<OID-F>/g" \ + -e "s/$(echo $G | cut -c1-7)[0-9a-f]*/<OID-G>/g" \ + -e "s/$(echo $H | cut -c1-7)[0-9a-f]*/<OID-H>/g" \ + -e "s/$(echo $I | cut -c1-7)[0-9a-f]*/<OID-I>/g" \ + -e "s/$(echo $J | cut -c1-7)[0-9a-f]*/<OID-J>/g" \ + -e "s/$(echo $K | cut -c1-7)[0-9a-f]*/<OID-K>/g" \ + -e "s/$(echo $L | cut -c1-7)[0-9a-f]*/<OID-L>/g" \ + -e "s/$(echo $M | cut -c1-7)[0-9a-f]*/<OID-M>/g" \ + -e "s/$(echo $N | cut -c1-7)[0-9a-f]*/<OID-N>/g" \ + -e "s/$(echo $O | cut -c1-7)[0-9a-f]*/<OID-O>/g" \ + -e "s/$(echo $P | cut -c1-7)[0-9a-f]*/<OID-P>/g" \ + -e "s/$TAG1/<TAG-1>/g" \ + -e "s/$TAG2/<TAG-2>/g" \ + -e "s/$TAG3/<TAG-3>/g" \ + -e "s/$(echo $TAG1 | cut -c1-7)[0-9a-f]*/<OID-TAG-1>/g" \ + -e "s/$(echo $TAG2 | cut -c1-7)[0-9a-f]*/<OID-TAG-2>/g" \ + -e "s/$(echo $TAG3 | cut -c1-7)[0-9a-f]*/<OID-TAG-3>/g" \ + -e "s/$ZERO_OID/<ZERO-OID>/g" +} + +# (C) (D, pull/1/head, topic/1) +# o --- o +# / \ (L) +# / \ o (H, topic/2) (M, tag:v2) +# / (F) \ / (N, tag:v3) +# / o --------- o (G, pull/2/head) o --- o --- o (release) +# / / \ \ / \ +# o --- o --- o -------- o -- o ------------------ o ------- o --- o (main) +# (A) (B) (E, tag:v1) (I) (J) (K) (O) (P) +# +test_expect_success 'setup' ' + # commit A & B + cat >main.txt <<-EOF && + Commit A + EOF + git add main.txt && + test_tick && + git commit -m "Commit A" && + + cat >main.txt <<-EOF && + Commit B + EOF + git add main.txt && + test_tick && + git commit -m "Commit B" && + A=$(git rev-parse HEAD~) && + B=$(git rev-parse HEAD) && + + # branch topic/1 + git checkout -b topic/1 && + cat >topic-1.txt <<-EOF && + Commit C + EOF + git add topic-1.txt && + test_tick && + git commit -m "Commit C" && + + cat >topic-1.txt <<-EOF && + Commit D + EOF + git add -u && + test_tick && + git commit -m "Commit D" && + git update-ref refs/pull/1/head HEAD && + C=$(git rev-parse topic/1~) && + D=$(git rev-parse topic/1) && + + # commit E + git checkout main && + cat >main.txt <<-EOF && + Commit E + EOF + git add main.txt && + test_tick && + git commit -m "Commit E" && + E=$(git rev-parse HEAD) && + test_tick && + git tag -m "v1" v1 HEAD && + TAG1=$(git rev-parse refs/tags/v1) && + + # branch topic/2 + git checkout -b topic/2 && + cat >topic-2.txt <<-EOF && + Commit F + EOF + git add topic-2.txt && + test_tick && + git commit -m "Commit F" && + + cat >topic-2.txt <<-EOF && + Commit G + EOF + git add -u && + test_tick && + git commit -m "Commit G" && + git update-ref refs/pull/2/head HEAD && + + cat >topic-2.txt <<-EOF && + Commit H + EOF + git add -u && + test_tick && + git commit -m "Commit H" && + F=$(git rev-parse topic/2~2) && + G=$(git rev-parse topic/2~) && + H=$(git rev-parse topic/2) && + + # merge commit I & J + git checkout main && + test_tick && + git merge --no-ff --no-edit topic/1 && + test_tick && + git merge --no-ff --no-edit refs/pull/2/head && + I=$(git rev-parse HEAD~) && + J=$(git rev-parse HEAD) && + + # commit K + git checkout main && + cat >main.txt <<-EOF && + Commit K + EOF + git add main.txt && + test_tick && + git commit -m "Commit K" && + K=$(git rev-parse HEAD) && + + # branch release + git checkout -b release && + cat >release.txt <<-EOF && + Commit L + EOF + git add release.txt && + test_tick && + git commit -m "Commit L" && + + cat >release.txt <<-EOF && + Commit M + EOF + git add -u && + test_tick && + git commit -m "Commit M" && + test_tick && + git tag -m "v2" v2 HEAD && + + cat >release.txt <<-EOF && + Commit N + EOF + git add -u && + test_tick && + git commit -m "Commit N" && + test_tick && + git tag -m "v3" v3 HEAD && + L=$(git rev-parse HEAD~2) && + M=$(git rev-parse HEAD~) && + N=$(git rev-parse HEAD) && + TAG2=$(git rev-parse refs/tags/v2) && + TAG3=$(git rev-parse refs/tags/v3) && + + # merge commit O + git checkout main && + test_tick && + git merge --no-ff --no-edit tags/v2 && + O=$(git rev-parse HEAD) && + + # commit P + git checkout main && + cat >main.txt <<-EOF && + Commit P + EOF + git add main.txt && + test_tick && + git commit -m "Commit P" && + P=$(git rev-parse HEAD) +' + +test_expect_success 'create bundle from special rev: main^!' ' + git bundle create special-rev.bdl "main^!" && + + git bundle list-heads special-rev.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-P> refs/heads/main + EOF + test_i18ncmp expect actual && + + git bundle verify special-rev.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + The bundle contains this ref: + <COMMIT-P> refs/heads/main + The bundle requires this ref: + <COMMIT-O> + EOF + test_i18ncmp expect actual && + + test_bundle_object_count special-rev.bdl 3 +' + +test_expect_success 'create bundle with --max-count option' ' + git bundle create max-count.bdl --max-count 1 \ + main \ + "^release" \ + refs/tags/v1 \ + refs/pull/1/head \ + refs/pull/2/head && + + git bundle list-heads max-count.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-P> refs/heads/main + <TAG-1> refs/tags/v1 + EOF + test_i18ncmp expect actual && + + git bundle verify max-count.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + The bundle contains these 2 refs: + <COMMIT-P> refs/heads/main + <TAG-1> refs/tags/v1 + The bundle requires this ref: + <COMMIT-O> + EOF + test_i18ncmp expect actual && + + test_bundle_object_count max-count.bdl 4 +' + +test_expect_success 'create bundle with --since option' ' + git bundle create since.bdl \ + --since "Thu Apr 7 15:26:13 2005 -0700" \ + --all && + + git bundle list-heads since.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-P> refs/heads/main + <COMMIT-N> refs/heads/release + <TAG-2> refs/tags/v2 + <TAG-3> refs/tags/v3 + <COMMIT-P> HEAD + EOF + test_i18ncmp expect actual && + + git bundle verify since.bdl | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + The bundle contains these 5 refs: + <COMMIT-P> refs/heads/main + <COMMIT-N> refs/heads/release + <TAG-2> refs/tags/v2 + <TAG-3> refs/tags/v3 + <COMMIT-P> HEAD + The bundle requires these 2 refs: + <COMMIT-L> + <COMMIT-K> + EOF + test_i18ncmp expect actual && + + test_thin_bundle_object_count since.bdl 16 +' + +test_expect_success 'create bundle 1 - no prerequisites' ' + git bundle create 1.bdl topic/1 topic/2 && + + cat >expect <<-EOF && + The bundle contains these 2 refs: + <COMMIT-D> refs/heads/topic/1 + <COMMIT-H> refs/heads/topic/2 + The bundle records a complete history. + EOF + + # verify bundle, which has no prerequisites + git bundle verify 1.bdl | + make_user_friendly_and_stable_output >actual && + test_i18ncmp expect actual && + + test_bundle_object_count 1.bdl 24 +' + +test_expect_success 'create bundle 2 - has prerequisites' ' + git bundle create 2.bdl \ + --ignore-missing \ + ^topic/deleted \ + ^$D \ + ^topic/2 \ + release && + + cat >expect <<-EOF && + The bundle contains this ref: + <COMMIT-N> refs/heads/release + The bundle requires these 3 refs: + <COMMIT-D> + <COMMIT-E> + <COMMIT-G> + EOF + + git bundle verify 2.bdl | + make_user_friendly_and_stable_output >actual && + test_i18ncmp expect actual && + + test_bundle_object_count 2.bdl 16 +' + +test_expect_success 'fail to verify bundle without prerequisites' ' + git init --bare test1.git && + + cat >expect <<-EOF && + error: Repository lacks these prerequisite commits: + error: <COMMIT-D> + error: <COMMIT-E> + error: <COMMIT-G> + EOF + + test_must_fail git -C test1.git bundle verify ../2.bdl 2>&1 | + make_user_friendly_and_stable_output >actual && + test_i18ncmp expect actual +' + +test_expect_success 'create bundle 3 - two refs, same object' ' + git bundle create --version=3 3.bdl \ + ^release \ + ^topic/1 \ + ^topic/2 \ + main \ + HEAD && + + cat >expect <<-EOF && + The bundle contains these 2 refs: + <COMMIT-P> refs/heads/main + <COMMIT-P> HEAD + The bundle requires these 2 refs: + <COMMIT-M> + <COMMIT-K> + EOF + + git bundle verify 3.bdl | + make_user_friendly_and_stable_output >actual && + test_i18ncmp expect actual && + + test_bundle_object_count 3.bdl 4 +' + +test_expect_success 'create bundle 4 - with tags' ' + git bundle create 4.bdl \ + ^main \ + ^release \ + ^topic/1 \ + ^topic/2 \ + --all && + + cat >expect <<-EOF && + The bundle contains these 3 refs: + <TAG-1> refs/tags/v1 + <TAG-2> refs/tags/v2 + <TAG-3> refs/tags/v3 + The bundle records a complete history. + EOF + + git bundle verify 4.bdl | + make_user_friendly_and_stable_output >actual && + test_i18ncmp expect actual && + + test_bundle_object_count 4.bdl 3 +' + +test_expect_success 'clone from bundle' ' + git clone --mirror 1.bdl mirror.git && + git -C mirror.git show-ref | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-D> refs/heads/topic/1 + <COMMIT-H> refs/heads/topic/2 + EOF + test_cmp expect actual && + + git -C mirror.git fetch ../2.bdl "+refs/*:refs/*" && + git -C mirror.git show-ref | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-N> refs/heads/release + <COMMIT-D> refs/heads/topic/1 + <COMMIT-H> refs/heads/topic/2 + EOF + test_cmp expect actual && + + git -C mirror.git fetch ../3.bdl "+refs/*:refs/*" && + git -C mirror.git show-ref | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-P> refs/heads/main + <COMMIT-N> refs/heads/release + <COMMIT-D> refs/heads/topic/1 + <COMMIT-H> refs/heads/topic/2 + EOF + test_cmp expect actual && + + git -C mirror.git fetch ../4.bdl "+refs/*:refs/*" && + git -C mirror.git show-ref | + make_user_friendly_and_stable_output >actual && + cat >expect <<-EOF && + <COMMIT-P> refs/heads/main + <COMMIT-N> refs/heads/release + <COMMIT-D> refs/heads/topic/1 + <COMMIT-H> refs/heads/topic/2 + <TAG-1> refs/tags/v1 + <TAG-2> refs/tags/v2 + <TAG-3> refs/tags/v3 + EOF + test_cmp expect actual +' + +test_done