From 63e95beb085ce8ae359d728ec0a54c3dcfe741f5 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 15 Apr 2016 17:50:12 -0700 Subject: [PATCH 1/5] submodule: port resolve_relative_url from shell to C Later on we want to automatically call `git submodule init` from other commands, such that the users don't have to initialize the submodule themselves. As these other commands are written in C already, we'd need the init functionality in C, too. The `resolve_relative_url` function is a large part of that init functionality, so start by porting this function to C. To create the tests in t0060, the function `resolve_relative_url` was temporarily enhanced to write all inputs and output to disk when running the test suite. The added tests in this patch are a small selection thereof. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 209 +++++++++++++++++++++++++++++++++++- git-submodule.sh | 81 +------------- t/t0060-path-utils.sh | 46 ++++++++ 3 files changed, 258 insertions(+), 78 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 864dd18786..46946b010a 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -9,6 +9,211 @@ #include "submodule-config.h" #include "string-list.h" #include "run-command.h" +#include "remote.h" +#include "refs.h" +#include "connect.h" + +static char *get_default_remote(void) +{ + char *dest = NULL, *ret; + unsigned char sha1[20]; + struct strbuf sb = STRBUF_INIT; + const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, NULL); + + if (!refname) + die(_("No such ref: %s"), "HEAD"); + + /* detached HEAD */ + if (!strcmp(refname, "HEAD")) + return xstrdup("origin"); + + if (!skip_prefix(refname, "refs/heads/", &refname)) + die(_("Expecting a full ref name, got %s"), refname); + + strbuf_addf(&sb, "branch.%s.remote", refname); + if (git_config_get_string(sb.buf, &dest)) + ret = xstrdup("origin"); + else + ret = dest; + + strbuf_release(&sb); + return ret; +} + +static int starts_with_dot_slash(const char *str) +{ + return str[0] == '.' && is_dir_sep(str[1]); +} + +static int starts_with_dot_dot_slash(const char *str) +{ + return str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]); +} + +/* + * Returns 1 if it was the last chop before ':'. + */ +static int chop_last_dir(char **remoteurl, int is_relative) +{ + char *rfind = find_last_dir_sep(*remoteurl); + if (rfind) { + *rfind = '\0'; + return 0; + } + + rfind = strrchr(*remoteurl, ':'); + if (rfind) { + *rfind = '\0'; + return 1; + } + + if (is_relative || !strcmp(".", *remoteurl)) + die(_("cannot strip one component off url '%s'"), + *remoteurl); + + free(*remoteurl); + *remoteurl = xstrdup("."); + return 0; +} + +/* + * The `url` argument is the URL that navigates to the submodule origin + * repo. When relative, this URL is relative to the superproject origin + * URL repo. The `up_path` argument, if specified, is the relative + * path that navigates from the submodule working tree to the superproject + * working tree. Returns the origin URL of the submodule. + * + * Return either an absolute URL or filesystem path (if the superproject + * origin URL is an absolute URL or filesystem path, respectively) or a + * relative file system path (if the superproject origin URL is a relative + * file system path). + * + * When the output is a relative file system path, the path is either + * relative to the submodule working tree, if up_path is specified, or to + * the superproject working tree otherwise. + * + * NEEDSWORK: This works incorrectly on the domain and protocol part. + * remote_url url outcome expectation + * http://a.com/b ../c http://a.com/c as is + * http://a.com/b ../../c http://c error out + * http://a.com/b ../../../c http:/c error out + * http://a.com/b ../../../../c http:c error out + * http://a.com/b ../../../../../c .:c error out + * NEEDSWORK: Given how chop_last_dir() works, this function is broken + * when a local part has a colon in its path component, too. + */ +static char *relative_url(const char *remote_url, + const char *url, + const char *up_path) +{ + int is_relative = 0; + int colonsep = 0; + char *out; + char *remoteurl = xstrdup(remote_url); + struct strbuf sb = STRBUF_INIT; + size_t len = strlen(remoteurl); + + if (is_dir_sep(remoteurl[len])) + remoteurl[len] = '\0'; + + if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl)) + is_relative = 0; + else { + is_relative = 1; + /* + * Prepend a './' to ensure all relative + * remoteurls start with './' or '../' + */ + if (!starts_with_dot_slash(remoteurl) && + !starts_with_dot_dot_slash(remoteurl)) { + strbuf_reset(&sb); + strbuf_addf(&sb, "./%s", remoteurl); + free(remoteurl); + remoteurl = strbuf_detach(&sb, NULL); + } + } + /* + * When the url starts with '../', remove that and the + * last directory in remoteurl. + */ + while (url) { + if (starts_with_dot_dot_slash(url)) { + url += 3; + colonsep |= chop_last_dir(&remoteurl, is_relative); + } else if (starts_with_dot_slash(url)) + url += 2; + else + break; + } + strbuf_reset(&sb); + strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url); + free(remoteurl); + + if (starts_with_dot_slash(sb.buf)) + out = xstrdup(sb.buf + 2); + else + out = xstrdup(sb.buf); + strbuf_reset(&sb); + + if (!up_path || !is_relative) + return out; + + strbuf_addf(&sb, "%s%s", up_path, out); + free(out); + return strbuf_detach(&sb, NULL); +} + +static int resolve_relative_url(int argc, const char **argv, const char *prefix) +{ + char *remoteurl = NULL; + char *remote = get_default_remote(); + const char *up_path = NULL; + char *res; + const char *url; + struct strbuf sb = STRBUF_INIT; + + if (argc != 2 && argc != 3) + die("resolve-relative-url only accepts one or two arguments"); + + url = argv[1]; + strbuf_addf(&sb, "remote.%s.url", remote); + free(remote); + + if (git_config_get_string(sb.buf, &remoteurl)) + /* the repository is its own authoritative upstream */ + remoteurl = xgetcwd(); + + if (argc == 3) + up_path = argv[2]; + + res = relative_url(remoteurl, url, up_path); + puts(res); + free(res); + free(remoteurl); + return 0; +} + +static int resolve_relative_url_test(int argc, const char **argv, const char *prefix) +{ + char *remoteurl, *res; + const char *up_path, *url; + + if (argc != 4) + die("resolve-relative-url-test only accepts three arguments: "); + + up_path = argv[1]; + remoteurl = xstrdup(argv[2]); + url = argv[3]; + + if (!strcmp(up_path, "(null)")) + up_path = NULL; + + res = relative_url(remoteurl, url, up_path); + puts(res); + free(res); + free(remoteurl); + return 0; +} struct module_list { const struct cache_entry **entries; @@ -504,7 +709,9 @@ static struct cmd_struct commands[] = { {"list", module_list}, {"name", module_name}, {"clone", module_clone}, - {"update-clone", update_clone} + {"update-clone", update_clone}, + {"resolve-relative-url", resolve_relative_url}, + {"resolve-relative-url-test", resolve_relative_url_test}, }; int cmd_submodule__helper(int argc, const char **argv, const char *prefix) diff --git a/git-submodule.sh b/git-submodule.sh index 07290d07ae..2423d7c3ef 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -46,79 +46,6 @@ prefix= custom_name= depth= -# The function takes at most 2 arguments. The first argument is the -# URL that navigates to the submodule origin repo. When relative, this URL -# is relative to the superproject origin URL repo. The second up_path -# argument, if specified, is the relative path that navigates -# from the submodule working tree to the superproject working tree. -# -# The output of the function is the origin URL of the submodule. -# -# The output will either be an absolute URL or filesystem path (if the -# superproject origin URL is an absolute URL or filesystem path, -# respectively) or a relative file system path (if the superproject -# origin URL is a relative file system path). -# -# When the output is a relative file system path, the path is either -# relative to the submodule working tree, if up_path is specified, or to -# the superproject working tree otherwise. -resolve_relative_url () -{ - remote=$(get_default_remote) - remoteurl=$(git config "remote.$remote.url") || - remoteurl=$(pwd) # the repository is its own authoritative upstream - url="$1" - remoteurl=${remoteurl%/} - sep=/ - up_path="$2" - - case "$remoteurl" in - *:*|/*) - is_relative= - ;; - ./*|../*) - is_relative=t - ;; - *) - is_relative=t - remoteurl="./$remoteurl" - ;; - esac - - while test -n "$url" - do - case "$url" in - ../*) - url="${url#../}" - case "$remoteurl" in - */*) - remoteurl="${remoteurl%/*}" - ;; - *:*) - remoteurl="${remoteurl%:*}" - sep=: - ;; - *) - if test -z "$is_relative" || test "." = "$remoteurl" - then - die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")" - else - remoteurl=. - fi - ;; - esac - ;; - ./*) - url="${url#./}" - ;; - *) - break;; - esac - done - remoteurl="$remoteurl$sep${url%/}" - echo "${is_relative:+${up_path}}${remoteurl#./}" -} - # Resolve a path to be relative to another path. This is intended for # converting submodule paths when git-submodule is run in a subdirectory # and only handles paths where the directory separator is '/'. @@ -281,7 +208,7 @@ cmd_add() die "$(gettext "Relative path can only be used from the toplevel of the working tree")" # dereference source url relative to parent's url - realrepo=$(resolve_relative_url "$repo") || exit + realrepo=$(git submodule--helper resolve-relative-url "$repo") || exit ;; *:*|/*) # absolute url @@ -485,7 +412,7 @@ cmd_init() # Possibly a url relative to parent case "$url" in ./*|../*) - url=$(resolve_relative_url "$url") || exit + url=$(git submodule--helper resolve-relative-url "$url") || exit ;; esac git config submodule."$name".url "$url" || @@ -1202,9 +1129,9 @@ cmd_sync() # guarantee a trailing / up_path=${up_path%/}/ && # path from submodule work tree to submodule origin repo - sub_origin_url=$(resolve_relative_url "$url" "$up_path") && + sub_origin_url=$(git submodule--helper resolve-relative-url "$url" "$up_path") && # path from superproject work tree to submodule origin repo - super_config_url=$(resolve_relative_url "$url") || exit + super_config_url=$(git submodule--helper resolve-relative-url "$url") || exit ;; *) sub_origin_url="$url" diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 8532a028e7..bf2deee109 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -19,6 +19,13 @@ relative_path() { "test \"\$(test-path-utils relative_path '$1' '$2')\" = '$expected'" } +test_submodule_relative_url() { + test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" " + actual=\$(git submodule--helper resolve-relative-url-test '$1' '$2' '$3') && + test \"\$actual\" = '$4' + " +} + test_git_path() { test_expect_success "git-path $1 $2 => $3" " $1 git rev-parse --git-path $2 >actual && @@ -298,4 +305,43 @@ test_git_path GIT_COMMON_DIR=bar config bar/config test_git_path GIT_COMMON_DIR=bar packed-refs bar/packed-refs test_git_path GIT_COMMON_DIR=bar shallow bar/shallow +# In the tests below, the distinction between $PWD and $(pwd) is important: +# on Windows, $PWD is POSIX style (/c/foo), $(pwd) has drive letter (c:/foo). + +test_submodule_relative_url "../" "../foo" "../submodule" "../../submodule" +test_submodule_relative_url "../" "../foo/bar" "../submodule" "../../foo/submodule" +test_submodule_relative_url "../" "../foo/submodule" "../submodule" "../../foo/submodule" +test_submodule_relative_url "../" "./foo" "../submodule" "../submodule" +test_submodule_relative_url "../" "./foo/bar" "../submodule" "../foo/submodule" +test_submodule_relative_url "../../../" "../foo/bar" "../sub/a/b/c" "../../../../foo/sub/a/b/c" +test_submodule_relative_url "../" "$PWD/addtest" "../repo" "$(pwd)/repo" +test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule" +test_submodule_relative_url "../" "foo" "../submodule" "../submodule" + +test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c" +test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule" +test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule" +test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule" +test_submodule_relative_url "(null)" "./foo/bar" "../submodule" "foo/submodule" +test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule" +test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo" +test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r" +test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r" +test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/." +test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/." +test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo" +test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö" +test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule" +test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule" +test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1" +test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/." +test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo" +test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule" +test_submodule_relative_url "(null)" "foo" "../submodule" "submodule" +test_submodule_relative_url "(null)" "helper:://hostname/repo" "../subrepo" "helper:://hostname/subrepo" +test_submodule_relative_url "(null)" "ssh://hostname/repo" "../subrepo" "ssh://hostname/subrepo" +test_submodule_relative_url "(null)" "ssh://hostname:22/repo" "../subrepo" "ssh://hostname:22/subrepo" +test_submodule_relative_url "(null)" "user@host:path/to/repo" "../subrepo" "user@host:path/to/subrepo" +test_submodule_relative_url "(null)" "user@host:repo" "../subrepo" "user@host:subrepo" + test_done From 3604242f080a813d6f20a7394def422d1e55b30e Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 15 Apr 2016 17:50:13 -0700 Subject: [PATCH 2/5] submodule: port init from shell to C By having the `submodule init` functionality in C, we can reference it easier from other parts in the code in later patches. The code is split up to have one function to initialize one submodule and a calling function that takes care of the rest, such as argument handling and translating the arguments to the paths of the submodules. This is the first submodule subcommand that is fully converted to C except for the usage string, so this is actually removing a call to the `submodule--helper list` function, which is supposed to be used in this transition. Instead we'll make a direct call to `module_list_compute`. An explanation why we need to edit the prefixes in cmd_update in git-submodule.sh in this patch: By having no processing in the shell part, we need to convey the notion of wt_prefix and prefix to the C parts, which former patches punted on and did the processing of displaying path in the shell. `wt_prefix` used to hold the path from the repository root to the current directory, e.g. wt_prefix would be t/ if the user invoked the `git submodule` command in ~/repo/t and ~repo is the GIT_DIR. `prefix` used to hold the relative path from the repository root to the operation, e.g. if you have recursive submodules, the shell script would modify the `prefix` in each recursive step by adding the submodule path. We will pass `wt_prefix` into the C helper via `git -C ` as that will setup git in the directory the user actually called git-submodule.sh from. The `prefix` will be passed in via the `--prefix` option. Having `prefix` and `wt_prefix` relative to the GIT_DIR of the calling superproject is unfortunate with this patch as the C code doesn't know about a possible recursion from a superproject via `submodule update --init --recursive`. To fix this, we change the meaning of `wt_prefix` to point to the current project instead of the superproject and `prefix` to include any relative paths issues in the superproject. That way `prefix` will become the leading part for displaying paths and `wt_prefix` will be empty in recursive calls for now. The new notion of `wt_prefix` and `prefix` still allows us to reconstruct the calling directory in the superproject by just traveling reverse of `prefix`. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 115 ++++++++++++++++++++++++++++++++++++ git-submodule.sh | 48 +-------------- submodule.c | 21 +++++++ submodule.h | 1 + 4 files changed, 140 insertions(+), 45 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 46946b010a..b6d4f27648 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -305,6 +305,120 @@ static int module_list(int argc, const char **argv, const char *prefix) return 0; } +static void init_submodule(const char *path, const char *prefix, int quiet) +{ + const struct submodule *sub; + struct strbuf sb = STRBUF_INIT; + char *upd = NULL, *url = NULL, *displaypath; + + /* Only loads from .gitmodules, no overlay with .git/config */ + gitmodules_config(); + + sub = submodule_from_path(null_sha1, path); + + if (prefix) { + strbuf_addf(&sb, "%s%s", prefix, path); + displaypath = strbuf_detach(&sb, NULL); + } else + displaypath = xstrdup(sub->path); + + /* + * Copy url setting when it is not set yet. + * To look up the url in .git/config, we must not fall back to + * .gitmodules, so look it up directly. + */ + strbuf_reset(&sb); + strbuf_addf(&sb, "submodule.%s.url", sub->name); + if (git_config_get_string(sb.buf, &url)) { + url = xstrdup(sub->url); + + if (!url) + die(_("No url found for submodule path '%s' in .gitmodules"), + displaypath); + + /* Possibly a url relative to parent */ + if (starts_with_dot_dot_slash(url) || + starts_with_dot_slash(url)) { + char *remoteurl, *relurl; + char *remote = get_default_remote(); + struct strbuf remotesb = STRBUF_INIT; + strbuf_addf(&remotesb, "remote.%s.url", remote); + free(remote); + + if (git_config_get_string(remotesb.buf, &remoteurl)) + /* + * The repository is its own + * authoritative upstream + */ + remoteurl = xgetcwd(); + relurl = relative_url(remoteurl, url, NULL); + strbuf_release(&remotesb); + free(remoteurl); + free(url); + url = relurl; + } + + if (git_config_set_gently(sb.buf, url)) + die(_("Failed to register url for submodule path '%s'"), + displaypath); + if (!quiet) + printf(_("Submodule '%s' (%s) registered for path '%s'\n"), + sub->name, url, displaypath); + } + + /* Copy "update" setting when it is not set yet */ + strbuf_reset(&sb); + strbuf_addf(&sb, "submodule.%s.update", sub->name); + if (git_config_get_string(sb.buf, &upd) && + sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) { + if (sub->update_strategy.type == SM_UPDATE_COMMAND) { + fprintf(stderr, _("warning: command update mode suggested for submodule '%s'\n"), + sub->name); + upd = xstrdup("none"); + } else + upd = xstrdup(submodule_strategy_to_string(&sub->update_strategy)); + + if (git_config_set_gently(sb.buf, upd)) + die(_("Failed to register update mode for submodule path '%s'"), displaypath); + } + strbuf_release(&sb); + free(displaypath); + free(url); + free(upd); +} + +static int module_init(int argc, const char **argv, const char *prefix) +{ + struct pathspec pathspec; + struct module_list list = MODULE_LIST_INIT; + int quiet = 0; + int i; + + struct option module_init_options[] = { + OPT_STRING(0, "prefix", &prefix, + N_("path"), + N_("alternative anchor for relative paths")), + OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")), + OPT_END() + }; + + const char *const git_submodule_helper_usage[] = { + N_("git submodule--helper init []"), + NULL + }; + + argc = parse_options(argc, argv, prefix, module_init_options, + git_submodule_helper_usage, 0); + + if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) + return 1; + + for (i = 0; i < list.nr; i++) + init_submodule(list.entries[i]->name, prefix, quiet); + + return 0; +} + static int module_name(int argc, const char **argv, const char *prefix) { const struct submodule *sub; @@ -712,6 +826,7 @@ static struct cmd_struct commands[] = { {"update-clone", update_clone}, {"resolve-relative-url", resolve_relative_url}, {"resolve-relative-url-test", resolve_relative_url_test}, + {"init", module_init} }; int cmd_submodule__helper(int argc, const char **argv, const char *prefix) diff --git a/git-submodule.sh b/git-submodule.sh index 2423d7c3ef..82e95a923b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -394,50 +394,7 @@ cmd_init() shift done - git submodule--helper list --prefix "$wt_prefix" "$@" | - while read mode sha1 stage sm_path - do - die_if_unmatched "$mode" - name=$(git submodule--helper name "$sm_path") || exit - - displaypath=$(relative_path "$prefix$sm_path") - - # Copy url setting when it is not set yet - if test -z "$(git config "submodule.$name.url")" - then - url=$(git config -f .gitmodules submodule."$name".url) - test -z "$url" && - die "$(eval_gettext "No url found for submodule path '\$displaypath' in .gitmodules")" - - # Possibly a url relative to parent - case "$url" in - ./*|../*) - url=$(git submodule--helper resolve-relative-url "$url") || exit - ;; - esac - git config submodule."$name".url "$url" || - die "$(eval_gettext "Failed to register url for submodule path '\$displaypath'")" - - say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$displaypath'")" - fi - - # Copy "update" setting when it is not set yet - if upd="$(git config -f .gitmodules submodule."$name".update)" && - test -n "$upd" && - test -z "$(git config submodule."$name".update)" - then - case "$upd" in - checkout | rebase | merge | none) - ;; # known modes of updating - *) - echo >&2 "warning: unknown update mode '$upd' suggested for submodule '$name'" - upd=none - ;; - esac - git config submodule."$name".update "$upd" || - die "$(eval_gettext "Failed to register update mode for submodule path '\$displaypath'")" - fi - done + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} "$@" } # @@ -740,7 +697,8 @@ cmd_update() if test -n "$recursive" then ( - prefix="$prefix$sm_path/" + prefix=$(relative_path "$prefix$sm_path/") + wt_prefix= clear_local_git_env cd "$sm_path" && eval cmd_update diff --git a/submodule.c b/submodule.c index 90825e17fa..4cc1c27931 100644 --- a/submodule.c +++ b/submodule.c @@ -237,6 +237,27 @@ int parse_submodule_update_strategy(const char *value, return 0; } +const char *submodule_strategy_to_string(const struct submodule_update_strategy *s) +{ + struct strbuf sb = STRBUF_INIT; + switch (s->type) { + case SM_UPDATE_CHECKOUT: + return "checkout"; + case SM_UPDATE_MERGE: + return "merge"; + case SM_UPDATE_REBASE: + return "rebase"; + case SM_UPDATE_NONE: + return "none"; + case SM_UPDATE_UNSPECIFIED: + return NULL; + case SM_UPDATE_COMMAND: + strbuf_addf(&sb, "!%s", s->command); + return strbuf_detach(&sb, NULL); + } + return NULL; +} + void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *arg) { diff --git a/submodule.h b/submodule.h index 7ef3775184..ff4c4f33a5 100644 --- a/submodule.h +++ b/submodule.h @@ -39,6 +39,7 @@ int submodule_config(const char *var, const char *value, void *cb); void gitmodules_config(void); int parse_submodule_update_strategy(const char *value, struct submodule_update_strategy *dst); +const char *submodule_strategy_to_string(const struct submodule_update_strategy *s); void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *); void show_submodule_summary(FILE *f, const char *path, const char *line_prefix, From d92028a575dde9c325e23f89c3d2b24f13868c57 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 28 Apr 2016 13:02:45 -0700 Subject: [PATCH 3/5] submodule init: fail gracefully with a missing .gitmodules file When there is no .gitmodules file availabe to initialize a submodule from, `submodule_from_path` just returns NULL. We need to check for that and abort gracefully. When `submodule init` was implemented in shell, a missing .gitmodules file would result in an error message No url found for submodule path '%s' in .gitmodules Replicate that error message for now. When the .gitmodules file is missing we can probably fail even earlier for all of the submodules with an improved error message. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 10 +++++++--- t/t7400-submodule-basic.sh | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b6d4f27648..ce9d11e8d3 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -314,13 +314,17 @@ static void init_submodule(const char *path, const char *prefix, int quiet) /* Only loads from .gitmodules, no overlay with .git/config */ gitmodules_config(); - sub = submodule_from_path(null_sha1, path); - if (prefix) { strbuf_addf(&sb, "%s%s", prefix, path); displaypath = strbuf_detach(&sb, NULL); } else - displaypath = xstrdup(sub->path); + displaypath = xstrdup(path); + + sub = submodule_from_path(null_sha1, path); + + if (!sub) + die(_("No url found for submodule path '%s' in .gitmodules"), + displaypath); /* * Copy url setting when it is not set yet. diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index f99f674ac7..df6b4da2ba 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -18,6 +18,14 @@ test_expect_success 'setup - initial commit' ' git branch initial ' +test_expect_success 'submodule init aborts on missing .gitmodules file' ' + test_when_finished "git update-index --remove sub" && + git update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub && + # missing the .gitmodules file here + test_must_fail git submodule init 2>actual && + test_i18ngrep "No url found for submodule path" actual +' + test_expect_success 'configuration parsing' ' test_when_finished "rm -f .gitmodules" && cat >.gitmodules <<-\EOF && From 08fdbdb153c138f0a2dc85949475fafb20853d96 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 28 Apr 2016 13:02:46 -0700 Subject: [PATCH 4/5] submodule--helper update-clone: abort gracefully on missing .gitmodules When there is no .gitmodules file availabe to initialize a submodule from, `submodule_from_path` just returns NULL. We need to check for that and abort gracefully. When `git submodule update` was implemented in shell, this error out with the warning Submodule path '%s' not initialized Maybe you want to use 'update --init'? Replicate that behavior for now instead of crashing. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 38 ++++++++++++++++++++++++------------- t/t7400-submodule-basic.sh | 8 ++++++++ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index ce9d11e8d3..5d05393e7b 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -593,6 +593,25 @@ struct submodule_update_clone { SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \ STRING_LIST_INIT_DUP, 0} + +static void next_submodule_warn_missing(struct submodule_update_clone *suc, + struct strbuf *out, const char *displaypath) +{ + /* + * Only mention uninitialized submodules when their + * paths have been specified. + */ + if (suc->warn_if_uninitialized) { + strbuf_addf(out, + _("Submodule path '%s' not initialized"), + displaypath); + strbuf_addch(out, '\n'); + strbuf_addstr(out, + _("Maybe you want to use 'update --init'?")); + strbuf_addch(out, '\n'); + } +} + /** * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise. @@ -627,6 +646,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, else displaypath = ce->name; + if (!sub) { + next_submodule_warn_missing(suc, out, displaypath); + goto cleanup; + } + if (suc->update.type == SM_UPDATE_NONE || (suc->update.type == SM_UPDATE_UNSPECIFIED && sub->update_strategy.type == SM_UPDATE_NONE)) { @@ -644,19 +668,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, strbuf_addf(&sb, "submodule.%s.url", sub->name); git_config_get_string(sb.buf, &url); if (!url) { - /* - * Only mention uninitialized submodules when their - * path have been specified - */ - if (suc->warn_if_uninitialized) { - strbuf_addf(out, - _("Submodule path '%s' not initialized"), - displaypath); - strbuf_addch(out, '\n'); - strbuf_addstr(out, - _("Maybe you want to use 'update --init'?")); - strbuf_addch(out, '\n'); - } + next_submodule_warn_missing(suc, out, displaypath); goto cleanup; } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index df6b4da2ba..814ee63194 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -26,6 +26,14 @@ test_expect_success 'submodule init aborts on missing .gitmodules file' ' test_i18ngrep "No url found for submodule path" actual ' +test_expect_success 'submodule update aborts on missing .gitmodules file' ' + test_when_finished "git update-index --remove sub" && + git update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub && + # missing the .gitmodules file here + git submodule update sub 2>actual && + test_i18ngrep "Submodule path .sub. not initialized" actual +' + test_expect_success 'configuration parsing' ' test_when_finished "rm -f .gitmodules" && cat >.gitmodules <<-\EOF && From c66410ed32a807cefca6f679ad5583eda2e9527b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Mon, 2 May 2016 15:24:04 -0700 Subject: [PATCH 5/5] submodule init: redirect stdout to stderr Reroute the output of stdout to stderr as it is just informative messages, not to be consumed by machines. This should not regress any scripts that try to parse the current output, as the output is already internationalized and therefore unstable. We want to init submodules from the helper for `submodule update` in a later patch and the stdout output of said helper is consumed by the parts of `submodule update` which are still written in shell. So we have to be careful which messages are on stdout. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 3 ++- t/t7406-submodule-update.sh | 24 ++++++++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5d05393e7b..7f0941ddc3 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -366,7 +366,8 @@ static void init_submodule(const char *path, const char *prefix, int quiet) die(_("Failed to register url for submodule path '%s'"), displaypath); if (!quiet) - printf(_("Submodule '%s' (%s) registered for path '%s'\n"), + fprintf(stderr, + _("Submodule '%s' (%s) registered for path '%s'\n"), sub->name, url, displaypath); } diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index fd741f506f..5f278799d5 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -108,24 +108,36 @@ pwd=$(pwd) cat <expect Submodule path '../super': checked out '$supersha1' -Submodule 'merging' ($pwd/merging) registered for path '../super/merging' -Submodule 'none' ($pwd/none) registered for path '../super/none' -Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing' -Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule' Submodule path '../super/merging': checked out '$mergingsha1' Submodule path '../super/none': checked out '$nonesha1' Submodule path '../super/rebasing': checked out '$rebasingsha1' Submodule path '../super/submodule': checked out '$submodulesha1' EOF +cat <expect2 +Submodule 'merging' ($pwd/merging) registered for path '../super/merging' +Submodule 'none' ($pwd/none) registered for path '../super/none' +Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing' +Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule' +Cloning into '$pwd/recursivesuper/super/merging'... +done. +Cloning into '$pwd/recursivesuper/super/none'... +done. +Cloning into '$pwd/recursivesuper/super/rebasing'... +done. +Cloning into '$pwd/recursivesuper/super/submodule'... +done. +EOF + test_expect_success 'submodule update --init --recursive from subdirectory' ' git -C recursivesuper/super reset --hard HEAD^ && (cd recursivesuper && mkdir tmp && cd tmp && - git submodule update --init --recursive ../super >../../actual + git submodule update --init --recursive ../super >../../actual 2>../../actual2 ) && - test_cmp expect actual + test_cmp expect actual && + test_cmp expect2 actual2 ' apos="'";