[GSoC] Update: Week 8

[GSoC] Update: Week 8

[Prathamesh Chavan]

SUMMARY OF MY PROJECT:

Git submodule subcommands are currently implemented by using shell script
'git-submodule.sh'. There are several reasons why we'll prefer not to
use the shell script. My project intends to convert the subcommands into
C code, thus making them builtins. This will increase Git's portability
and hence the efficiency of working with the git-submodule commands.
Link to the complete proposal: [1]

Mentors:
Stefan Beller <sbeller@google.com>
Christian Couder <christian.couder@gmail.com>

UPDATES:

Following are the updates about my ongoing project:

1. deinit: This patch is updated after its last review.
   and the updated one is attached with this update.

2. summary: Most of the time of the week was utilized for debugging
   this patch. Its debugging is completed and the patch also went
   under some review off the mailing list. Hence, this patch is also
   attached for review in the latest update.

PLAN FOR WEEK-9 (11 July 2017 to 17 July 2017):

1. In this week a new version of 'deinit' patch is included, and well
   as the first version of 'summary' is also included. In the following
   week, I aim to work on improvising these patches.

2. Apart from that, I also aim to work on getting the rest of the patches
   ('status', 'sync', and other functions) merged.

3. There is still work left with the foreach patch, and I wasn't able
   to work on this week. Hence, I will work on finding a way of generating
   the path variable without any hacks.

[1]: https://docs.google.com/document/d/1krxVLooWl--75Pot3dazhfygR3wCUUWZWzTXtK1L-xU/

Thanks,
Prathamesh Chavan

[GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Prathamesh Chavan]

Introduce function get_submodule_displaypath() to replace the code
occurring in submodule_init() for generating displaypath of the
submodule with a call to it.

This new function will also be used in other parts of the system
in later patches.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6abdad329..7af4de09b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -220,6 +220,27 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
 	return 0;
 }
 
