[RFC PATCH 19/20] submodule: make it a built-in, remove git-submodule.sh

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 10, 2022, 02:01 UTC
Message-ID: <RFC-patch-19.20-1423950de08-20220610T011725Z-avarab@gmail.com>
In-Reply-To: <RFC-cover-00.20-00000000000-20220610T011725Z-avarab@gmail.com>

Replace the now-trivial git-submodule.sh script with a built-in builtin/submodule.c. For now this new command is only a dumb dispatcher that uses run-command.c to invoke "git submodule--helper", just as "git-submodule.sh" used to do.

This is obviously not ideal, and we should follow-up and merge the builtin/submodule--helper.c code into builtin/submodule.c, but doing it this way makes it easy to review that this new C implementation isn't doing anything more clever than the old shellscript implementation.

The "define BUILTIN_" macros will help with that, i.e. the usage information we emit can be merged with what builtin/submodule--helper.c is now emitting. See 8757b35d443 (commit-graph: define common usage with a macro, 2021-08-23) and 1e91d3faf6c (reflog: move "usage" variables and use macros, 2022-03-17) for prior art using this pattern.

The "(argc < 2 || !strcmp(argv[1], "-h"))" path at the top of cmd_submodule__helper() could now be a "(argc < 2)" if not for t0012-help.sh (which invokes all built-ins manually with "-h"). Let's leave it for now, eventually we'll consolidate the two.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile            |   2 +-
 builtin.h           |   1 +
 builtin/submodule.c | 138 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh    |  80 -------------------------
 git.c               |   1 +
 5 files changed, 141 insertions(+), 81 deletions(-)
 create mode 100644 builtin/submodule.c
 delete mode 100755 git-submodule.sh

diff --git a/Makefile b/Makefile
index 790382bd1e3..635ab791a8a 100644
--- a/Makefile
+++ b/Makefile
@@ -622,7 +622,6 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_LIB += git-mergetool--lib
@@ -1213,6 +1212,7 @@ BUILTIN_OBJS += builtin/show-ref.o
 BUILTIN_OBJS += builtin/sparse-checkout.o
 BUILTIN_OBJS += builtin/stash.o
 BUILTIN_OBJS += builtin/stripspace.o
+BUILTIN_OBJS += builtin/submodule.o
 BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
 BUILTIN_OBJS += builtin/tag.o
diff --git a/builtin.h b/builtin.h
index 40e9ecc8485..c8cf4a2ae3b 100644
--- a/builtin.h
+++ b/builtin.h
@@ -223,6 +223,7 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix);
 int cmd_status(int argc, const char **argv, const char *prefix);
 int cmd_stash(int argc, const char **argv, const char *prefix);
 int cmd_stripspace(int argc, const char **argv, const char *prefix);
+int cmd_submodule(int argc, const char **argv, const char *prefix);
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix);
 int cmd_switch(int argc, const char **argv, const char *prefix);
 int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/submodule.c b/builtin/submodule.c
