* [GSoC][PATCH 01/13] submodule--helper: introduce get_submodule_displaypath()
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 02/13] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
` (11 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 02/13] submodule--helper: introduce for_each_submodule_list()
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 01/13] submodule--helper: introduce get_submodule_displaypath() Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 03/13] submodule: port set_name_rev() from shell to C Prathamesh Chavan
` (10 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 03/13] submodule: port set_name_rev() from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 01/13] submodule--helper: introduce get_submodule_displaypath() Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 02/13] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' " Prathamesh Chavan
` (9 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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>
---
In this new version, the following changes have been made:
* The variable namerev from print_name_rev is now freed at the end of the
function.
builtin/submodule--helper.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 16 ++----------
2 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e41572f7a..2cb72d68e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -244,6 +244,69 @@ 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");
+
+ free(namerev);
+ return 0;
+}
+
struct module_list {
const struct cache_entry **entries;
int alloc, nr;
@@ -1242,6 +1305,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (2 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 03/13] submodule: port set_name_rev() from shell to C Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 21:12 ` Stefan Beller
2017-07-31 20:56 ` [GSoC][PATCH 05/13] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
` (8 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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>
---
In this new version, the following changes have been made:
* parameters passed to the function print_status() have been changed.
Instead of passing char *sub_sha1, instead the object_id is being passed.
* Also, since the passed parameter displaypath's value isn't changed
by the function, it is passed to the funcition as const char *displaypath
instead of char *displaypath.
* the output type of the function handle_submodule_head_ref() is changed
from strbuf to object_id, as we will use the object_id instead of the
hex of sha1 being stored in a struct strbuf.
* diff_files_args is cleared after using it by passing it as args in the
function cmd_diff_files.
* In the function status_submodule(), for checking if a submodule has merge
conflicts, the patch currently checks if the value of any of the ce_flags
is non-zero. Currently, I think the we aren't interested in a partiular flag,
but I'm not sure on this.
* Debugging leftovers and suprious new-lines are removed.
* The confusion with displaypath being passed as te super-prefix in many
of the ported subcommands may be a result of the fact that the
function generating the displaypath: get_submodule_displaypath()
uses the super-prefix as simply a path concatenated with the current
submodule name to denote our current location.
The function get_super_prefix() is declared in cache.h and defined in
environment.c, but is majorly used in the builtin/submodule--helper.c
and also in unpack-trees.c
Also, for generating any submodule's displaypath, it would be important to
have ".." passed to the submodule, and currently it is possible only via the
super-prefix.
This is also other instaces where the super-prefix contained ".." as well.
One of such instance is Test 4 from t7406-submodule-update.sh
Hence, maybe documenting the value of displaypath might a solution
for the above problem.
I'm just stating my views and would like to recieve your opinion on this
matter.
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 2cb72d68e..a6e6a48cc 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -561,6 +561,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,
+ const struct object_id *oid, const char *displaypath)
+{
+ if (info->quiet)
+ return;
+
+ printf("%c%s %s", state, oid_to_hex(oid), displaypath);
+
+ if (state == ' ' || state == '+') {
+ struct argv_array name_rev_args = ARGV_ARRAY_INIT;
+
+ argv_array_pushl(&name_rev_args, "print-name-rev",
+ path, oid_to_hex(oid), 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 object_id *output = cb_data;
+ if (oid)
+ oidcpy(output, oid);
+
+ return 0;
+}
+
+static void status_submodule(const struct cache_entry *list_item, void *cb_data)
+{
+ struct status_cb *info = cb_data;
+ 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,
+ &null_oid, displaypath);
+ goto cleanup;
+ }
+
+ if (!is_submodule_active(the_repository, list_item->name)) {
+ print_status(info, '-', list_item->name, &list_item->oid,
+ displaypath);
+ goto cleanup;
+ }
+
+ argv_array_pushl(&diff_files_args, "diff-files",
+ "--ignore-submodules=dirty", "--quiet", "--",
+ list_item->name, NULL);
+
+ if (!cmd_diff_files(diff_files_args.argc, diff_files_args.argv,
+ info->prefix)) {
+ print_status(info, ' ', list_item->name, &list_item->oid,
+ displaypath);
+ } else {
+ if (!info->cached) {
+ struct object_id oid;
+
+ if (head_ref_submodule(list_item->name,
+ handle_submodule_head_ref, &oid))
+ die(_("could not resolve HEAD ref inside the"
+ "submodule '%s'"), list_item->name);
+
+ print_status(info, '+', list_item->name, &oid,
+ displaypath);
+ } else {
+ print_status(info, '+', list_item->name,
+ &list_item->oid, 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:
+ argv_array_clear(&diff_files_args);
+ free(displaypath);
+}
+
+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;
@@ -1307,6 +1460,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' " Prathamesh Chavan
@ 2017-07-31 21:12 ` Stefan Beller
2017-08-01 21:14 ` Prathamesh Chavan
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Beller @ 2017-07-31 21:12 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Christian Couder
On Mon, Jul 31, 2017 at 1:56 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>
> ---
> In this new version, the following changes have been made:
> * parameters passed to the function print_status() have been changed.
> Instead of passing char *sub_sha1, instead the object_id is being passed.
>
> * Also, since the passed parameter displaypath's value isn't changed
> by the function, it is passed to the funcition as const char *displaypath
> instead of char *displaypath.
>
> * the output type of the function handle_submodule_head_ref() is changed
> from strbuf to object_id, as we will use the object_id instead of the
> hex of sha1 being stored in a struct strbuf.
>
> * diff_files_args is cleared after using it by passing it as args in the
> function cmd_diff_files.
>
> * In the function status_submodule(), for checking if a submodule has merge
> conflicts, the patch currently checks if the value of any of the ce_flags
> is non-zero. Currently, I think the we aren't interested in a partiular flag,
> but I'm not sure on this.
>
> * Debugging leftovers and suprious new-lines are removed.
>
> * The confusion with displaypath being passed as te super-prefix in many
> of the ported subcommands may be a result of the fact that the
> function generating the displaypath: get_submodule_displaypath()
> uses the super-prefix as simply a path concatenated with the current
> submodule name to denote our current location.
> The function get_super_prefix() is declared in cache.h and defined in
> environment.c, but is majorly used in the builtin/submodule--helper.c
> and also in unpack-trees.c
> Also, for generating any submodule's displaypath, it would be important to
> have ".." passed to the submodule, and currently it is possible only via the
> super-prefix.
> This is also other instaces where the super-prefix contained ".." as well.
> One of such instance is Test 4 from t7406-submodule-update.sh
> Hence, maybe documenting the value of displaypath might a solution
> for the above problem.
> I'm just stating my views and would like to recieve your opinion on this
> matter.
Yes, I agree that the display path is not quite easily understood as it can be
ambiguous. I am confused by this paragraph:
* does test 4 from 7406 fail here, or was it just the starting point
of the discussion and it all works fine?
I have reviewed the patches up to (and including this) patch and
so far they look good to me.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' from shell to C
2017-07-31 21:12 ` Stefan Beller
@ 2017-08-01 21:14 ` Prathamesh Chavan
0 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-08-01 21:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Christian Couder, Brandon Williams
On Tue, Aug 1, 2017 at 2:42 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Jul 31, 2017 at 1:56 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>
>> ---
>> In this new version, the following changes have been made:
>> * parameters passed to the function print_status() have been changed.
>> Instead of passing char *sub_sha1, instead the object_id is being passed.
>>
>> * Also, since the passed parameter displaypath's value isn't changed
>> by the function, it is passed to the funcition as const char *displaypath
>> instead of char *displaypath.
>>
>> * the output type of the function handle_submodule_head_ref() is changed
>> from strbuf to object_id, as we will use the object_id instead of the
>> hex of sha1 being stored in a struct strbuf.
>>
>> * diff_files_args is cleared after using it by passing it as args in the
>> function cmd_diff_files.
>>
>> * In the function status_submodule(), for checking if a submodule has merge
>> conflicts, the patch currently checks if the value of any of the ce_flags
>> is non-zero. Currently, I think the we aren't interested in a partiular flag,
>> but I'm not sure on this.
>>
>> * Debugging leftovers and suprious new-lines are removed.
>>
>> * The confusion with displaypath being passed as te super-prefix in many
>> of the ported subcommands may be a result of the fact that the
>> function generating the displaypath: get_submodule_displaypath()
>> uses the super-prefix as simply a path concatenated with the current
>> submodule name to denote our current location.
>> The function get_super_prefix() is declared in cache.h and defined in
>> environment.c, but is majorly used in the builtin/submodule--helper.c
>> and also in unpack-trees.c
>> Also, for generating any submodule's displaypath, it would be important to
>> have ".." passed to the submodule, and currently it is possible only via the
>> super-prefix.
>> This is also other instaces where the super-prefix contained ".." as well.
>> One of such instance is Test 4 from t7406-submodule-update.sh
>> Hence, maybe documenting the value of displaypath might a solution
>> for the above problem.
>> I'm just stating my views and would like to recieve your opinion on this
>> matter.
>
> Yes, I agree that the display path is not quite easily understood as it can be
> ambiguous. I am confused by this paragraph:
> * does test 4 from 7406 fail here, or was it just the starting point
> of the discussion and it all works fine?
Sorry for misleading there. In the discussion earlier, Brandon mentioned that
displaypath's value may contain ".." as well.
To this, I replied pointing out one of the test which checks for the value of
displaypath.
In this test, ( Test 4 from t7406-submodule-update.sh), the value of displaypath
is being calculated via the submodule_init() function present in
submodule--helper.c
Here, I wanted to point out that, even in the case of this function,
the variable
displaypath is evaluated after receiving the value of "../super/" as
the value returned
by the function get_super_prefix().
Hence, to correct this confusion about the usage of super-prefix,
(which as pointed out by Brandon has been traditionally defined
as the path from the root of the superproject down to the root of
the submodule which further means that there should not ever be
any relative '..'s.), we may either have to get the value of super-prefix's
Documentation to accomodate this change, and to accept the way it is
used in submodule--helper and the current patch series, or else,
change the way it is being used in the various function, and used
another method for evaluation of displaypath.
Thanks,
Prathamesh Chavan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [GSoC][PATCH 05/13] submodule: port submodule subcommand 'sync' from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (3 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 04/13] submodule: port submodule subcommand 'status' " Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 21:19 ` Stefan Beller
2017-07-31 20:56 ` [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' " Prathamesh Chavan
` (7 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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>
---
In this new version, the following changes have been made:
* There was no good reason for using puts in the function print_default_remote()
Hence, in this patch, we instead use printf to do the same, as it is what
is generally used throughout the codebase.
* As suggested, this patch ensures a more efficient use of variables, and
removes most of the variables by reusing 'strbuf sb' at places required.
builtin/submodule--helper.c | 182 ++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 56 +-------------
2 files changed, 183 insertions(+), 55 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a6e6a48cc..91945337f 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)
+ printf("%s\n", remote);
+
+ return 0;
+}
+
static int starts_with_dot_slash(const char *str)
{
return str[0] == '.' && is_dir_sep(str[1]);
@@ -380,6 +394,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;
@@ -733,6 +766,153 @@ 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 *remote_key;
+ char *sub_origin_url, *super_config_url, *displaypath;
+ struct strbuf sb = STRBUF_INIT;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ char *sub_config_path = NULL;
+
+ if (!is_submodule_active(the_repository, list_item->name))
+ return;
+
+ sub = submodule_from_path(null_sha1, list_item->name);
+
+ if (sub && sub->url) {
+ if (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) {
+ char *remote_url, *up_path;
+ char *remote = get_default_remote();
+ strbuf_addf(&sb, "remote.%s.url", remote);
+
+ if (git_config_get_string(sb.buf, &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);
+ free(up_path);
+ free(remote_url);
+ } else {
+ sub_origin_url = xstrdup(sub->url);
+ super_config_url = xstrdup(sub->url);
+ }
+ } else {
+ sub_origin_url = "";
+ super_config_url = "";
+ }
+
+ displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+ if (!info->quiet)
+ printf(_("Synchronizing submodule url for '%s'\n"),
+ displaypath);
+
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "submodule.%s.url", sub->name);
+ if (git_config_set_gently(sb.buf, 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);
+
+ strbuf_reset(&sb);
+ 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_reset(&sb);
+ submodule_to_gitdir(&sb, list_item->name);
+ strbuf_addstr(&sb, "/config");
+
+ if (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url))
+ 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:
+ strbuf_release(&sb);
+ free(super_config_url);
+ free(displaypath);
+ free(sub_config_path);
+ 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)
@@ -1461,6 +1641,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 05/13] submodule: port submodule subcommand 'sync' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 05/13] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
@ 2017-07-31 21:19 ` Stefan Beller
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Beller @ 2017-07-31 21:19 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Christian Couder
On Mon, Jul 31, 2017 at 1:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> In this new version, the following changes have been made:
> * There was no good reason for using puts in the function print_default_remote()
> Hence, in this patch, we instead use printf to do the same, as it is what
> is generally used throughout the codebase.
Hah! I refrained on suggesting the opposite at the last patch.
In older code there are lots of "puts", c.f. "git grep puts", so I assumed
it to be a micro optimization, hence I would have suggested it.
Either way is fine with me.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (4 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 05/13] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 21:42 ` Stefan Beller
2017-07-31 20:56 ` [GSoC][PATCH 07/13] diff: change scope of the function count_lines() Prathamesh Chavan
` (6 subsequent siblings)
12 siblings, 1 reply; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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>
---
In this new version, the following changes have been made:
* In the function deinit_submodule, since the test is_git_directory()
adds an additional condition, instead is_directory() is used to check
if "sm_path/.git" is a directory.
* since it was possible in the previous path that the value st.st_mode passed
to the function mkdir contained a garbage value, instead we intrduce a
mode_t variable mode, initially containing a default mode value '0777'.
This is what the default of mode is set in case, that the value is
not set after the lstat call.
builtin/submodule--helper.c | 144 ++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 55 +----------------
2 files changed, 145 insertions(+), 54 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 91945337f..f642f9889 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -913,6 +913,149 @@ 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 *sub_git_dir = xstrfmt("%s/.git", list_item->name);
+ mode_t mode = 0777;
+
+ sub = submodule_from_path(null_sha1, list_item->name);
+
+ if (!sub || !sub->name)
+ goto cleanup;
+
+ displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+ /* remove the submodule work tree (unless the user already did it) */
+ if (is_directory(list_item->name)) {
+ struct stat st;
+ /* protect submodules containing a .git directory */
+ if (is_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",
+ list_item->name, NULL);
+
+ if (run_command(&cp_rm))
+ die(_("Submodule work tree '%s' contains local "
+ "modifications; use '-f' to discard them"),
+ displaypath);
+ }
+
+ if (!lstat(list_item->name, &st)) {
+ struct strbuf sb_rm = STRBUF_INIT;
+ const char *format;
+
+ strbuf_addstr(&sb_rm, list_item->name);
+
+ if (!remove_dir_recursively(&sb_rm, 0))
+ format = _("Cleared directory '%s'\n");
+ else
+ format = _("Could not remove submodule work tree '%s'\n");
+
+ if (!info->quiet)
+ printf(format, displaypath);
+
+ mode = st.st_mode;
+
+ strbuf_release(&sb_rm);
+ }
+ }
+
+ if (mkdir(list_item->name, 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);
+ 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)
@@ -1643,6 +1786,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' " Prathamesh Chavan
@ 2017-07-31 21:42 ` Stefan Beller
2017-08-01 21:19 ` Prathamesh Chavan
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Beller @ 2017-07-31 21:42 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Christian Couder
On Mon, Jul 31, 2017 at 1:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> 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>
> ---
> In this new version, the following changes have been made:
> * In the function deinit_submodule, since the test is_git_directory()
> adds an additional condition, instead is_directory() is used to check
> if "sm_path/.git" is a directory.
Thanks for writing these patches.
I wonder if (some of) these notes are best put into the code
as a comment such as
/* NEEDSWORK: convert to is_submodule_active */
such that people reading this code later realize that checking
for a directory may not be the "correct" thing, but a thing which
was easy to express using shell.
> +struct deinit_cb {
> + const char *prefix;
> + unsigned int quiet: 1;
> + unsigned int force: 1;
> + unsigned int all: 1;
The value 'all' seems to be unused, i.e. we assign it but never read it?
> +};
> +#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 *sub_git_dir = xstrfmt("%s/.git", list_item->name);
> + mode_t mode = 0777;
> +
> + sub = submodule_from_path(null_sha1, list_item->name);
> +
> + if (!sub || !sub->name)
> + goto cleanup;
> +
> + displaypath = get_submodule_displaypath(list_item->name, info->prefix);
> +
> + /* remove the submodule work tree (unless the user already did it) */
> + if (is_directory(list_item->name)) {
> + struct stat st;
> + /* protect submodules containing a .git directory */
Here may a good place to put:
/* NEEDSWORK: automatically call absorbgitdirs before warning/die. */
(It was not in the shell version, so feel free to ignore)
> + if (!info->force) {
> + struct child_process cp_rm = CHILD_PROCESS_INIT;
> + cp_rm.git_cmd = 1;
> + argv_array_pushl(&cp_rm.args, "rm", "-qn",
> + list_item->name, NULL);
A bug that exists in the shell version as well as here:
What if the submodule has the name '--cached', which happens
to be a valid argument for git-rm?
The call to git-rm would die claiming that the <file> is missing,
as the file name was miss-interpreted as another flag.
To solve this problem we would insert a '--' after the options,
before the file name to state that the last argument is a <file>.
Not sure if we want to fix the bug while we're here or if we rather
want to add
/* NEEDSWORK: add '--' to confirm <file> argument */
> + if (mkdir(list_item->name, mode))
> + die(_("could not create empty submodule directory %s"),
> + displaypath);
In the shell version we used the shell mkdir, which on failure
would print the error to stderr on its own. The added "|| say"
is just to clarify the bigger picture.
mkdir in C doesn't print the actual error (e.g.
no diskspace, permissions) so we have to do it.
Use 'die_errno' instead as then the reason will be given as well.
> + 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);
It would be nice if the commit message could give hints as why a literal
translation was chosen here and not e.g. one of the builtin config
calls, as that would omit spawning a process.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' from shell to C
2017-07-31 21:42 ` Stefan Beller
@ 2017-08-01 21:19 ` Prathamesh Chavan
0 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-08-01 21:19 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Christian Couder
On Tue, Aug 1, 2017 at 3:12 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Jul 31, 2017 at 1:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>> 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>
>> ---
>> In this new version, the following changes have been made:
>> * In the function deinit_submodule, since the test is_git_directory()
>> adds an additional condition, instead is_directory() is used to check
>> if "sm_path/.git" is a directory.
>
> Thanks for writing these patches.
> I wonder if (some of) these notes are best put into the code
> as a comment such as
>
> /* NEEDSWORK: convert to is_submodule_active */
>
> such that people reading this code later realize that checking
> for a directory may not be the "correct" thing, but a thing which
> was easy to express using shell.
>
>> +struct deinit_cb {
>> + const char *prefix;
>> + unsigned int quiet: 1;
>> + unsigned int force: 1;
>> + unsigned int all: 1;
>
> The value 'all' seems to be unused, i.e. we assign it but never read it?
>
>> +};
>> +#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 *sub_git_dir = xstrfmt("%s/.git", list_item->name);
>> + mode_t mode = 0777;
>> +
>> + sub = submodule_from_path(null_sha1, list_item->name);
>> +
>> + if (!sub || !sub->name)
>> + goto cleanup;
>> +
>> + displaypath = get_submodule_displaypath(list_item->name, info->prefix);
>> +
>> + /* remove the submodule work tree (unless the user already did it) */
>> + if (is_directory(list_item->name)) {
>> + struct stat st;
>> + /* protect submodules containing a .git directory */
>
> Here may a good place to put:
> /* NEEDSWORK: automatically call absorbgitdirs before warning/die. */
> (It was not in the shell version, so feel free to ignore)
>
>> + if (!info->force) {
>> + struct child_process cp_rm = CHILD_PROCESS_INIT;
>> + cp_rm.git_cmd = 1;
>> + argv_array_pushl(&cp_rm.args, "rm", "-qn",
>> + list_item->name, NULL);
>
> A bug that exists in the shell version as well as here:
> What if the submodule has the name '--cached', which happens
> to be a valid argument for git-rm?
>
> The call to git-rm would die claiming that the <file> is missing,
> as the file name was miss-interpreted as another flag.
>
> To solve this problem we would insert a '--' after the options,
> before the file name to state that the last argument is a <file>.
>
> Not sure if we want to fix the bug while we're here or if we rather
> want to add
>
> /* NEEDSWORK: add '--' to confirm <file> argument */
>
IMO, I would first port the subcommand, and add an additional comment
for pointing the bug out. And then later, we may have a bug-fix patch in this
series of patch itself for tackling this bug out.
Thanks,
Prathamesh Chavan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [GSoC][PATCH 07/13] diff: change scope of the function count_lines()
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (5 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 06/13] submodule: port submodule subcommand 'deinit' " Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C Prathamesh Chavan
` (5 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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 85e714f6c..03ed64f93 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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (6 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 07/13] diff: change scope of the function count_lines() Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 22:15 ` Stefan Beller
2017-07-31 23:27 ` Christian Couder
2017-07-31 20:56 ` [GSoC][PATCH 09/13] submodule foreach: correct '$path' in nested submodules from a subdirectory Prathamesh Chavan
` (4 subsequent siblings)
12 siblings, 2 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, 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>
---
In this new version, the following changes have been made:
* Firstly, about the function compute_summary_module_list().
This function is created to generate the list of modules, for which
we will generate the summary further. Since the list is actually
generated using the git-diff-files or git-diff-index command, but for
porting this, we required to create a function similar to the builtin
functions of the above commands. But we can't directly call cmd_diff_files()
and cmd_diff_index() since we don't have to display the output and instead
need to store it. Hence, this function is introduced.
* Also, the module_cb_list *list is not freed since it is a non-heap object.
Hence, free() can't be using on the non-heap objects.
* In the function prepare_submodule_summary(), as suggested
'git_config_get_string_const' was used instead of instead of '_value'
* Some variables which weren't modified throughout the function-call were
passed as const.
* The '!!' trick, which wasn't used in the last patch, is now used in this
new version .
* the variables sha1_dst and sha1_src are removed from the function
print_submodule_summary(), and instead the p->oid_src and p->oid_dst are
used.
* The variable sm_git_dir is freed at the end of the function.
* variable head was no longer used in module_summary() and instead the strbuf
was utilized.
builtin/submodule--helper.c | 425 ++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 182 +------------------
2 files changed, 426 insertions(+), 181 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f642f9889..94438d6ce 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);
@@ -766,6 +769,427 @@ 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 int verify_submodule_object_name(const char *sm_path, const char *sha1)
+{
+ struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
+
+ cp_rev_parse.git_cmd = 1;
+ cp_rev_parse.no_stdout = 1;
+ cp_rev_parse.dir = sm_path;
+ prepare_submodule_repo_env(&cp_rev_parse.env_array);
+
+ argv_array_pushl(&cp_rev_parse.args, "rev-parse", "-q",
+ "--verify", NULL);
+ argv_array_pushf(&cp_rev_parse.args, "%s^0", sha1);
+
+ if (run_command(&cp_rev_parse))
+ return 1;
+
+ return 0;
+}
+
+static void print_submodule_summary(struct summary_cb *info,
+ struct module_cb *p)
+{
+ int missing_src = 0;
+ int missing_dst = 0;
+ char *displaypath;
+ const char *sha1_abbr_src;
+ const char *sha1_abbr_dst;
+ int errmsg = 0;
+ int total_commits = -1;
+ char *sm_git_dir = xstrfmt("%s/.git", p->sm_path);
+ int is_sm_git_dir = 0;
+
+ if (!info->cached && !oidcmp(&p->oid_dst, &null_oid)) {
+ 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;
+ cp_rev_parse.dir = p->sm_path;
+ prepare_submodule_repo_env(&cp_rev_parse.env_array);
+
+ 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");
+
+ get_oid_hex(sb_rev_parse.buf, &p->oid_dst);
+ }
+ 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");
+
+ get_oid_hex(sb_hash_object.buf, &p->oid_dst);
+ }
+ strbuf_release(&sb_hash_object);
+ } else {
+ if (p->mod_dst)
+ 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))
+ missing_src = verify_submodule_object_name(p->sm_path,
+ oid_to_hex(&p->oid_src));
+
+ if (is_sm_git_dir && S_ISGITLINK(p->mod_dst))
+ missing_dst = verify_submodule_object_name(p->sm_path,
+ oid_to_hex(&p->oid_dst));
+
+ 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;
+ char *range;
+
+ if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst))
+ range = xstrfmt("%s...%s", oid_to_hex(&p->oid_src), oid_to_hex(&p->oid_dst));
+ else if (S_ISGITLINK(p->mod_src))
+ range = xstrdup(oid_to_hex(&p->oid_src));
+ else
+ range = xstrdup(oid_to_hex(&p->oid_dst));
+
+ cp_rev_list.git_cmd = 1;
+ cp_rev_list.dir = p->sm_path;
+ prepare_submodule_repo_env(&cp_rev_list.env_array);
+
+ 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;
+ }
+
+ free(range);
+ strbuf_release(&sb_rev_list);
+ }
+ } else {
+ errmsg = 1;
+ }
+
+ sha1_abbr_src = find_unique_abbrev(p->oid_src.hash, 7);
+ sha1_abbr_dst = find_unique_abbrev(p->oid_dst.hash, 7);
+
+ if (p->status == 'T') {
+ if (S_ISGITLINK(p->mod_dst))
+ printf(_("* %s %s(blob)->%s(submodule)"),
+ displaypath, sha1_abbr_src,
+ sha1_abbr_dst);
+ else
+ printf(_("* %s %s(submodule)->%s(blob)"),
+ displaypath, sha1_abbr_src,
+ sha1_abbr_dst);
+ } else {
+ printf("* %s %s...%s", displaypath, sha1_abbr_src,
+ sha1_abbr_dst);
+ }
+
+ if (total_commits < 0)
+ printf(":\n");
+ else
+ printf(" (%d):\n", 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, oid_to_hex(&p->oid_src), oid_to_hex(&p->oid_dst));
+ } else if (missing_src) {
+ printf(_(" Warn: %s doesn't contain commit %s\n"),
+ displaypath, oid_to_hex(&p->oid_src));
+ } else {
+ printf(_(" Warn: %s doesn't contain commit %s\n"),
+ displaypath, oid_to_hex(&p->oid_dst));
+ }
+ }
+ } else if (is_sm_git_dir) {
+ struct child_process cp_log = CHILD_PROCESS_INIT;
+
+ cp_log.git_cmd = 1;
+ cp_log.dir = p->sm_path;
+ prepare_submodule_repo_env(&cp_log.env_array);
+ argv_array_pushl(&cp_log.args, "log", NULL);
+
+ if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst)) {
+ if (info->summary_limits > 0)
+ argv_array_pushf(&cp_log.args, "-%d", info->summary_limits);
+
+ argv_array_pushl(&cp_log.args, "--pretty= %m %s",
+ "--first-parent", NULL);
+ argv_array_pushf(&cp_log.args, "%s...%s", oid_to_hex(&p->oid_src), oid_to_hex(&p->oid_dst));
+ } else if (S_ISGITLINK(p->mod_dst)) {
+ argv_array_pushl(&cp_log.args, "--pretty= > %s",
+ "-1", oid_to_hex(&p->oid_dst), NULL);
+ } else {
+ argv_array_pushl(&cp_log.args, "--pretty= < %s",
+ "-1", oid_to_hex(&p->oid_src), NULL);
+ }
+
+ run_command(&cp_log);
+ }
+ printf("\n");
+
+ free(displaypath);
+ free(sm_git_dir);
+}
+
+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 && p->status != 'A') {
+ config_key = xstrfmt("submodule.%s.ignore",
+ sub->name);
+ if (!git_config_get_string_const(config_key, &value))
+ ignore_config = value;
+ else if (sub->ignore)
+ ignore_config = sub->ignore;
+
+ free(config_key);
+
+ if (!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);
+
+ free(head);
+ 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;
+ struct child_process cp_rev = CHILD_PROCESS_INIT;
+ struct strbuf sb = 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",
+ argc ? argv[0] : "HEAD", NULL);
+
+ if (!capture_command(&cp_rev, &sb, 0)) {
+ strbuf_strip_suffix(&sb, "\n");
+ if (argc) {
+ argv++;
+ argc--;
+ }
+ } else if (!argc || !strcmp(argv[0], "HEAD")) {
+ /* before the first commit: compare with an empty tree */
+ struct stat st;
+ struct object_id oid;
+ if (fstat(0, &st) < 0 || index_fd(oid.hash, 0, &st, 2, prefix, 3))
+ die("Unable to add %s to database", oid.hash);
+ strbuf_addstr(&sb, oid_to_hex(&oid));
+ if (argc) {
+ argv++;
+ argc--;
+ }
+ } else {
+ strbuf_addstr(&sb, "HEAD");
+ }
+
+ if (files) {
+ if (cached)
+ die(_("The --cached option cannot be used with the --files option"));
+ diff_cmd = "diff-files";
+
+ strbuf_reset(&sb);
+ sb.buf = 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(strbuf_detach(&sb, NULL), &info);
+}
+
struct sync_cb {
const char *prefix;
unsigned int quiet: 1;
@@ -1787,6 +2211,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C Prathamesh Chavan
@ 2017-07-31 22:15 ` Stefan Beller
2017-07-31 23:27 ` Christian Couder
1 sibling, 0 replies; 25+ messages in thread
From: Stefan Beller @ 2017-07-31 22:15 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Christian Couder
On Mon, Jul 31, 2017 at 1:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> 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>
> ---
> In this new version, the following changes have been made:
>
> * Firstly, about the function compute_summary_module_list().
> This function is created to generate the list of modules, for which
> we will generate the summary further. Since the list is actually
> generated using the git-diff-files or git-diff-index command, but for
> porting this, we required to create a function similar to the builtin
> functions of the above commands. But we can't directly call cmd_diff_files()
> and cmd_diff_index() since we don't have to display the output and instead
> need to store it. Hence, this function is introduced.
>
> * Also, the module_cb_list *list is not freed since it is a non-heap object.
> Hence, free() can't be using on the non-heap objects.
>
> * In the function prepare_submodule_summary(), as suggested
> 'git_config_get_string_const' was used instead of instead of '_value'
>
> * Some variables which weren't modified throughout the function-call were
> passed as const.
>
> * The '!!' trick, which wasn't used in the last patch, is now used in this
> new version .
>
> * the variables sha1_dst and sha1_src are removed from the function
> print_submodule_summary(), and instead the p->oid_src and p->oid_dst are
> used.
>
> * The variable sm_git_dir is freed at the end of the function.
>
> * variable head was no longer used in module_summary() and instead the strbuf
> was utilized.
>
> builtin/submodule--helper.c | 425 ++++++++++++++++++++++++++++++++++++++++++++
> git-submodule.sh | 182 +------------------
> 2 files changed, 426 insertions(+), 181 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index f642f9889..94438d6ce 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);
> @@ -766,6 +769,427 @@ 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 int verify_submodule_object_name(const char *sm_path, const char *sha1)
> +{
> + struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
> +
> + cp_rev_parse.git_cmd = 1;
> + cp_rev_parse.no_stdout = 1;
> + cp_rev_parse.dir = sm_path;
> + prepare_submodule_repo_env(&cp_rev_parse.env_array);
> +
> + argv_array_pushl(&cp_rev_parse.args, "rev-parse", "-q",
> + "--verify", NULL);
> + argv_array_pushf(&cp_rev_parse.args, "%s^0", sha1);
> +
> + if (run_command(&cp_rev_parse))
> + return 1;
> +
> + return 0;
> +}
> +
> +static void print_submodule_summary(struct summary_cb *info,
> + struct module_cb *p)
> +{
> + int missing_src = 0;
> + int missing_dst = 0;
> + char *displaypath;
> + const char *sha1_abbr_src;
> + const char *sha1_abbr_dst;
> + int errmsg = 0;
> + int total_commits = -1;
> + char *sm_git_dir = xstrfmt("%s/.git", p->sm_path);
> + int is_sm_git_dir = 0;
> +
> + if (!info->cached && !oidcmp(&p->oid_dst, &null_oid)) {
> + 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;
> + cp_rev_parse.dir = p->sm_path;
> + prepare_submodule_repo_env(&cp_rev_parse.env_array);
> +
> + 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");
> +
> + get_oid_hex(sb_rev_parse.buf, &p->oid_dst);
> + }
> + strbuf_release(&sb_rev_parse);
I think this could be replaced via
head_ref_submodule(sub->path, callback function, &where_to_store)
or is there some trickery going on, that this also works on
non-compliant submodules?
(Maybe add that as a NEEDSWORK/TODO)
> +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;
Recently there was a discussion how to operate the
revision machinery best (search for earlier versions of
js/rebase-i-final if interested), whether we can and want
to directly set flags such as .abbrev or if we'd rather
want to push "--abbrev=0" to the diff_args before the --
bisect and archive both assign abbrev directly, so I think
we're fine here.
> + 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"))
This strcmp smells like we're encoding the state not optimally
in 'info'. Maybe we can have an enum { DIFF_FILES, DIFF_INDEX }
instead of a string (that we assign earlier) and then have to
compare to it again.
> + 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;
> + }
This cascaded decision whether to use
setup_work_tree / read_cache_preload / read_cache
seems quite optimized, hence complicated to read. :)
I like it, though.
> +
> + if (!summary_limits)
> + return 0;
Good call for converting "test $summary_limit = 0 && return".
I suspected this may be an overeager optimization (as no
error checking is done at all, but that is what it is)
f2dc06a344 (git-submodule summary: limit summary size,
2008-03-11) introduced it like this.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C Prathamesh Chavan
2017-07-31 22:15 ` Stefan Beller
@ 2017-07-31 23:27 ` Christian Couder
2017-08-05 10:28 ` Prathamesh Chavan
1 sibling, 1 reply; 25+ messages in thread
From: Christian Couder @ 2017-07-31 23:27 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git, Stefan Beller
On Mon, Jul 31, 2017 at 10:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> * variable head was no longer used in module_summary() and instead the strbuf
> was utilized.
Good but there might be a few problems in the way it is used. See below.
> +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);
> +
> + free(head);
It is a bit strange to have this function free() an argument it is passed.
> + return 0;
> +
Spurious new line.
> +}
> +
> +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;
> + struct child_process cp_rev = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
[...]
> + if (!capture_command(&cp_rev, &sb, 0)) {
> + strbuf_strip_suffix(&sb, "\n");
> + if (argc) {
> + argv++;
> + argc--;
> + }
> + } else if (!argc || !strcmp(argv[0], "HEAD")) {
> + /* before the first commit: compare with an empty tree */
> + struct stat st;
> + struct object_id oid;
> + if (fstat(0, &st) < 0 || index_fd(oid.hash, 0, &st, 2, prefix, 3))
> + die("Unable to add %s to database", oid.hash);
> + strbuf_addstr(&sb, oid_to_hex(&oid));
> + if (argc) {
> + argv++;
> + argc--;
> + }
> + } else {
> + strbuf_addstr(&sb, "HEAD");
> + }
> +
> + if (files) {
> + if (cached)
> + die(_("The --cached option cannot be used with the --files option"));
> + diff_cmd = "diff-files";
> +
> + strbuf_reset(&sb);
strbuf_reset() does not free the memory allocated to sb.buf...
> + sb.buf = NULL;
...then this makes sure that the memory previously allocated to sb.buf
will not be free()d.
Maybe instead of the above 2 lines just: strbuf_release(&sb)
Or maybe just don't set sb.buf to 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(strbuf_detach(&sb, NULL), &info);
Maybe you could pass: "sb.len > 0 ? strbuf_detach(&sb, NULL) : NULL"
This way if sb has previously been released or reset, NULL will be passed.
I would suggest though to just pass sb.buf and to strbuf_release(&sb)
after calling compute_summary_module_list() and before returning from
module_summary() instead of having compute_summary_module_list() free
its first argument.
If you do that then compute_summary_module_list() should be changed so
that when it is passed "" it will behave in the same way as when it is
passed NULL.
> +}
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-07-31 23:27 ` Christian Couder
@ 2017-08-05 10:28 ` Prathamesh Chavan
2017-08-05 16:55 ` Christian Couder
0 siblings, 1 reply; 25+ messages in thread
From: Prathamesh Chavan @ 2017-08-05 10:28 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Stefan Beller
On Tue, Aug 1, 2017 at 4:57 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Mon, Jul 31, 2017 at 10:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>
>> * variable head was no longer used in module_summary() and instead the strbuf
>> was utilized.
>
> Good but there might be a few problems in the way it is used. See below.
>
>> +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);
>> +
>> + free(head);
>
> It is a bit strange to have this function free() an argument it is passed.
>
>> + return 0;
>> +
>
> Spurious new line.
>
>> +}
>> +
>> +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;
>> + struct child_process cp_rev = CHILD_PROCESS_INIT;
>> + struct strbuf sb = STRBUF_INIT;
>
> [...]
>
>> + if (!capture_command(&cp_rev, &sb, 0)) {
>> + strbuf_strip_suffix(&sb, "\n");
>> + if (argc) {
>> + argv++;
>> + argc--;
>> + }
>> + } else if (!argc || !strcmp(argv[0], "HEAD")) {
>> + /* before the first commit: compare with an empty tree */
>> + struct stat st;
>> + struct object_id oid;
>> + if (fstat(0, &st) < 0 || index_fd(oid.hash, 0, &st, 2, prefix, 3))
>> + die("Unable to add %s to database", oid.hash);
>> + strbuf_addstr(&sb, oid_to_hex(&oid));
>> + if (argc) {
>> + argv++;
>> + argc--;
>> + }
>> + } else {
>> + strbuf_addstr(&sb, "HEAD");
>> + }
>> +
>> + if (files) {
>> + if (cached)
>> + die(_("The --cached option cannot be used with the --files option"));
>> + diff_cmd = "diff-files";
>> +
>> + strbuf_reset(&sb);
>
> strbuf_reset() does not free the memory allocated to sb.buf...
>
>> + sb.buf = NULL;
>
> ...then this makes sure that the memory previously allocated to sb.buf
> will not be free()d.
>
> Maybe instead of the above 2 lines just: strbuf_release(&sb)
> Or maybe just don't set sb.buf to 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(strbuf_detach(&sb, NULL), &info);
>
> Maybe you could pass: "sb.len > 0 ? strbuf_detach(&sb, NULL) : NULL"
> This way if sb has previously been released or reset, NULL will be passed.
>
> I would suggest though to just pass sb.buf and to strbuf_release(&sb)
> after calling compute_summary_module_list() and before returning from
> module_summary() instead of having compute_summary_module_list() free
> its first argument.
> If you do that then compute_summary_module_list() should be changed so
> that when it is passed "" it will behave in the same way as when it is
> passed NULL.
>
We can avoid it to behave same for "" and NULL, by checking if diff_cmd
is "cmd_diff_files", since its value is set NULL by this case.
ret = compute_summary_module_list(strcmp(diff_cmd, "diff-files") ?
NULL: sb.buf, &info);
strbuf_release(&sb);
instead of:
ret = compute_summary_module_list(sb.len ? sb.buf : NULL, &info);
if (sb.len)
strbuf_release(&sb);
>> +}
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-08-05 10:28 ` Prathamesh Chavan
@ 2017-08-05 16:55 ` Christian Couder
2017-08-05 18:03 ` Prathamesh Chavan
0 siblings, 1 reply; 25+ messages in thread
From: Christian Couder @ 2017-08-05 16:55 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git, Stefan Beller
On Sat, Aug 5, 2017 at 12:28 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 4:57 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Mon, Jul 31, 2017 at 10:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>>
> We can avoid it to behave same for "" and NULL, by checking if diff_cmd
> is "cmd_diff_files", since its value is set NULL by this case.
>
> ret = compute_summary_module_list(strcmp(diff_cmd, "diff-files") ?
> NULL: sb.buf, &info);
> strbuf_release(&sb);
It looks error prone, more fagile and less efficient to me.
> instead of:
> ret = compute_summary_module_list(sb.len ? sb.buf : NULL, &info);
> if (sb.len)
> strbuf_release(&sb);
I think it is ok to call strbuf_release(&sb) many times, so the "if
(sb.len)" check above is not needed.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
2017-08-05 16:55 ` Christian Couder
@ 2017-08-05 18:03 ` Prathamesh Chavan
0 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-08-05 18:03 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Stefan Beller
On Sat, Aug 5, 2017 at 10:25 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Sat, Aug 5, 2017 at 12:28 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>> On Tue, Aug 1, 2017 at 4:57 AM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>>> On Mon, Jul 31, 2017 at 10:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>>>
>> We can avoid it to behave same for "" and NULL, by checking if diff_cmd
>> is "cmd_diff_files", since its value is set NULL by this case.
>>
>> ret = compute_summary_module_list(strcmp(diff_cmd, "diff-files") ?
>> NULL: sb.buf, &info);
>> strbuf_release(&sb);
>
> It looks error prone, more fagile and less efficient to me.
I think by using enum { DIFF_INDEX, DIFF_FILES },
we can avoid using strcmp() here, and make it more efficient
not only here but also in other parts of the code, since
all such steps involving strcmp could be removed.
Thanks,
Prathamesh Chavan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [GSoC][PATCH 09/13] submodule foreach: correct '$path' in nested submodules from a subdirectory
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (7 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 10/13] submodule foreach: document '$sm_path' instead of '$path' Prathamesh Chavan
` (3 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, Prathamesh Chavan
When running 'git submodule foreach' from a subdirectory of your
repository, nested submodules get a bogus value for $sm_path:
For a submodule 'sub' that contains a nested submodule 'nested',
running 'git -C dir submodule foreach echo $path' would report
path='../nested' for the nested submodule. The first part '../' is
derived from the logic computing the relative path from $pwd to the
root of the superproject. The second part is the submodule path inside
the submodule. This value is of little use and is hard to document.
There are two different possible solutions that have more value:
(a) The path value is documented as the path from the toplevel of the
superproject to the mount point of the submodule.
In this case we would want to have path='sub/nested'.
(b) As Ramsay noticed the documented value is wrong. For the non-nested
case the path is equal to the relative path from $pwd to the
submodules working directory. When following this model,
the expected value would be path='../sub/nested'.
The behavior for (b) was introduced in 091a6eb0fe (submodule: drop the
top-level requirement, 2013-06-16) the intent for $path seemed to be
relative to $cwd to the submodule worktree, but that did not work for
nested submodules, as the intermittent submodules were not included in
the path.
If we were to fix the meaning of the $path using (a) such that "path"
is "the path from the toplevel of the superproject to the mount point
of the submodule", we would break any existing submodule user that runs
foreach from non-root of the superproject as the non-nested submodule
'../sub' would change its path to 'sub'.
If we would fix the meaning of the $path using (b), such that "path"
is "the relative path from $pwd to the submodule", then we would break
any user that uses nested submodules (even from the root directory) as
the 'nested' would become 'sub/nested'.
Both groups can be found in the wild. The author has no data if one group
outweighs the other by large margin, and offending each one seems equally
bad at first. However in the authors imagination it is better to go with
(a) as running from a sub directory sounds like it is carried out
by a human rather than by some automation task. With a human on
the keyboard the feedback loop is short and the changed behavior can be
adapted to quickly unlike some automation that can break silently.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 1 -
t/t7407-submodule-foreach.sh | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index a427ddafd..493a64372 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -320,7 +320,6 @@ cmd_foreach()
prefix="$prefix$sm_path/"
sanitize_submodule_env
cd "$sm_path" &&
- sm_path=$(git submodule--helper relative-path "$sm_path" "$wt_prefix") &&
# we make $path available to scripts ...
path=$sm_path &&
if test $# -eq 1
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 6ba5daf42..0663622a4 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -82,9 +82,9 @@ test_expect_success 'test basic "submodule foreach" usage' '
cat >expect <<EOF
Entering '../sub1'
-$pwd/clone-foo1-../sub1-$sub1sha1
+$pwd/clone-foo1-sub1-$sub1sha1
Entering '../sub3'
-$pwd/clone-foo3-../sub3-$sub3sha1
+$pwd/clone-foo3-sub3-$sub3sha1
EOF
test_expect_success 'test "submodule foreach" from subdirectory' '
@@ -196,6 +196,38 @@ test_expect_success 'test messages from "foreach --recursive" from subdirectory'
) &&
test_i18ncmp expect actual
'
+sub1sha1=$(cd clone2/sub1 && git rev-parse HEAD)
+sub2sha1=$(cd clone2/sub2 && git rev-parse HEAD)
+sub3sha1=$(cd clone2/sub3 && git rev-parse HEAD)
+nested1sha1=$(cd clone2/nested1 && git rev-parse HEAD)
+nested2sha1=$(cd clone2/nested1/nested2 && git rev-parse HEAD)
+nested3sha1=$(cd clone2/nested1/nested2/nested3 && git rev-parse HEAD)
+submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEAD)
+
+cat >expect <<EOF
+Entering '../nested1'
+$pwd/clone2-nested1-nested1-$nested1sha1
+Entering '../nested1/nested2'
+$pwd/clone2/nested1-nested2-nested2-$nested2sha1
+Entering '../nested1/nested2/nested3'
+$pwd/clone2/nested1/nested2-nested3-nested3-$nested3sha1
+Entering '../nested1/nested2/nested3/submodule'
+$pwd/clone2/nested1/nested2/nested3-submodule-submodule-$submodulesha1
+Entering '../sub1'
+$pwd/clone2-foo1-sub1-$sub1sha1
+Entering '../sub2'
+$pwd/clone2-foo2-sub2-$sub2sha1
+Entering '../sub3'
+$pwd/clone2-foo3-sub3-$sub3sha1
+EOF
+
+test_expect_success 'test "submodule foreach --recursive" from subdirectory' '
+ (
+ cd clone2/untracked &&
+ git submodule foreach --recursive "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual
+ ) &&
+ test_i18ncmp expect actual
+'
cat > expect <<EOF
nested1-nested1
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 10/13] submodule foreach: document '$sm_path' instead of '$path'
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (8 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 09/13] submodule foreach: correct '$path' in nested submodules from a subdirectory Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 11/13] submodule foreach: clarify the '$toplevel' variable documentation Prathamesh Chavan
` (2 subsequent siblings)
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, Prathamesh Chavan
As using a variable '$path' may be harmful to users due to
capitalization issues, see 64394e3ae9 (git-submodule.sh: Don't
use $path variable in eval_gettext string, 2012-04-17). Adjust
the documentation to advocate for using $sm_path, which contains
the same value. We still make the 'path' variable available and
document it as a deprecated synonym of 'sm_path'.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
This patch is same as its previous version.
Although here I'll like to add a point that we aim to slowly drop the support
of the variable 'path'.
Documentation/git-submodule.txt | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index ff612001d..a23baef62 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -183,12 +183,14 @@ information too.
foreach [--recursive] <command>::
Evaluates an arbitrary shell command in each checked out submodule.
- The command has access to the variables $name, $path, $sha1 and
+ The command has access to the variables $name, $sm_path, $sha1 and
$toplevel:
$name is the name of the relevant submodule section in `.gitmodules`,
- $path is the name of the submodule directory relative to the
- superproject, $sha1 is the commit as recorded in the superproject,
- and $toplevel is the absolute path to the top-level of the superproject.
+ $sm_path is the path of the submodule as recorded in the superproject,
+ $sha1 is the commit as recorded in the superproject, and
+ $toplevel is the absolute path to the top-level of the superproject.
+ Note that to avoid conflicts with '$PATH' on Windows, the '$path'
+ variable is now a deprecated synonym of '$sm_path' variable.
Any submodules defined in the superproject but not checked out are
ignored by this command. Unless given `--quiet`, foreach prints the name
of each submodule before evaluating the command.
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 11/13] submodule foreach: clarify the '$toplevel' variable documentation
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (9 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 10/13] submodule foreach: document '$sm_path' instead of '$path' Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 12/13] submodule foreach: document variable '$displaypath' Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 13/13] submodule: port submodule subcommand 'foreach' from shell to C Prathamesh Chavan
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, Prathamesh Chavan
It does not contain the topmost superproject as the author assumed,
but the direct superproject, such that $toplevel/$sm_path is the
actual absolute path of the submodule.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
Documentation/git-submodule.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a23baef62..8e7930ebc 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -188,7 +188,8 @@ foreach [--recursive] <command>::
$name is the name of the relevant submodule section in `.gitmodules`,
$sm_path is the path of the submodule as recorded in the superproject,
$sha1 is the commit as recorded in the superproject, and
- $toplevel is the absolute path to the top-level of the superproject.
+ $toplevel is the absolute path to its superproject, such that
+ $toplevel/$sm_path is the absolute path of the submodule.
Note that to avoid conflicts with '$PATH' on Windows, the '$path'
variable is now a deprecated synonym of '$sm_path' variable.
Any submodules defined in the superproject but not checked out are
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 12/13] submodule foreach: document variable '$displaypath'
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (10 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 11/13] submodule foreach: clarify the '$toplevel' variable documentation Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 20:56 ` [GSoC][PATCH 13/13] submodule: port submodule subcommand 'foreach' from shell to C Prathamesh Chavan
12 siblings, 0 replies; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, Prathamesh Chavan
It was observed that the variable '$displaypath' was accessible but
undocumented. Hence, document it.
Discussed-with: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
In this new version, the following changes have been made:
* Spelling mistake in the commit message was corrected.
Documentation/git-submodule.txt | 6 ++++--
t/t7407-submodule-foreach.sh | 22 +++++++++++-----------
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e7930ebc..0cca702cb 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -183,10 +183,12 @@ information too.
foreach [--recursive] <command>::
Evaluates an arbitrary shell command in each checked out submodule.
- The command has access to the variables $name, $sm_path, $sha1 and
- $toplevel:
+ The command has access to the variables $name, $sm_path, $displaypath,
+ $sha1 and $toplevel:
$name is the name of the relevant submodule section in `.gitmodules`,
$sm_path is the path of the submodule as recorded in the superproject,
+ $displaypath contains the relative path from the current working
+ directory to the submodules root directory,
$sha1 is the commit as recorded in the superproject, and
$toplevel is the absolute path to its superproject, such that
$toplevel/$sm_path is the absolute path of the submodule.
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 0663622a4..6ad57e061 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -82,16 +82,16 @@ test_expect_success 'test basic "submodule foreach" usage' '
cat >expect <<EOF
Entering '../sub1'
-$pwd/clone-foo1-sub1-$sub1sha1
+$pwd/clone-foo1-sub1-../sub1-$sub1sha1
Entering '../sub3'
-$pwd/clone-foo3-sub3-$sub3sha1
+$pwd/clone-foo3-sub3-../sub3-$sub3sha1
EOF
test_expect_success 'test "submodule foreach" from subdirectory' '
mkdir clone/sub &&
(
cd clone/sub &&
- git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual
+ git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual
) &&
test_i18ncmp expect actual
'
@@ -206,25 +206,25 @@ submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEA
cat >expect <<EOF
Entering '../nested1'
-$pwd/clone2-nested1-nested1-$nested1sha1
+$pwd/clone2-nested1-nested1-../nested1-$nested1sha1
Entering '../nested1/nested2'
-$pwd/clone2/nested1-nested2-nested2-$nested2sha1
+$pwd/clone2/nested1-nested2-nested2-../nested1/nested2-$nested2sha1
Entering '../nested1/nested2/nested3'
-$pwd/clone2/nested1/nested2-nested3-nested3-$nested3sha1
+$pwd/clone2/nested1/nested2-nested3-nested3-../nested1/nested2/nested3-$nested3sha1
Entering '../nested1/nested2/nested3/submodule'
-$pwd/clone2/nested1/nested2/nested3-submodule-submodule-$submodulesha1
+$pwd/clone2/nested1/nested2/nested3-submodule-submodule-../nested1/nested2/nested3/submodule-$submodulesha1
Entering '../sub1'
-$pwd/clone2-foo1-sub1-$sub1sha1
+$pwd/clone2-foo1-sub1-../sub1-$sub1sha1
Entering '../sub2'
-$pwd/clone2-foo2-sub2-$sub2sha1
+$pwd/clone2-foo2-sub2-../sub2-$sub2sha1
Entering '../sub3'
-$pwd/clone2-foo3-sub3-$sub3sha1
+$pwd/clone2-foo3-sub3-../sub3-$sub3sha1
EOF
test_expect_success 'test "submodule foreach --recursive" from subdirectory' '
(
cd clone2/untracked &&
- git submodule foreach --recursive "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual
+ git submodule foreach --recursive "echo \$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual
) &&
test_i18ncmp expect actual
'
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [GSoC][PATCH 13/13] submodule: port submodule subcommand 'foreach' from shell to C
2017-07-31 20:56 [GSoC][PATCH 00/13] Update: Week-11 Prathamesh Chavan
` (11 preceding siblings ...)
2017-07-31 20:56 ` [GSoC][PATCH 12/13] submodule foreach: document variable '$displaypath' Prathamesh Chavan
@ 2017-07-31 20:56 ` Prathamesh Chavan
2017-07-31 22:20 ` Stefan Beller
12 siblings, 1 reply; 25+ messages in thread
From: Prathamesh Chavan @ 2017-07-31 20:56 UTC (permalink / raw)
To: git; +Cc: sbeller, christian.couder, Prathamesh Chavan
This aims to make git-submodule foreach a builtin. This is the very
first step taken in this direction. Hence, 'foreach' is ported to
submodule--helper, and submodule--helper is called from git-submodule.sh.
The code is split up to have one function to obtain all the list of
submodules. This function acts as the front-end of git-submodule foreach
subcommand. It calls the function for_each_submodule_list, which basically
loops through the list and calls function fn, which in this case is
runcommand_in_submodule. This third function is a calling function that
takes care of running the command in that submodule, and recursively
perform the same when --recursive is flagged.
The first function module_foreach first parses the options present in
argv, and then with the help of module_list_compute, generates the list of
submodules present in the current working tree.
The second function for_each_submodule_list traverses through the
list, and calls function fn (which in case of submodule subcommand
foreach is runcommand_in_submodule) is called for each entry.
The third function runcommand_in_submodule, generates a submodule struct sub
for $name, value and then later prepends name=sub->name; and other
value assignment to the env argv_array structure of a child_process.
Also the <command> of submodule-foreach is push to args argv_array
structure and finally, using run_command the commands are executed
using a shell.
The third function also takes care of the recursive flag, by creating
a separate child_process structure and prepending "--super-prefix displaypath",
to the args argv_array structure. Other required arguments and the
input <command> of submodule-foreach is also appended to this argv_array.
Helped-by: Brandon Williams <bmwill@google.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
---
In this new version, the following changes have been made:
* Comment style is improved in the function runcommand_in_submodule()
* Comment in added about why the variable "path" was exposed via args
argv_array instead of exposing it via the env_array.
* This patch exposes the various variables when argc == 1 only, just
for maintaining a faithful porting. You can also find discussion about
the same at [1].
[1]: https://public-inbox.org/git/CAME+mvUSGAFbN5j-_hv7QpAS57hq4wgH+yZ7XJMPuyQN1gALaA@mail.gmail.com/
builtin/submodule--helper.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
git-submodule.sh | 39 +------------
2 files changed, 137 insertions(+), 38 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 94438d6ce..935f56510 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -769,6 +769,141 @@ static int module_name(int argc, const char **argv, const char *prefix)
return 0;
}
+struct cb_foreach {
+ int argc;
+ const char **argv;
+ const char *prefix;
+ unsigned int quiet: 1;
+ unsigned int recursive: 1;
+};
+#define CB_FOREACH_INIT { 0, NULL, NULL, 0, 0 }
+
+static void runcommand_in_submodule(const struct cache_entry *list_item,
+ void *cb_data)
+{
+ struct cb_foreach *info = cb_data;
+ const struct submodule *sub;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ char *displaypath;
+
+ displaypath = get_submodule_displaypath(list_item->name, info->prefix);
+
+ sub = submodule_from_path(null_sha1, list_item->name);
+
+ if (!sub)
+ die(_("No url found for submodule path '%s' in .gitmodules"),
+ displaypath);
+
+ if (!is_submodule_populated_gently(list_item->name, NULL))
+ goto cleanup;
+
+ prepare_submodule_repo_env(&cp.env_array);
+
+ /*
+ * For the purpose of executing <command> in the submodule,
+ * separate shell is used for the purpose of running the
+ * child process.
+ */
+ cp.use_shell = 1;
+ cp.dir = list_item->name;
+
+ if (info->argc == 1) {
+ char *toplevel = xgetcwd();
+
+ argv_array_pushf(&cp.env_array, "name=%s", sub->name);
+ argv_array_pushf(&cp.env_array, "sm_path=%s", list_item->name);
+ argv_array_pushf(&cp.env_array, "displaypath=%s", displaypath);
+ argv_array_pushf(&cp.env_array, "sha1=%s",
+ oid_to_hex(&list_item->oid));
+ argv_array_pushf(&cp.env_array, "toplevel=%s", toplevel);
+
+ /*
+ * Since the path variable was accessible from the script
+ * before porting, it is also made available after porting.
+ * The environment variable "PATH" has a very special purpose
+ * on windows. And since environment variables are
+ * case-insensitive in windows, it interferes with the
+ * existing PATH variable. Hence, to avoid that, we expose
+ * path via the args argv_array and not via env_array.
+ */
+ argv_array_pushf(&cp.args, "path=%s; %s",
+ list_item->name, info->argv[0]);
+ free(toplevel);
+ } else {
+ argv_array_pushv(&cp.args, info->argv);
+ }
+
+ if (!info->quiet)
+ printf(_("Entering '%s'\n"), displaypath);
+
+ if (info->argv[0] && run_command(&cp))
+ die(_("run_command returned non-zero status for %s\n."),
+ 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", "foreach", "--recursive",
+ NULL);
+
+ if (info->quiet)
+ argv_array_push(&cpr.args, "--quiet");
+
+ argv_array_pushv(&cpr.args, info->argv);
+
+ if (run_command(&cpr))
+ die(_("run_command returned non-zero status while"
+ "recursing in the nested submodules of %s\n."),
+ displaypath);
+ }
+
+cleanup:
+ free(displaypath);
+}
+
+static int module_foreach(int argc, const char **argv, const char *prefix)
+{
+ struct cb_foreach info;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ int quiet = 0;
+ int recursive = 0;
+
+ struct option module_foreach_options[] = {
+ OPT__QUIET(&quiet, N_("Suppress output of entering each submodule command")),
+ OPT_BOOL(0, "recursive", &recursive,
+ N_("Recurse into nested submodules")),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper foreach [--quiet] [--recursive] <command>"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, module_foreach_options,
+ git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);
+
+ if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
+ BUG("module_list_compute should not choke on empty pathspec");
+
+ info.argc = argc;
+ info.argv = argv;
+ info.prefix = prefix;
+ info.quiet = !!quiet;
+ info.recursive = !!recursive;
+
+ gitmodules_config();
+ for_each_submodule_list(list, runcommand_in_submodule, &info);
+
+ return 0;
+}
+
struct module_cb {
unsigned int mod_src;
unsigned int mod_dst;
@@ -2206,6 +2341,7 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url", resolve_relative_url, 0},
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"print-name-rev", print_name_rev, 0},
+ {"foreach", module_foreach, SUPPORT_SUPER_PREFIX},
{"init", module_init, SUPPORT_SUPER_PREFIX},
{"status", module_status, SUPPORT_SUPER_PREFIX},
{"print-default-remote", print_default_remote, 0},
diff --git a/git-submodule.sh b/git-submodule.sh
index 493a64372..e25b2c613 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -298,44 +298,7 @@ cmd_foreach()
shift
done
- toplevel=$(pwd)
-
- # dup stdin so that it can be restored when running the external
- # command in the subshell (and a recursive call to this function)
- exec 3<&0
-
- {
- git submodule--helper list --prefix "$wt_prefix" ||
- echo "#unmatched" $?
- } |
- while read -r mode sha1 stage sm_path
- do
- die_if_unmatched "$mode" "$sha1"
- if test -e "$sm_path"/.git
- then
- displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
- say "$(eval_gettext "Entering '\$displaypath'")"
- name=$(git submodule--helper name "$sm_path")
- (
- prefix="$prefix$sm_path/"
- sanitize_submodule_env
- cd "$sm_path" &&
- # we make $path available to scripts ...
- path=$sm_path &&
- if test $# -eq 1
- then
- eval "$1"
- else
- "$@"
- fi &&
- if test -n "$recursive"
- then
- cmd_foreach "--recursive" "$@"
- fi
- ) <&3 3<&- ||
- die "$(eval_gettext "Stopping at '\$displaypath'; script returned non-zero status.")"
- fi
- done
+ git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@"
}
#
--
2.13.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [GSoC][PATCH 13/13] submodule: port submodule subcommand 'foreach' from shell to C
2017-07-31 20:56 ` [GSoC][PATCH 13/13] submodule: port submodule subcommand 'foreach' from shell to C Prathamesh Chavan
@ 2017-07-31 22:20 ` Stefan Beller
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Beller @ 2017-07-31 22:20 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git@vger.kernel.org, Christian Couder
On Mon, Jul 31, 2017 at 1:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> This aims to make git-submodule foreach a builtin. This is the very
> first step taken in this direction. Hence, 'foreach' is ported to
> submodule--helper, and submodule--helper is called from git-submodule.sh.
> The code is split up to have one function to obtain all the list of
> submodules. This function acts as the front-end of git-submodule foreach
> subcommand. It calls the function for_each_submodule_list, which basically
> loops through the list and calls function fn, which in this case is
> runcommand_in_submodule. This third function is a calling function that
> takes care of running the command in that submodule, and recursively
> perform the same when --recursive is flagged.
>
> The first function module_foreach first parses the options present in
> argv, and then with the help of module_list_compute, generates the list of
> submodules present in the current working tree.
>
> The second function for_each_submodule_list traverses through the
> list, and calls function fn (which in case of submodule subcommand
> foreach is runcommand_in_submodule) is called for each entry.
>
> The third function runcommand_in_submodule, generates a submodule struct sub
> for $name, value and then later prepends name=sub->name; and other
> value assignment to the env argv_array structure of a child_process.
> Also the <command> of submodule-foreach is push to args argv_array
> structure and finally, using run_command the commands are executed
> using a shell.
>
> The third function also takes care of the recursive flag, by creating
> a separate child_process structure and prepending "--super-prefix displaypath",
> to the args argv_array structure. Other required arguments and the
> input <command> of submodule-foreach is also appended to this argv_array.
>
> Helped-by: Brandon Williams <bmwill@google.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
> In this new version, the following changes have been made:
> * Comment style is improved in the function runcommand_in_submodule()
>
> * Comment in added about why the variable "path" was exposed via args
> argv_array instead of exposing it via the env_array.
>
> * This patch exposes the various variables when argc == 1 only, just
> for maintaining a faithful porting. You can also find discussion about
> the same at [1].
>
> [1]: https://public-inbox.org/git/CAME+mvUSGAFbN5j-_hv7QpAS57hq4wgH+yZ7XJMPuyQN1gALaA@mail.gmail.com/
Ah right, maybe add a NEEDSWORK or have the commit message explain
this behavior.
^ permalink raw reply [flat|nested] 25+ messages in thread