+static char *get_submodule_displaypath(const char *path, const char *prefix)
+{
+	const char *super_prefix = get_super_prefix();
+
+	if (prefix && super_prefix) {
+		BUG("cannot have prefix '%s' and superprefix '%s'",
+		    prefix, super_prefix);
+	} else if (prefix) {
+		struct strbuf sb = STRBUF_INIT;
+		char *displaypath = xstrdup(relative_path(path, prefix, &sb));
+		strbuf_release(&sb);
+		return displaypath;
+	} else if (super_prefix) {
+		int len = strlen(super_prefix);
+		const char *format = is_dir_sep(super_prefix[len - 1]) ? "%s%s" : "%s/%s";
+		return xstrfmt(format, super_prefix, path);
+	} else {
+		return xstrdup(path);
+	}
+}
+
 struct module_list {
 	const struct cache_entry **entries;
 	int alloc, nr;
@@ -339,16 +360,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 
 	/* Only loads from .gitmodules, no overlay with .git/config */
 	gitmodules_config();
-
-	if (prefix && get_super_prefix())
-		die("BUG: cannot have prefix and superprefix");
-	else if (prefix)
-		displaypath = xstrdup(relative_path(path, prefix, &sb));
-	else if (get_super_prefix()) {
-		strbuf_addf(&sb, "%s%s", get_super_prefix(), path);
-		displaypath = strbuf_detach(&sb, NULL);
-	} else
-		displaypath = xstrdup(path);
+	displaypath = get_submodule_displaypath(path, prefix);
 
 	sub = submodule_from_path(null_sha1, path);
 
@@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 	 * Set active flag for the submodule being initialized
 	 */
 	if (!is_submodule_active(the_repository, path)) {
-		strbuf_reset(&sb);
 		strbuf_addf(&sb, "submodule.%s.active", sub->name);
 		git_config_set_gently(sb.buf, "true");
 	}
-- 
2.13.0

[GSoC][PATCH 2/8] submodule--helper: introduce for_each_submodule_list()

[Prathamesh Chavan]

Introduce function for_each_submodule_list() and
replace a loop in module_init() with a call to it.

The new function will also be used in other parts of the
system in later patches.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7af4de09b..e41572f7a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -14,6 +14,9 @@
 #include "refs.h"
 #include "connect.h"
 
+typedef void (*submodule_list_func_t)(const struct cache_entry *list_item,
+				      void *cb_data);
+
 static char *get_default_remote(void)
 {
 	char *dest = NULL, *ret;
@@ -352,17 +355,30 @@ 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)
+static void for_each_submodule_list(const struct module_list list,
+				    submodule_list_func_t fn, void *cb_data)
 {
+	int i;
+	for (i = 0; i < list.nr; i++)
+		fn(list.entries[i], cb_data);
+}
+
+struct init_cb {
+	const char *prefix;
+	unsigned int quiet: 1;
+};
+#define INIT_CB_INIT { NULL, 0 }
+
+static void init_submodule(const struct cache_entry *list_item, void *cb_data)
+{
+	struct init_cb *info = cb_data;
 	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();
-	displaypath = get_submodule_displaypath(path, prefix);
+	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
 
-	sub = submodule_from_path(null_sha1, path);
+	sub = submodule_from_path(null_sha1, list_item->name);
 
 	if (!sub)
 		die(_("No url found for submodule path '%s' in .gitmodules"),
@@ -374,7 +390,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 	 *
 	 * Set active flag for the submodule being initialized
 	 */
-	if (!is_submodule_active(the_repository, path)) {
+	if (!is_submodule_active(the_repository, list_item->name)) {
 		strbuf_addf(&sb, "submodule.%s.active", sub->name);
 		git_config_set_gently(sb.buf, "true");
 	}
@@ -416,7 +432,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 		if (git_config_set_gently(sb.buf, url))
 			die(_("Failed to register url for submodule path '%s'"),
 			    displaypath);
-		if (!quiet)
+		if (!info->quiet)
 			fprintf(stderr,
 				_("Submodule '%s' (%s) registered for path '%s'\n"),
 				sub->name, url, displaypath);
@@ -445,10 +461,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 
 static int module_init(int argc, const char **argv, const char *prefix)
 {
+	struct init_cb info = INIT_CB_INIT;
 	struct pathspec pathspec;
 	struct module_list list = MODULE_LIST_INIT;
 	int quiet = 0;
-	int i;
 
 	struct option module_init_options[] = {
 		OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")),
@@ -473,8 +489,11 @@ static int module_init(int argc, const char **argv, const char *prefix)
 	if (!argc && git_config_get_value_multi("submodule.active"))
 		module_list_active(&list);
 
-	for (i = 0; i < list.nr; i++)
-		init_submodule(list.entries[i]->name, prefix, quiet);
+	info.prefix = prefix;
+	info.quiet = !!quiet;
+
+	gitmodules_config();
+	for_each_submodule_list(list, init_submodule, &info);
 
 	return 0;
 }
-- 
2.13.0

[GSoC][PATCH 3/8] submodule: port set_name_rev() from shell to C

[Prathamesh Chavan]

Function set_name_rev() is ported from git-submodule to the
submodule--helper builtin. The function get_name_rev() generates the
value of the revision name as required, and the function
print_name_rev() handles the formating and printing of the obtained
revision name.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            | 16 ++----------
 2 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e41572f7a..80f744407 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -244,6 +244,68 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)
 	}
 }
 
+static char *get_name_rev(const char *sub_path, const char* object_id)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char ***d;
+
+	static const char *describe_bare[] = {
+		NULL
+	};
+
+	static const char *describe_tags[] = {
+		"--tags", NULL
+	};
+
+	static const char *describe_contains[] = {
+		"--contains", NULL
+	};
+
+	static const char *describe_all_always[] = {
+		"--all", "--always", NULL
+	};
+
+	static const char **describe_argv[] = {
+		describe_bare, describe_tags, describe_contains,
+		describe_all_always, NULL
+	};
+
+	for (d = describe_argv; *d; d++) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+		prepare_submodule_repo_env(&cp.env_array);
+		cp.dir = sub_path;
+		cp.git_cmd = 1;
+		cp.no_stderr = 1;
+
+		argv_array_push(&cp.args, "describe");
+		argv_array_pushv(&cp.args, *d);
+		argv_array_push(&cp.args, object_id);
+
+		if (!capture_command(&cp, &sb, 0) && sb.len) {
+			strbuf_strip_suffix(&sb, "\n");
+			return strbuf_detach(&sb, NULL);
+		}
+
+	}
+
+	strbuf_release(&sb);
+	return NULL;
+}
+
+static int print_name_rev(int argc, const char **argv, const char *prefix)
+{
+	char *namerev;
+	if (argc != 3)
+		die("print-name-rev only accepts two arguments: <path> <sha1>");
+
+	namerev = get_name_rev(argv[1], argv[2]);
+	if (namerev && namerev[0])
+		printf(" (%s)", namerev);
+	printf("\n");
+
+	return 0;
+}
+
 struct module_list {
 	const struct cache_entry **entries;
 	int alloc, nr;
@@ -1242,6 +1304,7 @@ static struct cmd_struct commands[] = {
 	{"relative-path", resolve_relative_path, 0},
 	{"resolve-relative-url", resolve_relative_url, 0},
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
+	{"print-name-rev", print_name_rev, 0},
 	{"init", module_init, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
diff --git a/git-submodule.sh b/git-submodule.sh
index e131760ee..e988167e0 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -759,18 +759,6 @@ cmd_update()
 	}
 }
 
-set_name_rev () {
-	revname=$( (
-		sanitize_submodule_env
-		cd "$1" && {
-			git describe "$2" 2>/dev/null ||
-			git describe --tags "$2" 2>/dev/null ||
-			git describe --contains "$2" 2>/dev/null ||
-			git describe --all --always "$2"
-		}
-	) )
-	test -z "$revname" || revname=" ($revname)"
-}
 #
 # Show commit summary for submodules in index or working tree
 #
@@ -1042,14 +1030,14 @@ cmd_status()
 		fi
 		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
 		then
-			set_name_rev "$sm_path" "$sha1"
+			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
 			say " $sha1 $displaypath$revname"
 		else
 			if test -z "$cached"
 			then
 				sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
 			fi
-			set_name_rev "$sm_path" "$sha1"
+			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
 			say "+$sha1 $displaypath$revname"
 		fi
 
-- 
2.13.0

[GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Prathamesh Chavan]

This aims to make git-submodule 'status' a built-in. Hence, the function
cmd_status() is ported from shell to C. This is done by introducing
three functions: module_status(), submodule_status() and print_status().

The function module_status() acts as the front-end of the subcommand.
It parses subcommand's options and then calls the function
module_list_compute() for computing the list of submodules. Then
this functions calls for_each_submodule_list() looping through the
list obtained.

Then for_each_submodule_list() calls submodule_status() for each of the
submodule in its list. The function submodule_status() is responsible
for generating the status each submodule it is called for, and
then calls print_status().

Finally, the function print_status() handles the printing of submodule's
status.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 154 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  49 +-------------
 2 files changed, 155 insertions(+), 48 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 80f744407..980b8ed27 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -560,6 +560,159 @@ static int module_init(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+struct status_cb {
+	const char *prefix;
+	unsigned int quiet: 1;
+	unsigned int recursive: 1;
+	unsigned int cached: 1;
+};
+#define STATUS_CB_INIT { NULL, 0, 0, 0 }
+
+static void print_status(struct status_cb *info, char state, const char *path,
+			 char *sub_sha1, char *displaypath)
+{
+	if (info->quiet)
+		return;
+
+	printf("%c%s %s", state, sub_sha1, displaypath);
+
+	if (state == ' ' || state == '+') {
+		struct argv_array name_rev_args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&name_rev_args, "print-name-rev",
+				 path, sub_sha1, NULL);
+		print_name_rev(name_rev_args.argc, name_rev_args.argv,
+			       info->prefix);
+	} else {
+		printf("\n");
+	}
+}
+
+static void status_submodule(const struct cache_entry *list_item, void *cb_data)
+{
+	struct status_cb *info = cb_data;
+	char *sub_sha1 = xstrdup(oid_to_hex(&list_item->oid));
+	char *displaypath;
+	struct argv_array diff_files_args = ARGV_ARRAY_INIT;
+
+	if (!submodule_from_path(null_sha1, list_item->name))
+		die(_("no submodule mapping found in .gitmodules for path '%s'"),
+		      list_item->name);
+
+	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+	if (list_item->ce_flags) {
+		print_status(info, 'U', list_item->name,
+			     sha1_to_hex(null_sha1), displaypath);
+		goto cleanup;
+	}
+
+	if (!is_submodule_active(the_repository, list_item->name)) {
+		print_status(info, '-', list_item->name, sub_sha1, displaypath);
+		goto cleanup;
+	}
+
+	argv_array_pushl(&diff_files_args, "diff-files",
+			 "--ignore-submodules=dirty", "--quiet", "--",
+			 list_item->name, NULL);
+
+	/* NEEDSWORK: future optimization possible */
+	if (!cmd_diff_files(diff_files_args.argc, diff_files_args.argv,
+			    info->prefix)) {
+		print_status(info, ' ', list_item->name, sub_sha1, displaypath);
+	} else {
+		if (!info->cached) {
+			struct child_process cp = CHILD_PROCESS_INIT;
+			struct strbuf sb = STRBUF_INIT;
+
+			prepare_submodule_repo_env(&cp.env_array);
+			cp.git_cmd = 1;
+			cp.dir = list_item->name;
+
+			argv_array_pushl(&cp.args, "rev-parse",
+					 "--verify", "HEAD", NULL);
+
+			/* NEEDSWORK: future optimization possible */
+			if (capture_command(&cp, &sb, 0))
+				die(_("could not run 'git rev-parse --verify"
+				      "HEAD' in submodule %s"),
+				      list_item->name);
+
+			strbuf_strip_suffix(&sb, "\n");
+			print_status(info, '+', list_item->name, sb.buf,
+				     displaypath);
+			strbuf_release(&sb);
+		} else {
+			print_status(info, '+', list_item->name, sub_sha1,
+				     displaypath);
+		}
+	}
+
+	if (info->recursive) {
+		struct child_process cpr = CHILD_PROCESS_INIT;
+
+		cpr.git_cmd = 1;
+		cpr.dir = list_item->name;
+		prepare_submodule_repo_env(&cpr.env_array);
+
+		argv_array_pushl(&cpr.args, "--super-prefix", displaypath,
+				 "submodule--helper", "status", "--recursive",
+				 NULL);
+
+		if (info->cached)
+			argv_array_push(&cpr.args, "--cached");
+
+		if (info->quiet)
+			argv_array_push(&cpr.args, "--quiet");
+
+		if (run_command(&cpr))
+			die(_("failed to recurse into submodule '%s'"),
+			      list_item->name);
+	}
+
+cleanup:
+	free(displaypath);
+	free(sub_sha1);
+}
+
+static int module_status(int argc, const char **argv, const char *prefix)
+{
+	struct status_cb info = STATUS_CB_INIT;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	int quiet = 0;
+	int cached = 0;
+	int recursive = 0;
+
+	struct option module_status_options[] = {
+		OPT__QUIET(&quiet, N_("Suppress submodule status output")),
+		OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")),
+		OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, module_status_options,
+			     git_submodule_helper_usage, 0);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	info.prefix = prefix;
+	info.quiet = !!quiet;
+	info.recursive = !!recursive;
+	info.cached = !!cached;
+
+	gitmodules_config();
+	for_each_submodule_list(list, status_submodule, &info);
+
+	return 0;
+}
+
 static int module_name(int argc, const char **argv, const char *prefix)
 {
 	const struct submodule *sub;
@@ -1306,6 +1459,7 @@ static struct cmd_struct commands[] = {
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
 	{"print-name-rev", print_name_rev, 0},
 	{"init", module_init, SUPPORT_SUPER_PREFIX},
+	{"status", module_status, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index e988167e0..51b057d82 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1005,54 +1005,7 @@ cmd_status()
 		shift
 	done
 
-	{
-		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched" $?
-	} |
-	while read -r mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode" "$sha1"
-		name=$(git submodule--helper name "$sm_path") || exit
-		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
-		if test "$stage" = U
-		then
-			say "U$sha1 $displaypath"
-			continue
-		fi
-		if ! git submodule--helper is-active "$sm_path" ||
-		{
-			! test -d "$sm_path"/.git &&
-			! test -f "$sm_path"/.git
-		}
-		then
-			say "-$sha1 $displaypath"
-			continue;
-		fi
-		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
-		then
-			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
-			say " $sha1 $displaypath$revname"
-		else
-			if test -z "$cached"
-			then
-				sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
-			fi
-			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
-			say "+$sha1 $displaypath$revname"
-		fi
-
-		if test -n "$recursive"
-		then
-			(
-				prefix="$displaypath/"
-				sanitize_submodule_env
-				wt_prefix=
-				cd "$sm_path" &&
-				eval cmd_status
-			) ||
-			die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
-		fi
-	done
+	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@"
 }
 #
 # Sync remote urls for submodules
-- 
2.13.0

Re: [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Brandon Williams]

On 07/11, Prathamesh Chavan wrote:
> This aims to make git-submodule 'status' a built-in. Hence, the function
> cmd_status() is ported from shell to C. This is done by introducing
> three functions: module_status(), submodule_status() and print_status().
> 
> The function module_status() acts as the front-end of the subcommand.
> It parses subcommand's options and then calls the function
> module_list_compute() for computing the list of submodules. Then
> this functions calls for_each_submodule_list() looping through the
> list obtained.
> 
> Then for_each_submodule_list() calls submodule_status() for each of the
> submodule in its list. The function submodule_status() is responsible
> for generating the status each submodule it is called for, and
> then calls print_status().
> 
> Finally, the function print_status() handles the printing of submodule's
> status.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
>  builtin/submodule--helper.c | 154 ++++++++++++++++++++++++++++++++++++++++++++
>  git-submodule.sh            |  49 +-------------
>  2 files changed, 155 insertions(+), 48 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 80f744407..980b8ed27 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -560,6 +560,159 @@ static int module_init(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> +struct status_cb {
> +	const char *prefix;
> +	unsigned int quiet: 1;
> +	unsigned int recursive: 1;
> +	unsigned int cached: 1;
> +};
> +#define STATUS_CB_INIT { NULL, 0, 0, 0 }
> +
> +static void print_status(struct status_cb *info, char state, const char *path,
> +			 char *sub_sha1, char *displaypath)
> +{
> +	if (info->quiet)
> +		return;
> +
> +	printf("%c%s %s", state, sub_sha1, displaypath);
> +
> +	if (state == ' ' || state == '+') {
> +		struct argv_array name_rev_args = ARGV_ARRAY_INIT;

This struct needs to be cleared to prevent a memory leak.

> +
> +		argv_array_pushl(&name_rev_args, "print-name-rev",
> +				 path, sub_sha1, NULL);
> +		print_name_rev(name_rev_args.argc, name_rev_args.argv,
> +			       info->prefix);
> +	} else {
> +		printf("\n");
> +	}
> +}
> +
> +static void status_submodule(const struct cache_entry *list_item, void *cb_data)
> +{
> +	struct status_cb *info = cb_data;
> +	char *sub_sha1 = xstrdup(oid_to_hex(&list_item->oid));
> +	char *displaypath;
> +	struct argv_array diff_files_args = ARGV_ARRAY_INIT;
> +
> +	if (!submodule_from_path(null_sha1, list_item->name))
> +		die(_("no submodule mapping found in .gitmodules for path '%s'"),
> +		      list_item->name);
> +
> +	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
> +
> +	if (list_item->ce_flags) {
> +		print_status(info, 'U', list_item->name,
> +			     sha1_to_hex(null_sha1), displaypath);
> +		goto cleanup;
> +	}
> +
> +	if (!is_submodule_active(the_repository, list_item->name)) {
> +		print_status(info, '-', list_item->name, sub_sha1, displaypath);
> +		goto cleanup;
> +	}
> +
> +	argv_array_pushl(&diff_files_args, "diff-files",
> +			 "--ignore-submodules=dirty", "--quiet", "--",
> +			 list_item->name, NULL);
> +
> +	/* NEEDSWORK: future optimization possible */

What sort of optimization? maybe you could document that?

> +	if (!cmd_diff_files(diff_files_args.argc, diff_files_args.argv,
> +			    info->prefix)) {
> +		print_status(info, ' ', list_item->name, sub_sha1, displaypath);
> +	} else {
> +		if (!info->cached) {
> +			struct child_process cp = CHILD_PROCESS_INIT;
> +			struct strbuf sb = STRBUF_INIT;
> +
> +			prepare_submodule_repo_env(&cp.env_array);
> +			cp.git_cmd = 1;
> +			cp.dir = list_item->name;
> +
> +			argv_array_pushl(&cp.args, "rev-parse",
> +					 "--verify", "HEAD", NULL);
> +
> +			/* NEEDSWORK: future optimization possible */

Same here.

> +			if (capture_command(&cp, &sb, 0))
> +				die(_("could not run 'git rev-parse --verify"
> +				      "HEAD' in submodule %s"),
> +				      list_item->name);
> +
> +			strbuf_strip_suffix(&sb, "\n");
> +			print_status(info, '+', list_item->name, sb.buf,
> +				     displaypath);
> +			strbuf_release(&sb);
> +		} else {
> +			print_status(info, '+', list_item->name, sub_sha1,
> +				     displaypath);
> +		}
> +	}
> +
> +	if (info->recursive) {
> +		struct child_process cpr = CHILD_PROCESS_INIT;
> +
> +		cpr.git_cmd = 1;
> +		cpr.dir = list_item->name;
> +		prepare_submodule_repo_env(&cpr.env_array);
> +
> +		argv_array_pushl(&cpr.args, "--super-prefix", displaypath,
> +				 "submodule--helper", "status", "--recursive",
> +				 NULL);
> +
> +		if (info->cached)
> +			argv_array_push(&cpr.args, "--cached");
> +
> +		if (info->quiet)
> +			argv_array_push(&cpr.args, "--quiet");
> +
> +		if (run_command(&cpr))
> +			die(_("failed to recurse into submodule '%s'"),
> +			      list_item->name);
> +	}
> +
> +cleanup:
> +	free(displaypath);
> +	free(sub_sha1);
> +}
> +
> +static int module_status(int argc, const char **argv, const char *prefix)
> +{
> +	struct status_cb info = STATUS_CB_INIT;
> +	struct pathspec pathspec;
> +	struct module_list list = MODULE_LIST_INIT;
> +	int quiet = 0;
> +	int cached = 0;
> +	int recursive = 0;
> +
> +	struct option module_status_options[] = {
> +		OPT__QUIET(&quiet, N_("Suppress submodule status output")),
> +		OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")),
> +		OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")),
> +		OPT_END()
> +	};
> +
> +	const char *const git_submodule_helper_usage[] = {
> +		N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"),
> +		NULL
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, module_status_options,
> +			     git_submodule_helper_usage, 0);
> +
> +	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
> +		return 1;
> +
> +	info.prefix = prefix;
> +	info.quiet = !!quiet;
> +	info.recursive = !!recursive;
> +	info.cached = !!cached;
> +
> +	gitmodules_config();
> +	for_each_submodule_list(list, status_submodule, &info);
> +
> +	return 0;
> +}
> +
>  static int module_name(int argc, const char **argv, const char *prefix)
>  {
>  	const struct submodule *sub;
> @@ -1306,6 +1459,7 @@ static struct cmd_struct commands[] = {
>  	{"resolve-relative-url-test", resolve_relative_url_test, 0},
>  	{"print-name-rev", print_name_rev, 0},
>  	{"init", module_init, SUPPORT_SUPER_PREFIX},
> +	{"status", module_status, SUPPORT_SUPER_PREFIX},
>  	{"remote-branch", resolve_remote_submodule_branch, 0},
>  	{"push-check", push_check, 0},
>  	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> diff --git a/git-submodule.sh b/git-submodule.sh
> index e988167e0..51b057d82 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1005,54 +1005,7 @@ cmd_status()
>  		shift
>  	done
>  
> -	{
> -		git submodule--helper list --prefix "$wt_prefix" "$@" ||
> -		echo "#unmatched" $?
> -	} |
> -	while read -r mode sha1 stage sm_path
> -	do
> -		die_if_unmatched "$mode" "$sha1"
> -		name=$(git submodule--helper name "$sm_path") || exit
> -		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
> -		if test "$stage" = U
> -		then
> -			say "U$sha1 $displaypath"
> -			continue
> -		fi
> -		if ! git submodule--helper is-active "$sm_path" ||
> -		{
> -			! test -d "$sm_path"/.git &&
> -			! test -f "$sm_path"/.git
> -		}
> -		then
> -			say "-$sha1 $displaypath"
> -			continue;
> -		fi
> -		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
> -		then
> -			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
> -			say " $sha1 $displaypath$revname"
> -		else
> -			if test -z "$cached"
> -			then
> -				sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
> -			fi
> -			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
> -			say "+$sha1 $displaypath$revname"
> -		fi
> -
> -		if test -n "$recursive"
> -		then
> -			(
> -				prefix="$displaypath/"
> -				sanitize_submodule_env
> -				wt_prefix=
> -				cd "$sm_path" &&
> -				eval cmd_status
> -			) ||
> -			die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
> -		fi
> -	done
> +	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@"
>  }
>  #
>  # Sync remote urls for submodules
> -- 
> 2.13.0
> 

-- 
Brandon Williams

[GSoC][PATCH 5/8] submodule: port submodule subcommand 'sync' from shell to C

[Prathamesh Chavan]

Port the submodule subcommand 'sync' from shell to C using the same
mechanism as that used for porting submodule subcommand 'status'.
Hence, here the function cmd_sync() is ported from shell to C.
This is done by introducing three functions: module_sync(),
sync_submodule() and print_default_remote().

The function print_default_remote() is introduced for getting
the default remote as stdout.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 181 +++++++++++++++++++++++++++++++++++++++++++-
 git-submodule.sh            |  56 +-------------
 2 files changed, 181 insertions(+), 56 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 980b8ed27..4e04b93f3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -44,6 +44,20 @@ static char *get_default_remote(void)
 	return ret;
 }
 
+static int print_default_remote(int argc, const char **argv, const char *prefix)
+{
+	const char *remote;
+
+	if (argc != 1)
+		die(_("submodule--helper print-default-remote takes no arguments"));
+
+	remote = get_default_remote();
+	if (remote)
+		puts(remote);
+
+	return 0;
+}
+
 static int starts_with_dot_slash(const char *str)
 {
 	return str[0] == '.' && is_dir_sep(str[1]);
@@ -379,6 +393,25 @@ static void module_list_active(struct module_list *list)
 	*list = active_modules;
 }
 
+static char *get_up_path(const char *path)
+{
+	int i;
+	struct strbuf sb = STRBUF_INIT;
+
+	for (i = count_slashes(path); i; i--)
+		strbuf_addstr(&sb, "../");
+
+	/*
+	 * Check if 'path' ends with slash or not
+	 * for having the same output for dir/sub_dir
+	 * and dir/sub_dir/
+	 */
+	if (!is_dir_sep(path[strlen(path) - 1]))
+		strbuf_addstr(&sb, "../");
+
+	return strbuf_detach(&sb, NULL);
+}
+
 static int module_list(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -478,16 +511,18 @@ static void init_submodule(const struct cache_entry *list_item, void *cb_data)
 			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)) {
 				warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
 				remoteurl = xgetcwd();
 			}
 			relurl = relative_url(remoteurl, url, NULL);
+
+			free(remote);
 			strbuf_release(&remotesb);
 			free(remoteurl);
 			free(url);
+
 			url = relurl;
 		}
 
@@ -732,6 +767,148 @@ static int module_name(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+struct sync_cb {
+	const char *prefix;
+	unsigned int quiet: 1;
+	unsigned int recursive: 1;
+};
+#define SYNC_CB_INIT { NULL, 0, 0 }
+
+static void sync_submodule(const struct cache_entry *list_item, void *cb_data)
+{
+	struct sync_cb *info = cb_data;
+	const struct submodule *sub;
+	char *sub_key, *remote_key;
+	char *sub_origin_url, *super_config_url, *displaypath;
+	struct strbuf sb = STRBUF_INIT;
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	if (!is_submodule_active(the_repository, list_item->name))
+		return;
+
+	sub = submodule_from_path(null_sha1, list_item->name);
+
+	if (!sub || !sub->url)
+		die(_("no url found for submodule path '%s' in .gitmodules"),
+		      list_item->name);
+
+	if (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) {
+		char *remote_url, *up_path;
+		char *remote = get_default_remote();
+		char *remote_key = xstrfmt("remote.%s.url", remote);
+		free(remote);
+
+		if (git_config_get_string(remote_key, &remote_url))
+			remote_url = xgetcwd();
+		up_path = get_up_path(list_item->name);
+		sub_origin_url = relative_url(remote_url, sub->url, up_path);
+		super_config_url = relative_url(remote_url, sub->url, NULL);
+		free(remote_key);
+		free(up_path);
+		free(remote_url);
+	} else {
+		sub_origin_url = xstrdup(sub->url);
+		super_config_url = xstrdup(sub->url);
+	}
+
+	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+	if (!info->quiet)
+		printf(_("Synchronizing submodule url for '%s'\n"),
+			 displaypath);
+
+	sub_key = xstrfmt("submodule.%s.url", sub->name);
+	if (git_config_set_gently(sub_key, super_config_url))
+		die(_("failed to register url for submodule path '%s'"),
+		      displaypath);
+
+	if (!is_submodule_populated_gently(list_item->name, NULL))
+		goto cleanup;
+
+	prepare_submodule_repo_env(&cp.env_array);
+	cp.git_cmd = 1;
+	cp.dir = list_item->name;
+	argv_array_pushl(&cp.args, "submodule--helper",
+			 "print-default-remote", NULL);
+	if (capture_command(&cp, &sb, 0))
+		die(_("failed to get the default remote for submodule '%s'"),
+		      list_item->name);
+
+	strbuf_strip_suffix(&sb, "\n");
+	remote_key = xstrfmt("remote.%s.url", sb.buf);
+	strbuf_release(&sb);
+
+	child_process_init(&cp);
+	prepare_submodule_repo_env(&cp.env_array);
+	cp.git_cmd = 1;
+	cp.dir = list_item->name;
+	argv_array_pushl(&cp.args, "config", remote_key, sub_origin_url, NULL);
+	if (run_command(&cp))
+		die(_("failed to update remote for submodule '%s'"),
+		      list_item->name);
+
+	if (info->recursive) {
+		struct child_process cpr = CHILD_PROCESS_INIT;
+
+		cpr.git_cmd = 1;
+		cpr.dir = list_item->name;
+		prepare_submodule_repo_env(&cpr.env_array);
+
+		argv_array_pushl(&cpr.args, "--super-prefix", displaypath,
+				 "submodule--helper", "sync", "--recursive",
+				 NULL);
+
+		if (info->quiet)
+			argv_array_push(&cpr.args, "--quiet");
+
+		if (run_command(&cpr))
+			die(_("failed to recurse into submodule '%s'"),
+			      list_item->name);
+	}
+
+cleanup:
+	free(sub_key);
+	free(super_config_url);
+	free(displaypath);
+	free(sub_origin_url);
+}
+
+static int module_sync(int argc, const char **argv, const char *prefix)
+{
+	struct sync_cb info = SYNC_CB_INIT;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	int quiet = 0;
+	int recursive = 0;
+
+	struct option module_sync_options[] = {
+		OPT__QUIET(&quiet, N_("Suppress output of synchronizing submodule url")),
+		OPT_BOOL(0, "recursive", &recursive,
+			N_("Recurse into nested submodules")),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule--helper sync [--quiet] [--recursive] [<path>]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, module_sync_options,
+			     git_submodule_helper_usage, 0);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	info.prefix = prefix;
+	info.quiet = !!quiet;
+	info.recursive = !!recursive;
+
+	gitmodules_config();
+	for_each_submodule_list(list, sync_submodule, &info);
+
+	return 0;
+}
+
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
 			   const char *depth, struct string_list *reference,
 			   int quiet, int progress)
@@ -1460,6 +1637,8 @@ static struct cmd_struct commands[] = {
 	{"print-name-rev", print_name_rev, 0},
 	{"init", module_init, SUPPORT_SUPER_PREFIX},
 	{"status", module_status, SUPPORT_SUPER_PREFIX},
+	{"print-default-remote", print_default_remote, 0},
+	{"sync", module_sync, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index 51b057d82..6bfc5e17d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1037,63 +1037,9 @@ cmd_sync()
 			;;
 		esac
 	done
-	cd_to_toplevel
-	{
-		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched" $?
-	} |
-	while read -r mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode" "$sha1"
-
-		# skip inactive submodules
-		if ! git submodule--helper is-active "$sm_path"
-		then
-			continue
-		fi
-
-		name=$(git submodule--helper name "$sm_path")
-		url=$(git config -f .gitmodules --get submodule."$name".url)
-
-		# Possibly a url relative to parent
-		case "$url" in
-		./*|../*)
-			# rewrite foo/bar as ../.. to find path from
-			# submodule work tree to superproject work tree
-			up_path="$(printf '%s\n' "$sm_path" | sed "s/[^/][^/]*/../g")" &&
-			# guarantee a trailing /
-			up_path=${up_path%/}/ &&
-			# path from submodule work tree to submodule origin repo
-			sub_origin_url=$(git submodule--helper resolve-relative-url "$url" "$up_path") &&
-			# path from superproject work tree to submodule origin repo
-			super_config_url=$(git submodule--helper resolve-relative-url "$url") || exit
-			;;
-		*)
-			sub_origin_url="$url"
-			super_config_url="$url"
-			;;
-		esac
 
-		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
-		say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
-		git config submodule."$name".url "$super_config_url"
-
-		if test -e "$sm_path"/.git
-		then
-		(
-			sanitize_submodule_env
-			cd "$sm_path"
-			remote=$(get_default_remote)
-			git config remote."$remote".url "$sub_origin_url"
+	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@"
 
-			if test -n "$recursive"
-			then
-				prefix="$prefix$sm_path/"
-				eval cmd_sync
-			fi
-		)
-		fi
-	done
 }
 
 cmd_absorbgitdirs()
-- 
2.13.0

Re: [GSoC][PATCH 5/8] submodule: port submodule subcommand 'sync' from shell to C

[Brandon Williams]

On 07/11, Prathamesh Chavan wrote:
> Port the submodule subcommand 'sync' from shell to C using the same
> mechanism as that used for porting submodule subcommand 'status'.
> Hence, here the function cmd_sync() is ported from shell to C.
> This is done by introducing three functions: module_sync(),
> sync_submodule() and print_default_remote().
> 
> The function print_default_remote() is introduced for getting
> the default remote as stdout.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
>  builtin/submodule--helper.c | 181 +++++++++++++++++++++++++++++++++++++++++++-
>  git-submodule.sh            |  56 +-------------
>  2 files changed, 181 insertions(+), 56 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 980b8ed27..4e04b93f3 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -44,6 +44,20 @@ static char *get_default_remote(void)
>  	return ret;
>  }
>  
> +static int print_default_remote(int argc, const char **argv, const char *prefix)
> +{
> +	const char *remote;
> +
> +	if (argc != 1)
> +		die(_("submodule--helper print-default-remote takes no arguments"));
> +
> +	remote = get_default_remote();
> +	if (remote)
> +		puts(remote);
> +
> +	return 0;
> +}
> +
>  static int starts_with_dot_slash(const char *str)
>  {
>  	return str[0] == '.' && is_dir_sep(str[1]);
> @@ -379,6 +393,25 @@ static void module_list_active(struct module_list *list)
>  	*list = active_modules;
>  }
>  
> +static char *get_up_path(const char *path)
> +{
> +	int i;
> +	struct strbuf sb = STRBUF_INIT;
> +
> +	for (i = count_slashes(path); i; i--)
> +		strbuf_addstr(&sb, "../");
> +
> +	/*
> +	 * Check if 'path' ends with slash or not
> +	 * for having the same output for dir/sub_dir
> +	 * and dir/sub_dir/
> +	 */
> +	if (!is_dir_sep(path[strlen(path) - 1]))
> +		strbuf_addstr(&sb, "../");
> +
> +	return strbuf_detach(&sb, NULL);
> +}
> +
>  static int module_list(int argc, const char **argv, const char *prefix)
>  {
>  	int i;
> @@ -478,16 +511,18 @@ static void init_submodule(const struct cache_entry *list_item, void *cb_data)
>  			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)) {
>  				warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
>  				remoteurl = xgetcwd();
>  			}
>  			relurl = relative_url(remoteurl, url, NULL);
> +
> +			free(remote);
>  			strbuf_release(&remotesb);
>  			free(remoteurl);
>  			free(url);
> +
>  			url = relurl;

