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

Re: [PATCH v2 08/12] submodule--helper: report "submodule" as our name in "-h" output

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 15, 2022, 09:42 UTC
Message-ID
<220615.86r13qxmjx.gmgdl@evledraar.gmail.com>
In-Reply-To
<kl6lczfa60d3.fsf@chooglen-macbookpro.roam.corp.google.com>
On Tue, Jun 14 2022, Glen Choo wrote:
Show 127 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 574d6e0a79b..c2f55779cb1 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -286,7 +286,7 @@ static int module_list(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper list [--prefix=<path>] [<path>...]"),
>> +		N_("git submodule list [--prefix=<path>] [<path>...]"),
>>  		NULL
>>  	};
>> @@ -444,7 +444,7 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper foreach [--quiet] [--recursive] [--] <command>"),
>> +		N_("git submodule foreach [--quiet] [--recursive] [--] <command>"),
>>  		NULL
>>  	};
>>  
>> @@ -582,7 +582,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper init [<options>] [<path>]"),
>> +		N_("git submodule init [<options>] [<path>]"),
>>  		NULL
>>  	};
>>  
>> @@ -786,7 +786,7 @@ static int module_name(int argc, const char **argv, const char *prefix)
>>  	const struct submodule *sub;
>>  
>>  	if (argc != 2)
>> -		usage(_("git submodule--helper name <path>"));
>> +		usage(_("git submodule name <path>"));
>>  
>>  	sub = submodule_from_path(the_repository, null_oid(), argv[1]);
>>  
>> @@ -1185,7 +1185,7 @@ static int module_summary(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper summary [<options>] [<commit>] [--] [<path>]"),
>> +		N_("git submodule summary [<options>] [<commit>] [--] [<path>]"),
>>  		NULL
>>  	};
>>  
>> @@ -1349,7 +1349,7 @@ static int module_sync(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper sync [--quiet] [--recursive] [<path>]"),
>> +		N_("git submodule sync [--quiet] [--recursive] [<path>]"),
>>  		NULL
>>  	};
>>  
>> @@ -1789,7 +1789,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper clone [--prefix=<path>] [--quiet] "
>> +		N_("git submodule clone [--prefix=<path>] [--quiet] "
>>  		   "[--reference <repository>] [--name <name>] [--depth <depth>] "
>>  		   "[--single-branch] [--filter <filter-spec>] "
>>  		   "--url <url> --path <path>"),
>> @@ -2787,7 +2787,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper absorbgitdirs [<options>] [<path>...]"),
>> +		N_("git submodule absorbgitdirs [<options>] [<path>...]"),
>>  		NULL
>>  	};
>>  
>> @@ -2851,9 +2851,9 @@ static int module_config(int argc, const char **argv, const char *prefix)
>>  		OPT_END()
>>  	};
>>  	const char *const git_submodule_helper_usage[] = {
>> -		N_("git submodule--helper config <name> [<value>]"),
>> -		N_("git submodule--helper config --unset <name>"),
>> -		"git submodule--helper config --check-writeable",
>> +		N_("git submodule config <name> [<value>]"),
>> +		N_("git submodule config --unset <name>"),
>> +		"git submodule config --check-writeable",
>>  		NULL
>>  	};
>>  
>> @@ -2892,7 +2892,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix)
>>  		OPT_END()
>>  	};
>>  	const char *const usage[] = {
>> -		N_("git submodule--helper set-url [--quiet] <path> <newurl>"),
>> +		N_("git submodule set-url [--quiet] <path> <newurl>"),
>>  		NULL
>>  	};
>>  
>> @@ -2931,8 +2931,8 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
>>  		OPT_END()
>>  	};
>>  	const char *const usage[] = {
>> -		N_("git submodule--helper set-branch [-q|--quiet] (-d|--default) <path>"),
>> -		N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
>> +		N_("git submodule set-branch [-q|--quiet] (-d|--default) <path>"),
>> +		N_("git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
>>  		NULL
>>  	};
>>  
>> @@ -2973,7 +2973,7 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)
>>  		OPT_END()
>>  	};
>>  	const char *const usage[] = {
>> -		N_("git submodule--helper create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> <start-name>"),
>> +		N_("git submodule create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> <start-name>"),
>>  		NULL
>>  	};
>>  
>> @@ -3276,7 +3276,7 @@ static int module_add(int argc, const char **argv, const char *prefix)
>>  	};
>>  
>>  	const char *const usage[] = {
>> -		N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),
>> +		N_("git submodule add [<options>] [--] <repository> [<path>]"),
>>  		NULL
>>  	};

