git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 2/2] submodule: Port init from shell to C
  2016-01-15 23:37 [PATCH 0/2] Port `git submodule init` " Stefan Beller
@ 2016-01-15 23:37 ` Stefan Beller
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-01-15 23:37 UTC (permalink / raw)
  To: git; +Cc: gitster, j6t, sunshine, Jens.Lehmann, Stefan Beller

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>
---
 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..c7bd38a 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)
+{
+	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); // as overlayed by .gitmodules
+
+		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);
+
+		printf(_("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);
+
+	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
 }
 
-- 
2.7.0.rc0.37.gb4a0220.dirty

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] submodule: port init from shell to C
  2016-02-12 23:39 [PATCH 0/2] Port `git submodule init` " Stefan Beller
@ 2016-02-12 23:39 ` Stefan Beller
  2016-02-18 20:46   ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-02-12 23:39 UTC (permalink / raw)
  To: git, gitster, jrnieder, Jens.Lehmann; +Cc: Stefan Beller

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>
---
 builtin/submodule--helper.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  39 +---------------
 submodule.c                 |  21 +++++++++
 submodule.h                 |   1 +
 4 files changed, 130 insertions(+), 38 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d1e9118..30e623a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -214,6 +214,112 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
 	return 0;
 }
 
+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;
+	char *cwd = xgetcwd();
+	const char *displaypath = relative_path(cwd, 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);
+			free(remoteurl);
+		}
+
+		if (git_config_set(sb.buf, url))
+			die(_("Failed to register url for submodule path '%s'"),
+			    displaypath);
+		if (!quiet)
+			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+				sub->name, url, displaypath);
+	}
+
+	/* 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_strategy.type != SM_UPDATE_UNSPECIFIED) {
+		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
+			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
+				sub->name);
+			upd = "none";
+		} else
+			upd = submodule_strategy_to_string(&sub->update_strategy);
+
+		if (git_config_set(sb.buf, upd))
+			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+	}
+	strbuf_release(&sb);
+	free(cwd);
+	free(url);
+}
+
+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;
@@ -709,6 +815,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
 }
 
diff --git a/submodule.c b/submodule.c
index 263cb2a..37d6b8d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -235,6 +235,27 @@ int parse_submodule_update_strategy(const char *value,
 	return 0;
 }
 
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (s->type) {
+	case SM_UPDATE_CHECKOUT:
+		return "checkout";
+	case SM_UPDATE_MERGE:
+		return "merge";
+	case SM_UPDATE_REBASE:
+		return "rebase";
+	case SM_UPDATE_NONE:
+		return "none";
+	case SM_UPDATE_UNSPECIFIED:
+		return NULL;
+	case SM_UPDATE_COMMAND:
+		strbuf_addf(&sb, "!%s", s->command);
+		return strbuf_detach(&sb, 0);
+	}
+	return NULL;
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
diff --git a/submodule.h b/submodule.h
index 7ef3775..ff4c4f3 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,7 @@ int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
-- 
2.7.1.292.g18a4ced.dirty

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] submodule: port init from shell to C
  2016-02-12 23:39 ` [PATCH 2/2] submodule: port init " Stefan Beller
@ 2016-02-18 20:46   ` Junio C Hamano
  2016-02-18 23:15     ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2016-02-18 20:46 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, jrnieder, Jens.Lehmann

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>
> ---
>  builtin/submodule--helper.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
>  git-submodule.sh            |  39 +---------------
>  submodule.c                 |  21 +++++++++
>  submodule.h                 |   1 +
>  4 files changed, 130 insertions(+), 38 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index d1e9118..30e623a 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -214,6 +214,112 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
>  	return 0;
>  }
>  
> +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;
> +	char *cwd = xgetcwd();
> +	const char *displaypath = relative_path(cwd, prefix, &sb);
> +
> +	/* Only loads from .gitmodules, no overlay with .git/config */
> +	gitmodules_config();

This feeds submodule_config() function with the contents of
".gitmodules".

> +	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);

I am assuming that this corresponds to these lines in the original
scripted version:

		url=$(git config -f .gitmodules submodule."$name".url)
		test -z "$url" &&
		die "$(eval_gettext "No url found for submodule path...

but what makes git_config_get_string() to read from ".gitmodules"
file?  Doesn't it read from $GIT_DIR/config & ~/.gitconfig instead?

> +		/* 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);
> +			free(remoteurl);

Does the code inside this block correspond to this single line in
the original?

	url=$(git submodule--helper resolve-relative-url "$url") || exit

It seems to be doing quite a different thing, though.

> +		}
> +
> +		if (git_config_set(sb.buf, url))
> +			die(_("Failed to register url for submodule path '%s'"),
> +			    displaypath);
> +		if (!quiet)
> +			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
> +				sub->name, url, displaypath);
> +	}
> +
> +	/* 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) &&



This part of the code is supposed to read from in-tree ".gitmodules"
and copy to $GIT_DIR/config (i.e. git_config_set() below), but
again, I am not sure what makes this read from ".gitmodules".

Puzzled.

> +	    sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {
> +		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
> +			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
> +				sub->name);
> +			upd = "none";
> +		} else
> +			upd = submodule_strategy_to_string(&sub->update_strategy);
> +
> +		if (git_config_set(sb.buf, upd))
> +			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
> +	}
> +	strbuf_release(&sb);
> +	free(cwd);
> +	free(url);
> +}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] submodule: port init from shell to C
  2016-02-18 20:46   ` Junio C Hamano
@ 2016-02-18 23:15     ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2016-02-18 23:15 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, jrnieder, Jens.Lehmann

Junio C Hamano <gitster@pobox.com> writes:

>> +	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);
>
> I am assuming that this corresponds to these lines in the original
> scripted version:
>
> 		url=$(git config -f .gitmodules submodule."$name".url)
> 		test -z "$url" &&
> 		die "$(eval_gettext "No url found for submodule path...
>
> but what makes git_config_get_string() to read from ".gitmodules"
> file?  Doesn't it read from $GIT_DIR/config & ~/.gitconfig instead?

I am starting to suspect that reading of ".gitmodules" in the
context of "git submodule" command is a good use case for the
configset API.  It wants to read variables from ".gitmodules" and
the regular configuration file, and cares about where they come
from, illustrated by this codepath.  Read URL from .gitmodules, do
something to it, and update the regular configuration file.  Read
Update from .gitmodules, do some verification, and selectively store
it in the regular configuration file.  There may be cases where you
want to check if a variable is in the regular configuration file
(i.e. read from there), see its value in ".gitmodules", and
conditionally update the regular configuration file (i.e. write into
it).  The configset API was designed to help implement this kind of
thing in a clean manner (i.e. initialize one, add ".gitmodules"
file, and then configset_get_* on it, without affecting what you
read and write with the regular config_get_*/config_set_* to the
regular configuration file).

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] submodule: port init from shell to C
  2016-03-15  0:15 [PATCH 0/2] Port `git submodule init` " Stefan Beller