The changes in this function seem unintended.

>  		}
>  
> @@ -732,6 +767,148 @@ static int module_name(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> +struct sync_cb {
> +	const char *prefix;
> +	unsigned int quiet: 1;
> +	unsigned int recursive: 1;
> +};
> +#define SYNC_CB_INIT { NULL, 0, 0 }
> +
> +static void sync_submodule(const struct cache_entry *list_item, void *cb_data)
> +{
> +	struct sync_cb *info = cb_data;
> +	const struct submodule *sub;
> +	char *sub_key, *remote_key;
> +	char *sub_origin_url, *super_config_url, *displaypath;
> +	struct strbuf sb = STRBUF_INIT;
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +
> +	if (!is_submodule_active(the_repository, list_item->name))
> +		return;
> +
> +	sub = submodule_from_path(null_sha1, list_item->name);
> +
> +	if (!sub || !sub->url)
> +		die(_("no url found for submodule path '%s' in .gitmodules"),
> +		      list_item->name);
> +
> +	if (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) {
> +		char *remote_url, *up_path;
> +		char *remote = get_default_remote();
> +		char *remote_key = xstrfmt("remote.%s.url", remote);
> +		free(remote);
> +
> +		if (git_config_get_string(remote_key, &remote_url))
> +			remote_url = xgetcwd();
> +		up_path = get_up_path(list_item->name);
> +		sub_origin_url = relative_url(remote_url, sub->url, up_path);
> +		super_config_url = relative_url(remote_url, sub->url, NULL);
> +		free(remote_key);
> +		free(up_path);
> +		free(remote_url);
> +	} else {
> +		sub_origin_url = xstrdup(sub->url);
> +		super_config_url = xstrdup(sub->url);
> +	}
> +
> +	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
> +
> +	if (!info->quiet)
> +		printf(_("Synchronizing submodule url for '%s'\n"),
> +			 displaypath);
> +
> +	sub_key = xstrfmt("submodule.%s.url", sub->name);
> +	if (git_config_set_gently(sub_key, super_config_url))
> +		die(_("failed to register url for submodule path '%s'"),
> +		      displaypath);
> +
> +	if (!is_submodule_populated_gently(list_item->name, NULL))
> +		goto cleanup;
> +
> +	prepare_submodule_repo_env(&cp.env_array);
> +	cp.git_cmd = 1;
> +	cp.dir = list_item->name;
> +	argv_array_pushl(&cp.args, "submodule--helper",
> +			 "print-default-remote", NULL);
> +	if (capture_command(&cp, &sb, 0))
> +		die(_("failed to get the default remote for submodule '%s'"),
> +		      list_item->name);
> +
> +	strbuf_strip_suffix(&sb, "\n");
> +	remote_key = xstrfmt("remote.%s.url", sb.buf);
> +	strbuf_release(&sb);
> +
> +	child_process_init(&cp);
> +	prepare_submodule_repo_env(&cp.env_array);
> +	cp.git_cmd = 1;
> +	cp.dir = list_item->name;
> +	argv_array_pushl(&cp.args, "config", remote_key, sub_origin_url, NULL);
> +	if (run_command(&cp))
> +		die(_("failed to update remote for submodule '%s'"),
> +		      list_item->name);
> +
> +	if (info->recursive) {
> +		struct child_process cpr = CHILD_PROCESS_INIT;
> +
> +		cpr.git_cmd = 1;
> +		cpr.dir = list_item->name;
> +		prepare_submodule_repo_env(&cpr.env_array);
> +
> +		argv_array_pushl(&cpr.args, "--super-prefix", displaypath,
> +				 "submodule--helper", "sync", "--recursive",
> +				 NULL);
> +
> +		if (info->quiet)
> +			argv_array_push(&cpr.args, "--quiet");
> +
> +		if (run_command(&cpr))
> +			die(_("failed to recurse into submodule '%s'"),
> +			      list_item->name);
> +	}
> +
> +cleanup:
> +	free(sub_key);
> +	free(super_config_url);
> +	free(displaypath);
> +	free(sub_origin_url);
> +}
> +
> +static int module_sync(int argc, const char **argv, const char *prefix)
> +{
> +	struct sync_cb info = SYNC_CB_INIT;
> +	struct pathspec pathspec;
> +	struct module_list list = MODULE_LIST_INIT;
> +	int quiet = 0;
> +	int recursive = 0;
> +
> +	struct option module_sync_options[] = {
> +		OPT__QUIET(&quiet, N_("Suppress output of synchronizing submodule url")),
> +		OPT_BOOL(0, "recursive", &recursive,
> +			N_("Recurse into nested submodules")),
> +		OPT_END()
> +	};
> +
> +	const char *const git_submodule_helper_usage[] = {
> +		N_("git submodule--helper sync [--quiet] [--recursive] [<path>]"),
> +		NULL
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, module_sync_options,
> +			     git_submodule_helper_usage, 0);
> +
> +	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
> +		return 1;
> +
> +	info.prefix = prefix;
> +	info.quiet = !!quiet;
> +	info.recursive = !!recursive;
> +
> +	gitmodules_config();
> +	for_each_submodule_list(list, sync_submodule, &info);
> +
> +	return 0;
> +}
> +
>  static int clone_submodule(const char *path, const char *gitdir, const char *url,
>  			   const char *depth, struct string_list *reference,
>  			   int quiet, int progress)
> @@ -1460,6 +1637,8 @@ static struct cmd_struct commands[] = {
>  	{"print-name-rev", print_name_rev, 0},
>  	{"init", module_init, SUPPORT_SUPER_PREFIX},
>  	{"status", module_status, SUPPORT_SUPER_PREFIX},
> +	{"print-default-remote", print_default_remote, 0},
> +	{"sync", module_sync, SUPPORT_SUPER_PREFIX},
>  	{"remote-branch", resolve_remote_submodule_branch, 0},
>  	{"push-check", push_check, 0},
>  	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 51b057d82..6bfc5e17d 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1037,63 +1037,9 @@ cmd_sync()
>  			;;
>  		esac
>  	done
> -	cd_to_toplevel
> -	{
> -		git submodule--helper list --prefix "$wt_prefix" "$@" ||
> -		echo "#unmatched" $?
> -	} |
> -	while read -r mode sha1 stage sm_path
> -	do
> -		die_if_unmatched "$mode" "$sha1"
> -
> -		# skip inactive submodules
> -		if ! git submodule--helper is-active "$sm_path"
> -		then
> -			continue
> -		fi
> -
> -		name=$(git submodule--helper name "$sm_path")
> -		url=$(git config -f .gitmodules --get submodule."$name".url)
> -
> -		# Possibly a url relative to parent
> -		case "$url" in
> -		./*|../*)
> -			# rewrite foo/bar as ../.. to find path from
> -			# submodule work tree to superproject work tree
> -			up_path="$(printf '%s\n' "$sm_path" | sed "s/[^/][^/]*/../g")" &&
> -			# guarantee a trailing /
> -			up_path=${up_path%/}/ &&
> -			# path from submodule work tree to submodule origin repo
> -			sub_origin_url=$(git submodule--helper resolve-relative-url "$url" "$up_path") &&
> -			# path from superproject work tree to submodule origin repo
> -			super_config_url=$(git submodule--helper resolve-relative-url "$url") || exit
> -			;;
> -		*)
> -			sub_origin_url="$url"
> -			super_config_url="$url"
> -			;;
> -		esac
>  
> -		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
> -		say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
> -		git config submodule."$name".url "$super_config_url"
> -
> -		if test -e "$sm_path"/.git
> -		then
> -		(
> -			sanitize_submodule_env
> -			cd "$sm_path"
> -			remote=$(get_default_remote)
> -			git config remote."$remote".url "$sub_origin_url"
> +	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@"
>  
> -			if test -n "$recursive"
> -			then
> -				prefix="$prefix$sm_path/"
> -				eval cmd_sync
> -			fi
> -		)
> -		fi
> -	done
>  }
>  
>  cmd_absorbgitdirs()
> -- 
> 2.13.0
> 

