@matheusbsbs / matheus-bagliano / commits / ecd2d3efe0

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>

Jeff King committed Aug 25, 2022 at 06:47 UTC ecd2d3efe04c982eb9b95a2997841634f6a54e45

4 files changed +26 -22

builtin/commit-graph.c

+2 -2

@@ -80,7 +80,7 @@ static int graph_verify(int argc, const char **argv, const char *prefix)

80 trace2_cmd_mode("verify");

82 opts.progress = isatty(2);

83 - argc = parse_options(argc, argv, NULL,

83 + argc = parse_options(argc, argv, prefix,

84 options,

85 builtin_commit_graph_verify_usage, 0);

86 if (argc)

@@ -241,7 +241,7 @@ static int graph_write(int argc, const char **argv, const char *prefix)

241

242 git_config(git_commit_graph_write_config, &opts);

243

244 - argc = parse_options(argc, argv, NULL,

244 + argc = parse_options(argc, argv, prefix,

245 options,

246 builtin_commit_graph_write_usage, 0);

247 if (argc)

builtin/multi-pack-index.c

+4 -4

@@ -133,7 +133,7 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,

133

134 if (isatty(2))

135 opts.flags |= MIDX_PROGRESS;

136 - argc = parse_options(argc, argv, NULL,

136 + argc = parse_options(argc, argv, prefix,

137 options, builtin_multi_pack_index_write_usage,

138 0);

139 if (argc)

@@ -176,7 +176,7 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv,

176

177 if (isatty(2))

178 opts.flags |= MIDX_PROGRESS;

179 - argc = parse_options(argc, argv, NULL,

179 + argc = parse_options(argc, argv, prefix,

180 options, builtin_multi_pack_index_verify_usage,

181 0);

182 if (argc)

@@ -203,7 +203,7 @@ static int cmd_multi_pack_index_expire(int argc, const char **argv,

203

204 if (isatty(2))

205 opts.flags |= MIDX_PROGRESS;

206 - argc = parse_options(argc, argv, NULL,

206 + argc = parse_options(argc, argv, prefix,

207 options, builtin_multi_pack_index_expire_usage,

208 0);

209 if (argc)

@@ -233,7 +233,7 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv,

233

234 if (isatty(2))

235 opts.flags |= MIDX_PROGRESS;

236 - argc = parse_options(argc, argv, NULL,

236 + argc = parse_options(argc, argv, prefix,

237 options,

238 builtin_multi_pack_index_repack_usage,

239 0);

builtin/remote.c

+16 -12

@@ -177,8 +177,8 @@ static int add(int argc, const char **argv, const char *prefix)

177 OPT_END()

178 };

179

180 - argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,

181 - 0);

180 + argc = parse_options(argc, argv, prefix, options,

181 + builtin_remote_add_usage, 0);

182

183 if (argc != 2)

184 usage_with_options(builtin_remote_add_usage, options);

@@ -695,7 +695,7 @@ static int mv(int argc, const char **argv, const char *prefix)

695 int i, refs_renamed_nr = 0, refspec_updated = 0;

696 struct progress *progress = NULL;

697

698 - argc = parse_options(argc, argv, NULL, options,

698 + argc = parse_options(argc, argv, prefix, options,

699 builtin_remote_rename_usage, 0);

700

701 if (argc != 2)

@@ -1264,7 +1264,8 @@ static int show(int argc, const char **argv, const char *prefix)

1264 };

1265 struct show_info info = SHOW_INFO_INIT;

1266

1267 - argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,

1267 + argc = parse_options(argc, argv, prefix, options,

1268 + builtin_remote_show_usage,

1269 0);

1270

1271 if (argc < 1)

@@ -1371,8 +1372,8 @@ static int set_head(int argc, const char **argv, const char *prefix)

1372 N_("delete refs/remotes/<name>/HEAD")),

1373 OPT_END()

1374 };

1374 - argc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,

1375 - 0);

1375 + argc = parse_options(argc, argv, prefix, options,

1376 + builtin_remote_sethead_usage, 0);

1377 if (argc)

1378 strbuf_addf(&buf, "refs/remotes/%s/HEAD", argv[0]);

1379

@@ -1471,8 +1472,8 @@ static int prune(int argc, const char **argv, const char *prefix)

1472 OPT_END()

1473 };

1474

1474 - argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage,

1475 - 0);