@ 2016-03-15  0:15 ` Stefan Beller
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-03-15  0:15 UTC (permalink / raw)
  To: git; +Cc: gitster, jrnieder, j6t, pclouds, Stefan Beller

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>
---
 builtin/submodule--helper.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  39 +---------------
 submodule.c                 |  21 +++++++++
 submodule.h                 |   1 +
 4 files changed, 130 insertions(+), 38 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bc7cf87..d942463 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -216,6 +216,112 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
 	return 0;
 }
 
+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;
+	char *cwd = xgetcwd();
+	const char *displaypath = relative_path(cwd, 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);
+			free(remoteurl);
+		}
+
+		if (git_config_set(sb.buf, url))
+			die(_("Failed to register url for submodule path '%s'"),
+			    displaypath);
+		if (!quiet)
+			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+				sub->name, url, displaypath);
+	}
+
+	/* 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_strategy.type != SM_UPDATE_UNSPECIFIED) {
+		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
+			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
+				sub->name);
+			upd = "none";
+		} else
+			upd = submodule_strategy_to_string(&sub->update_strategy);
+
+		if (git_config_set(sb.buf, upd))
+			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+	}
+	strbuf_release(&sb);
+	free(cwd);
+	free(url);
+}
+
+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, N_("Suppress output for initializing 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;
@@ -724,6 +830,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 b3290f8..97a3097 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
 }
 
diff --git a/submodule.c b/submodule.c
index 4bd14de..458189c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -237,6 +237,27 @@ int parse_submodule_update_strategy(const char *value,
 	return 0;
 }
 
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (s->type) {
+	case SM_UPDATE_CHECKOUT:
+		return "checkout";
+	case SM_UPDATE_MERGE:
+		return "merge";
+	case SM_UPDATE_REBASE:
+		return "rebase";
+	case SM_UPDATE_NONE:
+		return "none";
+	case SM_UPDATE_UNSPECIFIED:
+		return NULL;
+	case SM_UPDATE_COMMAND:
+		strbuf_addf(&sb, "!%s", s->command);
+		return strbuf_detach(&sb, 0);
+	}
+	return NULL;
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
diff --git a/submodule.h b/submodule.h
index 3166608..8cdc4bc 100644
--- a/submodule.h
+++ b/submodule.h
@@ -38,6 +38,7 @@ int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
-- 
2.7.0.rc0.46.g8f16ed4.dirty

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] submodule: port init from shell to C
  2016-04-13  0:18 [PATCH 0/2] Port `git submodule init` " Stefan Beller
@ 2016-04-13  0:18 ` Stefan Beller
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-04-13  0:18 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, j6t, Stefan Beller

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>
---
 builtin/submodule--helper.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  39 +---------------
 submodule.c                 |  21 +++++++++
 submodule.h                 |   1 +
 4 files changed, 130 insertions(+), 38 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2ab3662..3078790 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -215,6 +215,112 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
 	return 0;
 }
 