-- 
Brandon Williams

[GSoC][PATCH 6/8] submodule: port submodule subcommand 'deinit' from shell to C

[Prathamesh Chavan]

The same mechanism is used even for porting this submodule
subcommand, as used in the ported subcommands till now.
The function cmd_deinit in split up after porting into three
functions: module_deinit(), for_each_submodule_list() and
deinit_submodule().

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 143 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  55 +----------------
 2 files changed, 144 insertions(+), 54 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4e04b93f3..05d430846 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -909,6 +909,148 @@ static int module_sync(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+struct deinit_cb {
+	const char *prefix;
+	unsigned int quiet: 1;
+	unsigned int force: 1;
+	unsigned int all: 1;
+};
+#define DEINIT_CB_INIT { NULL, 0, 0, 0 }
+
+static void deinit_submodule(const struct cache_entry *list_item,
+			     void *cb_data)
+{
+	struct deinit_cb *info = cb_data;
+	const struct submodule *sub;
+	char *displaypath = NULL;
+	struct child_process cp_config = CHILD_PROCESS_INIT;
+	struct strbuf sb_config = STRBUF_INIT;
+	char *sm_path = xstrdup(list_item->name);
+	char *sub_git_dir = xstrfmt("%s/.git", sm_path);
+	struct stat st;
+
+	sub = submodule_from_path(null_sha1, sm_path);
+
+	if (!sub || !sub->name)
+		goto cleanup;
+
+	displaypath = get_submodule_displaypath(sm_path, info->prefix);
+
+	/* remove the submodule work tree (unless the user already did it) */
+	if (is_directory(sm_path)) {
+		/* protect submodules containing a .git directory */
+		if (is_git_directory(sub_git_dir))
+			die(_("Submodule work tree '%s' contains a .git "
+			      "directory use 'rm -rf' if you really want "
+			      "to remove it including all of its history"),
+			      displaypath);
+
+		if (!info->force) {
+			struct child_process cp_rm = CHILD_PROCESS_INIT;
+			cp_rm.git_cmd = 1;
+			argv_array_pushl(&cp_rm.args, "rm", "-qn", sm_path,
+					 NULL);
+
+			/* list_item->name is changed by cmd_rm() below */
+			if (run_command(&cp_rm))
+				die(_("Submodule work tree '%s' contains local "
+				      "modifications; use '-f' to discard them"),
+				      displaypath);
+		}
+
+		if (!lstat(sm_path, &st)) {
+			struct strbuf sb_rm = STRBUF_INIT;
+			strbuf_addstr(&sb_rm, sm_path);
+
+			if (!remove_dir_recursively(&sb_rm, 0)) {
+				if (!info->quiet)
+					printf(_("Cleared directory '%s'\n"),
+						 displaypath);
+			} else {
+				if (!info->quiet)
+					printf(_("Could not remove submodule work tree '%s'\n"),
+						 displaypath);
+			}
+			strbuf_release(&sb_rm);
+		}
+	}
+
+	if (mkdir(sm_path, st.st_mode))
+		die(_("could not create empty submodule directory %s"),
+		      displaypath);
+
+	cp_config.git_cmd = 1;
+	argv_array_pushl(&cp_config.args, "config", "--get-regexp", NULL);
+	argv_array_pushf(&cp_config.args, "submodule.%s\\.", sub->name);
+
+	/* remove the .git/config entries (unless the user already did it) */
+	if (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {
+		char *sub_key = xstrfmt("submodule.%s", sub->name);
+		/*
+		 * remove the whole section so we have a clean state when
+		 * the user later decides to init this submodule again
+		 */
+		git_config_rename_section_in_file(NULL, sub_key, NULL);
+		if (!info->quiet)
+			printf(_("Submodule '%s' (%s) unregistered for path '%s'\n"),
+				 sub->name, sub->url, displaypath);
+		free(sub_key);
+	}
+
+cleanup:
+	free(displaypath);
+	free(sub_git_dir);
+	free(sm_path);
+	strbuf_release(&sb_config);
+}
+
+static int module_deinit(int argc, const char **argv, const char *prefix)
+{
+	struct deinit_cb info = DEINIT_CB_INIT;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	int quiet = 0;
+	int force = 0;
+	int all = 0;
+
+	struct option module_deinit_options[] = {
+		OPT__QUIET(&quiet, N_("Suppress submodule status output")),
+		OPT__FORCE(&force, N_("Remove submodule working trees even if they contain local changes")),
+		OPT_BOOL(0, "all", &all, N_("Unregister all submodules")),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, module_deinit_options,
+			     git_submodule_helper_usage, 0);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		BUG("module_list_compute should not choke on empty pathspec");
+
+	info.prefix = prefix;
+	info.quiet = !!quiet;
+	info.all = !!all;
+	info.force = !!force;
+
+	if (all && argc) {
+		error("pathspec and --all are incompatible");
+		usage_with_options(git_submodule_helper_usage,
+				   module_deinit_options);
+	}
+
+	if (!argc && !all)
+		die(_("Use '--all' if you really want to deinitialize all submodules"));
+
+	gitmodules_config();
+	for_each_submodule_list(list, deinit_submodule, &info);
+
+	return 0;
+}
+
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
 			   const char *depth, struct string_list *reference,
 			   int quiet, int progress)
@@ -1639,6 +1781,7 @@ static struct cmd_struct commands[] = {
 	{"status", module_status, SUPPORT_SUPER_PREFIX},
 	{"print-default-remote", print_default_remote, 0},
 	{"sync", module_sync, SUPPORT_SUPER_PREFIX},
+	{"deinit", module_deinit, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index 6bfc5e17d..73e6f093f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -428,60 +428,7 @@ cmd_deinit()
 		shift
 	done
 
-	if test -n "$deinit_all" && test "$#" -ne 0
-	then
-		echo >&2 "$(eval_gettext "pathspec and --all are incompatible")"
-		usage
-	fi
-	if test $# = 0 && test -z "$deinit_all"
-	then
-		die "$(eval_gettext "Use '--all' if you really want to deinitialize all submodules")"
-	fi
-
-	{
-		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched" $?
-	} |
-	while read -r mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode" "$sha1"
-		name=$(git submodule--helper name "$sm_path") || exit
-
-		displaypath=$(git submodule--helper relative-path "$sm_path" "$wt_prefix")
-
-		# Remove the submodule work tree (unless the user already did it)
-		if test -d "$sm_path"
-		then
-			# Protect submodules containing a .git directory
-			if test -d "$sm_path/.git"
-			then
-				die "$(eval_gettext "\
-Submodule work tree '\$displaypath' contains a .git directory
-(use 'rm -rf' if you really want to remove it including all of its history)")"
-			fi
-
-			if test -z "$force"
-			then
-				git rm -qn "$sm_path" ||
-				die "$(eval_gettext "Submodule work tree '\$displaypath' contains local modifications; use '-f' to discard them")"
-			fi
-			rm -rf "$sm_path" &&
-			say "$(eval_gettext "Cleared directory '\$displaypath'")" ||
-			say "$(eval_gettext "Could not remove submodule work tree '\$displaypath'")"
-		fi
-
-		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$displaypath'")"
-
-		# Remove the .git/config entries (unless the user already did it)
-		if test -n "$(git config --get-regexp submodule."$name\.")"
-		then
-			# Remove the whole section so we have a clean state when
-			# the user later decides to init this submodule again
-			url=$(git config submodule."$name".url)
-			git config --remove-section submodule."$name" 2>/dev/null &&
-			say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
-		fi
-	done
+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${force:+--force} ${deinit_all:+--all} "$@"
 }
 
 is_tip_reachable () (
-- 
2.13.0

[GSoC][PATCH 7/8] diff: change scope of the function count_lines()

[Prathamesh Chavan]

Change the scope of function count_lines for allowing the function
to be reused in other parts of the code as well.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 diff.c | 2 +-
 diff.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 00b4c8669..3240f8283 100644
--- a/diff.c
+++ b/diff.c
@@ -425,7 +425,7 @@ struct emit_callback {
 	struct strbuf *header;
 };
 
-static int count_lines(const char *data, int size)
+int count_lines(const char *data, int size)
 {
 	int count, ch, completely_empty = 1, nl_just_seen = 0;
 	count = 0;
diff --git a/diff.h b/diff.h
index 2d442e296..8522514e9 100644
--- a/diff.h
+++ b/diff.h
@@ -273,6 +273,7 @@ extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *pat
 extern int parse_long_opt(const char *opt, const char **argv,
 			 const char **optarg);
 
+extern int count_lines(const char *data, int size);
 extern int git_diff_basic_config(const char *var, const char *value, void *cb);
 extern int git_diff_heuristic_config(const char *var, const char *value, void *cb);
 extern void init_diff_ui_defaults(void);
-- 
2.13.0

[GSoC][PATCH 8/8] submodule: port submodule subcommand 'summary' from shell to C

[Prathamesh Chavan]

The submodule subcommand 'summary' is ported in the process of
making git-submodule a builtin. The function cmd_summary() from
git-submodule.sh is ported to functions module_summary(),
compute_summary_module_list(), prepare_submodule_summary() and
print_submodule_summary().

The first function module_summary() parses the options of submodule
subcommand and also acts as the front-end of this subcommand.
After parsing them, it calls the compute_summary_module_list()

The functions compute_summary_module_list() runs the diff_cmd,
and generates the modules list, as required by the subcommand.
The generation of this module list is done by the using the
callback function submodule_summary_callback(), and stored in the
structure module_cb.

Once the module list is generated, prepare_submodule_summary()
further goes through the list and filters the list, for
eventually calling the print_submodule_summary() function.

Finally, the print_submodule_summary() takes care of generating
and printing the summary for each submodule.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
This is the first version of submodule-summary posted on the mailing list.

This patch has previously being reviewed off the mailing list as well, and
following are the changes made after the previous update:

An initial check for is_sm_git_dir is added.

Instead of changing the dir to sm_path in a child_process, now we instead
are adding the env_variable GIT_DIR. This helped in avoiding the usage of
shell in the child_process as well for getting all the tests cleared
from t7508-status.

Also, the sentences which earlier were translated unnecessarily were
changed for getting all the test cleared with the env_variable
GETTEXT_POISON, out of which 13 test from t7401-submodule-summary
failed earlier.

Still I'm looking into a better way for generating the abbrevation of
sha1_src and sha1_dst.

Complete build report of this series is available at:
https://travis-ci.org/pratham-pc/git/builds/
Branch: week-8
Build #129

 builtin/submodule--helper.c | 466 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            | 182 +----------------
 2 files changed, 467 insertions(+), 181 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 05d430846..1dc53c2b2 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -13,6 +13,9 @@
 #include "remote.h"
 #include "refs.h"
 #include "connect.h"
+#include "revision.h"
+#include "diffcore.h"
+#include "diff.h"
 
 typedef void (*submodule_list_func_t)(const struct cache_entry *list_item,
 				      void *cb_data);
@@ -767,6 +770,468 @@ static int module_name(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+struct module_cb {
+	unsigned int mod_src;
+	unsigned int mod_dst;
+	struct object_id oid_src;
+	struct object_id oid_dst;
+	char status;
+	const char *sm_path;
+};
+#define MODULE_CB_INIT { 0, 0, NULL, NULL, '\0', NULL }
+
+struct module_cb_list {
+	struct module_cb **entries;
+	int alloc, nr;
+};
+#define MODULE_CB_LIST_INIT { NULL, 0, 0 }
+
+struct summary_cb {
+	int argc;
+	const char **argv;
+	const char *prefix;
+	char *diff_cmd;
+	unsigned int cached: 1;
+	unsigned int for_status: 1;
+	unsigned int quiet: 1;
+	unsigned int files: 1;
+	int summary_limits;
+};
+#define SUMMARY_CB_INIT { 0, NULL, NULL, NULL, 0, 0, 0, 0, 0 }
+
+static void print_submodule_summary(struct summary_cb *info,
+				    struct module_cb *p)
+{
+	int missing_src = 0;
+	int missing_dst = 0;
+	char *displaypath;
+	char *sha1_abbr_src;
+	char *sha1_abbr_dst;
+	int errmsg = 0;
+	int total_commits = -1;
+	struct strbuf sb_sha1_src = STRBUF_INIT;
+	struct strbuf sb_sha1_dst = STRBUF_INIT;
+	char *sha1_dst = oid_to_hex(&p->oid_dst);
+	char *sha1_src = oid_to_hex(&p->oid_src);
+	char *sm_git_dir = xstrfmt("%s/.git", p->sm_path);
+	int is_sm_git_dir = 0;
+
+	if (!info->cached && !strcmp(sha1_dst, sha1_to_hex(null_sha1))) {
+		if (S_ISGITLINK(p->mod_dst)) {
+			struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
+			struct strbuf sb_rev_parse = STRBUF_INIT;
+
+			cp_rev_parse.git_cmd = 1;
+			cp_rev_parse.no_stderr = 1;
+
+			argv_array_pushf(&cp_rev_parse.env_array,
+					 "GIT_DIR=%s/.git", p->sm_path);
+			argv_array_pushl(&cp_rev_parse.args,
+					 "rev-parse", "HEAD", NULL);
+			if (!capture_command(&cp_rev_parse, &sb_rev_parse, 0)) {
+				strbuf_strip_suffix(&sb_rev_parse, "\n");
+				sha1_dst = xstrdup(sb_rev_parse.buf);
+			}
+			strbuf_release(&sb_rev_parse);
+		} else if (S_ISLNK(p->mod_dst) || S_ISREG(p->mod_dst)) {
+			struct child_process cp_hash_object = CHILD_PROCESS_INIT;
+			struct strbuf sb_hash_object = STRBUF_INIT;
+
+			cp_hash_object.git_cmd = 1;
+			argv_array_pushl(&cp_hash_object.args,
+					 "hash-object", p->sm_path,
+					 NULL);
+			if (!capture_command(&cp_hash_object,
+					     &sb_hash_object, 0)) {
+				strbuf_strip_suffix(&sb_hash_object, "\n");
+				sha1_dst = xstrdup(sb_hash_object.buf);
+			}
+			strbuf_release(&sb_hash_object);
+		} else {
+			if (p->mod_dst != 0)
+				die(_("unexpected mode %d\n"), p->mod_dst);
+		}
+	}
+
+	if (is_git_directory(sm_git_dir))
+		is_sm_git_dir = 1;
+
+	if (is_sm_git_dir && S_ISGITLINK(p->mod_src)) {
+		struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
+
+		cp_rev_parse.git_cmd = 1;
+		cp_rev_parse.no_stdout = 1;
+
+		argv_array_pushf(&cp_rev_parse.env_array, "GIT_DIR=%s/.git",
+				 p->sm_path);
+		argv_array_pushl(&cp_rev_parse.args, "rev-parse", "-q",
+				 "--verify", NULL);
+		argv_array_pushf(&cp_rev_parse.args, "%s^0", sha1_src);
+
+		if (run_command(&cp_rev_parse))
+			missing_src = 1;
+	}
+
+	if (is_sm_git_dir && S_ISGITLINK(p->mod_dst)) {
+		struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
+
+		cp_rev_parse.git_cmd = 1;
+		cp_rev_parse.no_stdout = 1;
+
+		argv_array_pushf(&cp_rev_parse.env_array, "GIT_DIR=%s/.git",
+				 p->sm_path);
+		argv_array_pushl(&cp_rev_parse.args, "rev-parse", "-q",
+				 "--verify", NULL);
+		argv_array_pushf(&cp_rev_parse.args, "%s^0", sha1_dst);
+
+		if (run_command(&cp_rev_parse))
+			missing_dst = 1;
+	}
+
+	displaypath = get_submodule_displaypath(p->sm_path, info->prefix);
+
+	if (!missing_dst && !missing_src) {
+		if (is_sm_git_dir) {
+			struct child_process cp_rev_list = CHILD_PROCESS_INIT;
+			struct strbuf sb_rev_list = STRBUF_INIT;
+			const char *range;
+
+			if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst))
+				range = xstrfmt("%s...%s", sha1_src, sha1_dst);
+			else if (S_ISGITLINK(p->mod_src))
+				range = xstrdup(sha1_src);
+			else
+				range = xstrdup(sha1_dst);
+
+			cp_rev_list.git_cmd = 1;
+			argv_array_pushf(&cp_rev_list.env_array,
+					 "GIT_DIR=%s/.git", p->sm_path);
+			argv_array_pushl(&cp_rev_list.args, "rev-list",
+					 "--first-parent", range, "--", NULL);
+			if (!capture_command(&cp_rev_list, &sb_rev_list, 0)) {
+				if (sb_rev_list.len)
+					total_commits = count_lines(sb_rev_list.buf,
+								    sb_rev_list.len);
+				else
+					total_commits = 0;
+			}
+			strbuf_release(&sb_rev_list);
+		}
+	} else {
+		errmsg = 1;
+	}
+
+	strbuf_addstr(&sb_sha1_src, sha1_src);
+	strbuf_addstr(&sb_sha1_dst, sha1_dst);
+
+	strbuf_remove(&sb_sha1_src, 7, 33);
+	strbuf_remove(&sb_sha1_dst, 7, 33);
+
+	sha1_abbr_src = strbuf_detach(&sb_sha1_src, NULL);
+	sha1_abbr_dst = strbuf_detach(&sb_sha1_dst, NULL);
+
+	if (p->status == 'T') {
+		if (S_ISGITLINK(p->mod_dst)) {
+			if (total_commits < 0)
+				printf(_("* %s %s(blob)->%s(submodule):\n"),
+					 displaypath, sha1_abbr_src,
+					 sha1_abbr_dst);
+			else
+				printf(_("* %s %s(blob)->%s(submodule) (%d):\n"),
+					 displaypath, sha1_abbr_src,
+					 sha1_abbr_dst, total_commits);
+		} else {
+			if (total_commits < 0)
+				printf(_("* %s %s(submodule)->%s(blob):\n"),
+					 displaypath, sha1_abbr_src,
+					 sha1_abbr_dst);
+			else
+				printf(_("* %s %s(submodule)->%s(blob) (%d):\n"),
+					 displaypath, sha1_abbr_src,
+					 sha1_abbr_dst, total_commits);
+		}
+	} else {
+		if (total_commits < 0)
+			printf("* %s %s...%s:\n", displaypath, sha1_abbr_src,
+				 sha1_abbr_dst);
+		else
+			printf("* %s %s...%s (%d):\n", displaypath,
+				 sha1_abbr_src, sha1_abbr_dst, total_commits);
+	}
+
+	if (errmsg) {
+		/*
+		 * Don't give error msg for modification whose dst is not
+		 * submodule, i.e. deleted or changed to blob
+		 */
+		if (S_ISGITLINK(p->mod_src)) {
+			if (missing_src && missing_dst) {
+				printf(_("  Warn: %s doesn't contain commits %s and %s\n"),
+				 displaypath, sha1_src, sha1_dst);
+			} else if (missing_src) {
+				printf(_("  Warn: %s doesn't contain commit %s\n"),
+				 displaypath, sha1_src);
+			} else {
+				printf(_("  Warn: %s doesn't contain commit %s\n"),
+				 displaypath, sha1_dst);
+			}
+		}
+
+	} else if (is_sm_git_dir) {
+		if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst)) {
+			struct child_process cp_log = CHILD_PROCESS_INIT;
+			char *limit = NULL;
+
+			if (info->summary_limits > 0)
+				limit = xstrfmt("-%d", info->summary_limits);
+
+			cp_log.git_cmd = 1;
+
+			argv_array_pushf(&cp_log.env_array, "GIT_DIR=%s/.git",
+					 p->sm_path);
+			argv_array_pushl(&cp_log.args, "log", NULL);
+			if (limit)
+				argv_array_push(&cp_log.args, limit);
+			argv_array_pushl(&cp_log.args, "--pretty=  %m %s",
+					 "--first-parent", NULL);
+			argv_array_pushf(&cp_log.args, "%s...%s", sha1_src,
+					 sha1_dst);
+
+			run_command(&cp_log);
+
+		} else if (S_ISGITLINK(p->mod_dst)) {
+			struct child_process cp_log = CHILD_PROCESS_INIT;
+
+			cp_log.git_cmd = 1;
+			argv_array_pushf(&cp_log.env_array, "GIT_DIR=%s/.git",
+					 p->sm_path);
+			argv_array_pushl(&cp_log.args, "log",
+					 "--pretty=  > %s", "-1",
+					 sha1_dst, NULL);
+
+			run_command(&cp_log);
+		} else {
+			struct child_process cp_log = CHILD_PROCESS_INIT;
+
+			cp_log.git_cmd = 1;
+			argv_array_pushf(&cp_log.env_array, "GIT_DIR=%s/.git",
+					 p->sm_path);
+			argv_array_pushl(&cp_log.args, "log",
+					 "--pretty=  < %s",
+					 "-1", sha1_src, NULL);
+
+			run_command(&cp_log);
+		}
+	}
+	printf("\n");
+}
+
+static void prepare_submodule_summary(struct summary_cb *info,
+				      struct module_cb_list *list)
+{
+	int i;
+	for (i = 0; i < list->nr; i++) {
+		struct module_cb *p = list->entries[i];
+		struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
+
+		if (p->status == 'D' || p->status == 'T') {
+			print_submodule_summary(info, p);
+			continue;
+		}
+
+		if (info->for_status) {
+			char *config_key;
+			const char *ignore_config = "none";
+			const char *value;
+			const struct submodule *sub = submodule_from_path(null_sha1, p->sm_path);
+
+			if (sub) {
+				config_key = xstrfmt("submodule.%s.ignore",
+						     sub->name);
+				if (!git_config_get_value(config_key, &value))
+					ignore_config = value;
+				else if (sub->ignore)
+					ignore_config = sub->ignore;
+
+				if (p->status != 'A' && !strcmp(ignore_config,
+								"all"))
+					continue;
+			}
+		}
+
+		/* Also show added or modified modules which are checked out */
+		cp_rev_parse.dir = p->sm_path;
+		cp_rev_parse.git_cmd = 1;
+		cp_rev_parse.no_stderr = 1;
+		cp_rev_parse.no_stdout = 1;
+
+		argv_array_pushl(&cp_rev_parse.args, "rev-parse",
+				 "--git-dir", NULL);
+
+		if (!run_command(&cp_rev_parse))
+			print_submodule_summary(info, p);
+	}
+}
+
+static void submodule_summary_callback(struct diff_queue_struct *q,
+				       struct diff_options *options,
+				       void *data)
+{
+	int i;
+	struct module_cb_list *list = data;
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		struct module_cb *temp;
+
+		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
+			continue;
+		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
+		temp->mod_src = p->one->mode;
+		temp->mod_dst = p->two->mode;
+		temp->oid_src = p->one->oid;
+		temp->oid_dst = p->two->oid;
+		temp->status = p->status;
+		temp->sm_path = xstrdup(p->one->path);
+
+		ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
+		list->entries[list->nr++] = temp;
+	}
+}
+
+static int compute_summary_module_list(char *head, struct summary_cb *info)
+{
+	struct argv_array diff_args = ARGV_ARRAY_INIT;
+	struct rev_info rev;
+	struct module_cb_list list = MODULE_CB_LIST_INIT;
+
+	argv_array_push(&diff_args, info->diff_cmd);
+	if (info->cached)
+		argv_array_push(&diff_args, "--cached");
+	argv_array_pushl(&diff_args, "--ignore-submodules=dirty", "--raw",
+			 NULL);
+	if (head)
+		argv_array_push(&diff_args, head);
+	argv_array_push(&diff_args, "--");
+	if (info->argc)
+		argv_array_pushv(&diff_args, info->argv);
+
+	git_config(git_diff_basic_config, NULL);
+	init_revisions(&rev, info->prefix);
+	gitmodules_config();
+	rev.abbrev = 0;
+	precompose_argv(diff_args.argc, diff_args.argv);
+
+	diff_args.argc = setup_revisions(diff_args.argc, diff_args.argv,
+					 &rev, NULL);
+	rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;
+	rev.diffopt.format_callback = submodule_summary_callback;
+	rev.diffopt.format_callback_data = &list;
+
+	if (!info->cached) {
+		if (!strcmp(info->diff_cmd, "diff-index"))
+			setup_work_tree();
+		if (read_cache_preload(&rev.diffopt.pathspec) < 0) {
+			perror("read_cache_preload");
+			return -1;
+		}
+	} else if (read_cache() < 0) {
+		perror("read_cache");
+		return -1;
+	}
+
+	if (!strcmp(info->diff_cmd, "diff-index"))
+		run_diff_index(&rev, info->cached);
+	else
+		run_diff_files(&rev, 0);
+	prepare_submodule_summary(info, &list);
+	return 0;
+
+}
+
+static int module_summary(int argc, const char **argv, const char *prefix)
+{
+	struct summary_cb info = SUMMARY_CB_INIT;
+	int cached = 0;
+	char *diff_cmd = "diff-index";
+	int for_status = 0;
+	int quiet = 0;
+	int files = 0;
+	int summary_limits = -1;
+	char *head = NULL;
+	struct child_process cp_rev = CHILD_PROCESS_INIT;
+	struct strbuf sb_rev = STRBUF_INIT;
+
+	struct option module_summary_options[] = {
+		OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")),
+		OPT_BOOL(0, "cached", &cached, N_("Use the commit stored in the index instead of the submodule HEAD")),
+		OPT_BOOL(0, "files", &files, N_("To compares the commit in the index with that in the submodule HEAD")),
+		OPT_BOOL(0, "for-status", &for_status, N_("Skip submodules with 'all' ignore_config value")),
+		OPT_INTEGER('n', "summary-limits", &summary_limits, N_("Limit the summary size")),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule--helper summary [<options>] [--] [<path>]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, module_summary_options,
+			     git_submodule_helper_usage, 0);
+
+	if (!summary_limits)
+		return 0;
+
+	cp_rev.git_cmd = 1;
+	argv_array_pushl(&cp_rev.args, "rev-parse", "-q", "--verify",
+			 NULL);
+	if (argc)
+		argv_array_push(&cp_rev.args, argv[0]);
+	else
+		argv_array_pushl(&cp_rev.args, "HEAD", NULL);
+
+	if (!capture_command(&cp_rev, &sb_rev, 0)) {
+		strbuf_strip_suffix(&sb_rev, "\n");
+		head = xstrdup(sb_rev.buf);
+		if (argc) {
+			argv++;
+			argc--;
+		}
+		strbuf_release(&sb_rev);
+	} else if (!argc || !strcmp(argv[0], "HEAD")) {
+		/* before the first commit: compare with an empty tree */
+		struct stat st;
+		unsigned char sha1[20];
+		if (fstat(0, &st) < 0 || index_fd(sha1, 0, &st, 2, prefix, 3))
+			die("Unable to add %s to database", sha1);
+		head = sha1_to_hex(sha1);
+		if (argc) {
+			argv++;
+			argc--;
+		}
+	} else {
+		head = "HEAD";
+	}
+
+	if (files) {
+		if (cached)
+			die(_("The --cached option cannot be used with the --files option"));
+		diff_cmd = "diff-files";
+		head = NULL;
+	}
+
+	info.argc = argc;
+	info.argv = argv;
+	info.prefix = prefix;
+	info.cached = cached;
+	info.for_status = for_status;
+	info.quiet = quiet;
+	info.files = files;
+	info.summary_limits = summary_limits;
+	info.diff_cmd = diff_cmd;
+
+	return compute_summary_module_list(head, &info);
+}
+
 struct sync_cb {
 	const char *prefix;
 	unsigned int quiet: 1;
@@ -1782,6 +2247,7 @@ static struct cmd_struct commands[] = {
 	{"print-default-remote", print_default_remote, 0},
 	{"sync", module_sync, SUPPORT_SUPER_PREFIX},
 	{"deinit", module_deinit, SUPPORT_SUPER_PREFIX},
+	{"summary", module_summary, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index 73e6f093f..a427ddafd 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -51,31 +51,6 @@ die_if_unmatched ()
 	fi
 }
 
-#
-# Print a submodule configuration setting
-#
-# $1 = submodule name
-# $2 = option name
-# $3 = default value
-#
-# Checks in the usual git-config places first (for overrides),
-# otherwise it falls back on .gitmodules.  This allows you to
-# distribute project-wide defaults in .gitmodules, while still
-# customizing individual repositories if necessary.  If the option is
-# not in .gitmodules either, print a default value.
-#
-get_submodule_config () {
-	name="$1"
-	option="$2"
-	default="$3"
-	value=$(git config submodule."$name"."$option")
-	if test -z "$value"
-	then
-		value=$(git config -f .gitmodules submodule."$name"."$option")
-	fi
-	printf '%s' "${value:-$default}"
-}
-
 isnumber()
 {
 	n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
@@ -755,163 +730,8 @@ cmd_summary() {
 		shift
 	done
 
-	test $summary_limit = 0 && return
-
-	if rev=$(git rev-parse -q --verify --default HEAD ${1+"$1"})
-	then
-		head=$rev
-		test $# = 0 || shift
-	elif test -z "$1" || test "$1" = "HEAD"
-	then
-		# before the first commit: compare with an empty tree
-		head=$(git hash-object -w -t tree --stdin </dev/null)
-		test -z "$1" || shift
-	else
-		head="HEAD"
-	fi
-
-	if [ -n "$files" ]
-	then
-		test -n "$cached" &&
-		die "$(gettext "The --cached option cannot be used with the --files option")"
-		diff_cmd=diff-files
-		head=
-	fi
-
-	cd_to_toplevel
-	eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
-	# Get modified modules cared by user
-	modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- "$@" |
-		sane_egrep '^:([0-7]* )?160000' |
-		while read -r mod_src mod_dst sha1_src sha1_dst status sm_path
-		do
-			# Always show modules deleted or type-changed (blob<->module)
-			if test "$status" = D || test "$status" = T
-			then
-				printf '%s\n' "$sm_path"
-				continue
-			fi
-			# Respect the ignore setting for --for-status.
-			if test -n "$for_status"
-			then
-				name=$(git submodule--helper name "$sm_path")
-				ignore_config=$(get_submodule_config "$name" ignore none)
-				test $status != A && test $ignore_config = all && continue
-			fi
-			# Also show added or modified modules which are checked out
-			GIT_DIR="$sm_path/.git" git-rev-parse --git-dir >/dev/null 2>&1 &&
-			printf '%s\n' "$sm_path"
-		done
-	)
-
-	test -z "$modules" && return
-
-	git $diff_cmd $cached --ignore-submodules=dirty --raw $head -- $modules |
-	sane_egrep '^:([0-7]* )?160000' |
-	cut -c2- |
-	while read -r mod_src mod_dst sha1_src sha1_dst status name
-	do
-		if test -z "$cached" &&
-			test $sha1_dst = 0000000000000000000000000000000000000000
-		then
-			case "$mod_dst" in
-			160000)
-				sha1_dst=$(GIT_DIR="$name/.git" git rev-parse HEAD)
-				;;
-			100644 | 100755 | 120000)
-				sha1_dst=$(git hash-object $name)
-				;;
-			000000)
-				;; # removed
-			*)
-				# unexpected type
-				eval_gettextln "unexpected mode \$mod_dst" >&2
-				continue ;;
-			esac
-		fi
-		missing_src=
-		missing_dst=
-
-		test $mod_src = 160000 &&
-		! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&
-		missing_src=t
-
-		test $mod_dst = 160000 &&
-		! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_dst^0 >/dev/null &&
-		missing_dst=t
+	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper summary ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${for_status:+--for-status} ${files:+--files} ${cached:+--cached} ${summary_limit:+-n $summary_limit} "$@"
 
