Re: [PATCH] bundle: arguments can be read from stdin

11 messages, 3 authors, 2021-01-08 · open the first message on its own page

Re: [PATCH] bundle: arguments can be read from stdin

From: Junio C Hamano <hidden>
Date: 2021-01-04 23:42:12 

Jiang Xin [off-list ref] writes:

Therefore, before calling `prepare_revision_walk()` function, make a
copy on `revs.pending` for later use.
The in-core objects pointed by the list elements are shared between
the original and the copy, and the object flag bits that are used to
control the traversal (like SEEN, SHOWN and BOUNDARY bits) would be
smudged during the traversal.  So depending on how the "later use"
uses the copied list, it may or may not be sufficient to just copy
the list.

Apparently, you've tested the updated code well enough to send to
the list, so it must be sufficient to make a copy of the list to
support the way the updated code uses it, but it is not clear how it
is so, only from what is in the proposed log message.

[PATCH v2 1/2] bundle: lost objects when removing duplicate pendings

From: Jiang Xin <hidden>
Date: 2021-01-05 16:31:40 

From: Jiang Xin <redacted>

`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 <redacted>
---
 object.c               |  10 +-
 t/t6020-bundle-misc.sh | 413 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 419 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))
 			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..c6613a4162
--- /dev/null
+++ b/t/t6020-bundle-misc.sh
@@ -0,0 +1,413 @@
+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 () {
+	git verify-pack -v "$1" >verify.out &&
+	count=$(grep "^$OID_REGEX " verify.out | wc -l) &&
+	test $2 = $count && return 0
+	echo object count 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, tags/v1)
+#           /         \        o (H, topic/2)             (M, tags/v2)
+#          /    (F)    \      /                                 (N, tags/v3)
+#         /      o --------- o (G, pull/2/head)      o --- o --- o (release)
+#        /      /        \    \                      /       \
+#  o --- o --- o -------- o -- o ------------------ o ------- o --- o (main)
+# (A)   (B)   (E)        (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) &&
+
+	# 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" &&
+	test_tick &&
+	git tag -m "v1" v1 HEAD &&
+
+	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) &&
+	TAG1=$(git rev-parse refs/tags/v1) &&
+	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 &&
+
+	convert_bundle_to_pack <special-rev.bdl >special-rev.pack &&
+	git index-pack special-rev.pack &&
+	test_bundle_object_count special-rev.pack 3
+'
+
+test_expect_success 'create bundle 1 - no prerequisites' '
+	# create bundle from args
+	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 &&
+
+	convert_bundle_to_pack <1.bdl >1.pack &&
+	git index-pack 1.pack &&
+	test_bundle_object_count 1.pack 24
+'
+
+test_expect_success 'create bundle 2 - has prerequisites' '
+	# create bundle from args
+	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 &&
+
+	convert_bundle_to_pack <2.bdl >2.pack &&
+	git index-pack 2.pack &&
+	test_bundle_object_count 2.pack 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' '
+	# create bundle from args
+	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 &&
+
+	convert_bundle_to_pack <3.bdl >3.pack &&
+	git index-pack 3.pack &&
+	test_bundle_object_count 3.pack 4
+'
+
+test_expect_success 'create bundle 4 - with tags' '
+	# create bundle from args
+	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 &&
+
+	convert_bundle_to_pack <4.bdl >4.pack &&
+	git index-pack 4.pack &&
+	test_bundle_object_count 4.pack 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.4.g09ee524cd5

[PATCH v2 2/2] bundle: arguments can be read from stdin

From: Jiang Xin <hidden>
Date: 2021-01-05 16:31:56 

From: Jiang Xin <redacted>

In order to create an incremental bundle, we need to pass many arguments
to let git-bundle ignore some already packed commits.  It will be more
convenient to pass args via stdin.  But the current implementation does
not allow us to do this.

This is because args are parsed twice when creating bundle.  The first
time for parsing args is in `compute_and_write_prerequisites()` by
running `git-rev-list` command to write prerequisites in bundle file,
and stdin is consumed in this step if "--stdin" option is provided for
`git-bundle`.  Later nothing can be read from stdin when running
`setup_revisions()` in `create_bundle()`.

Remove the entire `compute_and_write_prerequisites()` function, and
parse the args once by `setup_revisions()`.  The first step for creating
a bundle is to write prerequisites ("-" obj-id SP comment LF), but after
calling `prepare_revision_walk()`, the `revs.pending` is left empty.
Following steps could not work properly without data in `revs.pending`.
Therefore, before calling `prepare_revision_walk()` function, make a
copy `revs_copy` from `revs` for later use.  Even though `revs_copy` and
`revs` share the same objects, but changes on these objects will not
change the behavior of function `write_bundle_refs()` and
`write_pack_data()`.

Also add testcases for git bundle in t6020, which read args from stdin.

Signed-off-by: Jiang Xin <redacted>
---
 bundle.c                | 109 ++++++++++++++++++++++------------------
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  |  85 +++++++++++++++++++++++++++++--
 3 files changed, 142 insertions(+), 56 deletions(-)

diff --git a/bundle.c b/bundle.c
index cb0e5931ac..693d619551 100644
--- a/bundle.c
+++ b/bundle.c
@@ -338,48 +338,6 @@ static int write_pack_data(int bundle_fd, struct rev_info *revs, struct strvec *
 	return 0;
 }
 
-static int compute_and_write_prerequisites(int bundle_fd,
-					   struct rev_info *revs,
-					   int argc, const char **argv)
-{
-	struct child_process rls = CHILD_PROCESS_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	FILE *rls_fout;
-	int i;
-
-	strvec_pushl(&rls.args,
-		     "rev-list", "--boundary", "--pretty=oneline",
-		     NULL);
-	for (i = 1; i < argc; i++)
-		strvec_push(&rls.args, argv[i]);
-	rls.out = -1;
-	rls.git_cmd = 1;
-	if (start_command(&rls))
-		return -1;
-	rls_fout = xfdopen(rls.out, "r");
-	while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
-		struct object_id oid;
-		if (buf.len > 0 && buf.buf[0] == '-') {
-			write_or_die(bundle_fd, buf.buf, buf.len);
-			if (!get_oid_hex(buf.buf + 1, &oid)) {
-				struct object *object = parse_object_or_die(&oid,
-									    buf.buf);
-				object->flags |= UNINTERESTING;
-				add_pending_object(revs, object, buf.buf);
-			}
-		} else if (!get_oid_hex(buf.buf, &oid)) {
-			struct object *object = parse_object_or_die(&oid,
-								    buf.buf);
-			object->flags |= SHOWN;
-		}
-	}
-	strbuf_release(&buf);
-	fclose(rls_fout);
-	if (finish_command(&rls))
-		return error(_("rev-list died"));
-	return 0;
-}
-
 /*
  * Write out bundle refs based on the tips already
  * parsed into revs.pending. As a side effect, may
@@ -474,6 +432,38 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 	return ref_count;
 }
 
+struct bundle_prerequisites_info {
+	struct object_array *pending;
+	int fd;
+};
+
+static void write_bundle_prerequisites(struct commit *commit, void *data)
+{
+	struct bundle_prerequisites_info *bpi = data;
+	struct object *object;
+	struct pretty_print_context ctx = { 0 };
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!(commit->object.flags & BOUNDARY))
+		return;
+	strbuf_addf(&buf, "-%s ", oid_to_hex(&commit->object.oid));
+	write_or_die(bpi->fd, buf.buf, buf.len);
+
+	ctx.fmt = CMIT_FMT_ONELINE;
+	ctx.output_encoding = get_log_output_encoding();
+	strbuf_reset(&buf);
+	pretty_print_commit(&ctx, commit, &buf);
+	strbuf_trim(&buf);
+
+	object = (struct object *)commit;
+	object->flags |= UNINTERESTING;
+	add_object_array_with_path(object, buf.buf, bpi->pending, S_IFINVALID,
+				   NULL);
+	strbuf_addch(&buf, '\n');
+	write_or_die(bpi->fd, buf.buf, buf.len);
+	strbuf_release(&buf);
+}
+
 int create_bundle(struct repository *r, const char *path,
 		  int argc, const char **argv, struct strvec *pack_options, int version)
 {
@@ -481,8 +471,10 @@ int create_bundle(struct repository *r, const char *path,
 	int bundle_fd = -1;
 	int bundle_to_stdout;
 	int ref_count = 0;
-	struct rev_info revs;
+	struct rev_info revs, revs_copy;
 	int min_version = the_hash_algo == &hash_algos[GIT_HASH_SHA1] ? 2 : 3;
+	struct bundle_prerequisites_info bpi;
+	int i;
 
 	bundle_to_stdout = !strcmp(path, "-");
 	if (bundle_to_stdout)
@@ -512,10 +504,6 @@ int create_bundle(struct repository *r, const char *path,
 	save_commit_buffer = 0;
 	repo_init_revisions(r, &revs, NULL);
 
-	/* write prerequisites */
-	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
-		goto err;
-
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	if (argc > 1) {
@@ -523,16 +511,37 @@ int create_bundle(struct repository *r, const char *path,
 		goto err;
 	}
 
-	object_array_remove_duplicates(&revs.pending);
+	/* save revs.pending in revs_copy for later use */
+	memcpy(&revs_copy, &revs, sizeof(revs));
+	revs_copy.pending.nr = 0;
+	revs_copy.pending.alloc = 0;
+	revs_copy.pending.objects = NULL;
+	for (i = 0; i < revs.pending.nr; i++) {
+		struct object_array_entry *e = revs.pending.objects + i;
+		if (e)
+			add_object_array_with_path(e->item, e->name,
+						   &revs_copy.pending,
+						   e->mode, e->path);
+	}
 
-	ref_count = write_bundle_refs(bundle_fd, &revs);
+	/* write prerequisites */
+	revs.boundary = 1;
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
+	bpi.fd = bundle_fd;
+	bpi.pending = &revs_copy.pending;
+	traverse_commit_list(&revs, write_bundle_prerequisites, NULL, &bpi);
+	object_array_remove_duplicates(&revs_copy.pending);
+
+	/* write bundle refs */
+	ref_count = write_bundle_refs(bundle_fd, &revs_copy);
 	if (!ref_count)
 		die(_("Refusing to create empty bundle."));
 	else if (ref_count < 0)
 		goto err;
 
 	/* write pack */
-	if (write_pack_data(bundle_fd, &revs, pack_options))
+	if (write_pack_data(bundle_fd, &revs_copy, pack_options))
 		goto err;
 
 	if (!bundle_to_stdout) {
diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index 26985f4b44..425258767d 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -38,13 +38,13 @@ test_expect_success 'die if bundle file cannot be created' '
 	test_must_fail git bundle create adir --all
 '
 
-test_expect_failure 'bundle --stdin' '
+test_expect_success 'bundle --stdin' '
 	echo master | git bundle create stdin-bundle.bdl --stdin &&
 	git ls-remote stdin-bundle.bdl >output &&
 	grep master output
 '
 
-test_expect_failure 'bundle --stdin <rev-list options>' '
+test_expect_success 'bundle --stdin <rev-list options>' '
 	echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
 	git ls-remote hybrid-bundle.bdl >output &&
 	grep master output
diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index c6613a4162..780a66dcc5 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -252,6 +252,13 @@ test_expect_success 'create bundle 1 - no prerequisites' '
 	# create bundle from args
 	git bundle create 1.bdl topic/1 topic/2 &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		topic/1
+		topic/2
+		EOF
+	git bundle create stdin-1.bdl --stdin <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-D> refs/heads/topic/1
@@ -264,9 +271,17 @@ test_expect_success 'create bundle 1 - no prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
+	git bundle verify stdin-1.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
 	convert_bundle_to_pack <1.bdl >1.pack &&
 	git index-pack 1.pack &&
-	test_bundle_object_count 1.pack 24
+	test_bundle_object_count 1.pack 24 &&
+
+	convert_bundle_to_pack <stdin-1.bdl >stdin-1.pack &&
+	git index-pack stdin-1.pack &&
+	test_bundle_object_count stdin-1.pack 24
 '
 
 test_expect_success 'create bundle 2 - has prerequisites' '
@@ -278,6 +293,18 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		^topic/2 \
 		release &&
 
+	# create bundle from stdin
+	# input has a non-exist reference: "topic/deleted"
+	cat >input <<-EOF &&
+		^topic/deleted
+		^$D
+		^topic/2
+		EOF
+	git bundle create stdin-2.bdl \
+		--ignore-missing \
+		--stdin \
+		release <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains this ref:
 		<COMMIT-N> refs/heads/release
@@ -291,9 +318,17 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
+	git bundle verify stdin-2.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
 	convert_bundle_to_pack <2.bdl >2.pack &&
 	git index-pack 2.pack &&
-	test_bundle_object_count 2.pack 16
+	test_bundle_object_count 2.pack 16 &&
+
+	convert_bundle_to_pack <stdin-2.bdl >stdin-2.pack &&
+	git index-pack stdin-2.pack &&
+	test_bundle_object_count stdin-2.pack 16
 '
 
 test_expect_success 'fail to verify bundle without prerequisites' '
@@ -308,6 +343,10 @@ test_expect_success 'fail to verify bundle without prerequisites' '
 
 	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_must_fail git -C test1.git bundle verify ../stdin-2.bdl 2>&1 |
+		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual
 '
 
@@ -320,6 +359,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		main \
 		HEAD &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create --version=3 stdin-3.bdl \
+		--stdin \
+		main HEAD <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-P> refs/heads/main
@@ -333,9 +382,17 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
+	git bundle verify stdin-3.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
 	convert_bundle_to_pack <3.bdl >3.pack &&
 	git index-pack 3.pack &&
-	test_bundle_object_count 3.pack 4
+	test_bundle_object_count 3.pack 4 &&
+
+	convert_bundle_to_pack <stdin-3.bdl >stdin-3.pack &&
+	git index-pack stdin-3.pack &&
+	test_bundle_object_count stdin-3.pack 4
 '
 
 test_expect_success 'create bundle 4 - with tags' '
@@ -347,6 +404,18 @@ test_expect_success 'create bundle 4 - with tags' '
 		^topic/2 \
 		--all &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^main
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create stdin-4.bdl \
+		--ignore-missing \
+		--stdin \
+		--all <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 3 refs:
 		<TAG-1> refs/tags/v1
@@ -359,9 +428,17 @@ test_expect_success 'create bundle 4 - with tags' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
+	git bundle verify stdin-4.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
 	convert_bundle_to_pack <4.bdl >4.pack &&
 	git index-pack 4.pack &&
-	test_bundle_object_count 4.pack 3
+	test_bundle_object_count 4.pack 3 &&
+
+	convert_bundle_to_pack <stdin-4.bdl >stdin-4.pack &&
+	git index-pack stdin-4.pack &&
+	test_bundle_object_count stdin-4.pack 3
 '
 
 test_expect_success 'clone from bundle' '
-- 
2.30.0.4.g09ee524cd5

[PATCH v3 2/2] bundle: arguments can be read from stdin

From: Jiang Xin <hidden>
Date: 2021-01-07 13:51:29 

From: Jiang Xin <redacted>

In order to create an incremental bundle, we need to pass many arguments
to let git-bundle ignore some already packed commits.  It will be more
convenient to pass args via stdin.  But the current implementation does
not allow us to do this.

This is because args are parsed twice when creating bundle.  The first
time for parsing args is in `compute_and_write_prerequisites()` by
running `git-rev-list` command to write prerequisites in bundle file,
and stdin is consumed in this step if "--stdin" option is provided for
`git-bundle`.  Later nothing can be read from stdin when running
`setup_revisions()` in `create_bundle()`.

The solution is to parse args once by removing the entire function
`compute_and_write_prerequisites()` and then calling function
`setup_revisions()`.  In order to write prerequisites for bundle, will
call `prepare_revision_walk()` and `traverse_commit_list()`.  But after
calling `prepare_revision_walk()`, the object array `revs.pending` is
left empty, and the following steps could not work properly with the
empty object array (`revs.pending`).  Therefore, make a copy of `revs`
to `revs_copy` for later use right after calling `setup_revisions()`.

The copy of `revs_copy` is not a deep copy, it shares the same objects
with `revs`. The object array of `revs` has been cleared, but objects
themselves are still kept.  Flags of objects may change after calling
`prepare_revision_walk()`, we can use these changed flags without
calling the `git rev-list` command and parsing its output like the
former implementation.

Also add testcases for git bundle in t6020, which read args from stdin.

Signed-off-by: Jiang Xin <redacted>
---
 bundle.c                | 111 ++++++++++++++++++++++------------------
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  |  77 ++++++++++++++++++++++++++--
 3 files changed, 135 insertions(+), 57 deletions(-)

diff --git a/bundle.c b/bundle.c
index cb0e5931ac..72970d962a 100644
--- a/bundle.c
+++ b/bundle.c
@@ -338,48 +338,6 @@ static int write_pack_data(int bundle_fd, struct rev_info *revs, struct strvec *
 	return 0;
 }
 
-static int compute_and_write_prerequisites(int bundle_fd,
-					   struct rev_info *revs,
-					   int argc, const char **argv)
-{
-	struct child_process rls = CHILD_PROCESS_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	FILE *rls_fout;
-	int i;
-
-	strvec_pushl(&rls.args,
-		     "rev-list", "--boundary", "--pretty=oneline",
-		     NULL);
-	for (i = 1; i < argc; i++)
-		strvec_push(&rls.args, argv[i]);
-	rls.out = -1;
-	rls.git_cmd = 1;
-	if (start_command(&rls))
-		return -1;
-	rls_fout = xfdopen(rls.out, "r");
-	while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
-		struct object_id oid;
-		if (buf.len > 0 && buf.buf[0] == '-') {
-			write_or_die(bundle_fd, buf.buf, buf.len);
-			if (!get_oid_hex(buf.buf + 1, &oid)) {
-				struct object *object = parse_object_or_die(&oid,
-									    buf.buf);
-				object->flags |= UNINTERESTING;
-				add_pending_object(revs, object, buf.buf);
-			}
-		} else if (!get_oid_hex(buf.buf, &oid)) {
-			struct object *object = parse_object_or_die(&oid,
-								    buf.buf);
-			object->flags |= SHOWN;
-		}
-	}
-	strbuf_release(&buf);
-	fclose(rls_fout);
-	if (finish_command(&rls))
-		return error(_("rev-list died"));
-	return 0;
-}
-
 /*
  * Write out bundle refs based on the tips already
  * parsed into revs.pending. As a side effect, may
@@ -425,7 +383,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		 * constraints.
 		 */
 		if (!(e->item->flags & SHOWN) && e->item->type == OBJ_COMMIT) {
-			warning(_("ref '%s' is excluded by the rev-list options"),
+			warning(_("ref '%s' is excluded by the limiting options"),
 				e->name);
 			goto skip_write_ref;
 		}
@@ -474,6 +432,38 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 	return ref_count;
 }
 
+struct bundle_prerequisites_info {
+	struct object_array *pending;
+	int fd;
+};
+
+static void write_bundle_prerequisites(struct commit *commit, void *data)
+{
+	struct bundle_prerequisites_info *bpi = data;
+	struct object *object;
+	struct pretty_print_context ctx = { 0 };
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!(commit->object.flags & BOUNDARY))
+		return;
+	strbuf_addf(&buf, "-%s ", oid_to_hex(&commit->object.oid));
+	write_or_die(bpi->fd, buf.buf, buf.len);
+
+	ctx.fmt = CMIT_FMT_ONELINE;
+	ctx.output_encoding = get_log_output_encoding();
+	strbuf_reset(&buf);
+	pretty_print_commit(&ctx, commit, &buf);
+	strbuf_trim(&buf);
+
+	object = (struct object *)commit;
+	object->flags |= UNINTERESTING;
+	add_object_array_with_path(object, buf.buf, bpi->pending, S_IFINVALID,
+				   NULL);
+	strbuf_addch(&buf, '\n');
+	write_or_die(bpi->fd, buf.buf, buf.len);
+	strbuf_release(&buf);
+}
+
 int create_bundle(struct repository *r, const char *path,
 		  int argc, const char **argv, struct strvec *pack_options, int version)
 {
@@ -481,8 +471,10 @@ int create_bundle(struct repository *r, const char *path,
 	int bundle_fd = -1;
 	int bundle_to_stdout;
 	int ref_count = 0;
-	struct rev_info revs;
+	struct rev_info revs, revs_copy;
 	int min_version = the_hash_algo == &hash_algos[GIT_HASH_SHA1] ? 2 : 3;
+	struct bundle_prerequisites_info bpi;
+	int i;
 
 	bundle_to_stdout = !strcmp(path, "-");
 	if (bundle_to_stdout)
@@ -512,10 +504,6 @@ int create_bundle(struct repository *r, const char *path,
 	save_commit_buffer = 0;
 	repo_init_revisions(r, &revs, NULL);
 
-	/* write prerequisites */
-	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
-		goto err;
-
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	if (argc > 1) {
@@ -523,16 +511,37 @@ int create_bundle(struct repository *r, const char *path,
 		goto err;
 	}
 
-	object_array_remove_duplicates(&revs.pending);
+	/* save revs.pending in revs_copy for later use */
+	memcpy(&revs_copy, &revs, sizeof(revs));
+	revs_copy.pending.nr = 0;
+	revs_copy.pending.alloc = 0;
+	revs_copy.pending.objects = NULL;
+	for (i = 0; i < revs.pending.nr; i++) {
+		struct object_array_entry *e = revs.pending.objects + i;
+		if (e)
+			add_object_array_with_path(e->item, e->name,
+						   &revs_copy.pending,
+						   e->mode, e->path);
+	}
 
-	ref_count = write_bundle_refs(bundle_fd, &revs);
+	/* write prerequisites */
+	revs.boundary = 1;
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
+	bpi.fd = bundle_fd;
+	bpi.pending = &revs_copy.pending;
+	traverse_commit_list(&revs, write_bundle_prerequisites, NULL, &bpi);
+	object_array_remove_duplicates(&revs_copy.pending);
+
+	/* write bundle refs */
+	ref_count = write_bundle_refs(bundle_fd, &revs_copy);
 	if (!ref_count)
 		die(_("Refusing to create empty bundle."));
 	else if (ref_count < 0)
 		goto err;
 
 	/* write pack */
-	if (write_pack_data(bundle_fd, &revs, pack_options))
+	if (write_pack_data(bundle_fd, &revs_copy, pack_options))
 		goto err;
 
 	if (!bundle_to_stdout) {
diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index 26985f4b44..425258767d 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -38,13 +38,13 @@ test_expect_success 'die if bundle file cannot be created' '
 	test_must_fail git bundle create adir --all
 '
 
-test_expect_failure 'bundle --stdin' '
+test_expect_success 'bundle --stdin' '
 	echo master | git bundle create stdin-bundle.bdl --stdin &&
 	git ls-remote stdin-bundle.bdl >output &&
 	grep master output
 '
 
-test_expect_failure 'bundle --stdin <rev-list options>' '
+test_expect_success 'bundle --stdin <rev-list options>' '
 	echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
 	git ls-remote hybrid-bundle.bdl >output &&
 	grep master output
diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index d15e67c8f7..1f60fe180e 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -336,8 +336,16 @@ test_expect_success 'create bundle with --since option' '
 '
 
 test_expect_success 'create bundle 1 - no prerequisites' '
+	# create bundle from args
 	git bundle create 1.bdl topic/1 topic/2 &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		topic/1
+		topic/2
+		EOF
+	git bundle create stdin-1.bdl --stdin <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-D> refs/heads/topic/1
@@ -350,10 +358,16 @@ test_expect_success 'create bundle 1 - no prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 1.bdl 24
+	git bundle verify stdin-1.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       1.bdl 24 &&
+	test_bundle_object_count stdin-1.bdl 24
 '
 
 test_expect_success 'create bundle 2 - has prerequisites' '
+	# create bundle from args
 	git bundle create 2.bdl \
 		--ignore-missing \
 		^topic/deleted \
@@ -361,6 +375,18 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		^topic/2 \
 		release &&
 
+	# create bundle from stdin
+	# input has a non-exist reference: "topic/deleted"
+	cat >input <<-EOF &&
+		^topic/deleted
+		^$D
+		^topic/2
+		EOF
+	git bundle create stdin-2.bdl \
+		--ignore-missing \
+		--stdin \
+		release <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains this ref:
 		<COMMIT-N> refs/heads/release
@@ -374,7 +400,12 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 2.bdl 16
+	git bundle verify stdin-2.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       2.bdl 16 &&
+	test_bundle_object_count stdin-2.bdl 16
 '
 
 test_expect_success 'fail to verify bundle without prerequisites' '
@@ -389,10 +420,15 @@ test_expect_success 'fail to verify bundle without prerequisites' '
 
 	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_must_fail git -C test1.git bundle verify ../stdin-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' '
+	# create bundle from args
 	git bundle create --version=3 3.bdl \
 		^release \
 		^topic/1 \
@@ -400,6 +436,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		main \
 		HEAD &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create --version=3 stdin-3.bdl \
+		--stdin \
+		main HEAD <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-P> refs/heads/main
@@ -413,10 +459,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 3.bdl 4
+	git bundle verify stdin-3.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       3.bdl 4 &&
+	test_bundle_object_count stdin-3.bdl 4
 '
 
 test_expect_success 'create bundle 4 - with tags' '
+	# create bundle from args
 	git bundle create 4.bdl \
 		^main \
 		^release \
@@ -424,6 +476,18 @@ test_expect_success 'create bundle 4 - with tags' '
 		^topic/2 \
 		--all &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^main
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create stdin-4.bdl \
+		--ignore-missing \
+		--stdin \
+		--all <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 3 refs:
 		<TAG-1> refs/tags/v1
@@ -436,7 +500,12 @@ test_expect_success 'create bundle 4 - with tags' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 4.bdl 3
+	git bundle verify stdin-4.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       4.bdl 3 &&
+	test_bundle_object_count stdin-4.bdl 3
 '
 
 test_expect_success 'clone from bundle' '
-- 
2.30.0.2.g06d2f50715

[PATCH v3 1/2] bundle: lost objects when removing duplicate pendings

From: Jiang Xin <hidden>
Date: 2021-01-07 13:51:29 

From: Jiang Xin <redacted>

`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 <redacted>
---
 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))
 			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