new file mode 100644
index 00000000000..b777787169e
--- /dev/null
+++ b/builtin/submodule.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) 2007-2022 Lars Hjemli & others
+ * Copyright(c) 2022 Ævar Arnfjörð Bjarmason
+ */
+#include "builtin.h"
+#include "parse-options.h"
+#include "run-command.h"
+#include "strvec.h"
+
+#define BUILTIN_SUBMODULE_USAGE \
+	"git submodule [--quiet] [--cached]"
+
+#define BUILTIN_SUBMODULE_ADD_USAGE \
+	N_("git submodule [--quiet] add [-b <branch>] [-f | --force] [--name <name>]\n" \
+	   "              [--reference <repository>] [--] <repository> [<path>]")
+
+#define BUILTIN_SUBMODULE_STATUS_USAGE \
+	N_("git submodule [--quiet] status [--cached] [--recursive] [--] [<path>...]")
+
+#define BUILTIN_SUBMODULE_INIT_USAGE \
+	N_("git submodule [--quiet] init [--] [<path>...]")
+
+#define BUILTIN_SUBMODULE_DEINIT_USAGE \
+	N_("git submodule [--quiet] deinit [-f | --force] (--all | [--] <path>...)")
+
+#define BUILTIN_SUBMODULE_UPDATE_USAGE \
+	N_("git submodule [--quiet] update [-v] [--init [--filter=<filter-spec>]]\n" \
+	   "              [--remote] [-N | --no-fetch] [-f | --force] [--checkout |--merge | --rebase]\n" \
+	   "              [--[no-]recommend-shallow] [--reference <repository>] [--recursive]\n" \
+	   "              [--[no-]single-branch] [--] [<path>...]")
+
+#define BUILTIN_SUBMODULE_SET_BRANCH_USAGE \
+	N_("git submodule [--quiet] set-branch (--default | --branch <branch>) [--] <path>")
+
+#define BUILTIN_SUBMODULE_SET_URL_USAGE \
+	N_("git submodule [--quiet] set-url [--] <path> <newurl>")
+
+#define BUILTIN_SUBMODULE_SUMMARY_USAGE \
+	N_("git submodule [--quiet] summary [--cached | --files] [--summary-limit <n>]\n"  \
+	   "              [commit] [--] [<path>...]")
+#define BUILTIN_SUBMODULE_FOREACH_USAGE \
+	N_("git submodule [--quiet] foreach [--recursive] <command>")
+
+#define BUILTIN_SUBMODULE_SYNC_USAGE \
+	N_("git submodule [--quiet] sync [--recursive] [--] [<path>...]")
+
+#define BUILTIN_SUBMODULE_ABSORBGITDIRS_USAGE \
+	N_("git submodule [--quiet] absorbgitdirs [--] [<path>...]")
+
+static const char * const git_submodule_usage[] = {
+	BUILTIN_SUBMODULE_USAGE,
+	BUILTIN_SUBMODULE_ADD_USAGE,
+	BUILTIN_SUBMODULE_STATUS_USAGE,
+	BUILTIN_SUBMODULE_INIT_USAGE,
+	BUILTIN_SUBMODULE_DEINIT_USAGE,
+	BUILTIN_SUBMODULE_UPDATE_USAGE,
+	BUILTIN_SUBMODULE_SET_BRANCH_USAGE,
+	BUILTIN_SUBMODULE_SET_URL_USAGE,
+	BUILTIN_SUBMODULE_SUMMARY_USAGE,
+	BUILTIN_SUBMODULE_FOREACH_USAGE,
+	BUILTIN_SUBMODULE_SYNC_USAGE,
+	BUILTIN_SUBMODULE_ABSORBGITDIRS_USAGE,
+	NULL,
+};
+
+static void setup_helper_args(int argc, const char **argv, const char *prefix,
+			      int quiet, int cached, struct strvec *args)
+{
+	const char *cmd;
+	int do_quiet_cache = 1;
+	int do_prefix = 1;
+
+	strvec_push(args, "submodule--helper");
+
+	/* No command word defaults to "status" */
+	if (!argc) {
+		strvec_push(args, "status");
+		return;
+	}
+
+	/* Either a valid command, or submodule--helper will barf! */
+	cmd = argv[0];
+	strvec_push(args, cmd);
+	argv++;
+	argc--;
+
+	/* Options that need to go before user-supplied options */
+	if (!strcmp(cmd, "absorbgitdirs"))
+		do_quiet_cache = 0;
+	else if (!strcmp(cmd, "update"))
+		;
+	else
+		do_prefix = 0;
+	if (do_quiet_cache) {
+		if (quiet)
+			strvec_push(args, "--quiet");
+		if (cached)
+			strvec_push(args, "--cached");
+
+		if (prefix && do_prefix)
+			strvec_pushl(args, "--prefix", prefix, NULL);
+	}
+
+	/* All commands get argv, including a "--", if any */
+	strvec_pushv(args, argv);
+}
+
+int cmd_submodule(int argc, const char **argv, const char *prefix)
+{
+	int opt_quiet = 0;
+	int opt_cached = 0;
+	struct child_process cp = CHILD_PROCESS_INIT;
+	struct option options[] = {
+		OPT__QUIET(&opt_quiet, N_("be quiet")),
+		OPT_BOOL(0, "cached", &opt_cached,
+			 N_("print the OID of submodules")),
+		OPT_END()
+	};
+	int ret;
+
+	argc = parse_options(argc, argv, prefix, options, git_submodule_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	/*
+	 * Tell the rest of git that any URLs we get don't come
+	 * directly from the user, so it can apply policy as appropriate.
+	 */
+	strvec_push(&cp.env_array, "GIT_PROTOCOL_FROM_USER=0");
+	setup_helper_args(argc, argv, prefix, opt_quiet, opt_cached,
+			  &cp.args);
+
+	cp.git_cmd = 1;
+	cp.no_stdin = 0; /* for git submodule foreach */
+	cp.dir = startup_info->original_cwd;
+	ret = run_command(&cp);
+
+	return ret;
+}
diff --git a/git-submodule.sh b/git-submodule.sh
deleted file mode 100755
index 0274b253297..00000000000
--- a/git-submodule.sh
+++ /dev/null
@@ -1,80 +0,0 @@
-#!/bin/sh
-#
-# git-submodule.sh: add, init, update or list git submodules
-#
-# Copyright (c) 2007 Lars Hjemli
-
-dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] [--cached]
-   or: $dashless [--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
-   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
-   or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
-   or: $dashless [--quiet] update [-v] [--init [--filter=<filter-spec>]] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--[no-]single-branch] [--] [<path>...]
-   or: $dashless [--quiet] set-branch (--default|--branch <branch>) [--] <path>
-   or: $dashless [--quiet] set-url [--] <path> <newurl>
-   or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
-   or: $dashless [--quiet] foreach [--recursive] <command>
-   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
-   or: $dashless [--quiet] absorbgitdirs [--] [<path>...]"
-OPTIONS_SPEC=
-SUBDIRECTORY_OK=Yes
-. git-sh-setup
-require_work_tree
-wt_prefix=$(git rev-parse --show-prefix)
-cd_to_toplevel
-
-# Tell the rest of git that any URLs we get don't come
-# directly from the user, so it can apply policy as appropriate.
-GIT_PROTOCOL_FROM_USER=0
-export GIT_PROTOCOL_FROM_USER
-
-quiet=
-cached=
-
-while test $# != 0
-do
-	case "$1" in
-	-q|--quiet)
-		quiet=1 &&
-		shift
-		;;
-	--cached)
-		cached=1 &&
-		shift
-		;;
-	*)
-		break
-		;;
-	esac
-done
-
-# No command word defaults to "status"
-command=
-if test $# = 0
-then
-	command=status
-else
-	case "$1" in
-	add | foreach | init | deinit | update | set-branch | set-url | status | summary | sync | absorbgitdirs)
-		command=$1 &&
-		shift
-		;;
-	*)
-		usage
-	esac
-fi
-
-case "$command" in
-absorbgitdirs)
-	git submodule--helper "$command" --prefix "$wt_prefix" "$@"
-	;;
-update)
-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
-		${quiet:+--quiet} ${wt_prefix:+--prefix "$wt_prefix"} "$@"
-	;;
-*)
-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper "$command" \
-		${quiet:+--quiet} ${cached:+--cached} "$@"
-	;;
-esac
diff --git a/git.c b/git.c
index 5ff4f3e25b7..047fc262cd2 100644
--- a/git.c
+++ b/git.c
@@ -609,6 +609,7 @@ static struct cmd_struct commands[] = {
 	{ "stash", cmd_stash, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
+	{ "submodule", cmd_submodule, RUN_SETUP | NEED_WORK_TREE },
 	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
 	{ "switch", cmd_switch, RUN_SETUP | NEED_WORK_TREE },
 	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },

-- 
2.36.1.1178.gb5b1747c546