-		display_name=$(git submodule--helper relative-path "$name" "$wt_prefix")
-
-		total_commits=
-		case "$missing_src,$missing_dst" in
-		t,)
-			errmsg="$(eval_gettext "  Warn: \$display_name doesn't contain commit \$sha1_src")"
-			;;
-		,t)
-			errmsg="$(eval_gettext "  Warn: \$display_name doesn't contain commit \$sha1_dst")"
-			;;
-		t,t)
-			errmsg="$(eval_gettext "  Warn: \$display_name doesn't contain commits \$sha1_src and \$sha1_dst")"
-			;;
-		*)
-			errmsg=
-			total_commits=$(
-			if test $mod_src = 160000 && test $mod_dst = 160000
-			then
-				range="$sha1_src...$sha1_dst"
-			elif test $mod_src = 160000
-			then
-				range=$sha1_src
-			else
-				range=$sha1_dst
-			fi
-			GIT_DIR="$name/.git" \
-			git rev-list --first-parent $range -- | wc -l
-			)
-			total_commits=" ($(($total_commits + 0)))"
-			;;
-		esac
-
-		sha1_abbr_src=$(echo $sha1_src | cut -c1-7)
-		sha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)
-		if test $status = T
-		then
-			blob="$(gettext "blob")"
-			submodule="$(gettext "submodule")"
-			if test $mod_dst = 160000
-			then
-				echo "* $display_name $sha1_abbr_src($blob)->$sha1_abbr_dst($submodule)$total_commits:"
-			else
-				echo "* $display_name $sha1_abbr_src($submodule)->$sha1_abbr_dst($blob)$total_commits:"
-			fi
-		else
-			echo "* $display_name $sha1_abbr_src...$sha1_abbr_dst$total_commits:"
-		fi
-		if test -n "$errmsg"
-		then
-			# Don't give error msg for modification whose dst is not submodule
-			# i.e. deleted or changed to blob
-			test $mod_dst = 160000 && echo "$errmsg"
-		else
-			if test $mod_src = 160000 && test $mod_dst = 160000
-			then
-				limit=
-				test $summary_limit -gt 0 && limit="-$summary_limit"
-				GIT_DIR="$name/.git" \
-				git log $limit --pretty='format:  %m %s' \
-				--first-parent $sha1_src...$sha1_dst
-			elif test $mod_dst = 160000
-			then
-				GIT_DIR="$name/.git" \
-				git log --pretty='format:  > %s' -1 $sha1_dst
-			else
-				GIT_DIR="$name/.git" \
-				git log --pretty='format:  < %s' -1 $sha1_src
-			fi
-			echo
-		fi
-		echo
-	done
 }
 #
 # List all submodules, prefixed with:
-- 
2.13.0

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Brandon Williams]

On 07/11, Prathamesh Chavan wrote:
> Introduce function get_submodule_displaypath() to replace the code
> occurring in submodule_init() for generating displaypath of the
> submodule with a call to it.
> 
> This new function will also be used in other parts of the system
> in later patches.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
>  builtin/submodule--helper.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 6abdad329..7af4de09b 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -220,6 +220,27 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
>  	return 0;
>  }
>  
> +static char *get_submodule_displaypath(const char *path, const char *prefix)
> +{
> +	const char *super_prefix = get_super_prefix();
> +
> +	if (prefix && super_prefix) {
> +		BUG("cannot have prefix '%s' and superprefix '%s'",
> +		    prefix, super_prefix);
> +	} else if (prefix) {
> +		struct strbuf sb = STRBUF_INIT;
> +		char *displaypath = xstrdup(relative_path(path, prefix, &sb));
> +		strbuf_release(&sb);
> +		return displaypath;
> +	} else if (super_prefix) {
> +		int len = strlen(super_prefix);
> +		const char *format = is_dir_sep(super_prefix[len - 1]) ? "%s%s" : "%s/%s";
> +		return xstrfmt(format, super_prefix, path);
> +	} else {
> +		return xstrdup(path);
> +	}
> +}
> +
>  struct module_list {
>  	const struct cache_entry **entries;
>  	int alloc, nr;
> @@ -339,16 +360,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
>  
>  	/* Only loads from .gitmodules, no overlay with .git/config */
>  	gitmodules_config();
> -
> -	if (prefix && get_super_prefix())
> -		die("BUG: cannot have prefix and superprefix");
> -	else if (prefix)
> -		displaypath = xstrdup(relative_path(path, prefix, &sb));
> -	else if (get_super_prefix()) {
> -		strbuf_addf(&sb, "%s%s", get_super_prefix(), path);
> -		displaypath = strbuf_detach(&sb, NULL);
> -	} else
> -		displaypath = xstrdup(path);
> +	displaypath = get_submodule_displaypath(path, prefix);
>  
>  	sub = submodule_from_path(null_sha1, path);
>  
> @@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
>  	 * Set active flag for the submodule being initialized
>  	 */
>  	if (!is_submodule_active(the_repository, path)) {
> -		strbuf_reset(&sb);

Is this line removal intended?  It doesn't look related to the rest of
this patch.

>  		strbuf_addf(&sb, "submodule.%s.active", sub->name);
>  		git_config_set_gently(sb.buf, "true");
>  	}
> -- 
> 2.13.0
> 

-- 
Brandon Williams

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Stefan Beller]