-- 
2.30.0.2.g06d2f50715

[PATCH v3 0/2] improvements for git-bundle

From: Jiang Xin <hidden>
Date: 2021-01-07 13:51:29 

From: Jiang Xin <redacted>

## Introduce two improvements for git-bundle

+ Commit "bundle: lost objects when removing duplicate pendings",
  which fixes command like:

        $ git bundle create <file> 'master^!'
  
+ Commits "bundle: arguments can be read from stdin",
  which add "--stdin" option support for git-bundle, like:

        $ git bundle create <file> <input

## Changes of v3

1. Forgot to add shebang in file "t/t6020-bundle-misc.sh", which breaks
   build and test on Windows.

2. Add more testcases in t6020.

## Range diff of v2...v3

1:  ba13820340 ! 1:  9df48434f3 bundle: lost objects when removing duplicate pendings
    @@ object.c: void object_array_remove_duplicates(struct object_array *array)
     
      ## t/t6020-bundle-misc.sh (new) ##
     @@
    ++#!/bin/sh
    ++#
    ++# Copyright (c) 2021 Jiang Xin
    ++#
    ++
     +test_description='Test git-bundle'
     +
     +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
    @@ t/t6020-bundle-misc.sh (new)
     +. ./test-lib.sh
     +
     +test_bundle_object_count () {
    -+	git verify-pack -v "$1" >verify.out &&
    ++	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 is $count, not $2
    ++	echo object count for $bundle is $count, not $2
     +	return 1
     +}
     +
    @@ t/t6020-bundle-misc.sh (new)
     +
     +#            (C)   (D, pull/1/head, topic/1)
     +#             o --- o
    -+#            /       \                              (L, tags/v1)
    -+#           /         \        o (H, topic/2)             (M, tags/v2)
    -+#          /    (F)    \      /                                 (N, tags/v3)
    ++#            /       \                              (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)        (I)  (J)                  (K)       (O)   (P)
    ++# (A)   (B)  (E, tag:v1) (I)  (J)                  (K)       (O)   (P)
     +#
     +test_expect_success 'setup' '
     +	# commit A & B
    @@ t/t6020-bundle-misc.sh (new)
     +	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 &&
    @@ t/t6020-bundle-misc.sh (new)
     +	git add release.txt &&
     +	test_tick &&
     +	git commit -m "Commit L" &&
    -+	test_tick &&
    -+	git tag -m "v1" v1 HEAD &&
     +
     +	cat >release.txt <<-EOF &&
     +		Commit M
    @@ t/t6020-bundle-misc.sh (new)
     +	L=$(git rev-parse HEAD~2) &&
     +	M=$(git rev-parse HEAD~) &&
     +	N=$(git rev-parse HEAD) &&
    -+	TAG1=$(git rev-parse refs/tags/v1) &&
     +	TAG2=$(git rev-parse refs/tags/v2) &&
     +	TAG3=$(git rev-parse refs/tags/v3) &&
     +
    @@ t/t6020-bundle-misc.sh (new)
     +		EOF
     +	test_i18ncmp expect actual &&
     +
    -+	convert_bundle_to_pack <special-rev.bdl >special-rev.pack &&
    -+	git index-pack special-rev.pack &&
    -+	test_bundle_object_count special-rev.pack 3
    ++	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' '
    -+	# create bundle from args
     +	git bundle create 1.bdl topic/1 topic/2 &&
     +
     +	cat >expect <<-EOF &&
    @@ t/t6020-bundle-misc.sh (new)
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    -+	convert_bundle_to_pack <1.bdl >1.pack &&
    -+	git index-pack 1.pack &&
    -+	test_bundle_object_count 1.pack 24
    ++	test_bundle_object_count 1.bdl 24
     +'
     +
     +test_expect_success 'create bundle 2 - has prerequisites' '
    -+	# create bundle from args
     +	git bundle create 2.bdl \
     +		--ignore-missing \
     +		^topic/deleted \
    @@ t/t6020-bundle-misc.sh (new)
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    -+	convert_bundle_to_pack <2.bdl >2.pack &&
    -+	git index-pack 2.pack &&
    -+	test_bundle_object_count 2.pack 16
    ++	test_bundle_object_count 2.bdl 16
     +'
     +
     +test_expect_success 'fail to verify bundle without prerequisites' '
    @@ t/t6020-bundle-misc.sh (new)
     +'
     +
     +test_expect_success 'create bundle 3 - two refs, same object' '
    -+	# create bundle from args
     +	git bundle create --version=3 3.bdl \
     +		^release \
     +		^topic/1 \
    @@ t/t6020-bundle-misc.sh (new)
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    -+	convert_bundle_to_pack <3.bdl >3.pack &&
    -+	git index-pack 3.pack &&
    -+	test_bundle_object_count 3.pack 4
    ++	test_bundle_object_count 3.bdl 4
     +'
     +
     +test_expect_success 'create bundle 4 - with tags' '
    -+	# create bundle from args
     +	git bundle create 4.bdl \
     +		^main \
     +		^release \
    @@ t/t6020-bundle-misc.sh (new)
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    -+	convert_bundle_to_pack <4.bdl >4.pack &&
    -+	git index-pack 4.pack &&
    -+	test_bundle_object_count 4.pack 3
    ++	test_bundle_object_count 4.bdl 3
     +'
     +
     +test_expect_success 'clone from bundle' '
