Re: [PATCHv3 2/2] submodule: port init from shell to C

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> By having the `init` functionality in C, we can reference it easier
> from other parts in the code.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---

How faithful a conversion is this aiming to be?  For example, one
thing I noticed is that some messages that were originally given
with "say" and sent to the standard output, which is emitted to the
standard error with this rewrite.  I didn't read both patches
carefully, so there may be other discrepancies I didn't spot.

I think you would want to do this in three steps:

 - A faithful rewrite from shell to C;

 - s/printf/fprintf(stderr, / for some messages; and finally

 - Hiding of some messages under --quiet.

in the above order.

Thanks.

>  builtin/submodule--helper.c | 115 ++++++++++++++++++++++++++++++++++++++++++--
>  git-submodule.sh            |  39 +--------------
>  2 files changed, 111 insertions(+), 43 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 1484b36..4684f16 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -221,6 +221,115 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
>  	return 0;
>  }
>  
> +static int git_submodule_config(const char *var, const char *value, void *cb)
> +{
> +	return parse_submodule_config_option(var, value);
> +}
> +
> +static void init_submodule(const char *path, const char *prefix, int quiet)
> +{
> +	const struct submodule *sub;
> +	struct strbuf sb = STRBUF_INIT;
> +	char *url = NULL;
> +	const char *upd = NULL;
> +	const char *displaypath = relative_path(xgetcwd(), prefix, &sb);;
> +
> +	/* Only loads from .gitmodules, no overlay with .git/config */
> +	gitmodules_config();
> +
> +	sub = submodule_from_path(null_sha1, path);
> +
> +	/*
> +	 * Copy url setting when it is not set yet.
> +	 * To look up the url in .git/config, we must not fall back to
> +	 * .gitmodules, so look it up directly.
> +	 */
> +	strbuf_reset(&sb);
> +	strbuf_addf(&sb, "submodule.%s.url", sub->name);
> +	if (git_config_get_string(sb.buf, &url)) {
> +		url = xstrdup(sub->url);
> +
> +		if (!url)
> +			die(_("No url found for submodule path '%s' in .gitmodules"),
> +				displaypath);
> +
> +		/* Possibly a url relative to parent */
> +		if (starts_with_dot_dot_slash(url) ||
> +		    starts_with_dot_slash(url)) {
> +			char *remoteurl;
> +			char *remote = get_default_remote();
> +			struct strbuf remotesb = STRBUF_INIT;
> +			strbuf_addf(&remotesb, "remote.%s.url", remote);
> +			free(remote);
> +
> +			if (git_config_get_string(remotesb.buf, &remoteurl))
> +				/*
> +				 * The repository is its own
> +				 * authoritative upstream
> +				 */
> +				remoteurl = xgetcwd();
> +			url = relative_url(remoteurl, url, NULL);
> +			strbuf_release(&remotesb);
> +		}
> +
> +		if (git_config_set(sb.buf, url))
> +			die(_("Failed to register url for submodule path '%s'"),
> +			    displaypath);
> +		if (!quiet)
> +			fprintf(stderr, _("Submodule '%s' (%s) registered for path '%s'\n"),
> +				sub->name, url, displaypath);
> +		free(url);
> +	}
> +
> +	/* Copy "update" setting when it is not set yet */
> +	strbuf_reset(&sb);
> +	strbuf_addf(&sb, "submodule.%s.update", sub->name);
> +	if (git_config_get_string_const(sb.buf, &upd) && sub->update) {
> +		upd = sub->update;
> +		if (strcmp(sub->update, "checkout") &&
> +		    strcmp(sub->update, "rebase") &&
> +		    strcmp(sub->update, "merge") &&
> +		    strcmp(sub->update, "none")) {
> +			fprintf(stderr, _("warning: unknown update mode '%s' suggested for submodule '%s'\n"),
> +				upd, sub->name);
> +			upd = "none";
> +		}
> +		if (git_config_set(sb.buf, upd))
> +			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
> +	}
> +	strbuf_release(&sb);
> +}
> +
> +static int module_init(int argc, const char **argv, const char *prefix)
> +{
> +	int quiet = 0;
> +	int i;
> +
> +	struct option module_init_options[] = {
> +		OPT_STRING(0, "prefix", &prefix,
> +			   N_("path"),
> +			   N_("alternative anchor for relative paths")),
> +		OPT__QUIET(&quiet, "Suppress output for initialzing a submodule"),
> +		OPT_END()
> +	};
> +
> +	const char *const git_submodule_helper_usage[] = {
> +		N_("git submodule--helper init [<path>]"),
> +		NULL
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, module_init_options,
> +			     git_submodule_helper_usage, 0);
> +
> +	if (argc == 0)
> +		die(_("Pass at least one submodule"));
> +
> +	for (i = 0; i < argc; i++)
> +		init_submodule(argv[i], prefix, quiet);
> +
> +	return 0;
> +}
> +
>  struct module_list {
>  	const struct cache_entry **entries;
>  	int alloc, nr;
> @@ -466,11 +575,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> -static int git_submodule_config(const char *var, const char *value, void *cb)
> -{
> -	return parse_submodule_config_option(var, value);
> -}
> -
>  struct submodule_update_clone {
>  	/* states */
>  	int count;
> @@ -716,6 +820,7 @@ static struct cmd_struct commands[] = {
>  	{"update-clone", update_clone},
>  	{"resolve-relative-url", resolve_relative_url},
>  	{"resolve-relative-url-test", resolve_relative_url_test},
> +	{"init", module_init}
>  };
>  
>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 615ef9b..6fce0dc 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -398,45 +398,8 @@ cmd_init()
>  	while read mode sha1 stage sm_path
>  	do
>  		die_if_unmatched "$mode"
> -		name=$(git submodule--helper name "$sm_path") || exit
> -
> -		displaypath=$(relative_path "$sm_path")
> -
> -		# Copy url setting when it is not set yet
> -		if test -z "$(git config "submodule.$name.url")"
> -		then
> -			url=$(git config -f .gitmodules submodule."$name".url)
> -			test -z "$url" &&
> -			die "$(eval_gettext "No url found for submodule path '\$displaypath' in .gitmodules")"
> -
> -			# Possibly a url relative to parent
> -			case "$url" in
> -			./*|../*)
> -				url=$(git submodule--helper resolve-relative-url "$url") || exit
> -				;;
> -			esac
> -			git config submodule."$name".url "$url" ||
> -			die "$(eval_gettext "Failed to register url for submodule path '\$displaypath'")"
>  
> -			say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$displaypath'")"
> -		fi
> -
> -		# Copy "update" setting when it is not set yet
> -		if upd="$(git config -f .gitmodules submodule."$name".update)" &&
> -		   test -n "$upd" &&
> -		   test -z "$(git config submodule."$name".update)"
> -		then
> -			case "$upd" in
> -			checkout | rebase | merge | none)
> -				;; # known modes of updating
> -			*)
> -				echo >&2 "warning: unknown update mode '$upd' suggested for submodule '$name'"
> -				upd=none
> -				;;
> -			esac
> -			git config submodule."$name".update "$upd" ||
> -			die "$(eval_gettext "Failed to register update mode for submodule path '\$displaypath'")"
> -		fi
> +		git submodule--helper init ${GIT_QUIET:+--quiet} "$sm_path" || exit
>  	done
>  }