On Mon, Jul 10, 2017 at 4:32 PM, Brandon Williams <bmwill@google.com> wrote:
>>       if (!is_submodule_active(the_repository, path)) {
>> -             strbuf_reset(&sb);
>
> Is this line removal intended?  It doesn't look related to the rest of
> this patch.

It is, as &sb is re-used and has to be cleared first.
With the code above removed, &sb is unmodified since
struct strbuf sb = STRBUF_INIT; hence the removal is ok here.
It is related, but only when looking at the entirety of the code. :-/

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Brandon Williams]

On 07/10, Stefan Beller wrote:
> On Mon, Jul 10, 2017 at 4:32 PM, Brandon Williams <bmwill@google.com> wrote:
> >>       if (!is_submodule_active(the_repository, path)) {
> >> -             strbuf_reset(&sb);
> >
> > Is this line removal intended?  It doesn't look related to the rest of
> > this patch.
> 
> It is, as &sb is re-used and has to be cleared first.
> With the code above removed, &sb is unmodified since
> struct strbuf sb = STRBUF_INIT; hence the removal is ok here.
> It is related, but only when looking at the entirety of the code. :-/

Ah I see. Thanks!

-- 
Brandon Williams

[GSoC][PATCH 0/8] Update: Week 9

[Prathamesh Chavan]

SUMMARY OF MY PROJECT:

Git submodule subcommands are currently implemented by using shell script
'git-submodule.sh'. There are several reasons why we'll prefer not to
use the shell script. My project intends to convert the subcommands into
C code, thus making them builtins. This will increase Git's portability
and hence the efficiency of working with the git-submodule commands.
Link to the complete proposal: [1]

Mentors:
Stefan Beller <sbeller@google.com>
Christian Couder <christian.couder@gmail.com>

UPDATES:

Following are the updates about my ongoing project:

* status: Certain optimization were implemented as they were suggested.
  Also, the new version was posted for review[2]. This update also
  contains the above-stated version of this patch. 

* sync: The lasted version was posted on the mailing list[3].
  This update also contains the above-stated version of this patch.

* summary: This patch is updated after its last review.
  and the updated one is attached with this update.

* add: porting of this submodule subcommand has started.

* foreach: The former patch[4] posted on the mailing list has been split
  into smaller patches, along with certain additional changes which
  were suggested in the reviews. The patch is currently being posted
  discussion with the mentors and I aim to post it on the mailing
  list soon.

PLAN FOR WEEK-10 (18 July 2017 to 24 July 2017):

* foreach: After having a discussion with the mentors about the prepared
  patch, I'll post the patches.

* add: the porting of this subcommand has begun and will aim to finish
  it by the end of this week.

* Apart from that, I also aim to work on getting the rest of the patches
  ('status', 'sync', 'deinit', and other functions) merged.

Apart from this, sorry for posting the update late for this week. I arrived
at my college late yesterday and hence wasn't able to prepare this with
the ongoing classes. Also, I would do my best so that this doesn't occur
again.

[1]: https://docs.google.com/document/d/1krxVLooWl--75Pot3dazhfygR3wCUUWZWzTXtK1L-xU/
[2]: https://public-inbox.org/git/20170713200538.25806-4-pc44800@gmail.com/
[3]: https://public-inbox.org/git/20170713200538.25806-5-pc44800@gmail.com/
[4]: https://public-inbox.org/git/20170603003710.5558-1-sbeller@google.com/

Prathamesh Chavan (8):
  submodule--helper: introduce get_submodule_displaypath()
  submodule--helper: introduce for_each_submodule_list()
  submodule: port set_name_rev() from shell to C
  submodule: port submodule subcommand 'status' from shell to C
  submodule: port submodule subcommand 'sync' from shell to C
  submodule: port submodule subcommand 'deinit' from shell to C
  diff: change scope of the function count_lines()
  submodule: port submodule subcommand 'summary' from shell to C

 builtin/submodule--helper.c | 1070 ++++++++++++++++++++++++++++++++++++++++++-
 diff.c                      |    2 +-
 diff.h                      |    1 +
 git-submodule.sh            |  354 +-------------
 4 files changed, 1056 insertions(+), 371 deletions(-)

-- 
2.13.0

[GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Prathamesh Chavan]

This aims to make git-submodule 'status' a built-in. Hence, the function
cmd_status() is ported from shell to C. This is done by introducing
three functions: module_status(), submodule_status() and print_status().

The function module_status() acts as the front-end of the subcommand.
It parses subcommand's options and then calls the function
module_list_compute() for computing the list of submodules. Then
this functions calls for_each_submodule_list() looping through the
list obtained.

Then for_each_submodule_list() calls submodule_status() for each of the
submodule in its list. The function submodule_status() is responsible
for generating the status each submodule it is called for, and
then calls print_status().

Finally, the function print_status() handles the printing of submodule's
status.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
 builtin/submodule--helper.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  49 +--------------
 2 files changed, 147 insertions(+), 48 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 80f744407..9c1630495 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -560,6 +560,151 @@ static int module_init(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+struct status_cb {
+	const char *prefix;
+	unsigned int quiet: 1;
+	unsigned int recursive: 1;
+	unsigned int cached: 1;
+};
+#define STATUS_CB_INIT { NULL, 0, 0, 0 }
+
+static void print_status(struct status_cb *info, char state, const char *path,
+			 char *sub_sha1, char *displaypath)
+{
+	if (info->quiet)
+		return;
+
+	printf("%c%s %s", state, sub_sha1, displaypath);
+
+	if (state == ' ' || state == '+') {
+		struct argv_array name_rev_args = ARGV_ARRAY_INIT;
+
+		argv_array_pushl(&name_rev_args, "print-name-rev",
+				 path, sub_sha1, NULL);
+		print_name_rev(name_rev_args.argc, name_rev_args.argv,
+			       info->prefix);
+	} else {
+		printf("\n");
+	}
+}
+
+static int handle_submodule_head_ref(const char *refname,
+				     const struct object_id *oid, int flags,
+				     void *cb_data)
+{
+	struct strbuf *output = cb_data;
+	if (oid)
+		strbuf_addstr(output, oid_to_hex(oid));
+	return 0;
+}
+
+static void status_submodule(const struct cache_entry *list_item, void *cb_data)
+{
+	struct status_cb *info = cb_data;
+	char *sub_sha1 = xstrdup(oid_to_hex(&list_item->oid));
+	char *displaypath;
+	struct stat st;
+
+	if (!submodule_from_path(null_sha1, list_item->name))
+		die(_("no submodule mapping found in .gitmodules for path '%s'"),
+		      list_item->name);
+
+	displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+	if (list_item->ce_flags) {
+		print_status(info, 'U', list_item->name,
+			     sha1_to_hex(null_sha1), displaypath);
+		goto cleanup;
+	}
+
+	if (!is_submodule_active(the_repository, list_item->name)) {
+		print_status(info, '-', list_item->name, sub_sha1, displaypath);
+		goto cleanup;
+	}
+
+	if (!lstat(list_item->name, &st) && !ce_match_stat(list_item, &st, 0)) {
+		print_status(info, ' ', list_item->name, sub_sha1, displaypath);
+	} else {
+		if (!info->cached) {
+			struct strbuf sb = STRBUF_INIT;
+			if (head_ref_submodule(list_item->name,
+					       handle_submodule_head_ref, &sb))
+				die(_("could not resolve HEAD ref inside the"
+				      "submodule '%s'"), list_item->name);
+			print_status(info, '+', list_item->name, sb.buf,
+				     displaypath);
+			strbuf_release(&sb);
+		} else {
+			print_status(info, '+', list_item->name, sub_sha1,
+				     displaypath);
+		}
+	}
+
+	if (info->recursive) {
+		struct child_process cpr = CHILD_PROCESS_INIT;
+
+		cpr.git_cmd = 1;
+		cpr.dir = list_item->name;
+		prepare_submodule_repo_env(&cpr.env_array);
+
+		argv_array_pushl(&cpr.args, "--super-prefix", displaypath,
+				 "submodule--helper", "status", "--recursive",
+				 NULL);
+
+		if (info->cached)
+			argv_array_push(&cpr.args, "--cached");
+
+		if (info->quiet)
+			argv_array_push(&cpr.args, "--quiet");
+
+		if (run_command(&cpr))
+			die(_("failed to recurse into submodule '%s'"),
+			      list_item->name);
+	}
+
+cleanup:
+	free(displaypath);
+	free(sub_sha1);
+}
+
+static int module_status(int argc, const char **argv, const char *prefix)
+{
+	struct status_cb info = STATUS_CB_INIT;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+	int quiet = 0;
+	int cached = 0;
+	int recursive = 0;
+
+	struct option module_status_options[] = {
+		OPT__QUIET(&quiet, N_("Suppress submodule status output")),
+		OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")),
+		OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")),
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, module_status_options,
+			     git_submodule_helper_usage, 0);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	info.prefix = prefix;
+	info.quiet = !!quiet;
+	info.recursive = !!recursive;
+	info.cached = !!cached;
+
+	gitmodules_config();
+	for_each_submodule_list(list, status_submodule, &info);
+
+	return 0;
+}
+
 static int module_name(int argc, const char **argv, const char *prefix)
 {
 	const struct submodule *sub;
@@ -1306,6 +1451,7 @@ static struct cmd_struct commands[] = {
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
 	{"print-name-rev", print_name_rev, 0},
 	{"init", module_init, SUPPORT_SUPER_PREFIX},
+	{"status", module_status, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"push-check", push_check, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index e988167e0..51b057d82 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1005,54 +1005,7 @@ cmd_status()
 		shift
 	done
 
-	{
-		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched" $?
-	} |
-	while read -r mode sha1 stage sm_path
-	do
-		die_if_unmatched "$mode" "$sha1"
-		name=$(git submodule--helper name "$sm_path") || exit
-		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
-		if test "$stage" = U
-		then
-			say "U$sha1 $displaypath"
-			continue
-		fi
-		if ! git submodule--helper is-active "$sm_path" ||
-		{
-			! test -d "$sm_path"/.git &&
-			! test -f "$sm_path"/.git
-		}
-		then
-			say "-$sha1 $displaypath"
-			continue;
-		fi
-		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
-		then
-			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
-			say " $sha1 $displaypath$revname"
-		else
-			if test -z "$cached"
-			then
-				sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
-			fi
-			revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1")
-			say "+$sha1 $displaypath$revname"
-		fi
-
-		if test -n "$recursive"
-		then
-			(
-				prefix="$displaypath/"
-				sanitize_submodule_env
-				wt_prefix=
-				cd "$sm_path" &&
-				eval cmd_status
-			) ||
-			die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
-		fi
-	done
+	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@"
 }
 #
 # Sync remote urls for submodules
-- 
2.13.0

Re: [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Stefan Beller]

On Tue, Jul 18, 2017 at 1:49 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> This aims to make git-submodule 'status' a built-in. Hence, the function
> cmd_status() is ported from shell to C. This is done by introducing
> three functions: module_status(), submodule_status() and print_status().
>
> The function module_status() acts as the front-end of the subcommand.
> It parses subcommand's options and then calls the function
> module_list_compute() for computing the list of submodules. Then
> this functions calls for_each_submodule_list() looping through the
> list obtained.
>
> Then for_each_submodule_list() calls submodule_status() for each of the
> submodule in its list. The function submodule_status() is responsible
> for generating the status each submodule it is called for, and
> then calls print_status().
>
> Finally, the function print_status() handles the printing of submodule's
> status.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
>  builtin/submodule--helper.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
>  git-submodule.sh            |  49 +--------------
>  2 files changed, 147 insertions(+), 48 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 80f744407..9c1630495 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -560,6 +560,151 @@ static int module_init(int argc, const char **argv, const char *prefix)
>         return 0;
>  }
>
> +struct status_cb {
> +       const char *prefix;
> +       unsigned int quiet: 1;
> +       unsigned int recursive: 1;
> +       unsigned int cached: 1;
> +};
> +#define STATUS_CB_INIT { NULL, 0, 0, 0 }
> +
> +static void print_status(struct status_cb *info, char state, const char *path,
> +                        char *sub_sha1, char *displaypath)
> +{
> +       if (info->quiet)
> +               return;
> +
> +       printf("%c%s %s", state, sub_sha1, displaypath);
> +
> +       if (state == ' ' || state == '+') {
> +               struct argv_array name_rev_args = ARGV_ARRAY_INIT;
> +
> +               argv_array_pushl(&name_rev_args, "print-name-rev",
> +                                path, sub_sha1, NULL);
> +               print_name_rev(name_rev_args.argc, name_rev_args.argv,
> +                              info->prefix);
> +       } else {
> +               printf("\n");
> +       }
> +}
> +
> +static int handle_submodule_head_ref(const char *refname,
> +                                    const struct object_id *oid, int flags,
> +                                    void *cb_data)
> +{
> +       struct strbuf *output = cb_data;
> +       if (oid)
> +               strbuf_addstr(output, oid_to_hex(oid));
> +       return 0;
> +}
> +
> +static void status_submodule(const struct cache_entry *list_item, void *cb_data)
> +{
> +       struct status_cb *info = cb_data;
> +       char *sub_sha1 = xstrdup(oid_to_hex(&list_item->oid));
> +       char *displaypath;
> +       struct stat st;
> +
> +       if (!submodule_from_path(null_sha1, list_item->name))
> +               die(_("no submodule mapping found in .gitmodules for path '%s'"),
> +                     list_item->name);
> +
> +       displaypath = get_submodule_displaypath(list_item->name, info->prefix);
> +
> +       if (list_item->ce_flags) {
> +               print_status(info, 'U', list_item->name,
> +                            sha1_to_hex(null_sha1), displaypath);
> +               goto cleanup;
> +       }
> +
> +       if (!is_submodule_active(the_repository, list_item->name)) {
> +               print_status(info, '-', list_item->name, sub_sha1, displaypath);
> +               goto cleanup;
> +       }
> +
> +       if (!lstat(list_item->name, &st) && !ce_match_stat(list_item, &st, 0)) {
> +               print_status(info, ' ', list_item->name, sub_sha1, displaypath);

The question from the last round still stands
https://public-inbox.org/git/CAGZ79kb18z5zc9iu3Vv5aVZWJmoZzmwbMVpy89VC-t-ei2M+bw@mail.gmail.com/

  I am not an expert in the diff area  and wonder how
  the cmd_diff_files functionality is achieved with just a stat call
  and then comparing it to  ce_match_stat. 'Using "dirty" ignores
  all changes to the work tree of submodules, only changes to the
  commits stored in the superproject are shown.' So I'd have
  expected ce->oid to be compared (is there an index entry differing,
  i.e. more than one stage?)

Re: [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>> +       if (!lstat(list_item->name, &st) && !ce_match_stat(list_item, &st, 0)) {
>> +               print_status(info, ' ', list_item->name, sub_sha1, displaypath);
>
> The question from the last round still stands
> https://public-inbox.org/git/CAGZ79kb18z5zc9iu3Vv5aVZWJmoZzmwbMVpy89VC-t-ei2M+bw@mail.gmail.com/
>
>   I am not an expert in the diff area  and wonder how
>   the cmd_diff_files functionality is achieved with just a stat call
>   and then comparing it to  ce_match_stat. 'Using "dirty" ignores
>   all changes to the work tree of submodules, only changes to the
>   commits stored in the superproject are shown.' So I'd have
>   expected ce->oid to be compared (is there an index entry differing,
>   i.e. more than one stage?)

ce_match_stat() calls into ce_compare_gitlink() for a 160000 entry,
which would resolve HEAD ref there and compares ce->oid with it.

But as you said, this is probably insufficient to emulate the
original.  Shouldn't it call into run_diff_files(), which is the
in-core way to run the equivalent of "diff-files"?

Re: [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Stefan Beller]

On Tue, Jul 18, 2017 at 3:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> +       if (!lstat(list_item->name, &st) && !ce_match_stat(list_item, &st, 0)) {
>>> +               print_status(info, ' ', list_item->name, sub_sha1, displaypath);
>>
>> The question from the last round still stands
>> https://public-inbox.org/git/CAGZ79kb18z5zc9iu3Vv5aVZWJmoZzmwbMVpy89VC-t-ei2M+bw@mail.gmail.com/
>>
>>   I am not an expert in the diff area  and wonder how
>>   the cmd_diff_files functionality is achieved with just a stat call
>>   and then comparing it to  ce_match_stat. 'Using "dirty" ignores
>>   all changes to the work tree of submodules, only changes to the
>>   commits stored in the superproject are shown.' So I'd have
>>   expected ce->oid to be compared (is there an index entry differing,
>>   i.e. more than one stage?)
>
> ce_match_stat() calls into ce_compare_gitlink() for a 160000 entry,
> which would resolve HEAD ref there and compares ce->oid with it.

Oh in that case this should be fine, as in the original we did
"git diff-files --ignore-submodules=dirty <path>",
which did precisely that.

> But as you said, this is probably insufficient to emulate the
> original.  Shouldn't it call into run_diff_files(), which is the
> in-core way to run the equivalent of "diff-files"?

Oh, your comment in [1] was related to cmd_diff_files,
which is more complicated than run_diff_files?
run_diff_files also iterates over all cache entries.
I think we need to be looking at match_stat_with_submodule
to figure out what we need to do.

[1] https://public-inbox.org/git/xmqq60fdoyyt.fsf@gitster.mtv.corp.google.com/

Re: [GSoC][PATCH 4/8] submodule: port submodule subcommand 'status' from shell to C

[Junio C Hamano]

On Tue, Jul 18, 2017 at 3:44 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jul 18, 2017 at 3:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>>> +       if (!lstat(list_item->name, &st) && !ce_match_stat(list_item, &st, 0)) {
>>>> +               print_status(info, ' ', list_item->name, sub_sha1, displaypath);
>>>
>>> The question from the last round still stands
>>> https://public-inbox.org/git/CAGZ79kb18z5zc9iu3Vv5aVZWJmoZzmwbMVpy89VC-t-ei2M+bw@mail.gmail.com/
>>>
>>>   I am not an expert in the diff area  and wonder how
>>>   the cmd_diff_files functionality is achieved with just a stat call
>>>   and then comparing it to  ce_match_stat. 'Using "dirty" ignores
>>>   all changes to the work tree of submodules, only changes to the
>>>   commits stored in the superproject are shown.' So I'd have
>>>   expected ce->oid to be compared (is there an index entry differing,
>>>   i.e. more than one stage?)
>>
>> ce_match_stat() calls into ce_compare_gitlink() for a 160000 entry,
>> which would resolve HEAD ref there and compares ce->oid with it.
>
> Oh in that case this should be fine, as in the original we did
> "git diff-files --ignore-submodules=dirty <path>",
> which did precisely that.

Are you sure that this will work correctly when ce is unmerged?
run_diff_files() has quite a lot of code for that case.