2:  a4662f44a8 ! 2:  86ad41e4d4 bundle: arguments can be read from stdin
    @@ Commit message
         `git-bundle`.  Later nothing can be read from stdin when running
         `setup_revisions()` in `create_bundle()`.
     
    -    Remove the entire `compute_and_write_prerequisites()` function, and
    -    parse the args once by `setup_revisions()`.  The first step for creating
    -    a bundle is to write prerequisites ("-" obj-id SP comment LF), but after
    -    calling `prepare_revision_walk()`, the `revs.pending` is left empty.
    -    Following steps could not work properly without data in `revs.pending`.
    -    Therefore, before calling `prepare_revision_walk()` function, make a
    -    copy `revs_copy` from `revs` for later use.  Even though `revs_copy` and
    -    `revs` share the same objects, but changes on these objects will not
    -    change the behavior of function `write_bundle_refs()` and
    -    `write_pack_data()`.
    +    The solution is to parse args once by removing the entire function
    +    `compute_and_write_prerequisites()` and then calling function
    +    `setup_revisions()`.  In order to write prerequisites for bundle, will
    +    call `prepare_revision_walk()` and `traverse_commit_list()`.  But after
    +    calling `prepare_revision_walk()`, the object array `revs.pending` is
    +    left empty, and the following steps could not work properly with the
    +    empty object array (`revs.pending`).  Therefore, make a copy of `revs`
    +    to `revs_copy` for later use right after calling `setup_revisions()`.
    +
    +    The copy of `revs_copy` is not a deep copy, it shares the same objects
    +    with `revs`. The object array of `revs` has been cleared, but objects
    +    themselves are still kept.  Flags of objects may change after calling
    +    `prepare_revision_walk()`, we can use these changed flags without
    +    calling the `git rev-list` command and parsing its output like the
    +    former implementation.
     
         Also add testcases for git bundle in t6020, which read args from stdin.
     
    @@ bundle.c: static int write_pack_data(int bundle_fd, struct rev_info *revs, struc
      /*
       * Write out bundle refs based on the tips already
       * parsed into revs.pending. As a side effect, may
    +@@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
    + 		 * constraints.
    + 		 */
    + 		if (!(e->item->flags & SHOWN) && e->item->type == OBJ_COMMIT) {
    +-			warning(_("ref '%s' is excluded by the rev-list options"),
    ++			warning(_("ref '%s' is excluded by the limiting options"),
    + 				e->name);
    + 			goto skip_write_ref;
    + 		}
     @@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
      	return ref_count;
      }
    @@ t/t5607-clone-bundle.sh: test_expect_success 'die if bundle file cannot be creat
      	grep master output
     
      ## t/t6020-bundle-misc.sh ##
    -@@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 1 - no prerequisites' '
    - 	# create bundle from args
    +@@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle with --since option' '
    + '
    + 
    + test_expect_success 'create bundle 1 - no prerequisites' '
    ++	# create bundle from args
      	git bundle create 1.bdl topic/1 topic/2 &&
      
     +	# create bundle from stdin
    @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 1 - no prerequisites'
      		make_user_friendly_and_stable_output >actual &&
      	test_i18ncmp expect actual &&
      
    +-	test_bundle_object_count 1.bdl 24
     +	git bundle verify stdin-1.bdl |
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    - 	convert_bundle_to_pack <1.bdl >1.pack &&
    - 	git index-pack 1.pack &&
    --	test_bundle_object_count 1.pack 24
    -+	test_bundle_object_count 1.pack 24 &&
    -+
    -+	convert_bundle_to_pack <stdin-1.bdl >stdin-1.pack &&
    -+	git index-pack stdin-1.pack &&
    -+	test_bundle_object_count stdin-1.pack 24
    ++	test_bundle_object_count       1.bdl 24 &&
    ++	test_bundle_object_count stdin-1.bdl 24
      '
      
      test_expect_success 'create bundle 2 - has prerequisites' '
    ++	# create bundle from args
    + 	git bundle create 2.bdl \
    + 		--ignore-missing \
    + 		^topic/deleted \
     @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 2 - has prerequisites' '
      		^topic/2 \
      		release &&
    @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 2 - has prerequisites
      		make_user_friendly_and_stable_output >actual &&
      	test_i18ncmp expect actual &&
      
    +-	test_bundle_object_count 2.bdl 16
     +	git bundle verify stdin-2.bdl |
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    - 	convert_bundle_to_pack <2.bdl >2.pack &&
    - 	git index-pack 2.pack &&
    --	test_bundle_object_count 2.pack 16
    -+	test_bundle_object_count 2.pack 16 &&
    -+
    -+	convert_bundle_to_pack <stdin-2.bdl >stdin-2.pack &&
    -+	git index-pack stdin-2.pack &&
    -+	test_bundle_object_count stdin-2.pack 16
    ++	test_bundle_object_count       2.bdl 16 &&
    ++	test_bundle_object_count stdin-2.bdl 16
      '
      
      test_expect_success 'fail to verify bundle without prerequisites' '
    @@ t/t6020-bundle-misc.sh: test_expect_success 'fail to verify bundle without prere
      	test_i18ncmp expect actual
      '
      
    + test_expect_success 'create bundle 3 - two refs, same object' '
    ++	# create bundle from args
    + 	git bundle create --version=3 3.bdl \
    + 		^release \
    + 		^topic/1 \
     @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 3 - two refs, same object' '
      		main \
      		HEAD &&
    @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 3 - two refs, same ob
      		make_user_friendly_and_stable_output >actual &&
      	test_i18ncmp expect actual &&
      
    +-	test_bundle_object_count 3.bdl 4
     +	git bundle verify stdin-3.bdl |
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    - 	convert_bundle_to_pack <3.bdl >3.pack &&
    - 	git index-pack 3.pack &&
    --	test_bundle_object_count 3.pack 4
    -+	test_bundle_object_count 3.pack 4 &&
    -+
    -+	convert_bundle_to_pack <stdin-3.bdl >stdin-3.pack &&
    -+	git index-pack stdin-3.pack &&
    -+	test_bundle_object_count stdin-3.pack 4
    ++	test_bundle_object_count       3.bdl 4 &&
    ++	test_bundle_object_count stdin-3.bdl 4
      '
      
      test_expect_success 'create bundle 4 - with tags' '
    ++	# create bundle from args
    + 	git bundle create 4.bdl \
    + 		^main \
    + 		^release \
     @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 4 - with tags' '
      		^topic/2 \
      		--all &&
    @@ t/t6020-bundle-misc.sh: test_expect_success 'create bundle 4 - with tags' '
      		make_user_friendly_and_stable_output >actual &&
      	test_i18ncmp expect actual &&
      
    +-	test_bundle_object_count 4.bdl 3
     +	git bundle verify stdin-4.bdl |
     +		make_user_friendly_and_stable_output >actual &&
     +	test_i18ncmp expect actual &&
     +
    - 	convert_bundle_to_pack <4.bdl >4.pack &&
    - 	git index-pack 4.pack &&
    --	test_bundle_object_count 4.pack 3
    -+	test_bundle_object_count 4.pack 3 &&
    -+
    -+	convert_bundle_to_pack <stdin-4.bdl >stdin-4.pack &&
    -+	git index-pack stdin-4.pack &&
    -+	test_bundle_object_count stdin-4.pack 3
    ++	test_bundle_object_count       4.bdl 3 &&
    ++	test_bundle_object_count stdin-4.bdl 3
      '
      
      test_expect_success 'clone from bundle' '

--

Jiang Xin (2):
  bundle: lost objects when removing duplicate pendings
  bundle: arguments can be read from stdin

 bundle.c                | 111 ++++----
 object.c                |  10 +-
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  | 557 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 625 insertions(+), 57 deletions(-)
 create mode 100755 t/t6020-bundle-misc.sh

-- 
2.30.0.2.g06d2f50715

Re: [PATCH v3 1/2] bundle: lost objects when removing duplicate pendings

From: Đoàn Trần Công Danh <hidden>
Date: 2021-01-07 15:38:02 

On 2021-01-07 08:50:24-0500, Jiang Xin [off-list ref] wrote:
quoted hunk
From: Jiang Xin <redacted>

`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 <redacted>
---
 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`.
quoted hunk
 			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
-- 
Danh

Re: [PATCH v3 1/2] bundle: lost objects when removing duplicate pendings

From: Jiang Xin <hidden>
Date: 2021-01-08 13:15:31 

Đoàn Trần Công Danh [off-list ref] 于2021年1月7日周四 下午11:37写道：
quoted
-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.

quoted
+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.

quoted
+
+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?
quoted
+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





  

    
[PATCH v4 0/2] Improvements for git-bundle

    

      From: Jiang Xin <hidden>

      Date: 2021-01-08 14:46:01 
    

  

  
From: Jiang Xin <redacted>

## Two improvements for git-bundle

1. Commit "bundle: lost objects when removing duplicate pendings",
   which fixes command like:

        $ git bundle create <file> 'master^!'
  
2. Commits "bundle: arguments can be read from stdin",
   which add "--stdin" option support for git-bundle, like:

        $ git bundle create <file> <input


## Changes of v4

+ Refactor t6020.


## Range diff of v3...v4

1:  9df48434f3 ! 1:  0abad6486a bundle: lost objects when removing duplicate pendings
    @@ t/t6020-bundle-misc.sh (new)
     +
     +. ./test-lib.sh
     +
    ++# Check count of objects in a bundle file.
    ++# We can use "--thin" opiton to check thin pack, which must be fixed by
    ++# command `git-index-pack --fix-thin --stdin`.
     +test_bundle_object_count () {
    ++	thin= &&
    ++	if test "$1" = "--thin"
    ++	then
    ++		thin=yes
    ++		shift
    ++	fi &&
    ++	if test $# -ne 2
    ++	then
    ++		echo >&2 "args should be: <bundle> <count>"
    ++		return 1
    ++	fi
     +	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) &&
    ++	if test -n "$thin"
    ++	then
    ++		test_must_fail git index-pack "$pack" &&
    ++		mv "$pack" "$pack"-thin &&
    ++		cat "$pack"-thin |
    ++			git index-pack --stdin --fix-thin "$pack"
    ++	else
    ++		git index-pack "$pack"
    ++	fi &&
    ++	git verify-pack -v "$pack" >verify.out
    ++	if test $? -ne 0
    ++	then
    ++		echo >&2 "error: fail to convert $bundle to $pack"
    ++		return 1
    ++	fi
    ++	count=$(grep -c "^$OID_REGEX " verify.out) &&
     +	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
    ++	echo >&2 "error: object count for $bundle is $count, not $2"
     +	return 1
     +}
     +
    ++# Display the pack data contained in the bundle file, bypassing the
    ++# header that contains the signature, prerequisites and references.
     +convert_bundle_to_pack () {
     +	while read x && test -n "$x"
     +	do
    @@ t/t6020-bundle-misc.sh (new)
     +	cat
     +}
     +
    ++# Create a commit or tag and set the variable with the object ID.
    ++test_commit_setvar () {
    ++	notick= &&
    ++	signoff= &&
    ++	indir= &&
    ++	merge= &&
    ++	tag= &&
    ++	var= &&
    ++	while test $# != 0
    ++	do
    ++		case "$1" in
    ++		--merge)
    ++			merge=yes
    ++			;;
    ++		--tag)
    ++			tag=yes
    ++			;;
    ++		--notick)
    ++			notick=yes
    ++			;;
    ++		--signoff)
    ++			signoff="$1"
    ++			;;
    ++		-C)
    ++			indir="$2"
    ++			shift
    ++			;;
    ++		-*)
    ++			echo >&2 "error: unknown option $1"
    ++			return 1
    ++			;;
    ++		*)
    ++			test -n "$var" && break
    ++			var=$1
    ++			;;
    ++		esac
    ++		shift
    ++	done &&
    ++	indir=${indir:+"$indir"/} &&
    ++	if test $# -eq 0
    ++	then
    ++		echo >&2 "no args provided"
    ++		return 1
    ++	fi &&
    ++	if test -z "$notick"
    ++	then
    ++		test_tick
    ++	fi &&
    ++	if test -n "$merge"
    ++	then
    ++		git ${indir:+ -C "$indir"} merge --no-edit --no-ff \
    ++			${2:+-m "$2"} "$1" &&
    ++		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)
    ++	elif test -n "$tag"
    ++	then
    ++		git ${indir:+ -C "$indir"} tag -m "$1" "$1" &&
    ++		oid=$(git ${indir:+ -C "$indir"} rev-parse "$1")
    ++	else
    ++		file=${2:-"$1.t"} &&
    ++		echo "${3-$1}" > "$indir$file" &&
    ++		git ${indir:+ -C "$indir"} add "$file" &&
    ++		git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
    ++		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)
    ++	fi &&
    ++	eval $var=$oid
    ++}
    ++
    ++
     +# 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/$A/<COMMIT-A>/" \
    ++		-e "s/$B/<COMMIT-B>/" \
    ++		-e "s/$C/<COMMIT-C>/" \
    ++		-e "s/$D/<COMMIT-D>/" \
    ++		-e "s/$E/<COMMIT-E>/" \
    ++		-e "s/$F/<COMMIT-F>/" \
    ++		-e "s/$G/<COMMIT-G>/" \
    ++		-e "s/$H/<COMMIT-H>/" \
    ++		-e "s/$I/<COMMIT-I>/" \
    ++		-e "s/$J/<COMMIT-J>/" \
    ++		-e "s/$K/<COMMIT-K>/" \
    ++		-e "s/$L/<COMMIT-L>/" \
    ++		-e "s/$M/<COMMIT-M>/" \
    ++		-e "s/$N/<COMMIT-N>/" \
    ++		-e "s/$O/<COMMIT-O>/" \
    ++		-e "s/$P/<COMMIT-P>/" \
    ++		-e "s/$TAG1/<TAG-1>/" \
    ++		-e "s/$TAG2/<TAG-2>/" \
    ++		-e "s/$TAG3/<TAG-3>/" \
     +		-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" \
    @@ t/t6020-bundle-misc.sh (new)
     +		-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"
    ++		-e "s/ *\$//"
     +}
     +
     +#            (C)   (D, pull/1/head, topic/1)
    @@ t/t6020-bundle-misc.sh (new)
     +# (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" &&
    ++	# Try to make a stable fixed width for abbreviated commit ID,
    ++	# this fixed-width oid will be replaced with "<OID>".
    ++	git config core.abbrev 7 &&
     +
    -+	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 main: commit A & B
    ++	test_commit_setvar A "Commit A" main.txt &&
    ++	test_commit_setvar B "Commit B" main.txt &&
     +
    -+	# branch topic/1
    ++	# branch topic/1: commit C & D, refs/pull/1/head
     +	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" &&
    ++	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 &&
    -+	C=$(git rev-parse topic/1~) &&
    -+	D=$(git rev-parse topic/1) &&
     +
    -+	# commit E
    ++	# branch topic/1: commit E, tag v1
     +	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" &&
    ++	test_commit_setvar E "Commit E" main.txt &&
    ++	test_commit_setvar TAG1 --tag v1 &&
     +
    -+	cat >topic-2.txt <<-EOF &&
    -+		Commit G
    -+		EOF
    -+	git add -u &&
    -+	test_tick &&
    -+	git commit -m "Commit G" &&
    ++	# 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 &&
     +
    -+	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
    ++	# branch main: 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
    ++	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 &&
    -+	cat >main.txt <<-EOF &&
    -+		Commit K
    -+		EOF
    -+	git add main.txt &&
    -+	test_tick &&
    -+	git commit -m "Commit K" &&
    -+	K=$(git rev-parse HEAD) &&
    ++	test_commit_setvar K "Commit K" main.txt &&
     +
    -+	# branch release
    ++	# branch release:
     +	git checkout -b release &&
    -+	cat >release.txt <<-EOF &&
    -+		Commit L
    -+		EOF
    -+	git add release.txt &&
    -+	test_tick &&
    -+	git commit -m "Commit L" &&
    ++	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 &&
     +
    -+	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
    ++	# branch main: merge commit O, 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_commit_setvar O --merge tags/v2 "Merge commit O" &&
    ++	test_commit_setvar P "Commit P" main.txt
     +'
     +
     +test_expect_success 'create bundle from special rev: main^!' '
    @@ t/t6020-bundle-misc.sh (new)
     +'
     +
     +test_expect_success 'create bundle with --since option' '
    ++	since="Thu Apr 7 15:26:13 2005 -0700" &&
    ++	git log -1 --pretty="%ad" $M >actual &&
    ++	echo "$since" >expect &&
    ++	test_cmp expect actual &&
    ++
     +	git bundle create since.bdl \
    -+		--since "Thu Apr 7 15:26:13 2005 -0700" \
    -+		--all &&
    ++		--since "$since" --all &&
     +
     +	git bundle list-heads since.bdl |
     +		make_user_friendly_and_stable_output >actual &&
    @@ t/t6020-bundle-misc.sh (new)
     +		EOF
     +	test_i18ncmp expect actual &&
     +
    -+	test_thin_bundle_object_count since.bdl 16
    ++	test_bundle_object_count --thin since.bdl 16
     +'
     +
     +test_expect_success 'create bundle 1 - no prerequisites' '