First, I think this patch is premature, so let's just drop it in a re-roll. We don't *need* to fix this particular edge case or bug now, and can just do it as we do the final migration.

Show 6 quoted lines
> I don't feel positive about pretending to be "git submodule" when the
> subcommand is internal-only. Such commands are only invoked as "git
> submodule--helper foo" and we will probably never turn them into
> external-facing "git submodule foo" [1]. Even if it is internal-only,
> there is a debugging benefit to emitting a usage string with the
> correct, non-user facing command name.

I think we should really be most concerned about the end users here, nobody except git developers are going to want to invoke "submodule--helper" directly, so its help messages should be aimed at helping users.

As you mention in another E-Mail it's currently not a big deal, I *think* that "absorbgitdirs" is the only case where we'll implicitly suggest to a user that they might invoke "git submodule--helper" as we emit the usage info.

But I think for the patches that come after this we should definitely change this, i.e. we don't want an in-between state where we have both of builtin/submodule{,--helper}.c because we haven't merged the two yet, but are now telling users about the inernal-only helper.

Show 26 quoted lines
> I admit it might be a bit confusing to have a mix of usage strings with
> "submodule--helper" vs "submodule", but as long as we clearly label
> which commands are user-facing and which aren't, it shouldn't be too
> hard to maintain, e.g. this might just be a matter of adding comments to
> the array of commands like:
>
>   static struct cmd_struct commands[] = {
>     /* User facing commands - pretend to be "git submodule" */
>     {"name", module_name},
>     {"add", module_add},
>     [...]
>     /* Internal-only commands */
>     {"clone", module_clone},
>     {"resolve-relative-url-test", resolve_relative_url_test},
>     [...]
>   };
>
> Notably, even with this patch we still _kind of_ have 'pseudo usage
> strings' that reference "git submodule--helper". These are just
> internal-only commands that don't use the parse_options() API, e.g.
>
> 	if (argc < 3)
> 		die("submodule--helper push-check requires at least 2 arguments");
>
> I think it's ok to leave them as-is, provided we also leave the other
> internal-only commands as-is.

Yes let's leave them, we'll be converting these to not invoke sub-processes sooner than later anyway.

Show 13 quoted lines
> [1] For convenience, here is a list of all of the subcommands changed in
> this patch and whether they are user-facing/internal-only:
>
> User-facing                 Internal-only
>
> - foreach                   - list
> - init                      - name
> - summary                   - clone
> - sync                      - config
> - absorbgitdirs             - create-branch
> - set-url
> - set-branch
> - add

But yeah, if/when we change the public-facing usage strings we should be leaving out the "internal-only" commands, thanks. I just did a dumb search/replace.

