All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com,
	martin.agren@gmail.com
Subject: [PATCH v2 3/3] builtin/commit-graph.c: support '--input=none'
Date: Tue, 4 Feb 2020 16:28:36 -0800	[thread overview]
Message-ID: <4c6425f0da9a6e5ae86530a12f18959ada07404b.1580862307.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1580862307.git.me@ttaylorr.com>

In the previous commit, we introduced '--split=<no-merge|merge-all>',
and alluded to the fact that '--split=merge-all' would be useful for
callers who wish to always trigger a merge of an incremental chain.

There is a problem with the above approach, which is that there is no
way to specify to the commit-graph builtin that a caller only wants to
include commits already in the graph. One can specify '--input=append'
to include all commits in the existing graphs, but the absence of
'--input=stdin-{commits,packs}' causes the builtin to call
'fill_oids_from_all_packs()'.

Passing '--input=reachable' (as in 'git commit-graph write
--split=merge-all --input=reachable --input=append') works around this
issue by making '--input=reachable' effectively a no-op, but this can be
prohibitively expensive in large repositories, making it an undesirable
choice for some users.

Teach '--input=none' as an option to behave as if '--input=append' were
given, but to consider no other sources in addition.

This, in conjunction with the option introduced in the previous patch
offers the convenient way to force the commit-graph machinery to
condense a chain of incrementals without requiring any new commits:

  $ git commit-graph write --split=merge-all --input=none

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-commit-graph.txt |  8 +++++++-
 builtin/commit-graph.c             | 11 ++++++++---
 commit-graph.c                     |  6 ++++--
 commit-graph.h                     |  3 ++-
 t/t5324-split-commit-graph.sh      | 26 ++++++++++++++++++++++++++
 5 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt
index 2ae9de679a..633cfbe023 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -39,7 +39,7 @@ COMMANDS
 --------
 'write'::
 
-Write a commit-graph file based on the commits found in packfiles.
+Write a commit-graph file based on the specified sources of input:
 +
 With the `--input=stdin-packs` option, generate the new commit graph by
 walking objects only in the specified pack-indexes. (Cannot be combined
@@ -57,6 +57,12 @@ walking commits starting at all refs. (Cannot be combined with
 With the `--input=append` option, include all commits that are present
 in the existing commit-graph file.
 +
+With the `--input=none` option, behave as if `--input=append` were
+given, but do not walk other packs to find additional commits.
+
+If none of the above options are given, then generate the new
+commit-graph by walking over all pack-indexes.
++
 With the `--split[=<strategy>]` option, write the commit-graph as a
 chain of multiple commit-graph files stored in
 `<dir>/info/commit-graphs`. Commit-graph layers are merged based on the
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 0ff25896d0..a71af88815 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -11,7 +11,7 @@ static char const * const builtin_commit_graph_usage[] = {
 	N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),
 	N_("git commit-graph write [--object-dir <objdir>] "
 	   "[--split[=<strategy>]] "
-	   "[--input=<reachable|stdin-packs|stdin-commits|append>] "
+	   "[--input=<reachable|stdin-packs|stdin-commits|append|none>] "
 	   "[--[no-]progress] <split options>"),
 	NULL
 };
@@ -24,7 +24,7 @@ static const char * const builtin_commit_graph_verify_usage[] = {
 static const char * const builtin_commit_graph_write_usage[] = {
 	N_("git commit-graph write [--object-dir <objdir>] "
 	   "[--split[=<strategy>]] "
-	   "[--input=<reachable|stdin-packs|stdin-commits|append>] "
+	   "[--input=<reachable|stdin-packs|stdin-commits|append|none>] "
 	   "[--[no-]progress] <split options>"),
 	NULL
 };
@@ -33,7 +33,8 @@ enum commit_graph_input {
 	COMMIT_GRAPH_INPUT_REACHABLE     = (1 << 1),
 	COMMIT_GRAPH_INPUT_STDIN_PACKS   = (1 << 2),
 	COMMIT_GRAPH_INPUT_STDIN_COMMITS = (1 << 3),
-	COMMIT_GRAPH_INPUT_APPEND        = (1 << 4)
+	COMMIT_GRAPH_INPUT_APPEND        = (1 << 4),
+	COMMIT_GRAPH_INPUT_NONE          = (1 << 5)
 };
 
 static struct opts_commit_graph {
@@ -80,6 +81,8 @@ static int option_parse_input(const struct option *opt, const char *arg,
 		*to |= COMMIT_GRAPH_INPUT_STDIN_COMMITS;
 	else if (!strcmp(arg, "append"))
 		*to |= COMMIT_GRAPH_INPUT_APPEND;
+	else if (!strcmp(arg, "none"))
+		*to |= (COMMIT_GRAPH_INPUT_APPEND | COMMIT_GRAPH_INPUT_NONE);
 	else
 		die(_("unrecognized --input source, %s"), arg);
 	return 0;
@@ -225,6 +228,8 @@ static int graph_write(int argc, const char **argv)
 		opts.obj_dir = get_object_directory();
 	if (opts.input & COMMIT_GRAPH_INPUT_APPEND)
 		flags |= COMMIT_GRAPH_WRITE_APPEND;
+	if (opts.input & COMMIT_GRAPH_INPUT_NONE)
+		flags |= COMMIT_GRAPH_WRITE_NO_INPUT;
 	if (opts.split)
 		flags |= COMMIT_GRAPH_WRITE_SPLIT;
 	if (opts.progress)
diff --git a/commit-graph.c b/commit-graph.c
index 3a5cb23cd7..417b7eac9c 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -788,7 +788,8 @@ struct write_commit_graph_context {
 	unsigned append:1,
 		 report_progress:1,
 		 split:1,
-		 check_oids:1;
+		 check_oids:1,
+		 no_input:1;
 
 	const struct split_commit_graph_opts *split_opts;
 };
@@ -1785,6 +1786,7 @@ int write_commit_graph(struct object_directory *odb,
 	ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
 	ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
 	ctx->split_opts = split_opts;
+	ctx->no_input = flags & COMMIT_GRAPH_WRITE_NO_INPUT ? 1 : 0;
 
 	if (ctx->split) {
 		struct commit_graph *g;
@@ -1843,7 +1845,7 @@ int write_commit_graph(struct object_directory *odb,
 			goto cleanup;
 	}
 
-	if (!pack_indexes && !commit_hex)
+	if (!ctx->no_input && !pack_indexes && !commit_hex)
 		fill_oids_from_all_packs(ctx);
 
 	close_reachable(ctx);
diff --git a/commit-graph.h b/commit-graph.h
index 65a7d2edae..df7f3f5961 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -79,7 +79,8 @@ enum commit_graph_write_flags {
 	COMMIT_GRAPH_WRITE_PROGRESS   = (1 << 1),
 	COMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),
 	/* Make sure that each OID in the input is a valid commit OID. */
-	COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)
+	COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),
+	COMMIT_GRAPH_WRITE_NO_INPUT   = (1 << 4)
 };
 
 enum commit_graph_split_flags {
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 353523eca4..e3f317a1f4 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -369,4 +369,30 @@ test_expect_success '--split=no-merge always writes an incremental' '
 	test_line_count = 2 $graphdir/commit-graph-chain
 '
 
+test_expect_success '--split=no-merge, --input=none writes nothing' '
+	test_when_finished rm -rf a graphs.before graphs.after &&
+	rm -rf $graphdir &&
+	git reset --hard commits/2 &&
+	git rev-list -1 HEAD~1 >a &&
+	git commit-graph write --split=no-merge --input=stdin-commits <a &&
+	ls $graphdir/graph-*.graph >graphs.before &&
+	test_line_count = 1 $graphdir/commit-graph-chain &&
+	git commit-graph write --split --input=none &&
+	ls $graphdir/graph-*.graph >graphs.after &&
+	test_cmp graphs.before graphs.after
+'
+
+test_expect_success '--split=merge-all, --input=none merges the chain' '
+	test_when_finished rm -rf a b &&
+	rm -rf $graphdir &&
+	git reset --hard commits/2 &&
+	git rev-list -1 HEAD~1 >a &&
+	git rev-list -1 HEAD >b &&
+	git commit-graph write --split=no-merge --input=stdin-commits <a &&
+	git commit-graph write --split=no-merge --input=stdin-commits <b &&
+	test_line_count = 2 $graphdir/commit-graph-chain &&
+	git commit-graph write --split=merge-all --input=none &&
+	test_line_count = 1 $graphdir/commit-graph-chain
+'
+
 test_done
-- 
2.25.0.119.gaa12b7378b

  parent reply	other threads:[~2020-02-05  0:28 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31  0:28 [PATCH 0/3] builtin/commit-graph.c: new split/merge options Taylor Blau
2020-01-31  0:28 ` [PATCH 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Taylor Blau
2020-01-31 14:19   ` Derrick Stolee
2020-02-04  3:47     ` Taylor Blau
2020-01-31 19:27   ` Martin Ågren
2020-02-04  4:06     ` Taylor Blau
2020-02-06 19:15       ` Martin Ågren
2020-02-09 23:27         ` Taylor Blau
2020-01-31 23:34   ` SZEDER Gábor
2020-02-01 21:25     ` Johannes Schindelin
2020-02-03 10:47       ` SZEDER Gábor
2020-02-03 11:11         ` Jeff King
2020-02-04  3:58           ` Taylor Blau
2020-02-04 14:14             ` Jeff King
2020-02-04  3:59       ` Taylor Blau
2020-02-04  3:59     ` Taylor Blau
2020-01-31  0:28 ` [PATCH 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-01-31 14:40   ` Derrick Stolee
2020-02-04  4:21     ` Taylor Blau
2020-01-31 19:34   ` Martin Ågren
2020-02-04  4:51     ` Taylor Blau
2020-02-13 11:33       ` SZEDER Gábor
2020-02-13 11:48         ` SZEDER Gábor
2020-02-13 17:56           ` Taylor Blau
2020-01-31  0:28 ` [PATCH 3/3] builtin/commit-graph.c: support '--input=none' Taylor Blau
2020-01-31 14:40   ` Derrick Stolee
2020-01-31 19:45   ` Martin Ågren
2020-02-04  5:01     ` Taylor Blau
2020-01-31  0:32 ` [PATCH 0/3] builtin/commit-graph.c: new split/merge options Taylor Blau
2020-01-31 13:26   ` Derrick Stolee
2020-01-31 14:41 ` Derrick Stolee
2020-02-04 23:44 ` Junio C Hamano
2020-02-05  0:30   ` Taylor Blau
2020-02-05  0:28 ` [PATCH v2 " Taylor Blau
2020-02-05  0:28   ` [PATCH v2 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Taylor Blau
2020-02-06 19:41     ` Martin Ågren
2020-02-07 15:48       ` Derrick Stolee
2020-02-09 23:32         ` Taylor Blau
2020-02-12  6:03         ` Martin Ågren
2020-02-12 20:50           ` Taylor Blau
2020-02-09 23:30       ` Taylor Blau
2020-02-05  0:28   ` [PATCH v2 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-02-05  0:28   ` Taylor Blau [this message]
2020-02-06 19:50     ` [PATCH v2 3/3] builtin/commit-graph.c: support '--input=none' Martin Ågren
2020-02-09 23:32       ` Taylor Blau
2020-02-05 20:07   ` [PATCH v2 0/3] builtin/commit-graph.c: new split/merge options Junio C Hamano
2020-02-12  5:47 ` [PATCH v3 " Taylor Blau
2020-02-12  5:47   ` [PATCH v3 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Taylor Blau
2020-02-12  5:47   ` [PATCH v3 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-02-12  5:47   ` [PATCH v3 3/3] builtin/commit-graph.c: support '--input=none' Taylor Blau
2020-02-13 11:39     ` SZEDER Gábor
2020-02-13 12:31     ` SZEDER Gábor
2020-02-13 16:08       ` Junio C Hamano
2020-02-13 17:58         ` Taylor Blau
2020-02-13 17:56       ` Taylor Blau
2020-02-12 18:19   ` [PATCH v3 0/3] builtin/commit-graph.c: new split/merge options Junio C Hamano
2020-02-13 17:41     ` Taylor Blau
2020-02-17 18:24   ` Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4c6425f0da9a6e5ae86530a12f18959ada07404b.1580862307.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.