2:  86ad41e4d4 = 2:  48ef4aa44e bundle: arguments can be read from stdin

--
Jiang Xin (2):
  bundle: lost objects when removing duplicate pendings
  bundle: arguments can be read from stdin

 bundle.c                | 111 ++++----
 object.c                |  10 +-
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  | 546 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 614 insertions(+), 57 deletions(-)
 create mode 100755 t/t6020-bundle-misc.sh

-- 
2.30.0.2.g06d2f50715






  

    
[PATCH v4 2/2] bundle: arguments can be read from stdin

    

      From: Jiang Xin <hidden>

      Date: 2021-01-08 14:46:18 
    

  

  
From: Jiang Xin <redacted>

In order to create an incremental bundle, we need to pass many arguments
to let git-bundle ignore some already packed commits.  It will be more
convenient to pass args via stdin.  But the current implementation does
not allow us to do this.

This is because args are parsed twice when creating bundle.  The first
time for parsing args is in `compute_and_write_prerequisites()` by
running `git-rev-list` command to write prerequisites in bundle file,
and stdin is consumed in this step if "--stdin" option is provided for
`git-bundle`.  Later nothing can be read from stdin when running
`setup_revisions()` in `create_bundle()`.

The solution is to parse args once by removing the entire function
`compute_and_write_prerequisites()` and then calling function
`setup_revisions()`.  In order to write prerequisites for bundle, will
call `prepare_revision_walk()` and `traverse_commit_list()`.  But after
calling `prepare_revision_walk()`, the object array `revs.pending` is
left empty, and the following steps could not work properly with the
empty object array (`revs.pending`).  Therefore, make a copy of `revs`
to `revs_copy` for later use right after calling `setup_revisions()`.

