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>

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 1eb5492cbda49bcd050b37c2ca29f483a0c71867..dc3cc35394781282f136e3bf4999a9b0807d2e18 100644 (file)
--- 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)

diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index b8320d597ba4e8ce348d5ca5afedcf186f14cb71..248929f2e681b239421dafe0c00087c996cb4cb1 100644 (file)
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -133,7 +133,7 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,
 
        if (isatty(2))
                opts.flags |= MIDX_PROGRESS;
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             options, builtin_multi_pack_index_write_usage,
                             0);
        if (argc)
@@ -176,7 +176,7 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv,
 
        if (isatty(2))
                opts.flags |= MIDX_PROGRESS;
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             options, builtin_multi_pack_index_verify_usage,
                             0);
        if (argc)
@@ -203,7 +203,7 @@ static int cmd_multi_pack_index_expire(int argc, const char **argv,
 
        if (isatty(2))
                opts.flags |= MIDX_PROGRESS;
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             options, builtin_multi_pack_index_expire_usage,
                             0);
        if (argc)
@@ -233,7 +233,7 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv,
 
        if (isatty(2))
                opts.flags |= MIDX_PROGRESS;
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             options,
                             builtin_multi_pack_index_repack_usage,
                             0);

diff --git a/builtin/remote.c b/builtin/remote.c
index 4a6d47c03a4bb508f7543f07a7ac013577c9c893..96f562f00a230ca5dedba2f5f07a79b4df02308e 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -177,8 +177,8 @@ static int add(int argc, const char **argv, const char *prefix)
                OPT_END()
        };
 
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
-                            0);
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_add_usage, 0);
 
        if (argc != 2)
                usage_with_options(builtin_remote_add_usage, options);
@@ -695,7 +695,7 @@ static int mv(int argc, const char **argv, const char *prefix)
        int i, refs_renamed_nr = 0, refspec_updated = 0;
        struct progress *progress = NULL;
 
-       argc = parse_options(argc, argv, NULL, options,
+       argc = parse_options(argc, argv, prefix, options,
                             builtin_remote_rename_usage, 0);
 
        if (argc != 2)
@@ -1264,7 +1264,8 @@ static int show(int argc, const char **argv, const char *prefix)
        };
        struct show_info info = SHOW_INFO_INIT;
 
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_show_usage,
                             0);
 
        if (argc < 1)
@@ -1371,8 +1372,8 @@ static int set_head(int argc, const char **argv, const char *prefix)
                         N_("delete refs/remotes/<name>/HEAD")),
                OPT_END()
        };
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,
-                            0);
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_sethead_usage, 0);
        if (argc)
                strbuf_addf(&buf, "refs/remotes/%s/HEAD", argv[0]);
 
@@ -1471,8 +1472,8 @@ static int prune(int argc, const char **argv, const char *prefix)
                OPT_END()
        };
 
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage,
-                            0);
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_prune_usage, 0);
 
        if (argc < 1)
                usage_with_options(builtin_remote_prune_usage, options);
@@ -1504,7 +1505,8 @@ static int update(int argc, const char **argv, const char *prefix)
        int default_defined = 0;
        int retval;
 
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage,
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_update_usage,
                             PARSE_OPT_KEEP_ARGV0);
 
        strvec_push(&fetch_argv, "fetch");
@@ -1583,7 +1585,7 @@ static int set_branches(int argc, const char **argv, const char *prefix)
                OPT_END()
        };
 
-       argc = parse_options(argc, argv, NULL, options,
+       argc = parse_options(argc, argv, prefix, options,
                             builtin_remote_setbranches_usage, 0);
        if (argc == 0) {
                error(_("no remote specified"));
@@ -1608,7 +1610,8 @@ static int get_url(int argc, const char **argv, const char *prefix)
                         N_("return all URLs")),
                OPT_END()
        };
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_geturl_usage, 0);
 
        if (argc != 1)
                usage_with_options(builtin_remote_geturl_usage, options);
@@ -1668,7 +1671,8 @@ static int set_url(int argc, const char **argv, const char *prefix)
                            N_("delete URLs")),
                OPT_END()
        };
-       argc = parse_options(argc, argv, NULL, options, builtin_remote_seturl_usage,
+       argc = parse_options(argc, argv, prefix, options,
+                            builtin_remote_seturl_usage,
                             PARSE_OPT_KEEP_ARGV0);
 
        if (add_mode && delete_mode)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 7b39a150a99461039bca4c404f8198c58ae7b497..287716db68e4198bdec97517ff2cd9c053c9f0b8 100644 (file)
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -60,7 +60,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)
        if (!core_apply_sparse_checkout)
                die(_("this worktree is not sparse"));
 
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             builtin_sparse_checkout_list_options,
                             builtin_sparse_checkout_list_usage, 0);
 
@@ -452,7 +452,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
        init_opts.cone_mode = -1;
        init_opts.sparse_index = -1;
 
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             builtin_sparse_checkout_init_options,
                             builtin_sparse_checkout_init_usage, 0);
 
@@ -860,7 +860,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
        reapply_opts.cone_mode = -1;
        reapply_opts.sparse_index = -1;
 
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             builtin_sparse_checkout_reapply_options,
                             builtin_sparse_checkout_reapply_usage, 0);
 
@@ -897,7 +897,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
         * forcibly return to a dense checkout regardless of initial state.
         */
 
-       argc = parse_options(argc, argv, NULL,
+       argc = parse_options(argc, argv, prefix,
                             builtin_sparse_checkout_disable_options,
                             builtin_sparse_checkout_disable_usage, 0);