+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;
+	char *cwd = xgetcwd();
+	const char *displaypath = relative_path(cwd, 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);
+			free(remoteurl);
+		}
+
+		if (git_config_set(sb.buf, url))
+			die(_("Failed to register url for submodule path '%s'"),
+			    displaypath);
+		if (!quiet)
+			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+				sub->name, url, displaypath);
+	}
+
+	/* 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_strategy.type != SM_UPDATE_UNSPECIFIED) {
+		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
+			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
+				sub->name);
+			upd = "none";
+		} else
+			upd = submodule_strategy_to_string(&sub->update_strategy);
+
+		if (git_config_set(sb.buf, upd))
+			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+	}
+	strbuf_release(&sb);
+	free(cwd);
+	free(url);
+}
+
+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, N_("Suppress output for initializing 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;
@@ -723,6 +829,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 b3290f8..97a3097 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
 }
 
diff --git a/submodule.c b/submodule.c
index 4bd14de..458189c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -237,6 +237,27 @@ int parse_submodule_update_strategy(const char *value,
 	return 0;
 }
 
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (s->type) {
+	case SM_UPDATE_CHECKOUT:
+		return "checkout";
+	case SM_UPDATE_MERGE:
+		return "merge";
+	case SM_UPDATE_REBASE:
+		return "rebase";
+	case SM_UPDATE_NONE:
+		return "none";
+	case SM_UPDATE_UNSPECIFIED:
+		return NULL;
+	case SM_UPDATE_COMMAND:
+		strbuf_addf(&sb, "!%s", s->command);
+		return strbuf_detach(&sb, 0);
+	}
+	return NULL;
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
diff --git a/submodule.h b/submodule.h
index 3166608..8cdc4bc 100644
--- a/submodule.h
+++ b/submodule.h
@@ -38,6 +38,7 @@ int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
-- 
2.5.0.264.gc776916.dirty

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 0/2] Port `submodule init` to C
@ 2016-04-14 18:18 Stefan Beller
  2016-04-14 18:18 ` [PATCH 1/2] submodule: port resolve_relative_url from shell " Stefan Beller
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Stefan Beller @ 2016-04-14 18:18 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, j6t, Stefan Beller

This is another round for sb/submodule-init.

Changes since last round:
* Also iterate over the submodules in the C helper. With that missing piece
  `git submodule init` is completely handled in C now except for the usage
  string and the command->subcommand selection. (i.e. when calling
  `git submodule init`, we still go through git.c -> git-submodule.sh
  -> submodule--helper.c, but we do not go back into the shell after parsing
  `init` and calling module_init for it.
* This applies on another base commit, such that we make use of the tests
  written in origin/sb/submodule-path-misc-bugs. (I am not sure if I have too
  many series in flight stomping on each other here)
* This time I actually fix what Ramsay was hinting at:
  strbuf_detach(&sb, NULL) instead of strbuf_detach(&sb, 0);

Where do these patches apply?
=============================

I ran the following commands for a new starting point of this series:

    git checkout --detach origin/sb/submodule-parallel-update
    git merge origin/sb/submodule-helper-clone-regression-fix
    git merge origin/sb/submodule-path-misc-bugs

The second merge produces 2 conflicts, which can be resolved like this:
(I am unsure about the second comment in strbuf.h though)
diff --cc builtin/fetch.c
index 5aa1c2d,e4639d8..0000000
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@@ -37,7 -37,8 +37,8 @@@ static int prune = -1; /* unspecified *
  static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
  static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  static int tags = TAGS_DEFAULT, unshallow, update_shallow;
 -static int max_children = 1;
 +static int max_children = -1;
+ static enum transport_family family;
  static const char *depth;
  static const char *upload_pack;
  static struct strbuf default_rla = STRBUF_INIT;
diff --cc strbuf.h
index d4f2aa1,f72fd14..0000000
--- a/strbuf.h
+++ b/strbuf.h
@@@ -387,15 -387,10 +387,16 @@@ extern ssize_t strbuf_read_file(struct 
  extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
  
  /**
 + * Write the whole content of the strbuf to the stream not stopping at
 + * NUL bytes.
 + */
 +extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);
 +
 +/**
-  * Read a line from a FILE *, overwriting the existing contents
-  * of the strbuf. The second argument specifies the line
-  * terminator character, typically `'\n'`.
+  * Read a line from a FILE *, overwriting the existing contents of
+  * the strbuf.  The strbuf_getline*() family of functions share
+  * this signature, but have different line termination conventions.
+  *
   * Reading stops after the terminator or at EOF.  The terminator
   * is removed from the buffer before returning.  Returns 0 unless
   * there was nothing left before EOF, in which case it returns `EOF`.

As sb/submodule-parallel-update and sb/submodule-helper-clone-regression-fix
both touch builtin/submodule--helper.c, so we need those.

We need origin/sb/submodule-path-misc-bugs as it tests `submodule add`
output.

Thanks,
Stefan

Stefan Beller (2):
  submodule: port resolve_relative_url from shell to C
  submodule: port init from shell to C

 builtin/submodule--helper.c | 322 +++++++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh            | 127 +----------------
 submodule.c                 |  21 +++
 submodule.h                 |   1 +
 t/t0060-path-utils.sh       |  43 ++++++
 5 files changed, 392 insertions(+), 122 deletions(-)

-- 
2.8.0.26.g0341e85

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] submodule: port resolve_relative_url from shell to C
  2016-04-14 18:18 [PATCH 0/2] Port `submodule init` to C Stefan Beller
@ 2016-04-14 18:18 ` Stefan Beller
  2016-04-14 19:35   ` Johannes Sixt
  2016-04-14 18:18 ` [PATCH 2/2] submodule: port init " Stefan Beller
  2016-04-14 18:26 ` [PATCH 0/2] Port `submodule init` " Junio C Hamano
  2 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-04-14 18:18 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, j6t, Stefan Beller

Later on we want to automatically call `git submodule init` from
other commands, such that the users don't have to initialize the
submodule themselves.  As these other commands are written in C
already, we'd need the init functionality in C, too.  The
`resolve_relative_url` function is a large part of that init
functionality, so start by porting this function to C.

To create the tests in t0060, the function `resolve_relative_url`
was temporarily enhanced to write all inputs and output to disk
when running the test suite. The added tests in this patch are
a small selection thereof.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c | 209 +++++++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh            |  81 +----------------
 t/t0060-path-utils.sh       |  43 +++++++++
 3 files changed, 255 insertions(+), 78 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 864dd18..46946b0 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -9,6 +9,211 @@
 #include "submodule-config.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "remote.h"
+#include "refs.h"
+#include "connect.h"
+
+static char *get_default_remote(void)
+{
+	char *dest = NULL, *ret;
+	unsigned char sha1[20];
+	struct strbuf sb = STRBUF_INIT;
+	const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
+
+	if (!refname)
+		die(_("No such ref: %s"), "HEAD");
+
+	/* detached HEAD */
+	if (!strcmp(refname, "HEAD"))
+		return xstrdup("origin");
+
+	if (!skip_prefix(refname, "refs/heads/", &refname))
+		die(_("Expecting a full ref name, got %s"), refname);
+
+	strbuf_addf(&sb, "branch.%s.remote", refname);
+	if (git_config_get_string(sb.buf, &dest))
+		ret = xstrdup("origin");
+	else
+		ret = dest;
+
+	strbuf_release(&sb);
+	return ret;
+}
+
+static int starts_with_dot_slash(const char *str)
+{
+	return str[0] == '.' && is_dir_sep(str[1]);
+}
+
+static int starts_with_dot_dot_slash(const char *str)
+{
+	return str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);
+}
+
+/*
+ * Returns 1 if it was the last chop before ':'.
+ */
+static int chop_last_dir(char **remoteurl, int is_relative)
+{
+	char *rfind = find_last_dir_sep(*remoteurl);
+	if (rfind) {
+		*rfind = '\0';
+		return 0;
+	}
+
+	rfind = strrchr(*remoteurl, ':');
+	if (rfind) {
+		*rfind = '\0';
+		return 1;
+	}
+
+	if (is_relative || !strcmp(".", *remoteurl))
+		die(_("cannot strip one component off url '%s'"),
+			*remoteurl);
+
+	free(*remoteurl);
+	*remoteurl = xstrdup(".");
+	return 0;
+}
+
+/*
+ * The `url` argument is the URL that navigates to the submodule origin
+ * repo. When relative, this URL is relative to the superproject origin
+ * URL repo. The `up_path` argument, if specified, is the relative
+ * path that navigates from the submodule working tree to the superproject
+ * working tree. Returns the origin URL of the submodule.
+ *
+ * Return either an absolute URL or filesystem path (if the superproject
+ * origin URL is an absolute URL or filesystem path, respectively) or a
+ * relative file system path (if the superproject origin URL is a relative
+ * file system path).
+ *
+ * When the output is a relative file system path, the path is either
+ * relative to the submodule working tree, if up_path is specified, or to
+ * the superproject working tree otherwise.
+ *
+ * NEEDSWORK: This works incorrectly on the domain and protocol part.
+ * remote_url      url              outcome          expectation
+ * http://a.com/b  ../c             http://a.com/c   as is
+ * http://a.com/b  ../../c          http://c         error out
+ * http://a.com/b  ../../../c       http:/c          error out
+ * http://a.com/b  ../../../../c    http:c           error out
+ * http://a.com/b  ../../../../../c    .:c           error out
+ * NEEDSWORK: Given how chop_last_dir() works, this function is broken
+ * when a local part has a colon in its path component, too.
+ */
+static char *relative_url(const char *remote_url,
+				const char *url,
+				const char *up_path)
+{
+	int is_relative = 0;
+	int colonsep = 0;
+	char *out;
+	char *remoteurl = xstrdup(remote_url);
+	struct strbuf sb = STRBUF_INIT;
+	size_t len = strlen(remoteurl);
+
+	if (is_dir_sep(remoteurl[len]))
+		remoteurl[len] = '\0';
+
+	if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
+		is_relative = 0;
+	else {
+		is_relative = 1;
+		/*
+		 * Prepend a './' to ensure all relative
+		 * remoteurls start with './' or '../'
+		 */
+		if (!starts_with_dot_slash(remoteurl) &&
+		    !starts_with_dot_dot_slash(remoteurl)) {
+			strbuf_reset(&sb);
+			strbuf_addf(&sb, "./%s", remoteurl);
+			free(remoteurl);
+			remoteurl = strbuf_detach(&sb, NULL);
+		}
+	}
+	/*
+	 * When the url starts with '../', remove that and the
+	 * last directory in remoteurl.
+	 */
+	while (url) {
+		if (starts_with_dot_dot_slash(url)) {
+			url += 3;
+			colonsep |= chop_last_dir(&remoteurl, is_relative);
+		} else if (starts_with_dot_slash(url))
+			url += 2;
+		else
+			break;
+	}
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
+	free(remoteurl);
+
+	if (starts_with_dot_slash(sb.buf))
+		out = xstrdup(sb.buf + 2);
+	else
+		out = xstrdup(sb.buf);
+	strbuf_reset(&sb);
+
+	if (!up_path || !is_relative)
+		return out;
+
+	strbuf_addf(&sb, "%s%s", up_path, out);
+	free(out);
+	return strbuf_detach(&sb, NULL);
+}
+
+static int resolve_relative_url(int argc, const char **argv, const char *prefix)
+{
+	char *remoteurl = NULL;
+	char *remote = get_default_remote();
+	const char *up_path = NULL;
+	char *res;
+	const char *url;
+	struct strbuf sb = STRBUF_INIT;
+
+	if (argc != 2 && argc != 3)
+		die("resolve-relative-url only accepts one or two arguments");
+
+	url = argv[1];
+	strbuf_addf(&sb, "remote.%s.url", remote);
+	free(remote);
+
+	if (git_config_get_string(sb.buf, &remoteurl))
+		/* the repository is its own authoritative upstream */
+		remoteurl = xgetcwd();
+
+	if (argc == 3)
+		up_path = argv[2];
+
+	res = relative_url(remoteurl, url, up_path);
+	puts(res);
+	free(res);
+	free(remoteurl);
+	return 0;
+}
+
+static int resolve_relative_url_test(int argc, const char **argv, const char *prefix)
+{
+	char *remoteurl, *res;
+	const char *up_path, *url;
+
+	if (argc != 4)
+		die("resolve-relative-url-test only accepts three arguments: <up_path> <remoteurl> <url>");
+
+	up_path = argv[1];
+	remoteurl = xstrdup(argv[2]);
+	url = argv[3];
+
+	if (!strcmp(up_path, "(null)"))
+		up_path = NULL;
+
+	res = relative_url(remoteurl, url, up_path);
+	puts(res);
+	free(res);
+	free(remoteurl);
+	return 0;
+}
 
 struct module_list {
 	const struct cache_entry **entries;
@@ -504,7 +709,9 @@ static struct cmd_struct commands[] = {
 	{"list", module_list},
 	{"name", module_name},
 	{"clone", module_clone},
-	{"update-clone", update_clone}
+	{"update-clone", update_clone},
+	{"resolve-relative-url", resolve_relative_url},
+	{"resolve-relative-url-test", resolve_relative_url_test},
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index 07290d0..2423d7c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -46,79 +46,6 @@ prefix=
 custom_name=
 depth=
 
-# The function takes at most 2 arguments. The first argument is the
-# URL that navigates to the submodule origin repo. When relative, this URL
-# is relative to the superproject origin URL repo. The second up_path
-# argument, if specified, is the relative path that navigates
-# from the submodule working tree to the superproject working tree.
-#
-# The output of the function is the origin URL of the submodule.
-#
-# The output will either be an absolute URL or filesystem path (if the
-# superproject origin URL is an absolute URL or filesystem path,
-# respectively) or a relative file system path (if the superproject
-# origin URL is a relative file system path).
-#
-# When the output is a relative file system path, the path is either
-# relative to the submodule working tree, if up_path is specified, or to
-# the superproject working tree otherwise.
-resolve_relative_url ()
-{
-	remote=$(get_default_remote)
-	remoteurl=$(git config "remote.$remote.url") ||
-		remoteurl=$(pwd) # the repository is its own authoritative upstream
-	url="$1"
-	remoteurl=${remoteurl%/}
-	sep=/
-	up_path="$2"
-
-	case "$remoteurl" in
-	*:*|/*)
-		is_relative=
-		;;
-	./*|../*)
-		is_relative=t
-		;;
-	*)
-		is_relative=t
-		remoteurl="./$remoteurl"
-		;;
-	esac
-
-	while test -n "$url"
-	do
-		case "$url" in
-		../*)
-			url="${url#../}"
-			case "$remoteurl" in
-			*/*)
-				remoteurl="${remoteurl%/*}"
-				;;
-			*:*)
-				remoteurl="${remoteurl%:*}"
-				sep=:
-				;;
-			*)
-				if test -z "$is_relative" || test "." = "$remoteurl"
-				then
-					die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")"
-				else
-					remoteurl=.
-				fi
-				;;
-			esac
-			;;
-		./*)
-			url="${url#./}"
-			;;
-		*)
-			break;;
-		esac
-	done
-	remoteurl="$remoteurl$sep${url%/}"
-	echo "${is_relative:+${up_path}}${remoteurl#./}"
-}
-
 # Resolve a path to be relative to another path.  This is intended for
 # converting submodule paths when git-submodule is run in a subdirectory
 # and only handles paths where the directory separator is '/'.
@@ -281,7 +208,7 @@ cmd_add()
 		die "$(gettext "Relative path can only be used from the toplevel of the working tree")"
 
 		# dereference source url relative to parent's url
-		realrepo=$(resolve_relative_url "$repo") || exit
+		realrepo=$(git submodule--helper resolve-relative-url "$repo") || exit
 		;;
 	*:*|/*)
 		# absolute url
@@ -485,7 +412,7 @@ cmd_init()
 			# Possibly a url relative to parent
 			case "$url" in
 			./*|../*)
-				url=$(resolve_relative_url "$url") || exit
+				url=$(git submodule--helper resolve-relative-url "$url") || exit
 				;;
 			esac
 			git config submodule."$name".url "$url" ||
@@ -1202,9 +1129,9 @@ cmd_sync()
 			# guarantee a trailing /
 			up_path=${up_path%/}/ &&
 			# path from submodule work tree to submodule origin repo
-			sub_origin_url=$(resolve_relative_url "$url" "$up_path") &&
+			sub_origin_url=$(git submodule--helper resolve-relative-url "$url" "$up_path") &&
 			# path from superproject work tree to submodule origin repo
-			super_config_url=$(resolve_relative_url "$url") || exit
+			super_config_url=$(git submodule--helper resolve-relative-url "$url") || exit
 			;;
 		*)
 			sub_origin_url="$url"
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 8532a02..2e62548 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -19,6 +19,13 @@ relative_path() {
 	"test \"\$(test-path-utils relative_path '$1' '$2')\" = '$expected'"
 }
 
+test_submodule_relative_url() {
+	test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" "
+		actual=\$(git submodule--helper resolve-relative-url-test '$1' '$2' '$3') &&
+		test \"\$actual\" = '$4'
+	"
+}
+
 test_git_path() {
 	test_expect_success "git-path $1 $2 => $3" "
 		$1 git rev-parse --git-path $2 >actual &&
@@ -298,4 +305,40 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
 
+test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
+test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
+test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
+test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
+test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
+test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
+test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
+test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
+test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
+
+test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
+test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
+test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
+test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
+test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
+test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
+test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
+test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$PWD/subsubsuper_update_r"
+test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$PWD/subsuper_update_r"
+test_submodule_relative_url "(null)" "$PWD/." "../." "$PWD/."
+test_submodule_relative_url "(null)" "$PWD" "./." "$PWD/."
+test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$PWD/repo"
+test_submodule_relative_url "(null)" "$PWD" "./å äö" "$PWD/å äö"
+test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$PWD/submodule"
+test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$PWD/submodule"
+test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$PWD/submodule_update_repo/."
+test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
+test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
+test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
+test_submodule_relative_url "(null)" "helper:://hostname/repo" "../subrepo" "helper:://hostname/subrepo"
+test_submodule_relative_url "(null)" "ssh://hostname/repo" "../subrepo" "ssh://hostname/subrepo"
+test_submodule_relative_url "(null)" "ssh://hostname:22/repo" "../subrepo" "ssh://hostname:22/subrepo"
+test_submodule_relative_url "(null)" "user@host:path/to/repo" "../subrepo" "user@host:path/to/subrepo"
+test_submodule_relative_url "(null)" "user@host:repo" "../subrepo" "user@host:subrepo"
+
 test_done
-- 
2.8.0.26.g0341e85

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] submodule: port init from shell to C
  2016-04-14 18:18 [PATCH 0/2] Port `submodule init` to C Stefan Beller
  2016-04-14 18:18 ` [PATCH 1/2] submodule: port resolve_relative_url from shell " Stefan Beller
@ 2016-04-14 18:18 ` Stefan Beller
  2016-04-14 18:26 ` [PATCH 0/2] Port `submodule init` " Junio C Hamano
  2 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-04-14 18:18 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, j6t, Stefan Beller

By having the `submodule init` functionality in C, we can reference it
easier from other parts in the code in later patches. The code is split
up to have one function to initialize one submodule and a calling function
that takes care of the rest, such as argument handling and translating the
arguments to the paths of the submodules.

This is the first submodule subcommand that is fully converted to C
except for the usage string, so this is actually removing a call to
the `submodule--helper list` function, which is supposed to be used in
this transition. Instead we'll make a direct call to `module_list_compute`.

An explanation why we need to edit the prefixes in cmd_update in
git-submodule.sh in this patch:

By having no processing in the shell part, we need to convey the notion
of wt_prefix and prefix to the C parts, which former patches punted on
and did the processing of displaying path in the shell.

`wt_prefix` used to hold the path from the repository root to the current
directory, e.g. wt_prefix would be t/ if the user invoked the
`git submodule` command in ~/repo/t and ~repo is the GIT_DIR.

`prefix` used to hold the relative path from the repository root to the
operation, e.g. if you have recursive submodules, the shell script would
modify the `prefix` in each recursive step by adding the submodule path.

We will pass `wt_prefix` into the C helper via `git -C <dir>` as that
will setup git in the directory the user actually called git-submodule.sh
from. The `prefix` will be passed in via the `--prefix` option.

Having `prefix` and `wt_prefix` relative to the GIT_DIR of the
calling superproject is unfortunate with this patch as the C code doesn't
know about a possible recursion from a superproject via `submodule update
--init --recursive`.

To fix this, we change the meaning of `wt_prefix` to point to the current
project instead of the superproject and `prefix` to include any relative
paths issues in the superproject. That way `prefix` will become the leading
part for displaying paths and `wt_prefix` will be empty in recursive
calls for now.

The new notion of `wt_prefix` and `prefix` still allows us to reconstruct
the calling directory in the superproject by just traveling reverse of
`prefix`.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  48 ++-----------------
 submodule.c                 |  21 ++++++++
 submodule.h                 |   1 +
 4 files changed, 138 insertions(+), 45 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 46946b0..ad3cba6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -305,6 +305,118 @@ static int module_list(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+static void init_submodule(const char *path, const char *prefix, int quiet)
+{
+	const struct submodule *sub;
+	struct strbuf sb = STRBUF_INIT;
+	const char *upd = NULL;
+	char *url = NULL, *displaypath;
+
+	/* Only loads from .gitmodules, no overlay with .git/config */
+	gitmodules_config();
+
+	sub = submodule_from_path(null_sha1, path);
+
+	if (prefix) {
+		strbuf_addf(&sb, "%s%s", prefix, path);
+		displaypath = strbuf_detach(&sb, NULL);
+	} else
+		displaypath = xstrdup(sub->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);
+			free(remoteurl);
+		}
+
+		if (git_config_set_gently(sb.buf, url))
+			die(_("Failed to register url for submodule path '%s'"),
+			    displaypath);
+		if (!quiet)
+			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+				sub->name, url, displaypath);
+	}
+
+	/* 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_strategy.type != SM_UPDATE_UNSPECIFIED) {
+		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
+			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
+				sub->name);
+			upd = "none";
+		} else
+			upd = submodule_strategy_to_string(&sub->update_strategy);
+
+		if (git_config_set_gently(sb.buf, upd))
+			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+	}
+	strbuf_release(&sb);
+	free(displaypath);
+	free(url);
+}
+
+static int module_init(int argc, const char **argv, const char *prefix)
+{
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	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, N_("Suppress output for initializing 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 (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	for (i = 0; i < list.nr; i++)
+		init_submodule(list.entries[i]->name, prefix, quiet);
+
+	return 0;
+}
+
 static int module_name(int argc, const char **argv, const char *prefix)
 {
 	const struct submodule *sub;
@@ -712,6 +824,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 2423d7c..82e95a9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -394,50 +394,7 @@ cmd_init()
 		shift
 	done
 
-	git submodule--helper list --prefix "$wt_prefix" "$@" |
-	while read mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode"
-		name=$(git submodule--helper name "$sm_path") || exit
-
-		displaypath=$(relative_path "$prefix$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
-	done
+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} "$@"
 }
 
 #
@@ -740,7 +697,8 @@ cmd_update()
 		if test -n "$recursive"
 		then
 			(
-				prefix="$prefix$sm_path/"
+				prefix=$(relative_path "$prefix$sm_path/")
+				wt_prefix=
 				clear_local_git_env
 				cd "$sm_path" &&
 				eval cmd_update
diff --git a/submodule.c b/submodule.c
index 90825e1..4cc1c27 100644
--- a/submodule.c
+++ b/submodule.c
@@ -237,6 +237,27 @@ int parse_submodule_update_strategy(const char *value,
 	return 0;
 }
 
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (s->type) {
+	case SM_UPDATE_CHECKOUT:
+		return "checkout";
+	case SM_UPDATE_MERGE:
+		return "merge";
+	case SM_UPDATE_REBASE:
+		return "rebase";
+	case SM_UPDATE_NONE:
+		return "none";
+	case SM_UPDATE_UNSPECIFIED:
+		return NULL;
+	case SM_UPDATE_COMMAND:
+		strbuf_addf(&sb, "!%s", s->command);
+		return strbuf_detach(&sb, NULL);
+	}
+	return NULL;
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
diff --git a/submodule.h b/submodule.h
index 7ef3775..ff4c4f3 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,7 @@ int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
-- 
2.8.0.26.g0341e85

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Port `submodule init` to C
  2016-04-14 18:18 [PATCH 0/2] Port `submodule init` to C Stefan Beller
  2016-04-14 18:18 ` [PATCH 1/2] submodule: port resolve_relative_url from shell " Stefan Beller
  2016-04-14 18:18 ` [PATCH 2/2] submodule: port init " Stefan Beller
@ 2016-04-14 18:26 ` Junio C Hamano
  2 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2016-04-14 18:26 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, pclouds, j6t

Stefan Beller <sbeller@google.com> writes:

> * This applies on another base commit, such that we make use of the tests
>   written in origin/sb/submodule-path-misc-bugs. (I am not sure if I have too
>   many series in flight stomping on each other here)

I actually am quite sure that is the case ;-)

> * This time I actually fix what Ramsay was hinting at:
>   strbuf_detach(&sb, NULL) instead of strbuf_detach(&sb, 0);

Thanks, will take a look.

> Where do these patches apply?
> =============================
>
> I ran the following commands for a new starting point of this series:
>
>     git checkout --detach origin/sb/submodule-parallel-update
>     git merge origin/sb/submodule-helper-clone-regression-fix
>     git merge origin/sb/submodule-path-misc-bugs
>
> The second merge produces 2 conflicts, which can be resolved like this:
> (I am unsure about the second comment in strbuf.h though)
> diff --cc builtin/fetch.c
> index 5aa1c2d,e4639d8..0000000
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@@ -37,7 -37,8 +37,8 @@@ static int prune = -1; /* unspecified *
>   static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
>   static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
>   static int tags = TAGS_DEFAULT, unshallow, update_shallow;
>  -static int max_children = 1;
>  +static int max_children = -1;
> + static enum transport_family family;
>   static const char *depth;
>   static const char *upload_pack;
>   static struct strbuf default_rla = STRBUF_INIT;
> diff --cc strbuf.h
> index d4f2aa1,f72fd14..0000000
> --- a/strbuf.h
> +++ b/strbuf.h
> @@@ -387,15 -387,10 +387,16 @@@ extern ssize_t strbuf_read_file(struct 
>   extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
>   
>   /**
>  + * Write the whole content of the strbuf to the stream not stopping at
>  + * NUL bytes.
>  + */
>  +extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);
>  +
>  +/**
> -  * Read a line from a FILE *, overwriting the existing contents
> -  * of the strbuf. The second argument specifies the line
> -  * terminator character, typically `'\n'`.
> +  * Read a line from a FILE *, overwriting the existing contents of
> +  * the strbuf.  The strbuf_getline*() family of functions share
> +  * this signature, but have different line termination conventions.
> +  *
>    * Reading stops after the terminator or at EOF.  The terminator
>    * is removed from the buffer before returning.  Returns 0 unless
>    * there was nothing left before EOF, in which case it returns `EOF`.
>
> As sb/submodule-parallel-update and sb/submodule-helper-clone-regression-fix
> both touch builtin/submodule--helper.c, so we need those.
>
> We need origin/sb/submodule-path-misc-bugs as it tests `submodule add`
> output.
>
> Thanks,
> Stefan
>
> Stefan Beller (2):
>   submodule: port resolve_relative_url from shell to C
>   submodule: port init from shell to C
>
>  builtin/submodule--helper.c | 322 +++++++++++++++++++++++++++++++++++++++++++-
>  git-submodule.sh            | 127 +----------------
>  submodule.c                 |  21 +++
>  submodule.h                 |   1 +
>  t/t0060-path-utils.sh       |  43 ++++++
>  5 files changed, 392 insertions(+), 122 deletions(-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] submodule: port resolve_relative_url from shell to C
  2016-04-14 18:18 ` [PATCH 1/2] submodule: port resolve_relative_url from shell " Stefan Beller
@ 2016-04-14 19:35   ` Johannes Sixt
  2016-04-14 19:37     ` Stefan Beller
  0 siblings, 1 reply; 13+ messages in thread
From: Johannes Sixt @ 2016-04-14 19:35 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, gitster, pclouds

Am 14.04.2016 um 20:18 schrieb Stefan Beller:
> @@ -298,4 +305,40 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
>   test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
>   test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
>   
> +test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
> +test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
> +test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
> +test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
> +test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
> +test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
> +test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
> +test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
> +test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
> +
> +test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
> +test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
> +test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
> +test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
> +test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
> +test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
> +test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
> +test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$PWD/subsubsuper_update_r"
> +test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$PWD/subsuper_update_r"
> +test_submodule_relative_url "(null)" "$PWD/." "../." "$PWD/."
> +test_submodule_relative_url "(null)" "$PWD" "./." "$PWD/."
> +test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$PWD/repo"
> +test_submodule_relative_url "(null)" "$PWD" "./å äö" "$PWD/å äö"
> +test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$PWD/submodule"
> +test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$PWD/submodule"
> +test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"
> +test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$PWD/submodule_update_repo/."
> +test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
> +test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
> +test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
> +test_submodule_relative_url "(null)" "helper:://hostname/repo" "../subrepo" "helper:://hostname/subrepo"
> +test_submodule_relative_url "(null)" "ssh://hostname/repo" "../subrepo" "ssh://hostname/subrepo"
> +test_submodule_relative_url "(null)" "ssh://hostname:22/repo" "../subrepo" "ssh://hostname:22/subrepo"
> +test_submodule_relative_url "(null)" "user@host:path/to/repo" "../subrepo" "user@host:path/to/subrepo"
> +test_submodule_relative_url "(null)" "user@host:repo" "../subrepo" "user@host:subrepo"
> +
>   test_done
> 

I am very sorry that I am chiming in again so late. I forgot to mention
that this requires a fixup on Windows as below. I won't mind to submit
the fixup as a follow-on patch, but you could also squash it in if yet
another round is required.

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 579c1fa..1d19fbb 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -293,13 +293,16 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
 
+# In the tests below, the distinction between $PWD and $(pwd) is important:
+# on Windows, $PWD is POSIX style (/c/foo), $(pwd) has drive letter (c:/foo).
+
 test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
 test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
 test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
 test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
 test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
 test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
-test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
+test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$(pwd)/repo"
 test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
 test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
 
@@ -310,16 +313,16 @@ test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
 test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
 test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
 test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
-test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$PWD/subsubsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$PWD/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$PWD/."
-test_submodule_relative_url "(null)" "$PWD" "./." "$PWD/."
-test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$PWD/repo"
-test_submodule_relative_url "(null)" "$PWD" "./å äö" "$PWD/å äö"
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$PWD/submodule"
-test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$PWD/submodule"
-test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"
-test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$PWD/submodule_update_repo/."
+test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
+test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
+test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
+test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
+test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
 test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
 test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
 test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
-- 
2.7.0.118.g90056ae

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] submodule: port resolve_relative_url from shell to C
  2016-04-14 19:35   ` Johannes Sixt
@ 2016-04-14 19:37     ` Stefan Beller
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-04-14 19:37 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen

On Thu, Apr 14, 2016 at 12:35 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 14.04.2016 um 20:18 schrieb Stefan Beller:
>> @@ -298,4 +305,40 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
>>   test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
>>   test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
>>
>> +test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
>> +test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
>> +test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
>> +test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
>> +test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
>> +test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
>> +test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
>> +test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
>> +test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
>> +
>> +test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
>> +test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
>> +test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
>> +test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
>> +test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
>> +test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
>> +test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
>> +test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$PWD/subsubsuper_update_r"
>> +test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$PWD/subsuper_update_r"
>> +test_submodule_relative_url "(null)" "$PWD/." "../." "$PWD/."
>> +test_submodule_relative_url "(null)" "$PWD" "./." "$PWD/."
>> +test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$PWD/repo"
>> +test_submodule_relative_url "(null)" "$PWD" "./å äö" "$PWD/å äö"
>> +test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$PWD/submodule"
>> +test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$PWD/submodule"
>> +test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"
>> +test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$PWD/submodule_update_repo/."
>> +test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
>> +test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
>> +test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
>> +test_submodule_relative_url "(null)" "helper:://hostname/repo" "../subrepo" "helper:://hostname/subrepo"
>> +test_submodule_relative_url "(null)" "ssh://hostname/repo" "../subrepo" "ssh://hostname/subrepo"
>> +test_submodule_relative_url "(null)" "ssh://hostname:22/repo" "../subrepo" "ssh://hostname:22/subrepo"
>> +test_submodule_relative_url "(null)" "user@host:path/to/repo" "../subrepo" "user@host:path/to/subrepo"
>> +test_submodule_relative_url "(null)" "user@host:repo" "../subrepo" "user@host:subrepo"
>> +
>>   test_done
>>
>
> I am very sorry that I am chiming in again so late. I forgot to mention
> that this requires a fixup on Windows as below. I won't mind to submit
> the fixup as a follow-on patch, but you could also squash it in if yet
> another round is required.

Thanks a lot for testing a patch on Windows!
I'll pick this up in a resend.


>
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 579c1fa..1d19fbb 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -293,13 +293,16 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config
>  test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
>  test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
>
> +# In the tests below, the distinction between $PWD and $(pwd) is important:
> +# on Windows, $PWD is POSIX style (/c/foo), $(pwd) has drive letter (c:/foo).
> +
>  test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule"
>  test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule"
>  test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule"
>  test_submodule_relative_url "../" "./foo" "../submodule" "../submodule"
>  test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule"
>  test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c"
> -test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$PWD/repo"
> +test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$(pwd)/repo"
>  test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
>  test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
>
> @@ -310,16 +313,16 @@ test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
>  test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule"
>  test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
>  test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
> -test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$PWD/subsubsuper_update_r"
> -test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$PWD/subsuper_update_r"
> -test_submodule_relative_url "(null)" "$PWD/." "../." "$PWD/."
> -test_submodule_relative_url "(null)" "$PWD" "./." "$PWD/."
> -test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$PWD/repo"
> -test_submodule_relative_url "(null)" "$PWD" "./å äö" "$PWD/å äö"
> -test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$PWD/submodule"
> -test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$PWD/submodule"
> -test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$PWD/home2/../bundle1"
> -test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$PWD/submodule_update_repo/."
> +test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
> +test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
> +test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
> +test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
> +test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
> +test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
> +test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
> +test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
> +test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
> +test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
>  test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
>  test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
>  test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
> --
> 2.7.0.118.g90056ae
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] submodule: port init from shell to C
  2016-04-16  0:50 [PATCH 0/2] Another reroll of sb/submodule-init Stefan Beller
@ 2016-04-16  0:50 ` Stefan Beller
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-04-16  0:50 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, j6t, Stefan Beller

By having the `submodule init` functionality in C, we can reference it
easier from other parts in the code in later patches. The code is split
up to have one function to initialize one submodule and a calling function
that takes care of the rest, such as argument handling and translating the
arguments to the paths of the submodules.

This is the first submodule subcommand that is fully converted to C
except for the usage string, so this is actually removing a call to
the `submodule--helper list` function, which is supposed to be used in
this transition. Instead we'll make a direct call to `module_list_compute`.

An explanation why we need to edit the prefixes in cmd_update in
git-submodule.sh in this patch:

By having no processing in the shell part, we need to convey the notion
of wt_prefix and prefix to the C parts, which former patches punted on
and did the processing of displaying path in the shell.

`wt_prefix` used to hold the path from the repository root to the current
directory, e.g. wt_prefix would be t/ if the user invoked the
`git submodule` command in ~/repo/t and ~repo is the GIT_DIR.

`prefix` used to hold the relative path from the repository root to the
operation, e.g. if you have recursive submodules, the shell script would
modify the `prefix` in each recursive step by adding the submodule path.

We will pass `wt_prefix` into the C helper via `git -C <dir>` as that
will setup git in the directory the user actually called git-submodule.sh
from. The `prefix` will be passed in via the `--prefix` option.

Having `prefix` and `wt_prefix` relative to the GIT_DIR of the
calling superproject is unfortunate with this patch as the C code doesn't
know about a possible recursion from a superproject via `submodule update
--init --recursive`.

To fix this, we change the meaning of `wt_prefix` to point to the current
project instead of the superproject and `prefix` to include any relative
paths issues in the superproject. That way `prefix` will become the leading
part for displaying paths and `wt_prefix` will be empty in recursive
calls for now.

The new notion of `wt_prefix` and `prefix` still allows us to reconstruct
the calling directory in the superproject by just traveling reverse of
`prefix`.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 115 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  48 ++----------------
 submodule.c                 |  21 ++++++++
 submodule.h                 |   1 +
 4 files changed, 140 insertions(+), 45 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 46946b0..b6d4f27 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -305,6 +305,120 @@ static int module_list(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+static void init_submodule(const char *path, const char *prefix, int quiet)
+{
+	const struct submodule *sub;
+	struct strbuf sb = STRBUF_INIT;
+	char *upd = NULL, *url = NULL, *displaypath;
+
+	/* Only loads from .gitmodules, no overlay with .git/config */
+	gitmodules_config();
+
+	sub = submodule_from_path(null_sha1, path);
+
+	if (prefix) {
+		strbuf_addf(&sb, "%s%s", prefix, path);
+		displaypath = strbuf_detach(&sb, NULL);
+	} else
+		displaypath = xstrdup(sub->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, *relurl;
+			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();
+			relurl = relative_url(remoteurl, url, NULL);
+			strbuf_release(&remotesb);
+			free(remoteurl);
+			free(url);
+			url = relurl;
+		}
+
+		if (git_config_set_gently(sb.buf, url))
+			die(_("Failed to register url for submodule path '%s'"),
+			    displaypath);
+		if (!quiet)
+			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+				sub->name, url, displaypath);
+	}
+
+	/* 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(sb.buf, &upd) &&
+	    sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {
+		if (sub->update_strategy.type == SM_UPDATE_COMMAND) {
+			fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"),
+				sub->name);
+			upd = xstrdup("none");
+		} else
+			upd = xstrdup(submodule_strategy_to_string(&sub->update_strategy));
+
+		if (git_config_set_gently(sb.buf, upd))
+			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+	}
+	strbuf_release(&sb);
+	free(displaypath);
+	free(url);
+	free(upd);
+}
+
+static int module_init(int argc, const char **argv, const char *prefix)
+{
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	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, N_("Suppress output for initializing 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 (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	for (i = 0; i < list.nr; i++)
+		init_submodule(list.entries[i]->name, prefix, quiet);
+
+	return 0;
+}
+
 static int module_name(int argc, const char **argv, const char *prefix)
 {
 	const struct submodule *sub;
@@ -712,6 +826,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 2423d7c..82e95a9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -394,50 +394,7 @@ cmd_init()
 		shift
 	done
 
-	git submodule--helper list --prefix "$wt_prefix" "$@" |
-	while read mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode"
-		name=$(git submodule--helper name "$sm_path") || exit
-
-		displaypath=$(relative_path "$prefix$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
-	done
+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} "$@"
 }
 
 #
@@ -740,7 +697,8 @@ cmd_update()
 		if test -n "$recursive"
 		then
 			(
-				prefix="$prefix$sm_path/"
+				prefix=$(relative_path "$prefix$sm_path/")
+				wt_prefix=
 				clear_local_git_env
 				cd "$sm_path" &&
 				eval cmd_update
diff --git a/submodule.c b/submodule.c
index 90825e1..4cc1c27 100644
--- a/submodule.c
+++ b/submodule.c
@@ -237,6 +237,27 @@ int parse_submodule_update_strategy(const char *value,
 	return 0;
 }
 
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s)
+{
+	struct strbuf sb = STRBUF_INIT;
+	switch (s->type) {
+	case SM_UPDATE_CHECKOUT:
+		return "checkout";
+	case SM_UPDATE_MERGE:
+		return "merge";
+	case SM_UPDATE_REBASE:
+		return "rebase";
+	case SM_UPDATE_NONE:
+		return "none";
+	case SM_UPDATE_UNSPECIFIED:
+		return NULL;
+	case SM_UPDATE_COMMAND:
+		strbuf_addf(&sb, "!%s", s->command);
+		return strbuf_detach(&sb, NULL);
+	}
+	return NULL;
+}
+
 void handle_ignore_submodules_arg(struct diff_options *diffopt,
 				  const char *arg)
 {
diff --git a/submodule.h b/submodule.h
index 7ef3775..ff4c4f3 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,7 @@ int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config(void);
 int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
+const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
-- 
2.8.0.26.gba39a1b.dirty

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-04-16  0:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-14 18:18 [PATCH 0/2] Port `submodule init` to C Stefan Beller
2016-04-14 18:18 ` [PATCH 1/2] submodule: port resolve_relative_url from shell " Stefan Beller
2016-04-14 19:35   ` Johannes Sixt
2016-04-14 19:37     ` Stefan Beller
2016-04-14 18:18 ` [PATCH 2/2] submodule: port init " Stefan Beller
2016-04-14 18:26 ` [PATCH 0/2] Port `submodule init` " Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2016-04-16  0:50 [PATCH 0/2] Another reroll of sb/submodule-init Stefan Beller
2016-04-16  0:50 ` [PATCH 2/2] submodule: port init from shell to C Stefan Beller
2016-04-13  0:18 [PATCH 0/2] Port `git submodule init` " Stefan Beller
2016-04-13  0:18 ` [PATCH 2/2] submodule: port init " Stefan Beller
2016-03-15  0:15 [PATCH 0/2] Port `git submodule init` " Stefan Beller
2016-03-15  0:15 ` [PATCH 2/2] submodule: port init " Stefan Beller
2016-02-12 23:39 [PATCH 0/2] Port `git submodule init` " Stefan Beller
2016-02-12 23:39 ` [PATCH 2/2] submodule: port init " Stefan Beller
2016-02-18 20:46   ` Junio C Hamano
2016-02-18 23:15     ` Junio C Hamano
2016-01-15 23:37 [PATCH 0/2] Port `git submodule init` " Stefan Beller
2016-01-15 23:37 ` [PATCH 2/2] submodule: Port init " Stefan Beller

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).