The copy of `revs_copy` is not a deep copy, it shares the same objects
with `revs`. The object array of `revs` has been cleared, but objects
themselves are still kept.  Flags of objects may change after calling
`prepare_revision_walk()`, we can use these changed flags without
calling the `git rev-list` command and parsing its output like the
former implementation.

Also add testcases for git bundle in t6020, which read args from stdin.

Signed-off-by: Jiang Xin <redacted>
---
 bundle.c                | 111 ++++++++++++++++++++++------------------
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  |  77 ++++++++++++++++++++++++++--
 3 files changed, 135 insertions(+), 57 deletions(-)

diff --git a/bundle.c b/bundle.c
index cb0e5931ac..72970d962a 100644
--- a/bundle.c
+++ b/bundle.c
@@ -338,48 +338,6 @@ static int write_pack_data(int bundle_fd, struct rev_info *revs, struct strvec *
 	return 0;
 }
 
-static int compute_and_write_prerequisites(int bundle_fd,
-					   struct rev_info *revs,
-					   int argc, const char **argv)
-{
-	struct child_process rls = CHILD_PROCESS_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	FILE *rls_fout;
-	int i;
-
-	strvec_pushl(&rls.args,
-		     "rev-list", "--boundary", "--pretty=oneline",
-		     NULL);
-	for (i = 1; i < argc; i++)
-		strvec_push(&rls.args, argv[i]);
-	rls.out = -1;
-	rls.git_cmd = 1;
-	if (start_command(&rls))
-		return -1;
-	rls_fout = xfdopen(rls.out, "r");
-	while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
-		struct object_id oid;
-		if (buf.len > 0 && buf.buf[0] == '-') {
-			write_or_die(bundle_fd, buf.buf, buf.len);
-			if (!get_oid_hex(buf.buf + 1, &oid)) {
-				struct object *object = parse_object_or_die(&oid,
-									    buf.buf);
-				object->flags |= UNINTERESTING;
-				add_pending_object(revs, object, buf.buf);
-			}
-		} else if (!get_oid_hex(buf.buf, &oid)) {
-			struct object *object = parse_object_or_die(&oid,
-								    buf.buf);
-			object->flags |= SHOWN;
-		}
-	}
-	strbuf_release(&buf);
-	fclose(rls_fout);
-	if (finish_command(&rls))
-		return error(_("rev-list died"));
-	return 0;
-}
-
 /*
  * Write out bundle refs based on the tips already
  * parsed into revs.pending. As a side effect, may
@@ -425,7 +383,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		 * constraints.
 		 */
 		if (!(e->item->flags & SHOWN) && e->item->type == OBJ_COMMIT) {
-			warning(_("ref '%s' is excluded by the rev-list options"),
+			warning(_("ref '%s' is excluded by the limiting options"),
 				e->name);
 			goto skip_write_ref;
 		}
@@ -474,6 +432,38 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 	return ref_count;
 }
 
