git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v6 3/3] bundle: arguments can be read from stdin

From
Jiang Xin <worldhello.net@gmail.com>
Date
Jan 12, 2021, 02:27 UTC
Message-ID
<20210112022703.1884-4-worldhello.net@gmail.com>
In-Reply-To
<xmqq1rer8cbz.fsf@gitster.c.googlers.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

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 <zhiyou.jx@alibaba-inc.com>
---
 bundle.c                | 109 ++++++++++++++++++++++------------------
 t/t5607-clone-bundle.sh |   4 +-
 t/t6020-bundle-misc.sh  |  77 ++++++++++++++++++++++++++--
 3 files changed, 134 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 b554538e00..6249420a80 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -242,8 +242,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
@@ -256,10 +264,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 \
@@ -267,6 +281,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
@@ -280,7 +306,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' '
@@ -295,10 +326,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 \
@@ -306,6 +342,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
@@ -319,10 +365,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 \
@@ -330,6 +382,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
@@ -342,7 +406,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.28.0.15.gba9e81f0bd
Previous: Jiang XinNext: Jiang Xin
Message 53 of 60 in “bundle: arguments can be read from stdin”
  1. bundle: arguments can be read from stdinJiang Xin, Jan 3, 2021
  2. Junio C HamanoJan 4, 2021
  3. 1/2 bundle: lost objects when removing duplicate pendingsJiang Xin, Jan 5, 2021
  4. 2/2 bundle: arguments can be read from stdinJiang Xin, Jan 5, 2021
  5. 2/2 bundle: arguments can be read from stdinJiang Xin, Jan 7, 2021
  6. 1/2 bundle: lost objects when removing duplicate pendingsJiang Xin, Jan 7, 2021
  7. Đoàn Trần Công DanhJan 7, 2021
  8. Jiang XinJan 8, 2021
  9. 0/2 Improvements for git-bundleJiang Xin, Jan 8, 2021
  10. 2/2 bundle: arguments can be read from stdinJiang Xin, Jan 8, 2021
  11. Junio C HamanoJan 9, 2021
  12. 1/2 bundle: lost objects when removing duplicate pendingsJiang Xin, Jan 8, 2021
  13. Junio C HamanoJan 9, 2021
  14. Jiang XinJan 9, 2021
  15. Junio C HamanoJan 9, 2021
  16. 0/3 improvements for git-bundleJiang Xin, Jan 10, 2021
  17. 1/3 test: add helper functions for git-bundleJiang Xin, Jan 10, 2021
  18. Junio C HamanoJan 11, 2021
  19. 0/3 improvements for git-bundleJiang Xin, Jan 12, 2021
  20. 1/3 test: add helper functions for git-bundleJiang Xin, Jan 12, 2021
  21. Runaway sed memory use in test on older sed+glibc (was "Re: [PATCH v6 1/3] test: add helper functions for git-bundle")Ævar Arnfjörð Bjarmason, May 26, 2021
  22. Jiang XinMay 27, 2021
  23. Ævar Arnfjörð BjarmasonMay 27, 2021
  24. Jeff KingMay 27, 2021
  25. Felipe ContrerasMay 27, 2021
  26. Jiang XinJun 1, 2021
  27. Jiang XinJun 1, 2021
  28. Ævar Arnfjörð BjarmasonJun 1, 2021
  29. Jiang XinJun 1, 2021
  30. 1/2 t6020: fix bash incompatible issueJiang Xin, Jun 1, 2021
  31. 2/2 t6020: do not mangle trailing spaces in outputJiang Xin, Jun 1, 2021
  32. Ævar Arnfjörð BjarmasonJun 5, 2021
  33. 3/4 sideband: append suffix for message whose CR in next pktlineJiang Xin, Jun 12, 2021
  34. Ævar Arnfjörð BjarmasonJun 13, 2021
  35. Junio C HamanoJun 14, 2021
  36. Jiang XinJun 14, 2021
  37. Junio C HamanoJun 15, 2021
  38. Jiang XinJun 15, 2021
  39. Nicolas PitreJun 15, 2021
  40. Jiang XinJun 15, 2021
  41. Nicolas PitreJun 15, 2021
  42. Junio C HamanoJun 15, 2021
  43. Jiang XinJun 15, 2021
  44. Nicolas PitreJun 15, 2021
  45. 0/4 Fixed t6020 bash compatible issue and fixed wrong sideband suffix issueJiang Xin, Jun 12, 2021
  46. Junio C HamanoJun 14, 2021
  47. Jiang XinJun 15, 2021
  48. t6020: fix incompatible parameter expansionJiang Xin, Jun 17, 2021
  49. Ævar Arnfjörð BjarmasonJun 21, 2021
  50. 1/4 t6020: fix bash incompatible issueJiang Xin, Jun 12, 2021
  51. 2/4 test: refactor create_commits_in() for t5411 and t5548Jiang Xin, Jun 12, 2021
  52. 4/4 test: compare raw output, not mangle tabs and spacesJiang Xin, Jun 12, 2021
  53. 3/3 bundle: arguments can be read from stdinJiang Xin, Jan 12, 2021
  54. 2/3 bundle: lost objects when removing duplicate pendingsJiang Xin, Jan 12, 2021
  55. 2/3 bundle: lost objects when removing duplicate pendingsJiang Xin, Jan 10, 2021
  56. Junio C HamanoJan 11, 2021
  57. 3/3 bundle: arguments can be read from stdinJiang Xin, Jan 10, 2021
  58. Jiang XinJan 9, 2021
  59. Junio C HamanoJan 9, 2021
  60. 0/2 improvements for git-bundleJiang Xin, Jan 7, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.