summaryrefslogtreecommitdiff
path: root/builtin/commit-graph.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2022-08-25 06:47:00 -0400
committerJunio C Hamano <gitster@pobox.com>2022-08-25 09:43:29 -0700
commitecd2d3efe04c982eb9b95a2997841634f6a54e45 (patch)
tree809fc7d6058aae77900196d761d9aebf74aac1b2 /builtin/commit-graph.c
parent398c4ff582646b099eb1f3b14a525dfe5eeb5420 (diff)
downloadgit-ecd2d3efe04c982eb9b95a2997841634f6a54e45.tar.gz
pass subcommand "prefix" arguments to parse_options()
Recent commits such as bf0a6b65fc (builtin/multi-pack-index.c: let parse-options parse subcommands, 2022-08-19) converted a few functions to match our usual argc/argv/prefix conventions, but the prefix argument remains unused. However, there is a good use for it: they should pass it to their own parse_options() functions, where it may be used to adjust the value of any filename options. In all but one of these functions, there's no behavior change, since they don't use OPT_FILENAME. But this is an actual fix for one option, which you can see by modifying the test suite like so: diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 4fe57414c1..d0974d4371 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -186,7 +186,11 @@ test_expect_success 'writing a bitmap with --refs-snapshot' ' # Then again, but with a refs snapshot which only sees # refs/tags/one. - git multi-pack-index write --bitmap --refs-snapshot=snapshot && + ( + mkdir subdir && + cd subdir && + git multi-pack-index write --bitmap --refs-snapshot=../snapshot + ) && test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && I'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken all along, because the sub-function never got to see the prefix. It is that commit which is actually enabling us to fix it (and which also brought attention to the problem because it triggers -Wunused-parameter!) The other functions changed here don't use OPT_FILENAME at all. In their cases this isn't fixing anything visible, but it's following the usual pattern and future-proofing them against somebody adding new options and being surprised. I didn't include a test for the one visible case above. We don't generally test routine parse-options behavior for individual options. The challenge here was finding the problem, and now that this has been done, it's not likely to regress. Likewise, we could apply the patch above to cover it "for free" but it makes reading the rest of the test unnecessarily complicated. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/commit-graph.c')
-rw-r--r--builtin/commit-graph.c4
1 files changed, 2 insertions, 2 deletions
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 1eb5492cbd..dc3cc35394 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -80,7 +80,7 @@ static int graph_verify(int argc, const char **argv, const char *prefix)
trace2_cmd_mode("verify");
opts.progress = isatty(2);
- argc = parse_options(argc, argv, NULL,
+ argc = parse_options(argc, argv, prefix,
options,
builtin_commit_graph_verify_usage, 0);
if (argc)
@@ -241,7 +241,7 @@ static int graph_write(int argc, const char **argv, const char *prefix)
git_config(git_commit_graph_write_config, &opts);
- argc = parse_options(argc, argv, NULL,
+ argc = parse_options(argc, argv, prefix,
options,
builtin_commit_graph_write_usage, 0);
if (argc)