+struct bundle_prerequisites_info {
+	struct object_array *pending;
+	int fd;
+};
+
+static void write_bundle_prerequisites(struct commit *commit, void *data)
+{
+	struct bundle_prerequisites_info *bpi = data;
+	struct object *object;
+	struct pretty_print_context ctx = { 0 };
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!(commit->object.flags & BOUNDARY))
+		return;
+	strbuf_addf(&buf, "-%s ", oid_to_hex(&commit->object.oid));
+	write_or_die(bpi->fd, buf.buf, buf.len);
+
+	ctx.fmt = CMIT_FMT_ONELINE;
+	ctx.output_encoding = get_log_output_encoding();
+	strbuf_reset(&buf);
+	pretty_print_commit(&ctx, commit, &buf);
+	strbuf_trim(&buf);
+
+	object = (struct object *)commit;
+	object->flags |= UNINTERESTING;
+	add_object_array_with_path(object, buf.buf, bpi->pending, S_IFINVALID,
+				   NULL);
+	strbuf_addch(&buf, '\n');
+	write_or_die(bpi->fd, buf.buf, buf.len);
+	strbuf_release(&buf);
+}
+
 int create_bundle(struct repository *r, const char *path,
 		  int argc, const char **argv, struct strvec *pack_options, int version)
 {
@@ -481,8 +471,10 @@ int create_bundle(struct repository *r, const char *path,
 	int bundle_fd = -1;
 	int bundle_to_stdout;
 	int ref_count = 0;
-	struct rev_info revs;
+	struct rev_info revs, revs_copy;
 	int min_version = the_hash_algo == &hash_algos[GIT_HASH_SHA1] ? 2 : 3;
+	struct bundle_prerequisites_info bpi;
+	int i;
 
 	bundle_to_stdout = !strcmp(path, "-");
 	if (bundle_to_stdout)
@@ -512,10 +504,6 @@ int create_bundle(struct repository *r, const char *path,
 	save_commit_buffer = 0;
 	repo_init_revisions(r, &revs, NULL);
 
-	/* write prerequisites */
-	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
-		goto err;
-
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	if (argc > 1) {
@@ -523,16 +511,37 @@ int create_bundle(struct repository *r, const char *path,
 		goto err;
 	}
 
-	object_array_remove_duplicates(&revs.pending);
+	/* save revs.pending in revs_copy for later use */
+	memcpy(&revs_copy, &revs, sizeof(revs));
+	revs_copy.pending.nr = 0;
+	revs_copy.pending.alloc = 0;
+	revs_copy.pending.objects = NULL;
+	for (i = 0; i < revs.pending.nr; i++) {
+		struct object_array_entry *e = revs.pending.objects + i;
+		if (e)
+			add_object_array_with_path(e->item, e->name,
+						   &revs_copy.pending,
+						   e->mode, e->path);
+	}
 
-	ref_count = write_bundle_refs(bundle_fd, &revs);
+	/* write prerequisites */
+	revs.boundary = 1;
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
+	bpi.fd = bundle_fd;
+	bpi.pending = &revs_copy.pending;
+	traverse_commit_list(&revs, write_bundle_prerequisites, NULL, &bpi);
+	object_array_remove_duplicates(&revs_copy.pending);
+
+	/* write bundle refs */
+	ref_count = write_bundle_refs(bundle_fd, &revs_copy);
 	if (!ref_count)
 		die(_("Refusing to create empty bundle."));
 	else if (ref_count < 0)
 		goto err;
 
 	/* write pack */
-	if (write_pack_data(bundle_fd, &revs, pack_options))
+	if (write_pack_data(bundle_fd, &revs_copy, pack_options))
 		goto err;
 
 	if (!bundle_to_stdout) {
diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index 26985f4b44..425258767d 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -38,13 +38,13 @@ test_expect_success 'die if bundle file cannot be created' '
 	test_must_fail git bundle create adir --all
 '
 
-test_expect_failure 'bundle --stdin' '
+test_expect_success 'bundle --stdin' '
 	echo master | git bundle create stdin-bundle.bdl --stdin &&
 	git ls-remote stdin-bundle.bdl >output &&
 	grep master output
 '
 
-test_expect_failure 'bundle --stdin <rev-list options>' '
+test_expect_success 'bundle --stdin <rev-list options>' '
 	echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
 	git ls-remote hybrid-bundle.bdl >output &&
 	grep master output
diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index c4447ca88f..ace675a68b 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -325,8 +325,16 @@ test_expect_success 'create bundle with --since option' '
 '
 
 test_expect_success 'create bundle 1 - no prerequisites' '
+	# create bundle from args
 	git bundle create 1.bdl topic/1 topic/2 &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		topic/1
+		topic/2
+		EOF
+	git bundle create stdin-1.bdl --stdin <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-D> refs/heads/topic/1
@@ -339,10 +347,16 @@ test_expect_success 'create bundle 1 - no prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 1.bdl 24
+	git bundle verify stdin-1.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       1.bdl 24 &&
+	test_bundle_object_count stdin-1.bdl 24
 '
 
 test_expect_success 'create bundle 2 - has prerequisites' '
+	# create bundle from args
 	git bundle create 2.bdl \
 		--ignore-missing \
 		^topic/deleted \
@@ -350,6 +364,18 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		^topic/2 \
 		release &&
 
+	# create bundle from stdin
+	# input has a non-exist reference: "topic/deleted"
+	cat >input <<-EOF &&
+		^topic/deleted
+		^$D
+		^topic/2
+		EOF
+	git bundle create stdin-2.bdl \
+		--ignore-missing \
+		--stdin \
+		release <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains this ref:
 		<COMMIT-N> refs/heads/release
@@ -363,7 +389,12 @@ test_expect_success 'create bundle 2 - has prerequisites' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 2.bdl 16
+	git bundle verify stdin-2.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       2.bdl 16 &&
+	test_bundle_object_count stdin-2.bdl 16
 '
 
 test_expect_success 'fail to verify bundle without prerequisites' '
@@ -378,10 +409,15 @@ test_expect_success 'fail to verify bundle without prerequisites' '
 
 	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_must_fail git -C test1.git bundle verify ../stdin-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' '
+	# create bundle from args
 	git bundle create --version=3 3.bdl \
 		^release \
 		^topic/1 \
@@ -389,6 +425,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		main \
 		HEAD &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create --version=3 stdin-3.bdl \
+		--stdin \
+		main HEAD <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 2 refs:
 		<COMMIT-P> refs/heads/main
@@ -402,10 +448,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 3.bdl 4
+	git bundle verify stdin-3.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       3.bdl 4 &&
+	test_bundle_object_count stdin-3.bdl 4
 '
 
 test_expect_success 'create bundle 4 - with tags' '
+	# create bundle from args
 	git bundle create 4.bdl \
 		^main \
 		^release \
@@ -413,6 +465,18 @@ test_expect_success 'create bundle 4 - with tags' '
 		^topic/2 \
 		--all &&
 
+	# create bundle from stdin
+	cat >input <<-EOF &&
+		^main
+		^release
+		^topic/1
+		^topic/2
+		EOF
+	git bundle create stdin-4.bdl \
+		--ignore-missing \
+		--stdin \
+		--all <input &&
+
 	cat >expect <<-EOF &&
 		The bundle contains these 3 refs:
 		<TAG-1> refs/tags/v1
@@ -425,7 +489,12 @@ test_expect_success 'create bundle 4 - with tags' '
 		make_user_friendly_and_stable_output >actual &&
 	test_i18ncmp expect actual &&
 
-	test_bundle_object_count 4.bdl 3
+	git bundle verify stdin-4.bdl |
+		make_user_friendly_and_stable_output >actual &&
+	test_i18ncmp expect actual &&
+
+	test_bundle_object_count       4.bdl 3 &&
+	test_bundle_object_count stdin-4.bdl 3
 '
 
 test_expect_success 'clone from bundle' '
-- 
2.30.0.2.g06d2f50715






  

    
[PATCH v4 1/2] bundle: lost objects when removing duplicate pendings

    

      From: Jiang Xin <hidden>

      Date: 2021-01-08 14:46:18 
    

  

  
From: Jiang Xin <redacted>

`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 <redacted>
---
 object.c               |  10 +-
 t/t6020-bundle-misc.sh | 477 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 483 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))
 			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..c4447ca88f
--- /dev/null
+++ b/t/t6020-bundle-misc.sh
@@ -0,0 +1,477 @@
+#!/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
+
+# Check count of objects in a bundle file.
+# We can use "--thin" opiton to check thin pack, which must be fixed by
+# command `git-index-pack --fix-thin --stdin`.
+test_bundle_object_count () {
+	thin= &&
+	if test "$1" = "--thin"
+	then
+		thin=yes
+		shift
+	fi &&
+	if test $# -ne 2
+	then
+		echo >&2 "args should be: <bundle> <count>"
+		return 1
+	fi
+	bundle=$1 &&
+	pack=${bundle%.bdl}.pack &&
+	convert_bundle_to_pack <"$bundle" >"$pack" &&
+	if test -n "$thin"
+	then
+		test_must_fail git index-pack "$pack" &&
+		mv "$pack" "$pack"-thin &&
+		cat "$pack"-thin |
+			git index-pack --stdin --fix-thin "$pack"
+	else
+		git index-pack "$pack"
+	fi &&
+	git verify-pack -v "$pack" >verify.out
+	if test $? -ne 0
+	then
+		echo >&2 "error: fail to convert $bundle to $pack"
+		return 1
+	fi
+	count=$(grep -c "^$OID_REGEX " verify.out) &&
+	test $2 = $count && return 0
+	echo >&2 "error: object count for $bundle is $count, not $2"
+	return 1
+}
+
+# Display the pack data contained in the bundle file, bypassing the
+# header that contains the signature, prerequisites and references.
+convert_bundle_to_pack () {
+	while read x && test -n "$x"
+	do
+		:;
+	done
+	cat
+}
+
+# Create a commit or tag and set the variable with the object ID.
+test_commit_setvar () {
+	notick= &&
+	signoff= &&
+	indir= &&
+	merge= &&
+	tag= &&
+	var= &&
+	while test $# != 0
+	do
+		case "$1" in
+		--merge)
+			merge=yes
+			;;
+		--tag)
+			tag=yes
+			;;
+		--notick)
+			notick=yes
+			;;
+		--signoff)
+			signoff="$1"
+			;;
+		-C)
+			indir="$2"
+			shift
+			;;
+		-*)
+			echo >&2 "error: unknown option $1"
+			return 1
+			;;
+		*)
+			test -n "$var" && break
+			var=$1
+			;;
+		esac
+		shift
+	done &&
+	indir=${indir:+"$indir"/} &&
+	if test $# -eq 0
+	then
+		echo >&2 "no args provided"
+		return 1
+	fi &&
+	if test -z "$notick"
+	then
+		test_tick
+	fi &&
+	if test -n "$merge"
+	then
+		git ${indir:+ -C "$indir"} merge --no-edit --no-ff \
+			${2:+-m "$2"} "$1" &&
+		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)
+	elif test -n "$tag"
+	then
+		git ${indir:+ -C "$indir"} tag -m "$1" "$1" &&
+		oid=$(git ${indir:+ -C "$indir"} rev-parse "$1")
+	else
+		file=${2:-"$1.t"} &&
+		echo "${3-$1}" > "$indir$file" &&
+		git ${indir:+ -C "$indir"} add "$file" &&
+		git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)
+	fi &&
+	eval $var=$oid
+}
+
+
+# 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/$A/<COMMIT-A>/" \
+		-e "s/$B/<COMMIT-B>/" \
+		-e "s/$C/<COMMIT-C>/" \
+		-e "s/$D/<COMMIT-D>/" \
+		-e "s/$E/<COMMIT-E>/" \
+		-e "s/$F/<COMMIT-F>/" \
+		-e "s/$G/<COMMIT-G>/" \
+		-e "s/$H/<COMMIT-H>/" \
+		-e "s/$I/<COMMIT-I>/" \
+		-e "s/$J/<COMMIT-J>/" \
+		-e "s/$K/<COMMIT-K>/" \
+		-e "s/$L/<COMMIT-L>/" \
+		-e "s/$M/<COMMIT-M>/" \
+		-e "s/$N/<COMMIT-N>/" \
+		-e "s/$O/<COMMIT-O>/" \
+		-e "s/$P/<COMMIT-P>/" \
+		-e "s/$TAG1/<TAG-1>/" \
+		-e "s/$TAG2/<TAG-2>/" \
+		-e "s/$TAG3/<TAG-3>/" \
+		-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/$(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/ *\$//"
+}
+
+#            (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' '
+	# Try to make a stable fixed width for abbreviated commit ID,
+	# this fixed-width oid will be replaced with "<OID>".
+	git config core.abbrev 7 &&
+
+	# 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_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
+'
+
+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' '
+	since="Thu Apr 7 15:26:13 2005 -0700" &&
+	git log -1 --pretty="%ad" $M >actual &&
+	echo "$since" >expect &&
+	test_cmp expect actual &&
+
+	git bundle create since.bdl \
+		--since "$since" --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_bundle_object_count --thin 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





    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help