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

[PATCH v2 10/18] submodule--helper: eliminate internal "--update" option

From
Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 30, 2022, 21:19 UTC
Message-ID
<b788fc671bf1862a72f9fe7256affd7d152b8a6f.1656623978.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1282.v2.git.git.1656623978.gitgitgadget@gmail.com>
From: Glen Choo <chooglen@google.com>

Follow-up on the preceding commit which taught "git submodule--helper update" to understand "--merge", "--checkout" and "--rebase" and use those options instead of "--update=(rebase|merge|checkout|none)" when the command invokes itself.

Unlike the preceding change this isn't strictly necessary to eventually change "git-submodule.sh" so that it invokes "git submodule--helper update" directly, but let's remove this inconsistency in the command-line interface. We shouldn't need to carry special synonyms for existing options in "git submodule--helper" when that command can use the primary documented names instead.

But, as seen in the post-image this makes the control flow within "builtin/submodule--helper.c" simpler, we can now write directly to the "update_default" member of "struct update_data" when parsing the options in "module_update()".

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 53179472d85..389b900602f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1818,7 +1818,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 static void determine_submodule_update_strategy(struct repository *r,
 						int just_cloned,
 						const char *path,
-						const char *update,
+						enum submodule_update_type update,
 						struct submodule_update_strategy *out)
 {
 	const struct submodule *sub = submodule_from_path(r, null_oid(), path);
@@ -1828,9 +1828,7 @@ static void determine_submodule_update_strategy(struct repository *r,
 	key = xstrfmt("submodule.%s.update", sub->name);
 
 	if (update) {
-		if (parse_submodule_update_strategy(update, out) < 0)
-			die(_("Invalid update mode '%s' for submodule path '%s'"),
-				update, path);
+		out->type = update;
 	} else if (!repo_config_get_string_tmp(r, key, &val)) {
 		if (parse_submodule_update_strategy(val, out) < 0)
 			die(_("Invalid update mode '%s' configured for submodule path '%s'"),
@@ -1882,7 +1880,7 @@ struct update_data {
 	const char *prefix;
 	const char *recursive_prefix;
 	const char *displaypath;
-	const char *update_default;
+	enum submodule_update_type update_default;
 	struct object_id suboid;
 	struct string_list references;
 	struct submodule_update_strategy update_strategy;
@@ -2423,6 +2421,8 @@ static const char *submodule_update_type_to_label(enum submodule_update_type typ
 
 static void update_data_to_args(struct update_data *update_data, struct strvec *args)
 {
+	enum submodule_update_type update_type = update_data->update_default;
+
 	strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
 	strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
 	if (update_data->recursive_prefix)
@@ -2446,8 +2446,10 @@ static void update_data_to_args(struct update_data *update_data, struct strvec *
 		strvec_push(args, "--require-init");
 	if (update_data->depth)
 		strvec_pushf(args, "--depth=%d", update_data->depth);
-	if (update_data->update_default)
-		strvec_pushl(args, "--update", update_data->update_default, NULL);
+	if (update_type != SM_UPDATE_UNSPECIFIED)
+		strvec_pushf(args, "--%s",
+			     submodule_update_type_to_label(update_type));
+
 	if (update_data->references.nr) {
 		struct string_list_item *item;
 		for_each_string_list_item(item, &update_data->references)
@@ -2599,7 +2601,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
 	struct update_data opt = UPDATE_DATA_INIT;
 	struct list_objects_filter_options filter_options;
 	int ret;
-	enum submodule_update_type update_type = SM_UPDATE_UNSPECIFIED;
 
 	struct option module_update_options[] = {
 		OPT__FORCE(&opt.force, N_("force checkout updates"), 0),
@@ -2618,16 +2619,13 @@ static int module_update(int argc, const char **argv, const char *prefix)
 			   N_("path"),
 			   N_("path into the working tree, across nested "
 			      "submodule boundaries")),
-		OPT_STRING(0, "update", &opt.update_default,
-			   N_("string"),
-			   N_("rebase, merge, checkout or none")),
-		OPT_SET_INT(0, "checkout", &update_type,
+		OPT_SET_INT(0, "checkout", &opt.update_default,
 			N_("use the 'checkout' update strategy (default)"),
 			SM_UPDATE_CHECKOUT),
-		OPT_SET_INT('m', "merge", &update_type,
+		OPT_SET_INT('m', "merge", &opt.update_default,
 			N_("use the 'merge' update strategy"),
 			SM_UPDATE_MERGE),
-		OPT_SET_INT('r', "rebase", &update_type,
+		OPT_SET_INT('r', "rebase", &opt.update_default,
 			N_("use the 'rebase' update strategy"),
 			SM_UPDATE_REBASE),
 		OPT_STRING_LIST(0, "reference", &opt.references, N_("repo"),
@@ -2679,13 +2677,8 @@ static int module_update(int argc, const char **argv, const char *prefix)
 
 	opt.filter_options = &filter_options;
 
-	if (update_type != SM_UPDATE_UNSPECIFIED)
-		opt.update_default = submodule_update_type_to_label(update_type);
-
 	if (opt.update_default)
-		if (parse_submodule_update_strategy(opt.update_default,
-						    &opt.update_strategy) < 0)
-			die(_("bad value for update parameter"));
+		opt.update_strategy.type = opt.update_default;
 
 	if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
 		list_objects_filter_release(&filter_options);

-- 
gitgitgadget
Previous: Ævar Arnfjörð Bjarmason via GitGitGadgetNext: Ævar Arnfjörð Bjarmason via GitGitGadget
Message 22 of 48 in “submodule: remove "--recursive-prefix"”
  1. 0/5 submodule: remove "--recursive-prefix"Glen Choo via GitGitGadget, Jun 27, 2022
  2. 1/5 submodule--helper update: use display path helperGlen Choo via GitGitGadget, Jun 27, 2022
  3. Ævar Arnfjörð BjarmasonJun 28, 2022
  4. Glen ChooJun 28, 2022
  5. 2/5 submodule--helper: don't recreate recursive prefixGlen Choo via GitGitGadget, Jun 27, 2022
  6. 3/5 submodule--helper: use correct display path helperGlen Choo via GitGitGadget, Jun 27, 2022
  7. 4/5 submodule--helper update: use --super-prefixGlen Choo via GitGitGadget, Jun 27, 2022
  8. Ævar Arnfjörð BjarmasonJun 28, 2022
  9. Glen ChooJun 28, 2022
  10. 5/5 submodule--helper: remove display path helperGlen Choo via GitGitGadget, Jun 27, 2022
  11. Glen ChooJun 28, 2022
  12. 00/18 submodule: remove "--recursive-prefix"Glen Choo via GitGitGadget, Jun 30, 2022
  13. 01/18 git-submodule.sh: remove unused sanitize_submodule_env()Ævar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  14. 02/18 git-submodule.sh: remove unused $prefix variableÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  15. 03/18 git-submodule.sh: make the "$cached" variable a booleanÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  16. 04/18 git-submodule.sh: remove unused top-level "--branch" argumentÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  17. 05/18 submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  18. 06/18 submodule update: remove "-v" optionÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  19. 07/18 submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  20. 08/18 submodule--helper: report "submodule" as our name in some "-h" outputÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  21. 09/18 submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  22. 10/18 submodule--helper: eliminate internal "--update" optionGlen Choo via GitGitGadget, Jun 30, 2022
  23. 11/18 git-submodule.sh: use "$quiet", not "$GIT_QUIET"Ævar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  24. 12/18 git-sh-setup.sh: remove "say" function, change last usersÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  25. 13/18 submodule--helper update: use display path helperGlen Choo via GitGitGadget, Jun 30, 2022
  26. 14/18 submodule--helper: don't recreate recursive prefixGlen Choo via GitGitGadget, Jun 30, 2022
  27. 15/18 submodule--helper: use correct display path helperGlen Choo via GitGitGadget, Jun 30, 2022
  28. 16/18 submodule--helper: remove unused SUPPORT_SUPER_PREFIX flagsÆvar Arnfjörð Bjarmason via GitGitGadget, Jun 30, 2022
  29. 18/18 submodule--helper: remove display path helperGlen Choo via GitGitGadget, Jun 30, 2022
  30. 17/18 submodule--helper update: use --super-prefixGlen Choo via GitGitGadget, Jun 30, 2022
  31. Glen ChooJun 30, 2022
  32. 0/6 submodule: remove "--recursive-prefix"Glen Choo, Jun 30, 2022
  33. 1/6 submodule--helper update: use display path helperGlen Choo, Jun 30, 2022
  34. 2/6 submodule--helper: don't recreate recursive prefixGlen Choo, Jun 30, 2022
  35. 3/6 submodule--helper: use correct display path helperGlen Choo, Jun 30, 2022
  36. 4/6 submodule--helper: remove unused SUPPORT_SUPER_PREFIX flagsGlen Choo, Jun 30, 2022
  37. 5/6 submodule--helper update: use --super-prefixGlen Choo, Jun 30, 2022
  38. 6/6 submodule--helper: remove display path helperGlen Choo, Jun 30, 2022
  39. 0/7 submodule: remove "--recursive-prefix"Glen Choo, Jul 1, 2022
  40. 1/7 submodule--helper tests: add missing "display path" coverageGlen Choo, Jul 1, 2022
  41. 2/7 submodule--helper update: use display path helperGlen Choo, Jul 1, 2022
  42. 3/7 submodule--helper: don't recreate recursive prefixGlen Choo, Jul 1, 2022
  43. 4/7 submodule--helper: use correct display path helperGlen Choo, Jul 1, 2022
  44. 6/7 submodule--helper update: use --super-prefixGlen Choo, Jul 1, 2022
  45. 5/7 submodule--helper: remove unused SUPPORT_SUPER_PREFIX flagsGlen Choo, Jul 1, 2022
  46. 7/7 submodule--helper: remove display path helperGlen Choo, Jul 1, 2022
  47. Ævar Arnfjörð BjarmasonJun 30, 2022
  48. Glen ChooJun 30, 2022

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.