From 85775255f18e0a6a6b2e65394bc18ec440dba99d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:23 +0200 Subject: [PATCH 01/12] git-submodule.sh: remove unused sanitize_submodule_env() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sanitize_submodule_env() function was last used before b3c5f5cb048 (submodule: move core cmd_update() logic to C, 2022-03-15), let's remove it. This also allows us to remove clear_local_git_env() from git-sh-setup.sh. That function hasn't been documented in Documentation/git-sh-setup.sh, and since 14111fc4927 (git: submodule honor -c credential.* from command line, 2016-02-29) it had only been used in the sanitize_submodule_env() function being removed here. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-sh-setup.sh | 7 ------- git-submodule.sh | 11 ----------- 2 files changed, 18 deletions(-) diff --git a/git-sh-setup.sh b/git-sh-setup.sh index d92df37e99..ecb60d9e3c 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -285,13 +285,6 @@ get_author_ident_from_commit () { parse_ident_from_commit author AUTHOR } -# Clear repo-local GIT_* environment variables. Useful when switching to -# another repository (e.g. when entering a submodule). See also the env -# list in git_connect() -clear_local_git_env() { - unset $(git rev-parse --local-env-vars) -} - # Generate a virtual base file for a two-file merge. Uses git apply to # remove lines from $1 that are not in $2, leaving only common lines. create_virtual_base() { diff --git a/git-submodule.sh b/git-submodule.sh index fd0b4a2c94..bc436c4ca4 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -56,17 +56,6 @@ isnumber() n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1" } -# Sanitize the local git environment for use within a submodule. We -# can't simply use clear_local_git_env since we want to preserve some -# of the settings from GIT_CONFIG_PARAMETERS. -sanitize_submodule_env() -{ - save_config=$GIT_CONFIG_PARAMETERS - clear_local_git_env - GIT_CONFIG_PARAMETERS=$save_config - export GIT_CONFIG_PARAMETERS -} - # # Add a new submodule to the working tree, .gitmodules and the index # From 960fad98e8a8d4c123bb5040f616f856cb9925ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:24 +0200 Subject: [PATCH 02/12] git-submodule.sh: remove unused $prefix variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the $prefix variable which isn't used anymore, and hasn't been since b3c5f5cb048 (submodule: move core cmd_update() logic to C, 2022-03-15). Before that we'd use it to invoke "git submodule--helper" with the "--recursive-prefix" option, but since b3c5f5cb048 that "git submodule--helper" option is only used when it invokes itself. So the "--recursive-prefix" option is still in use, but at this point only when the helper invokes itself during submodule recursion. See the "--recursive-prefix" option added in c51f8f94e5b (submodule--helper: run update procedures from C, 2021-08-24). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-submodule.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index bc436c4ca4..53847bbf6e 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -41,7 +41,6 @@ files= remote= nofetch= update= -prefix= custom_name= depth= progress= @@ -127,7 +126,7 @@ cmd_add() usage fi - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@" } # @@ -189,7 +188,7 @@ cmd_init() shift done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@" } # @@ -346,7 +345,6 @@ cmd_update() ${init:+--init} \ ${nofetch:+--no-fetch} \ ${wt_prefix:+--prefix "$wt_prefix"} \ - ${prefix:+--recursive-prefix "$prefix"} \ ${update:+--update "$update"} \ ${reference:+"$reference"} \ ${dissociate:+"--dissociate"} \ From 757d0927976cf9d59a050f8f9ce1fe54d4dff653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:25 +0200 Subject: [PATCH 03/12] git-submodule.sh: make the "$cached" variable a boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the assignment of "$1" to the "$cached" variable. As seen in the initial implementation in 70c7ac22de6 (Add git-submodule command, 2007-05-26) we only need to keep track of if we've seen the --cached option, not save the "--cached" string for later use. In 28f9af5d25e (git-submodule summary: code framework, 2008-03-11) "$1" was assigned to it, but since there was no reason to do so let's stop doing it. This trivial change will make it easier to reason about an eventual change that'll remove the cmd_summary() function in favor of dispatching to "git submodule--helper summary" directly. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-submodule.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 53847bbf6e..b99a00d9f8 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -446,7 +446,7 @@ cmd_summary() { do case "$1" in --cached) - cached="$1" + cached=1 ;; --files) files="$1" @@ -583,7 +583,7 @@ do branch="$2"; shift ;; --cached) - cached="$1" + cached=1 ;; --) break From da3aae9e8476af3b23363d39dba86b679c14a498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:26 +0200 Subject: [PATCH 04/12] git-submodule.sh: remove unused top-level "--branch" argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 5c08dbbdf1a (git-submodule: fix subcommand parser, 2008-01-15) the "--branch" option was supported as an option to "git submodule" itself, i.e. "git submodule --branch" as a side-effect of its implementation. Then in b57e8119e6e (submodule: teach set-branch subcommand, 2019-02-08) when the "set-branch" subcommand was added the assertion that we shouldn't have "--branch" anywhere except as an argument to "add" and "set-branch" was copy/pasted from the adjacent check for "--cache" added (or rather modified) in 496eeeb19b9 (git-submodule.sh: avoid "test -a/-o ", 2014-06-10). But there's been a logic error in that check, which at a glance looked like it should be supporting: git submodule --branch (add | set-branch) [] But due to "||" in the condition (as opposed to "&&" for "--cache") if we have "--branch" here already we'll emit usage, even for "add" and "set-branch". So in addition to never having documented this form, it hasn't worked since b57e8119e6e was released with v2.22.0. So it's safe to remove this code. I.e. we don't want to support the form noted above, but only: git submodule (add | set-branch) --branch [] Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-submodule.sh | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index b99a00d9f8..20fc1b620f 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -574,14 +574,6 @@ do -q|--quiet) GIT_QUIET=1 ;; - -b|--branch) - case "$2" in - '') - usage - ;; - esac - branch="$2"; shift - ;; --cached) cached=1 ;; @@ -609,12 +601,6 @@ then fi fi -# "-b branch" is accepted only by "add" and "set-branch" -if test -n "$branch" && (test "$command" != add || test "$command" != set-branch) -then - usage -fi - # "--cached" is accepted only by "status" and "summary" if test -n "$cached" && test "$command" != status && test "$command" != summary then From d9c7f69aaa6b001949e9d2b693c22c595cc9d0d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:27 +0200 Subject: [PATCH 05/12] submodule--helper: have --require-init imply --init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjust code added in 0060fd1511b (clone --recurse-submodules: prevent name squatting on Windows, 2019-09-12) to have the internal --require-init option imply --init, rather than having "git-submodule.sh" add it implicitly. This change doesn't make any difference now, but eliminates another special-case where "git submodule--helper update"'s behavior was different from "git submodule update". This will make it easier to eventually replace the cmd_update() function in git-submodule.sh. We'll still need to keep the distinction between "--init" and "--require-init" in git-submodule.sh. Once cmd_update() gets re-implemented in C we'll be able to change variables and other code related to that, but not yet. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 5 ++++- git-submodule.sh | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5c77dfcffe..f0702d0cfa 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2618,7 +2618,7 @@ static int module_update(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "progress", &opt.progress, N_("force cloning progress")), OPT_BOOL(0, "require-init", &opt.require_init, - N_("disallow cloning into non-empty directory")), + N_("disallow cloning into non-empty directory, implies --init")), OPT_BOOL(0, "single-branch", &opt.single_branch, N_("clone only one branch, HEAD or --branch")), OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options), @@ -2642,6 +2642,9 @@ static int module_update(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, module_update_options, git_submodule_helper_usage, 0); + if (opt.require_init) + opt.init = 1; + if (filter_options.choice && !opt.init) { usage_with_options(git_submodule_helper_usage, module_update_options); diff --git a/git-submodule.sh b/git-submodule.sh index 20fc1b620f..5b9683bf76 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -251,7 +251,6 @@ cmd_update() init=1 ;; --require-init) - init=1 require_init=1 ;; --remote) From 0d68ee723e54330138450f29e358e5ebe1a47aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:28 +0200 Subject: [PATCH 06/12] submodule update: remove "-v" option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In e84c3cf3dc3 (git-submodule.sh: accept verbose flag in cmd_update to be non-quiet, 2018-08-14) the "git submodule update" sub-command was made to understand "-v", but the option was never documented. The only in-tree user has been this test added in 3ad0401e9e6 (submodule update: silence underlying merge/rebase with "--quiet", 2020-09-30), it wasn't per-se testing --quiet, but fixing a bug in e84c3cf3dc3: It used to set "GIT_QUIET=0" instead of unsetting it on "-v", and thus we'd end up passing "--quiet" to "git submodule--helper" on "-v", since the "--quiet" option was passed using the ${parameter:+word} construct. Furthermore, even if someone had used the "-v" option they'd only be getting the default output. Our default in both git-submodule.sh and "git submodule--helper" has been to be "verbose", so the only way this option could have matter is if it were used as e.g.: git submodule --quiet update -v [...] I.e. to undo the effect of a previous "--quiet" on the command-line. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-submodule.sh | 3 --- t/t7406-submodule-update.sh | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 5b9683bf76..0df6b0fc97 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -241,9 +241,6 @@ cmd_update() -q|--quiet) GIT_QUIET=1 ;; - -v) - unset GIT_QUIET - ;; --progress) progress=1 ;; diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 43f779d751..06d804e213 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -1074,7 +1074,7 @@ test_expect_success 'submodule update --quiet passes quietness to merge/rebase' git submodule update --rebase --quiet >out 2>err && test_must_be_empty out && test_must_be_empty err && - git submodule update --rebase -v >out 2>err && + git submodule update --rebase >out 2>err && test_file_not_empty out && test_must_be_empty err ) From 6e556c412e9283cfa9f9be4bfe4e9c813a53bf52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:29 +0200 Subject: [PATCH 07/12] submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the "absorb-git-dirs" subcommand to "absorbgitdirs", which is what the "git submodule" command itself has called it since the subcommand was implemented in f6f85861400 (submodule: add absorb-git-dir function, 2016-12-12). Having these two be different will make it more tedious to dispatch to eventually dispatch "git submodule--helper" directly, as we'd need to retain this name mapping. So let's get rid of this needless inconsistency. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 4 ++-- git-submodule.sh | 2 +- submodule.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index f0702d0cfa..1a84ae8efd 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -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 absorb-git-dirs [] [...]"), + N_("git submodule--helper absorbgitdirs [] [...]"), NULL }; @@ -3389,7 +3389,7 @@ static struct cmd_struct commands[] = { {"deinit", module_deinit, 0}, {"summary", module_summary, SUPPORT_SUPER_PREFIX}, {"push-check", push_check, 0}, - {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, + {"absorbgitdirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, {"is-active", is_active, 0}, {"check-name", check_name, 0}, {"config", module_config, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index 0df6b0fc97..1c1dc32092 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -552,7 +552,7 @@ cmd_sync() cmd_absorbgitdirs() { - git submodule--helper absorb-git-dirs --prefix "$wt_prefix" "$@" + git submodule--helper absorbgitdirs --prefix "$wt_prefix" "$@" } # This loop parses the command line arguments to find the diff --git a/submodule.c b/submodule.c index 4e299f578f..2af16c647d 100644 --- a/submodule.c +++ b/submodule.c @@ -2374,7 +2374,7 @@ void absorb_git_dir_into_superproject(const char *path, cp.no_stdin = 1; strvec_pushl(&cp.args, "--super-prefix", sb.buf, "submodule--helper", - "absorb-git-dirs", NULL); + "absorbgitdirs", NULL); prepare_submodule_repo_env(&cp.env); if (run_command(&cp)) die(_("could not recurse into submodule '%s'"), path); From 36d45163b6de05886c2c6feb2e626ce423b96b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:30 +0200 Subject: [PATCH 08/12] submodule--helper: report "submodule" as our name in some "-h" output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the user-facing "git submodule--helper" commands so that they'll report their name as being "git submodule". To a user these commands are internal implementation details, and it doesn't make sense to emit usage about an internal helper when "git submodule" is invoked with invalid options. Before this we'd emit e.g.: $ git submodule absorbgitdirs --blah error: unknown option `blah' usage: git submodule--helper absorbgitdirs [] [...] [...] And: $ git submodule set-url -- -- usage: git submodule--helper set-url [--quiet] [...] Now we'll start with "usage: git submodule [...]" in both of those cases. This change does not alter the "list", "name", "clone", "config" and "create-branch" commands, those are internal-only (as an aside; their usage info should probably invoke BUG(...)). This only changes the user-facing commands. The "status", "deinit" and "update" commands are not included in this change, because their usage information already used "submodule" rather than "submodule--helper". I don't think it's currently possible to emit some of this usage information in practice, as git-submodule.sh will catch unknown options, and e.g. it doesn't seem to be possible to get "add" to emit its usage information from "submodule--helper". Though that change may be superfluous now, it's also harmless, and will allow us to eventually dispatch further into "git submodule--helper" from git-submodule.sh, while emitting the correct usage output. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1a84ae8efd..04d2620fce 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -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] [--] "), + N_("git submodule foreach [--quiet] [--recursive] [--] "), 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 [] []"), + N_("git submodule init [] []"), NULL }; @@ -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 [] [] [--] []"), + N_("git submodule summary [] [] [--] []"), 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] []"), + N_("git submodule sync [--quiet] [--recursive] []"), NULL }; @@ -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 [] [...]"), + N_("git submodule absorbgitdirs [] [...]"), 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] "), + N_("git submodule set-url [--quiet] "), 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) "), - N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) "), + N_("git submodule set-branch [-q|--quiet] (-d|--default) "), + N_("git submodule set-branch [-q|--quiet] (-b|--branch) "), 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 [] [--] []"), + N_("git submodule add [] [--] []"), NULL }; From 8f12108c2951cdfa181d6be66b6def28cd007bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:31 +0200 Subject: [PATCH 09/12] submodule--helper: understand --checkout, --merge and --rebase synonyms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Understand --checkout, --merge and --rebase synonyms for --update={checkout,merge,rebase}, as well as the short options that 'git submodule' itself understands. This removes a difference between the CLI API of "git submodule" and "git submodule--helper", making it easier to make the latter an alias for the former. See 48308681b07 (git submodule update: have a dedicated helper for cloning, 2016-02-29) for the initial addition of --update. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 30 ++++++++++++++++++++++++++++++ git-submodule.sh | 14 +++++++++----- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 04d2620fce..53179472d8 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2404,6 +2404,23 @@ static void ensure_core_worktree(const char *path) } } +static const char *submodule_update_type_to_label(enum submodule_update_type type) +{ + switch (type) { + case SM_UPDATE_CHECKOUT: + return "checkout"; + case SM_UPDATE_MERGE: + return "merge"; + case SM_UPDATE_REBASE: + return "rebase"; + case SM_UPDATE_UNSPECIFIED: + case SM_UPDATE_NONE: + case SM_UPDATE_COMMAND: + break; + } + BUG("unreachable with type %d", type); +} + static void update_data_to_args(struct update_data *update_data, struct strvec *args) { strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL); @@ -2582,6 +2599,7 @@ 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), @@ -2603,6 +2621,15 @@ static int module_update(int argc, const char **argv, const char *prefix) OPT_STRING(0, "update", &opt.update_default, N_("string"), N_("rebase, merge, checkout or none")), + OPT_SET_INT(0, "checkout", &update_type, + N_("use the 'checkout' update strategy (default)"), + SM_UPDATE_CHECKOUT), + OPT_SET_INT('m', "merge", &update_type, + N_("use the 'merge' update strategy"), + SM_UPDATE_MERGE), + OPT_SET_INT('r', "rebase", &update_type, + N_("use the 'rebase' update strategy"), + SM_UPDATE_REBASE), OPT_STRING_LIST(0, "reference", &opt.references, N_("repo"), N_("reference repository")), OPT_BOOL(0, "dissociate", &opt.dissociate, @@ -2652,6 +2679,9 @@ 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) diff --git a/git-submodule.sh b/git-submodule.sh index 1c1dc32092..7fc7119fb2 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -40,7 +40,9 @@ require_init= files= remote= nofetch= -update= +rebase= +merge= +checkout= custom_name= depth= progress= @@ -260,7 +262,7 @@ cmd_update() force=$1 ;; -r|--rebase) - update="rebase" + rebase=1 ;; --reference) case "$2" in '') usage ;; esac @@ -274,13 +276,13 @@ cmd_update() dissociate=1 ;; -m|--merge) - update="merge" + merge=1 ;; --recursive) recursive=1 ;; --checkout) - update="checkout" + checkout=1 ;; --recommend-shallow) recommend_shallow="--recommend-shallow" @@ -341,7 +343,9 @@ cmd_update() ${init:+--init} \ ${nofetch:+--no-fetch} \ ${wt_prefix:+--prefix "$wt_prefix"} \ - ${update:+--update "$update"} \ + ${rebase:+--rebase} \ + ${merge:+--merge} \ + ${checkout:+--checkout} \ ${reference:+"$reference"} \ ${dissociate:+"--dissociate"} \ ${depth:+"$depth"} \ From b788fc671bf1862a72f9fe7256affd7d152b8a6f Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 28 Jun 2022 12:05:32 +0200 Subject: [PATCH 10/12] submodule--helper: eliminate internal "--update" option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- 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 53179472d8..389b900602 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); From 2eec463739745d2110aa5462ba9547fa8d255ebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:33 +0200 Subject: [PATCH 11/12] git-submodule.sh: use "$quiet", not "$GIT_QUIET" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the use of the "$GIT_QUIET" variable in favor of our own "$quiet", ever since b3c5f5cb048 (submodule: move core cmd_update() logic to C, 2022-03-15) we have not used the "say" function in git-sh-setup.sh, which is the only thing that's affected by using "GIT_QUIET". We still want to support --quiet for our own use though, but let's use our own variable for that. Now it's obvious that we only care about passing "--quiet" to "git submodule--helper", and not to change the output of any "say" invocation. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- git-submodule.sh | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 7fc7119fb2..5e5d21c010 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -30,6 +30,7 @@ GIT_PROTOCOL_FROM_USER=0 export GIT_PROTOCOL_FROM_USER command= +quiet= branch= force= reference= @@ -80,7 +81,7 @@ cmd_add() force=$1 ;; -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --progress) progress=1 @@ -128,7 +129,7 @@ cmd_add() usage fi - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper add ${quiet:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@" } # @@ -144,7 +145,7 @@ cmd_foreach() do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --recursive) recursive=1 @@ -159,7 +160,7 @@ cmd_foreach() shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper foreach ${quiet:+--quiet} ${recursive:+--recursive} -- "$@" } # @@ -174,7 +175,7 @@ cmd_init() do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --) shift @@ -190,7 +191,7 @@ cmd_init() shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${quiet:+--quiet} -- "$@" } # @@ -207,7 +208,7 @@ cmd_deinit() force=$1 ;; -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --all) deinit_all=t @@ -226,7 +227,7 @@ cmd_deinit() shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${force:+--force} ${deinit_all:+--all} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${quiet:+--quiet} ${force:+--force} ${deinit_all:+--all} -- "$@" } # @@ -241,7 +242,7 @@ cmd_update() do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --progress) progress=1 @@ -335,7 +336,7 @@ cmd_update() done git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update \ - ${GIT_QUIET:+--quiet} \ + ${quiet:+--quiet} \ ${force:+--force} \ ${progress:+"--progress"} \ ${remote:+--remote} \ @@ -396,7 +397,7 @@ cmd_set_branch() { shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-branch ${GIT_QUIET:+--quiet} ${branch:+--branch "$branch"} ${default:+--default} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-branch ${quiet:+--quiet} ${branch:+--branch "$branch"} ${default:+--default} -- "$@" } # @@ -409,7 +410,7 @@ cmd_set_url() { do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --) shift @@ -425,7 +426,7 @@ cmd_set_url() { shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-url ${GIT_QUIET:+--quiet} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper set-url ${quiet:+--quiet} -- "$@" } # @@ -496,7 +497,7 @@ cmd_status() do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --cached) cached=1 @@ -518,7 +519,7 @@ cmd_status() shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${quiet:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@" } # # Sync remote urls for submodules @@ -531,7 +532,7 @@ cmd_sync() do case "$1" in -q|--quiet) - GIT_QUIET=1 + quiet=1 shift ;; --recursive) @@ -551,7 +552,7 @@ cmd_sync() esac done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper sync ${quiet:+--quiet} ${recursive:+--recursive} -- "$@" } cmd_absorbgitdirs() @@ -572,7 +573,7 @@ do command=$1 ;; -q|--quiet) - GIT_QUIET=1 + quiet=1 ;; --cached) cached=1 From 5b893f7d81eb7feb43662ed8663e2af76a76b4c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 28 Jun 2022 12:05:34 +0200 Subject: [PATCH 12/12] git-sh-setup.sh: remove "say" function, change last users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the "say" function, with various rewrites of the remaining git-*.sh code to C and the preceding change to have git-submodule.sh stop using the GIT_QUIET variable there were only four uses in git-subtree.sh. Let's have it use an "arg_quiet" variable instead, and move the "say" function over to it. The only other use was a trivial message in git-instaweb.sh, since it has never supported the --quiet option (or similar) that code added in 0b624b4ceee (instaweb: restart server if already running, 2009-11-22) can simply use "echo" instead. The remaining in-tree hits from "say" are all for the sibling function defined in t/test-lib.sh. It's safe to remove this function since it has never been documented in Documentation/git-sh-setup.txt. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- contrib/subtree/git-subtree.sh | 15 ++++++++++++--- git-instaweb.sh | 2 +- git-sh-setup.sh | 9 --------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 1af1d9653e..7562a395c2 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -50,6 +50,14 @@ m,message= use the given message as the commit message for the merge commit indent=0 +# Usage: say [MSG...] +say () { + if test -z "$arg_quiet" + then + printf '%s\n' "$*" + fi +} + # Usage: debug [MSG...] debug () { if test -n "$arg_debug" @@ -60,7 +68,7 @@ debug () { # Usage: progress [MSG...] progress () { - if test -z "$GIT_QUIET" + if test -z "$arg_quiet" then if test -z "$arg_debug" then @@ -146,6 +154,7 @@ main () { eval "$set_args" # Begin "real" flag parsing. + arg_quiet= arg_debug= arg_prefix= arg_split_branch= @@ -161,7 +170,7 @@ main () { case "$opt" in -q) - GIT_QUIET=1 + arg_quiet=1 ;; -d) arg_debug=1 @@ -252,7 +261,7 @@ main () { dir="$(dirname "$arg_prefix/.")" debug "command: {$arg_command}" - debug "quiet: {$GIT_QUIET}" + debug "quiet: {$arg_quiet}" debug "dir: {$dir}" debug "opts: {$*}" debug diff --git a/git-instaweb.sh b/git-instaweb.sh index 4349566c89..c68f49454c 100755 --- a/git-instaweb.sh +++ b/git-instaweb.sh @@ -102,7 +102,7 @@ resolve_full_httpd () { start_httpd () { if test -f "$fqgitdir/pid"; then - say "Instance already running. Restarting..." + echo "Instance already running. Restarting..." stop_httpd fi diff --git a/git-sh-setup.sh b/git-sh-setup.sh index ecb60d9e3c..ce273fe0e4 100644 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -57,15 +57,6 @@ die_with_status () { exit "$status" } -GIT_QUIET= - -say () { - if test -z "$GIT_QUIET" - then - printf '%s\n' "$*" - fi -} - if test -n "$OPTIONS_SPEC"; then usage() { "$0" -h