Previous: Glen ChooNext: Ævar Arnfjörð Bjarmason
Message 46 of 90 in “[RFC] submodule update: parse all options in C”
  1. 0/8 [RFC] submodule update: parse all options in CGlen Choo via GitGitGadget, Jun 10, 2022
  2. 1/8 submodule update: remove intermediate parsingGlen Choo via GitGitGadget, Jun 10, 2022
  3. 2/8 submodule update: pass options containing "[no-]"Glen Choo via GitGitGadget, Jun 10, 2022
  4. 3/8 submodule update: pass options with stuck formsGlen Choo via GitGitGadget, Jun 10, 2022
  5. 4/8 submodule update: pass --require-init and --initGlen Choo via GitGitGadget, Jun 10, 2022
  6. 5/8 submodule--helper update: use one param per typeGlen Choo via GitGitGadget, Jun 10, 2022
  7. 7/8 submodule update: stop parsing options in .shGlen Choo via GitGitGadget, Jun 10, 2022
  8. 8/8 submodule update: remove never-used expansionGlen Choo via GitGitGadget, Jun 10, 2022
  9. 6/8 submodule update: remove -v, pass --quietGlen Choo via GitGitGadget, Jun 10, 2022
  10. 00/20 submodule: remove git-submodule.sh, create bare builtin/submodule.cÆvar Arnfjörð Bjarmason, Jun 10, 2022
  11. 01/20 git-submodule.sh: remove unused sanitize_submodule_env()Ævar Arnfjörð Bjarmason, Jun 10, 2022
  12. 02/20 git-submodule.sh: remove unused $prefix variableÆvar Arnfjörð Bjarmason, Jun 10, 2022
  13. 04/20 git-submodule.sh: normalize parsing of "--branch"Ævar Arnfjörð Bjarmason, Jun 10, 2022
  14. 03/20 git-submodule.sh: remove unused --super-prefix logicÆvar Arnfjörð Bjarmason, Jun 10, 2022
  15. 05/20 git-submodule.sh: normalize parsing of --cachedÆvar Arnfjörð Bjarmason, Jun 10, 2022
  16. 06/20 submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason, Jun 10, 2022
  17. 07/20 git-submodule.sh: create a "case" dispatch statementÆvar Arnfjörð Bjarmason, Jun 10, 2022
  18. 09/20 git-submodule.sh: dispatch "sync" to helperÆvar Arnfjörð Bjarmason, Jun 10, 2022
  19. 08/20 submodule--helper: pretend to be "git submodule" in "-h" outputÆvar Arnfjörð Bjarmason, Jun 10, 2022
  20. 10/20 git-submodule.sh: dispatch directly to helperÆvar Arnfjörð Bjarmason, Jun 10, 2022
  21. 12/20 submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason, Jun 10, 2022
  22. 11/20 git-submodule.sh: dispatch "foreach" to helperÆvar Arnfjörð Bjarmason, Jun 10, 2022
  23. 15/20 submodule--helper: understand -v option for "update"Ævar Arnfjörð Bjarmason, Jun 10, 2022
  24. 14/20 git-submodule doc: document the -v" option to "update"Ævar Arnfjörð Bjarmason, Jun 10, 2022
  25. 18/20 git-submodule.sh: simplify parsing loopÆvar Arnfjörð Bjarmason, Jun 10, 2022
  26. 20/20 submodule: add a subprocess-less submodule.useBuiltin settingÆvar Arnfjörð Bjarmason, Jun 10, 2022
  27. 19/20 submodule: make it a built-in, remove git-submodule.shÆvar Arnfjörð Bjarmason, Jun 10, 2022
  28. 17/20 git-submodule.sh: use "$quiet", not "$GIT_QUIET"Ævar Arnfjörð Bjarmason, Jun 10, 2022
  29. 13/20 submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason, Jun 10, 2022
  30. 16/20 git-submodule.sh: dispatch "update" to helperÆvar Arnfjörð Bjarmason, Jun 10, 2022
  31. Glen ChooJun 13, 2022
  32. 00/12 submodule: make "git submodule--helper" behave like "git submodule"Ævar Arnfjörð Bjarmason, Jun 13, 2022
  33. 01/12 git-submodule.sh: remove unused sanitize_submodule_env()Ævar Arnfjörð Bjarmason, Jun 13, 2022
  34. 02/12 git-submodule.sh: remove unused $prefix var and --super-prefixÆvar Arnfjörð Bjarmason, Jun 13, 2022
  35. 03/12 git-submodule.sh: make "$cached" variable a booleanÆvar Arnfjörð Bjarmason, Jun 13, 2022
  36. 04/12 git-submodule.sh: remove unused top-level "--branch" argumentÆvar Arnfjörð Bjarmason, Jun 13, 2022
  37. Glen ChooJun 15, 2022
  38. 05/12 submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason, Jun 13, 2022
  39. Glen ChooJun 15, 2022
  40. 06/12 submodule update: remove "-v" optionÆvar Arnfjörð Bjarmason, Jun 13, 2022
  41. Glen ChooJun 15, 2022
  42. 09/12 submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason, Jun 13, 2022
  43. 08/12 submodule--helper: report "submodule" as our name in "-h" outputÆvar Arnfjörð Bjarmason, Jun 13, 2022
  44. Glen ChooJun 15, 2022
  45. Glen ChooJun 15, 2022
  46. Ævar Arnfjörð BjarmasonJun 15, 2022
  47. 07/12 submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason, Jun 13, 2022
  48. 11/12 git-submodule.sh: use "$quiet", not "$GIT_QUIET"Ævar Arnfjörð Bjarmason, Jun 13, 2022
  49. 12/12 git-sh-setup.sh: remove "say" function, change last usersÆvar Arnfjörð Bjarmason, Jun 13, 2022
  50. Glen ChooJun 15, 2022
  51. 10/12 submodule--helper: eliminate internal "--update" optionÆvar Arnfjörð Bjarmason, Jun 13, 2022
  52. Glen ChooJun 15, 2022
  53. Glen ChooJun 13, 2022
  54. Ævar Arnfjörð BjarmasonJun 13, 2022
  55. Glen ChooJun 15, 2022
  56. Glen ChooJun 15, 2022
  57. 00/12 submodule: make "git submodule--helper" behave like "git submodule"Ævar Arnfjörð Bjarmason, Jun 22, 2022
  58. 12/12 git-sh-setup.sh: remove "say" function, change last usersÆvar Arnfjörð Bjarmason, Jun 22, 2022
  59. 01/12 git-submodule.sh: remove unused sanitize_submodule_env()Ævar Arnfjörð Bjarmason, Jun 22, 2022
  60. 02/12 git-submodule.sh: remove unused $prefix var and --super-prefixÆvar Arnfjörð Bjarmason, Jun 22, 2022
  61. Glen ChooJun 22, 2022
  62. Ævar Arnfjörð BjarmasonJun 24, 2022
  63. Glen ChooJun 24, 2022
  64. 03/12 git-submodule.sh: make the "$cached" variable a booleanÆvar Arnfjörð Bjarmason, Jun 22, 2022
  65. 04/12 git-submodule.sh: remove unused top-level "--branch" argumentÆvar Arnfjörð Bjarmason, Jun 22, 2022
  66. 06/12 submodule update: remove "-v" optionÆvar Arnfjörð Bjarmason, Jun 22, 2022
  67. 05/12 submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason, Jun 22, 2022
  68. 10/12 submodule--helper: eliminate internal "--update" optionÆvar Arnfjörð Bjarmason, Jun 22, 2022
  69. 09/12 submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason, Jun 22, 2022
  70. Glen ChooJun 22, 2022
  71. Glen ChooJun 22, 2022
  72. 08/12 submodule--helper: report "submodule" as our name in some "-h" outputÆvar Arnfjörð Bjarmason, Jun 22, 2022
  73. Glen ChooJun 22, 2022
  74. 07/12 submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason, Jun 22, 2022
  75. 11/12 git-submodule.sh: use "$quiet", not "$GIT_QUIET"Ævar Arnfjörð Bjarmason, Jun 22, 2022
  76. Glen ChooJun 24, 2022
  77. 00/12 submodule: make "git submodule--helper" behave like "git submodule"Ævar Arnfjörð Bjarmason, Jun 28, 2022
  78. 01/12 git-submodule.sh: remove unused sanitize_submodule_env()Ævar Arnfjörð Bjarmason, Jun 28, 2022
  79. 02/12 git-submodule.sh: remove unused $prefix variableÆvar Arnfjörð Bjarmason, Jun 28, 2022
  80. 03/12 git-submodule.sh: make the "$cached" variable a booleanÆvar Arnfjörð Bjarmason, Jun 28, 2022
  81. 08/12 submodule--helper: report "submodule" as our name in some "-h" outputÆvar Arnfjörð Bjarmason, Jun 28, 2022
  82. 07/12 submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason, Jun 28, 2022
  83. 06/12 submodule update: remove "-v" optionÆvar Arnfjörð Bjarmason, Jun 28, 2022
  84. 12/12 git-sh-setup.sh: remove "say" function, change last usersÆvar Arnfjörð Bjarmason, Jun 28, 2022
  85. 04/12 git-submodule.sh: remove unused top-level "--branch" argumentÆvar Arnfjörð Bjarmason, Jun 28, 2022
  86. 05/12 submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason, Jun 28, 2022
  87. 10/12 submodule--helper: eliminate internal "--update" optionÆvar Arnfjörð Bjarmason, Jun 28, 2022
  88. 11/12 git-submodule.sh: use "$quiet", not "$GIT_QUIET"Ævar Arnfjörð Bjarmason, Jun 28, 2022
  89. 09/12 submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason, Jun 28, 2022
  90. Glen ChooJun 28, 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.