1475 + argc = parse_options(argc, argv, prefix, options,

1476 + builtin_remote_prune_usage, 0);

1477

1478 if (argc < 1)

1479 usage_with_options(builtin_remote_prune_usage, options);

@@ -1504,7 +1505,8 @@ static int update(int argc, const char **argv, const char *prefix)

1505 int default_defined = 0;

1506 int retval;

1507

1507 - argc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage,

1508 + argc = parse_options(argc, argv, prefix, options,

1509 + builtin_remote_update_usage,

1510 PARSE_OPT_KEEP_ARGV0);

1511

1512 strvec_push(&fetch_argv, "fetch");

@@ -1583,7 +1585,7 @@ static int set_branches(int argc, const char **argv, const char *prefix)

1585 OPT_END()

1586 };

1587

1586 - argc = parse_options(argc, argv, NULL, options,

1588 + argc = parse_options(argc, argv, prefix, options,

1589 builtin_remote_setbranches_usage, 0);

1590 if (argc == 0) {

1591 error(_("no remote specified"));

@@ -1608,7 +1610,8 @@ static int get_url(int argc, const char **argv, const char *prefix)

1610 N_("return all URLs")),

1611 OPT_END()

1612 };

1611 - argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);

1613 + argc = parse_options(argc, argv, prefix, options,

1614 + builtin_remote_geturl_usage, 0);

1615

1616 if (argc != 1)

1617 usage_with_options(builtin_remote_geturl_usage, options);

@@ -1668,7 +1671,8 @@ static int set_url(int argc, const char **argv, const char *prefix)

1671 N_("delete URLs")),

1672 OPT_END()

1673 };

1671 - argc = parse_options(argc, argv, NULL, options, builtin_remote_seturl_usage,

1674 + argc = parse_options(argc, argv, prefix, options,

1675 + builtin_remote_seturl_usage,

1676 PARSE_OPT_KEEP_ARGV0);

1677

1678 if (add_mode && delete_mode)

builtin/sparse-checkout.c

+4 -4

@@ -60,7 +60,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)

60 if (!core_apply_sparse_checkout)

61 die(_("this worktree is not sparse"));

63 - argc = parse_options(argc, argv, NULL,

63 + argc = parse_options(argc, argv, prefix,

64 builtin_sparse_checkout_list_options,

65 builtin_sparse_checkout_list_usage, 0);

@@ -452,7 +452,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)

452 init_opts.cone_mode = -1;

453 init_opts.sparse_index = -1;

454

455 - argc = parse_options(argc, argv, NULL,

455 + argc = parse_options(argc, argv, prefix,

456 builtin_sparse_checkout_init_options,

457 builtin_sparse_checkout_init_usage, 0);

458

@@ -860,7 +860,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,

860 reapply_opts.cone_mode = -1;

861 reapply_opts.sparse_index = -1;

862

863 - argc = parse_options(argc, argv, NULL,

863 + argc = parse_options(argc, argv, prefix,

864 builtin_sparse_checkout_reapply_options,

865 builtin_sparse_checkout_reapply_usage, 0);

866

@@ -897,7 +897,7 @@ static int sparse_checkout_disable(int argc, const char **argv,

897 * forcibly return to a dense checkout regardless of initial state.

898 */

899

900 - argc = parse_options(argc, argv, NULL,

900 + argc = parse_options(argc, argv, prefix,

901 builtin_sparse_checkout_disable_options,

902 builtin_sparse_checkout_disable_usage, 0);

903