From: Jiang Xin <worldhello.net@gmail.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
Git List <git@vger.kernel.org>,
Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: Re: [PATCH v3 1/2] bundle: lost objects when removing duplicate pendings
Date: Fri, 8 Jan 2021 21:14:22 +0800 [thread overview]
Message-ID: <CANYiYbG84GuwnU5QEg0NoMfYS0fWzfDbz-yruA2fr2yQfyfMhg@mail.gmail.com> (raw)
In-Reply-To: <X/cqrTgilKAW9P9G@danh.dev>
Đ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
next prev parent reply other threads:[~2021-01-08 13:15 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-03 9:54 [PATCH] bundle: arguments can be read from stdin Jiang Xin
2021-01-04 23:41 ` Junio C Hamano
2021-01-05 16:30 ` [PATCH v2 1/2] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-05 16:30 ` [PATCH v2 2/2] bundle: arguments can be read from stdin Jiang Xin
2021-01-07 13:50 ` [PATCH v3 0/2] improvements for git-bundle Jiang Xin
2021-01-07 13:50 ` [PATCH v3 1/2] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-07 15:37 ` Đoàn Trần Công Danh
2021-01-08 13:14 ` Jiang Xin [this message]
2021-01-08 14:45 ` [PATCH v4 0/2] Improvements for git-bundle Jiang Xin
2021-01-08 14:45 ` [PATCH v4 1/2] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-09 2:10 ` Junio C Hamano
2021-01-09 13:32 ` Jiang Xin
2021-01-09 22:02 ` Junio C Hamano
2021-01-10 14:30 ` [PATCH v5 0/3] improvements for git-bundle Jiang Xin
2021-01-10 14:30 ` [PATCH v5 1/3] test: add helper functions " Jiang Xin
2021-01-11 20:09 ` Junio C Hamano
2021-01-12 2:27 ` [PATCH v6 0/3] improvements " Jiang Xin
2021-01-12 2:27 ` [PATCH v6 1/3] test: add helper functions " Jiang Xin
2021-05-26 18:49 ` Runaway sed memory use in test on older sed+glibc (was "Re: [PATCH v6 1/3] test: add helper functions for git-bundle") Ævar Arnfjörð Bjarmason
2021-05-27 11:52 ` Jiang Xin
2021-05-27 12:19 ` Ævar Arnfjörð Bjarmason
2021-05-27 13:48 ` Jeff King
2021-05-27 19:19 ` Felipe Contreras
2021-06-01 9:45 ` Jiang Xin
2021-06-01 9:42 ` Jiang Xin
2021-06-01 11:50 ` Ævar Arnfjörð Bjarmason
2021-06-01 13:20 ` Jiang Xin
2021-06-01 14:49 ` [PATCH 1/2] t6020: fix bash incompatible issue Jiang Xin
2021-06-01 14:49 ` [PATCH 2/2] t6020: do not mangle trailing spaces in output Jiang Xin
2021-06-05 17:02 ` Ævar Arnfjörð Bjarmason
2021-06-12 5:07 ` [PATCH v2 0/4] Fixed t6020 bash compatible issue and fixed wrong sideband suffix issue Jiang Xin
2021-06-14 4:10 ` Junio C Hamano
2021-06-15 3:11 ` Jiang Xin
2021-06-17 3:14 ` [PATCH v3] t6020: fix incompatible parameter expansion Jiang Xin
2021-06-21 8:41 ` Ævar Arnfjörð Bjarmason
2021-06-12 5:07 ` [PATCH v2 1/4] t6020: fix bash incompatible issue Jiang Xin
2021-06-12 5:07 ` [PATCH v2 2/4] test: refactor create_commits_in() for t5411 and t5548 Jiang Xin
2021-06-12 5:07 ` [PATCH v2 3/4] sideband: append suffix for message whose CR in next pktline Jiang Xin
2021-06-13 7:47 ` Ævar Arnfjörð Bjarmason
2021-06-14 3:50 ` Junio C Hamano
2021-06-14 11:51 ` Jiang Xin
2021-06-15 1:17 ` Junio C Hamano
2021-06-15 1:47 ` Jiang Xin
2021-06-15 2:11 ` Nicolas Pitre
2021-06-15 3:04 ` Jiang Xin
2021-06-15 3:26 ` Nicolas Pitre
2021-06-15 4:46 ` Junio C Hamano
2021-06-15 7:17 ` Jiang Xin
2021-06-15 14:46 ` Nicolas Pitre
2021-06-12 5:07 ` [PATCH v2 4/4] test: compare raw output, not mangle tabs and spaces Jiang Xin
2021-01-12 2:27 ` [PATCH v6 2/3] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-12 2:27 ` [PATCH v6 3/3] bundle: arguments can be read from stdin Jiang Xin
2021-01-10 14:30 ` [PATCH v5 2/3] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-11 20:12 ` Junio C Hamano
2021-01-10 14:30 ` [PATCH v5 3/3] bundle: arguments can be read from stdin Jiang Xin
2021-01-09 15:09 ` [PATCH v4 1/2] bundle: lost objects when removing duplicate pendings Jiang Xin
2021-01-09 22:02 ` Junio C Hamano
2021-01-08 14:45 ` [PATCH v4 2/2] bundle: arguments can be read from stdin Jiang Xin
2021-01-09 2:18 ` Junio C Hamano
2021-01-07 13:50 ` [PATCH v3 " Jiang Xin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANYiYbG84GuwnU5QEg0NoMfYS0fWzfDbz-yruA2fr2yQfyfMhg@mail.gmail.com \
--to=worldhello.net@gmail.com \
--cc=congdanhqx@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=zhiyou.jx@alibaba-inc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).