Re: [PATCH 1/4] submodule: implement `module_list` as a builtin helper

> Most of the submodule operations work on a set of submodules.
> Calculating and using this set is usually done via:
>
>         module_list "$@" | {
>             while read mode sha1 stage sm_path
>             do
>                  # the actual operation
>             done
>         }
>
> Currently the function `module_list` is implemented in the
> git-submodule.sh as a shell script wrapping a perl script.
> The rewrite is in C, such that it is faster and can later be
> easily adapted when other functions are rewritten in C.
>
> git-submodule.sh similar to the builtin commands will navigate
> to the top most directory of the repository and keeping the
> subdirectories as a variable. As the helper is called from
> within the git-submodule.sh script, we are already navigated
> to the root level, but the path arguments are stil relative
> to the subdirectory we were in when calling git-submodule.sh.
> That's why there is a `--prefix` option pointing to an alternative
> path where to anchor relative path arguments.

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> The same as yesterday evening, just an entry added to .gitignore.
>
> So we'll have a "git submodule--helper module_list" here.
>
>   .gitignore                  |   1 +
>   Makefile                    |   1 +
>   builtin.h                   |   1 +
>   builtin/submodule--helper.c | 111 ++++++++++++++++++++++++++++++++++++++++++++
>   git-submodule.sh            |  54 +++------------------
>   git.c                       |   1 +
>   6 files changed, 121 insertions(+), 48 deletions(-)
>   create mode 100644 builtin/submodule--helper.c
>
> diff --git a/.gitignore b/.gitignore
> index a685ec1..2a69ba0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -155,6 +155,7 @@
>   /git-status
>   /git-stripspace
>   /git-submodule
> +/git-submodule--helper
>   /git-svn
>   /git-symbolic-ref
>   /git-tag
> diff --git a/Makefile b/Makefile
> index 7efedbe..460d17a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -899,6 +899,7 @@ BUILTIN_OBJS += builtin/shortlog.o
>   BUILTIN_OBJS += builtin/show-branch.o
>   BUILTIN_OBJS += builtin/show-ref.o
>   BUILTIN_OBJS += builtin/stripspace.o
> +BUILTIN_OBJS += builtin/submodule--helper.o
>   BUILTIN_OBJS += builtin/symbolic-ref.o
>   BUILTIN_OBJS += builtin/tag.o
>   BUILTIN_OBJS += builtin/unpack-file.o
> diff --git a/builtin.h b/builtin.h
> index 839483d..924e6c4 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -119,6 +119,7 @@ extern int cmd_show(int argc, const char **argv, const char *prefix);
>   extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
>   extern int cmd_status(int argc, const char **argv, const char *prefix);
>   extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
> +extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);
>   extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
>   extern int cmd_tag(int argc, const char **argv, const char *prefix);
>   extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> new file mode 100644
> index 0000000..cb18ddf
> --- /dev/null
> +++ b/builtin/submodule--helper.c
> @@ -0,0 +1,111 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "parse-options.h"
> +#include "quote.h"
> +#include "pathspec.h"
> +#include "dir.h"
> +#include "utf8.h"
> +
> +static char *ps_matched;
> +static const struct cache_entry **ce_entries;
> +static int ce_alloc, ce_used;
> +static struct pathspec pathspec;
> +static const char *alternative_path;
> +
> +static void module_list_compute(int argc, const char **argv,
> +				const char *prefix,
> +				struct pathspec *pathspec)
> +{
> +	int i;
> +	char *max_prefix;
> +	int max_prefix_len;
> +	parse_pathspec(pathspec, 0,
> +		       PATHSPEC_PREFER_FULL |
> +		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
> +		       prefix, argv);
> +
> +	/* Find common prefix for all pathspec's */
> +	max_prefix = common_prefix(pathspec);
> +	max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
> +
> +	if (pathspec->nr)
> +		ps_matched = xcalloc(1, pathspec->nr);
> +
> +
> +	if (read_cache() < 0)
> +		die("index file corrupt");
> +
> +	for (i = 0; i < active_nr; i++) {
> +		const struct cache_entry *ce = active_cache[i];
> +
> +		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
> +				    max_prefix_len, ps_matched,
> +				    S_ISGITLINK(ce->ce_mode) | S_ISDIR(ce->ce_mode)))
> +			continue;
> +
> +		if (S_ISGITLINK(ce->ce_mode)) {
> +			ALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);
> +			ce_entries[ce_used++] = ce;
> +		}
> +	}
> +}
> +
> +static int module_list(int argc, const char **argv, const char *prefix)
> +{
> +	int i;
> +	struct string_list already_printed = STRING_LIST_INIT_NODUP;
> +
> +	struct option module_list_options[] = {
> +		OPT_STRING(0, "prefix", &alternative_path,
> +			   N_("path"),
> +			   N_("alternative anchor for relative paths")),
> +		OPT_END()
> +	};
> +
> +	static const char * const git_submodule_helper_usage[] = {
> +		N_("git submodule--helper module_list [--prefix=<path>] [<path>...]"),
> +		NULL
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, module_list_options,
> +			     git_submodule_helper_usage, 0);
> +
> +	module_list_compute(argc, argv, alternative_path
> +					? alternative_path
> +					: prefix, &pathspec);
> +
> +	if (ps_matched && report_path_error(ps_matched, &pathspec, prefix)) {
> +		printf("#unmatched\n");
> +		return 1;
> +	}
> +
> +	for (i = 0; i < ce_used; i++) {
> +		const struct cache_entry *ce = ce_entries[i];
> +
> +		if (string_list_has_string(&already_printed, ce->name))
> +			continue;
> +
> +		if (ce_stage(ce)) {
> +			printf("%06o %s U\t", ce->ce_mode, sha1_to_hex(null_sha1));
> +		} else {
> +			printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), ce_stage(ce));
> +		}
> +
> +		utf8_fprintf(stdout, "%s\n", ce->name);
> +
> +		string_list_insert(&already_printed, ce->name);
> +	}
> +	return 0;
> +}
> +
> +int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> +{
> +	if (argc < 2)
> +		goto usage;
> +
> +	if (!strcmp(argv[1], "module_list"))
> +		return module_list(argc - 1, argv + 1, prefix);
> +
> +usage:
> +	usage("git submodule--helper module_list\n");
> +}
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 36797c3..af9ecef 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -145,48 +145,6 @@ relative_path ()
>   	echo "$result$target"
>   }
>
> -#
> -# Get submodule info for registered submodules
> -# $@ = path to limit submodule list
> -#
> -module_list()
> -{
> -	eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
> -	(
> -		git ls-files -z --error-unmatch --stage -- "$@" ||
> -		echo "unmatched pathspec exists"
> -	) |
> -	@@PERL@@ -e '
> -	my %unmerged = ();
> -	my ($null_sha1) = ("0" x 40);
> -	my @out = ();
> -	my $unmatched = 0;
> -	$/ = "\0";
> -	while (<STDIN>) {
> -		if (/^unmatched pathspec/) {
> -			$unmatched = 1;
> -			next;
> -		}
> -		chomp;
> -		my ($mode, $sha1, $stage, $path) =
> -			/^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
> -		next unless $mode eq "160000";
> -		if ($stage ne "0") {
> -			if (!$unmerged{$path}++) {
> -				push @out, "$mode $null_sha1 U\t$path\n";
> -			}
> -			next;
> -		}
> -		push @out, "$_\n";
> -	}
> -	if ($unmatched) {
> -		print "#unmatched\n";
> -	} else {
> -		print for (@out);
> -	}
> -	'
> -}
> -
>   die_if_unmatched ()
>   {
>   	if test "$1" = "#unmatched"
> @@ -532,7 +490,7 @@ cmd_foreach()
>   	# command in the subshell (and a recursive call to this function)
>   	exec 3<&0
>
> -	module_list |
> +	git submodule--helper module_list --prefix "$wt_prefix"|
>   	while read mode sha1 stage sm_path
>   	do
>   		die_if_unmatched "$mode"
> @@ -592,7 +550,7 @@ cmd_init()
>   		shift
>   	done
>
> -	module_list "$@" |
> +	git submodule--helper module_list --prefix "$wt_prefix" "$@" |
>   	while read mode sha1 stage sm_path
>   	do
>   		die_if_unmatched "$mode"
> @@ -674,7 +632,7 @@ cmd_deinit()
>   		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
>   	fi
>
> -	module_list "$@" |
> +	git submodule--helper module_list --prefix "$wt_prefix" "$@" |
>   	while read mode sha1 stage sm_path
>   	do
>   		die_if_unmatched "$mode"
> @@ -790,7 +748,7 @@ cmd_update()
>   	fi
>
>   	cloned_modules=
> -	module_list "$@" | {
> +	git submodule--helper module_list --prefix "$wt_prefix" "$@" | {
>   	err=
>   	while read mode sha1 stage sm_path
>   	do
> @@ -1222,7 +1180,7 @@ cmd_status()
>   		shift
>   	done
>
> -	module_list "$@" |
> +	git submodule--helper module_list --prefix "$wt_prefix" "$@" |
>   	while read mode sha1 stage sm_path
>   	do
>   		die_if_unmatched "$mode"
> @@ -1299,7 +1257,7 @@ cmd_sync()
>   		esac
>   	done
>   	cd_to_toplevel
> -	module_list "$@" |
> +	git submodule--helper module_list --prefix "$wt_prefix" "$@" |
>   	while read mode sha1 stage sm_path
>   	do
>   		die_if_unmatched "$mode"
> diff --git a/git.c b/git.c
> index 55c327c..deecba0 100644
> --- a/git.c
> +++ b/git.c
> @@ -469,6 +469,7 @@ static struct cmd_struct commands[] = {
>   	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>   	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
>   	{ "stripspace", cmd_stripspace },
> +	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP },
>   	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
>   	{ "tag", cmd_tag, RUN_SETUP },
>   	{ "unpack-file", cmd_unpack_file, RUN_SETUP },
>