From 6baf4e4da44e8412d61118c5f35f383b679462ca Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:33 +0530 Subject: [PATCH 1/9] submodule--helper: add options for compute_submodule_clone_url() Let's modify the interface to `compute_submodule_clone_url()` function by adding two more arguments, so that we can reuse this in various parts of `submodule--helper.c` that follow a common pattern, which is--read the remote url configuration of the superproject and then call `relative_url()`. This function is nearly identical to `resolve_relative_url()`, the only difference being the extra warning message. We can add a quiet flag to it, to suppress that warning when not needed, and then refactor `resolve_relative_url()` by using this function, something we will do in the next patch. We also rename the local variable 'relurl' to avoid potential confusion with the 'rel_url' parameter while we are at it. Having this functionality factored out will be useful for converting the rest of `submodule add` in subsequent patches. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 674b45b4dd..c9a4862a77 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -590,24 +590,28 @@ static int module_foreach(int argc, const char **argv, const char *prefix) return 0; } -static char *compute_submodule_clone_url(const char *rel_url) +static char *compute_submodule_clone_url(const char *rel_url, const char *up_path, int quiet) { - char *remoteurl, *relurl; + char *remoteurl, *resolved_url; char *remote = get_default_remote(); struct strbuf remotesb = STRBUF_INIT; strbuf_addf(&remotesb, "remote.%s.url", remote); if (git_config_get_string(remotesb.buf, &remoteurl)) { - warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf); + if (!quiet) + warning(_("could not look up configuration '%s'. " + "Assuming this repository is its own " + "authoritative upstream."), + remotesb.buf); remoteurl = xgetcwd(); } - relurl = relative_url(remoteurl, rel_url, NULL); + resolved_url = relative_url(remoteurl, rel_url, up_path); free(remote); free(remoteurl); strbuf_release(&remotesb); - return relurl; + return resolved_url; } struct init_cb { @@ -660,7 +664,7 @@ static void init_submodule(const char *path, const char *prefix, if (starts_with_dot_dot_slash(url) || starts_with_dot_slash(url)) { char *oldurl = url; - url = compute_submodule_clone_url(oldurl); + url = compute_submodule_clone_url(oldurl, NULL, 0); free(oldurl); } @@ -2134,7 +2138,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, if (repo_config_get_string_tmp(the_repository, sb.buf, &url)) { if (starts_with_dot_slash(sub->url) || starts_with_dot_dot_slash(sub->url)) { - url = compute_submodule_clone_url(sub->url); + url = compute_submodule_clone_url(sub->url, NULL, 0); need_free_url = 1; } else url = sub->url; From ab6f23b75129bc20681c0941cdd01d2694bc54f4 Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:34 +0530 Subject: [PATCH 2/9] submodule--helper: refactor resolve_relative_url() helper Refactor the helper function to resolve a relative url, by reusing the existing `compute_submodule_clone_url()` function. `compute_submodule_clone_url()` performs the same work that `resolve_relative_url()` is doing, so we eliminate this code repetition by moving the former function's definition up, and calling it inside `resolve_relative_url()`. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 61 +++++++++++++++---------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index c9a4862a77..75179fca89 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url, return strbuf_detach(&sb, NULL); } +static char *compute_submodule_clone_url(const char *rel_url, const char *up_path, int quiet) +{ + char *remoteurl, *resolved_url; + char *remote = get_default_remote(); + struct strbuf remotesb = STRBUF_INIT; + + strbuf_addf(&remotesb, "remote.%s.url", remote); + if (git_config_get_string(remotesb.buf, &remoteurl)) { + if (!quiet) + warning(_("could not look up configuration '%s'. " + "Assuming this repository is its own " + "authoritative upstream."), + remotesb.buf); + remoteurl = xgetcwd(); + } + resolved_url = relative_url(remoteurl, rel_url, up_path); + + free(remote); + free(remoteurl); + strbuf_release(&remotesb); + + return resolved_url; +} + 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); + res = compute_submodule_clone_url(url, up_path, 1); puts(res); free(res); - free(remoteurl); return 0; } @@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix) return 0; } -static char *compute_submodule_clone_url(const char *rel_url, const char *up_path, int quiet) -{ - char *remoteurl, *resolved_url; - char *remote = get_default_remote(); - struct strbuf remotesb = STRBUF_INIT; - - strbuf_addf(&remotesb, "remote.%s.url", remote); - if (git_config_get_string(remotesb.buf, &remoteurl)) { - if (!quiet) - warning(_("could not look up configuration '%s'. " - "Assuming this repository is its own " - "authoritative upstream."), - remotesb.buf); - remoteurl = xgetcwd(); - } - resolved_url = relative_url(remoteurl, rel_url, up_path); - - free(remote); - free(remoteurl); - strbuf_release(&remotesb); - - return resolved_url; -} - struct init_cb { const char *prefix; unsigned int flags; From 0c61041ed6778db86184d7a0d9759085f3cb9a82 Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:35 +0530 Subject: [PATCH 3/9] submodule--helper: remove repeated code in sync_submodule() This part of `sync_submodule()` is doing the same thing that `compute_submodule_clone_url()` is doing. Let's reuse that helper here. Note that this change adds a small overhead where we allocate and free the 'remote' twice, but that is a small price to pay for the higher level of abstraction we get. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 75179fca89..3dd122b410 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1373,20 +1373,10 @@ static void sync_submodule(const char *path, const char *prefix, 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(path); - sub_origin_url = relative_url(remote_url, sub->url, up_path); - super_config_url = relative_url(remote_url, sub->url, NULL); - - free(remote); + char *up_path = get_up_path(path); + sub_origin_url = compute_submodule_clone_url(sub->url, up_path, 1); + super_config_url = compute_submodule_clone_url(sub->url, NULL, 1); free(up_path); - free(remote_url); } else { sub_origin_url = xstrdup(sub->url); super_config_url = xstrdup(sub->url); From ed86301f68fcbb17c5d1c7a3258e4705b3b1da9c Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:36 +0530 Subject: [PATCH 4/9] dir: libify and export helper functions from clone.c These functions can be useful to other parts of Git. Let's move them to dir.c, while renaming them to be make their functionality more explicit. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/clone.c | 118 +----------------------------------------------- dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++ dir.h | 11 +++++ 3 files changed, 127 insertions(+), 116 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 66fe66679c..5ba24b7ae7 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle) return canon; } -static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) -{ - const char *end = repo + strlen(repo), *start, *ptr; - size_t len; - char *dir; - - /* - * Skip scheme. - */ - start = strstr(repo, "://"); - if (start == NULL) - start = repo; - else - start += 3; - - /* - * Skip authentication data. The stripping does happen - * greedily, such that we strip up to the last '@' inside - * the host part. - */ - for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) { - if (*ptr == '@') - start = ptr + 1; - } - - /* - * Strip trailing spaces, slashes and /.git - */ - while (start < end && (is_dir_sep(end[-1]) || isspace(end[-1]))) - end--; - if (end - start > 5 && is_dir_sep(end[-5]) && - !strncmp(end - 4, ".git", 4)) { - end -= 5; - while (start < end && is_dir_sep(end[-1])) - end--; - } - - /* - * Strip trailing port number if we've got only a - * hostname (that is, there is no dir separator but a - * colon). This check is required such that we do not - * strip URI's like '/foo/bar:2222.git', which should - * result in a dir '2222' being guessed due to backwards - * compatibility. - */ - if (memchr(start, '/', end - start) == NULL - && memchr(start, ':', end - start) != NULL) { - ptr = end; - while (start < ptr && isdigit(ptr[-1]) && ptr[-1] != ':') - ptr--; - if (start < ptr && ptr[-1] == ':') - end = ptr - 1; - } - - /* - * Find last component. To remain backwards compatible we - * also regard colons as path separators, such that - * cloning a repository 'foo:bar.git' would result in a - * directory 'bar' being guessed. - */ - ptr = end; - while (start < ptr && !is_dir_sep(ptr[-1]) && ptr[-1] != ':') - ptr--; - start = ptr; - - /* - * Strip .{bundle,git}. - */ - len = end - start; - strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git"); - - if (!len || (len == 1 && *start == '/')) - die(_("No directory name could be guessed.\n" - "Please specify a directory on the command line")); - - if (is_bare) - dir = xstrfmt("%.*s.git", (int)len, start); - else - dir = xstrndup(start, len); - /* - * Replace sequences of 'control' characters and whitespace - * with one ascii space, remove leading and trailing spaces. - */ - if (*dir) { - char *out = dir; - int prev_space = 1 /* strip leading whitespace */; - for (end = dir; *end; ++end) { - char ch = *end; - if ((unsigned char)ch < '\x20') - ch = '\x20'; - if (isspace(ch)) { - if (prev_space) - continue; - prev_space = 1; - } else - prev_space = 0; - *out++ = ch; - } - *out = '\0'; - if (out > dir && prev_space) - out[-1] = '\0'; - } - return dir; -} - -static void strip_trailing_slashes(char *dir) -{ - char *end = dir + strlen(dir); - - while (dir < end - 1 && is_dir_sep(end[-1])) - end--; - *end = '\0'; -} - static int add_one_reference(struct string_list_item *item, void *cb_data) { struct strbuf err = STRBUF_INIT; @@ -1041,8 +927,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (argc == 2) dir = xstrdup(argv[1]); else - dir = guess_dir_name(repo_name, is_bundle, option_bare); - strip_trailing_slashes(dir); + dir = git_url_basename(repo_name, is_bundle, option_bare); + strip_dir_trailing_slashes(dir); dest_exists = path_exists(dir); if (dest_exists && !is_empty_dir(dir)) diff --git a/dir.c b/dir.c index 03c4d21267..6d26db3189 100644 --- a/dir.c +++ b/dir.c @@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path) return ret; } +char *git_url_basename(const char *repo, int is_bundle, int is_bare) +{ + const char *end = repo + strlen(repo), *start, *ptr; + size_t len; + char *dir; + + /* + * Skip scheme. + */ + start = strstr(repo, "://"); + if (start == NULL) + start = repo; + else + start += 3; + + /* + * Skip authentication data. The stripping does happen + * greedily, such that we strip up to the last '@' inside + * the host part. + */ + for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) { + if (*ptr == '@') + start = ptr + 1; + } + + /* + * Strip trailing spaces, slashes and /.git + */ + while (start < end && (is_dir_sep(end[-1]) || isspace(end[-1]))) + end--; + if (end - start > 5 && is_dir_sep(end[-5]) && + !strncmp(end - 4, ".git", 4)) { + end -= 5; + while (start < end && is_dir_sep(end[-1])) + end--; + } + + /* + * Strip trailing port number if we've got only a + * hostname (that is, there is no dir separator but a + * colon). This check is required such that we do not + * strip URI's like '/foo/bar:2222.git', which should + * result in a dir '2222' being guessed due to backwards + * compatibility. + */ + if (memchr(start, '/', end - start) == NULL + && memchr(start, ':', end - start) != NULL) { + ptr = end; + while (start < ptr && isdigit(ptr[-1]) && ptr[-1] != ':') + ptr--; + if (start < ptr && ptr[-1] == ':') + end = ptr - 1; + } + + /* + * Find last component. To remain backwards compatible we + * also regard colons as path separators, such that + * cloning a repository 'foo:bar.git' would result in a + * directory 'bar' being guessed. + */ + ptr = end; + while (start < ptr && !is_dir_sep(ptr[-1]) && ptr[-1] != ':') + ptr--; + start = ptr; + + /* + * Strip .{bundle,git}. + */ + len = end - start; + strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git"); + + if (!len || (len == 1 && *start == '/')) + die(_("No directory name could be guessed.\n" + "Please specify a directory on the command line")); + + if (is_bare) + dir = xstrfmt("%.*s.git", (int)len, start); + else + dir = xstrndup(start, len); + /* + * Replace sequences of 'control' characters and whitespace + * with one ascii space, remove leading and trailing spaces. + */ + if (*dir) { + char *out = dir; + int prev_space = 1 /* strip leading whitespace */; + for (end = dir; *end; ++end) { + char ch = *end; + if ((unsigned char)ch < '\x20') + ch = '\x20'; + if (isspace(ch)) { + if (prev_space) + continue; + prev_space = 1; + } else + prev_space = 0; + *out++ = ch; + } + *out = '\0'; + if (out > dir && prev_space) + out[-1] = '\0'; + } + return dir; +} + +void strip_dir_trailing_slashes(char *dir) +{ + char *end = dir + strlen(dir); + + while (dir < end - 1 && is_dir_sep(end[-1])) + end--; + *end = '\0'; +} + static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) { DIR *dir; diff --git a/dir.h b/dir.h index b3e1a54a97..63ff04f5ac 100644 --- a/dir.h +++ b/dir.h @@ -453,6 +453,17 @@ static inline int is_dot_or_dotdot(const char *name) int is_empty_dir(const char *dir); +/* + * Retrieve the "humanish" basename of the given Git URL. + * + * For example: + * /path/to/repo.git => "repo" + * host.xz:foo/.git => "foo" + * http://example.com/user/bar.baz => "bar.baz" + */ +char *git_url_basename(const char *repo, int is_bundle, int is_bare); +void strip_dir_trailing_slashes(char *dir); + void setup_standard_excludes(struct dir_struct *dir); char *get_sparse_checkout_filename(void); From a6226fd772b1dfff87fa8dc040d97efa75a3721c Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:37 +0530 Subject: [PATCH 5/9] submodule--helper: convert the bulk of cmd_add() to C Introduce the 'add' subcommand to `submodule--helper.c` that does all the work 'submodule add' past the parsing of flags. We also remove the constness of the sm_path field of the `add_data` struct. This is needed so that it can be modified by normalize_path_copy(). As with the previous conversions, this is meant to be a faithful conversion with no modification to the behaviour of `submodule add`. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Helped-by: Kaartic Sivaraam Mentored-by: Shourya Shukla Based-on-patch-by: Shourya Shukla Based-on-patch-by: Prathamesh Chavan Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 165 +++++++++++++++++++++++++++++++++++- git-submodule.sh | 96 +-------------------- 2 files changed, 166 insertions(+), 95 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 3dd122b410..0e94e16f4d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2748,7 +2748,7 @@ struct add_data { const char *prefix; const char *branch; const char *reference_path; - const char *sm_path; + char *sm_path; const char *sm_name; const char *repo; const char *realrepo; @@ -3047,6 +3047,168 @@ static int add_config(int argc, const char **argv, const char *prefix) return 0; } +static void die_on_index_match(const char *path, int force) +{ + struct pathspec ps; + const char *args[] = { path, NULL }; + parse_pathspec(&ps, 0, PATHSPEC_PREFER_CWD, NULL, args); + + if (read_cache_preload(NULL) < 0) + die(_("index file corrupt")); + + if (ps.nr) { + int i; + char *ps_matched = xcalloc(ps.nr, 1); + + /* TODO: audit for interaction with sparse-index. */ + ensure_full_index(&the_index); + + /* + * Since there is only one pathspec, we just need + * need to check ps_matched[0] to know if a cache + * entry matched. + */ + for (i = 0; i < active_nr; i++) { + ce_path_match(&the_index, active_cache[i], &ps, + ps_matched); + + if (ps_matched[0]) { + if (!force) + die(_("'%s' already exists in the index"), + path); + if (!S_ISGITLINK(active_cache[i]->ce_mode)) + die(_("'%s' already exists in the index " + "and is not a submodule"), path); + break; + } + } + free(ps_matched); + } +} + +static void die_on_repo_without_commits(const char *path) +{ + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(&sb, path); + if (is_nonbare_repository_dir(&sb)) { + struct object_id oid; + if (resolve_gitlink_ref(path, "HEAD", &oid) < 0) + die(_("'%s' does not have a commit checked out"), path); + } +} + +static int module_add(int argc, const char **argv, const char *prefix) +{ + int force = 0, quiet = 0, progress = 0, dissociate = 0; + struct add_data add_data = ADD_DATA_INIT; + + struct option options[] = { + OPT_STRING('b', "branch", &add_data.branch, N_("branch"), + N_("branch of repository to add as submodule")), + OPT__FORCE(&force, N_("allow adding an otherwise ignored submodule path"), + PARSE_OPT_NOCOMPLETE), + OPT__QUIET(&quiet, N_("print only error messages")), + OPT_BOOL(0, "progress", &progress, N_("force cloning progress")), + OPT_STRING(0, "reference", &add_data.reference_path, N_("repository"), + N_("reference repository")), + OPT_BOOL(0, "dissociate", &dissociate, N_("borrow the objects from reference repositories")), + OPT_STRING(0, "name", &add_data.sm_name, N_("name"), + N_("sets the submodule’s name to the given string " + "instead of defaulting to its path")), + OPT_INTEGER(0, "depth", &add_data.depth, N_("depth for shallow clones")), + OPT_END() + }; + + const char *const usage[] = { + N_("git submodule--helper add [] [--] []"), + NULL + }; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + + if (!is_writing_gitmodules_ok()) + die(_("please make sure that the .gitmodules file is in the working tree")); + + if (prefix && *prefix && + add_data.reference_path && !is_absolute_path(add_data.reference_path)) + add_data.reference_path = xstrfmt("%s%s", prefix, add_data.reference_path); + + if (argc == 0 || argc > 2) + usage_with_options(usage, options); + + add_data.repo = argv[0]; + if (argc == 1) + add_data.sm_path = git_url_basename(add_data.repo, 0, 0); + else + add_data.sm_path = xstrdup(argv[1]); + + if (prefix && *prefix && !is_absolute_path(add_data.sm_path)) + add_data.sm_path = xstrfmt("%s%s", prefix, add_data.sm_path); + + if (starts_with_dot_dot_slash(add_data.repo) || + starts_with_dot_slash(add_data.repo)) { + if (prefix) + die(_("Relative path can only be used from the toplevel " + "of the working tree")); + + /* dereference source url relative to parent's url */ + add_data.realrepo = compute_submodule_clone_url(add_data.repo, NULL, 1); + } else if (is_dir_sep(add_data.repo[0]) || strchr(add_data.repo, ':')) { + add_data.realrepo = add_data.repo; + } else { + die(_("repo URL: '%s' must be absolute or begin with ./|../"), + add_data.repo); + } + + /* + * normalize path: + * multiple //; leading ./; /./; /../; + */ + normalize_path_copy(add_data.sm_path, add_data.sm_path); + strip_dir_trailing_slashes(add_data.sm_path); + + die_on_index_match(add_data.sm_path, force); + die_on_repo_without_commits(add_data.sm_path); + + if (!force) { + int exit_code = -1; + struct strbuf sb = STRBUF_INIT; + struct child_process cp = CHILD_PROCESS_INIT; + cp.git_cmd = 1; + cp.no_stdout = 1; + strvec_pushl(&cp.args, "add", "--dry-run", "--ignore-missing", + "--no-warn-embedded-repo", add_data.sm_path, NULL); + if ((exit_code = pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0))) { + strbuf_complete_line(&sb); + fputs(sb.buf, stderr); + free(add_data.sm_path); + return exit_code; + } + strbuf_release(&sb); + } + + if(!add_data.sm_name) + add_data.sm_name = add_data.sm_path; + + if (check_submodule_name(add_data.sm_name)) + die(_("'%s' is not a valid submodule name"), add_data.sm_name); + + add_data.prefix = prefix; + add_data.force = !!force; + add_data.quiet = !!quiet; + add_data.progress = !!progress; + add_data.dissociate = !!dissociate; + + if (add_submodule(&add_data)) { + free(add_data.sm_path); + return 1; + } + configure_added_submodule(&add_data); + free(add_data.sm_path); + + return 0; +} + #define SUPPORT_SUPER_PREFIX (1<<0) struct cmd_struct { @@ -3061,6 +3223,7 @@ static struct cmd_struct commands[] = { {"clone", module_clone, 0}, {"add-clone", add_clone, 0}, {"add-config", add_config, 0}, + {"add", module_add, SUPPORT_SUPER_PREFIX}, {"update-module-mode", module_update_module_mode, 0}, {"update-clone", update_clone, 0}, {"ensure-core-worktree", ensure_core_worktree, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index 8c219ef382..1070540525 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -145,104 +145,12 @@ cmd_add() shift done - if ! git submodule--helper config --check-writeable >/dev/null 2>&1 + if test -z "$1" then - die "fatal: $(eval_gettext "please make sure that the .gitmodules file is in the working tree")" - fi - - if test -n "$reference_path" - then - is_absolute_path "$reference_path" || - reference_path="$wt_prefix$reference_path" - - reference="--reference=$reference_path" - fi - - repo=$1 - sm_path=$2 - - if test -z "$sm_path"; then - sm_path=$(printf '%s\n' "$repo" | - sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g') - fi - - if test -z "$repo" || test -z "$sm_path"; then usage fi - is_absolute_path "$sm_path" || sm_path="$wt_prefix$sm_path" - - # assure repo is absolute or relative to parent - case "$repo" in - ./*|../*) - test -z "$wt_prefix" || - die "fatal: $(gettext "Relative path can only be used from the toplevel of the working tree")" - - # dereference source url relative to parent's url - realrepo=$(git submodule--helper resolve-relative-url "$repo") || exit - ;; - *:*|/*) - # absolute url - realrepo=$repo - ;; - *) - die "fatal: $(eval_gettext "repo URL: '\$repo' must be absolute or begin with ./|../")" - ;; - esac - - # normalize path: - # multiple //; leading ./; /./; /../; trailing / - sm_path=$(printf '%s/\n' "$sm_path" | - sed -e ' - s|//*|/|g - s|^\(\./\)*|| - s|/\(\./\)*|/|g - :start - s|\([^/]*\)/\.\./|| - tstart - s|/*$|| - ') - if test -z "$force" - then - git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 && - die "fatal: $(eval_gettext "'\$sm_path' already exists in the index")" - else - git ls-files -s "$sm_path" | sane_grep -v "^160000" > /dev/null 2>&1 && - die "fatal: $(eval_gettext "'\$sm_path' already exists in the index and is not a submodule")" - fi - - if test -d "$sm_path" && - test -z $(git -C "$sm_path" rev-parse --show-cdup 2>/dev/null) - then - git -C "$sm_path" rev-parse --verify -q HEAD >/dev/null || - die "fatal: $(eval_gettext "'\$sm_path' does not have a commit checked out")" - fi - - if test -z "$force" - then - dryerr=$(git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" 2>&1 >/dev/null) - res=$? - if test $res -ne 0 - then - echo >&2 "$dryerr" - exit $res - fi - fi - - if test -n "$custom_name" - then - sm_name="$custom_name" - else - sm_name="$sm_path" - fi - - if ! git submodule--helper check-name "$sm_name" - then - die "fatal: $(eval_gettext "'$sm_name' is not a valid submodule name")" - fi - - git submodule--helper add-clone ${GIT_QUIET:+--quiet} ${force:+"--force"} ${progress:+"--progress"} ${branch:+--branch "$branch"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"--dissociate"} ${depth:+"$depth"} || exit - git submodule--helper add-config ${force:+--force} ${branch:+--branch "$branch"} --url "$repo" --resolved-url "$realrepo" --path "$sm_path" --name "$sm_name" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper add ${GIT_QUIET:+--quiet} ${force:+--force} ${progress:+"--progress"} ${branch:+--branch "$branch"} ${reference_path:+--reference "$reference_path"} ${dissociate:+--dissociate} ${custom_name:+--name "$custom_name"} ${depth:+"$depth"} -- "$@" } # From f006132c243249f0eef01ba31b415426650e0916 Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:38 +0530 Subject: [PATCH 6/9] submodule--helper: remove add-clone subcommand We no longer need this subcommand, as all of its functionality is being called by the newly-introduced `module_add()` directly within C. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 60 ------------------------------------- 1 file changed, 60 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 0e94e16f4d..1c81f15a24 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data) return 0; } -static int add_clone(int argc, const char **argv, const char *prefix) -{ - int force = 0, quiet = 0, dissociate = 0, progress = 0; - struct add_data add_data = ADD_DATA_INIT; - - struct option options[] = { - OPT_STRING('b', "branch", &add_data.branch, - N_("branch"), - N_("branch of repository to checkout on cloning")), - OPT_STRING(0, "prefix", &prefix, - N_("path"), - N_("alternative anchor for relative paths")), - OPT_STRING(0, "path", &add_data.sm_path, - N_("path"), - N_("where the new submodule will be cloned to")), - OPT_STRING(0, "name", &add_data.sm_name, - N_("string"), - N_("name of the new submodule")), - OPT_STRING(0, "url", &add_data.realrepo, - N_("string"), - N_("url where to clone the submodule from")), - OPT_STRING(0, "reference", &add_data.reference_path, - N_("repo"), - N_("reference repository")), - OPT_BOOL(0, "dissociate", &dissociate, - N_("use --reference only while cloning")), - OPT_INTEGER(0, "depth", &add_data.depth, - N_("depth for shallow clones")), - OPT_BOOL(0, "progress", &progress, - N_("force cloning progress")), - OPT__FORCE(&force, N_("allow adding an otherwise ignored submodule path"), - PARSE_OPT_NOCOMPLETE), - OPT__QUIET(&quiet, "suppress output for cloning a submodule"), - OPT_END() - }; - - const char *const usage[] = { - N_("git submodule--helper add-clone [...] " - "--url --path --name "), - NULL - }; - - argc = parse_options(argc, argv, prefix, options, usage, 0); - - if (argc != 0) - usage_with_options(usage, options); - - add_data.prefix = prefix; - add_data.progress = !!progress; - add_data.dissociate = !!dissociate; - add_data.force = !!force; - add_data.quiet = !!quiet; - - if (add_submodule(&add_data)) - return 1; - - return 0; -} - static int config_submodule_in_gitmodules(const char *name, const char *var, const char *value) { char *key; @@ -3221,7 +3162,6 @@ static struct cmd_struct commands[] = { {"list", module_list, 0}, {"name", module_name, 0}, {"clone", module_clone, 0}, - {"add-clone", add_clone, 0}, {"add-config", add_config, 0}, {"add", module_add, SUPPORT_SUPER_PREFIX}, {"update-module-mode", module_update_module_mode, 0}, From ba8a3b019ed796fcf5224c2b2280eb1ebc129075 Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:39 +0530 Subject: [PATCH 7/9] submodule--helper: remove add-config subcommand Also no longer needed is this subcommand, as all of its functionality is being called by the newly-introduced `module_add()` directly within C. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 49 ------------------------------------- 1 file changed, 49 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1c81f15a24..28610706f5 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2940,54 +2940,6 @@ static void configure_added_submodule(struct add_data *add_data) } } -static int add_config(int argc, const char **argv, const char *prefix) -{ - int force = 0; - struct add_data add_data = ADD_DATA_INIT; - - struct option options[] = { - OPT_STRING('b', "branch", &add_data.branch, - N_("branch"), - N_("branch of repository to store in " - "the submodule configuration")), - OPT_STRING(0, "url", &add_data.repo, - N_("string"), - N_("url to clone submodule from")), - OPT_STRING(0, "resolved-url", &add_data.realrepo, - N_("string"), - N_("url to clone the submodule from, after it has " - "been dereferenced relative to parent's url, " - "in the case where is a relative url")), - OPT_STRING(0, "path", &add_data.sm_path, - N_("path"), - N_("where the new submodule will be cloned to")), - OPT_STRING(0, "name", &add_data.sm_name, - N_("string"), - N_("name of the new submodule")), - OPT__FORCE(&force, N_("allow adding an otherwise ignored submodule path"), - PARSE_OPT_NOCOMPLETE), - OPT_END() - }; - - const char *const usage[] = { - N_("git submodule--helper add-config " - "[--force|-f] [--branch|-b ] " - "--url --resolved-url " - "--path --name "), - NULL - }; - - argc = parse_options(argc, argv, prefix, options, usage, 0); - - if (argc) - usage_with_options(usage, options); - - add_data.force = !!force; - configure_added_submodule(&add_data); - - return 0; -} - static void die_on_index_match(const char *path, int force) { struct pathspec ps; @@ -3162,7 +3114,6 @@ static struct cmd_struct commands[] = { {"list", module_list, 0}, {"name", module_name, 0}, {"clone", module_clone, 0}, - {"add-config", add_config, 0}, {"add", module_add, SUPPORT_SUPER_PREFIX}, {"update-module-mode", module_update_module_mode, 0}, {"update-clone", update_clone, 0}, From 15fe88d5a6275c79eb0a4e6e56b5bf5d0a30a4fc Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:40 +0530 Subject: [PATCH 8/9] submodule--helper: remove resolve-relative-url subcommand The shell subcommand `resolve-relative-url` is no longer required, as its last caller has been removed when it was converted to C. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 28610706f5..9d450b5860 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_pat return resolved_url; } -static int resolve_relative_url(int argc, const char **argv, const char *prefix) -{ - const char *up_path = NULL; - char *res; - const char *url; - - if (argc != 2 && argc != 3) - die("resolve-relative-url only accepts one or two arguments"); - - url = argv[1]; - if (argc == 3) - up_path = argv[2]; - - res = compute_submodule_clone_url(url, up_path, 1); - puts(res); - free(res); - return 0; -} - static int resolve_relative_url_test(int argc, const char **argv, const char *prefix) { char *remoteurl, *res; @@ -3119,7 +3100,6 @@ static struct cmd_struct commands[] = { {"update-clone", update_clone, 0}, {"ensure-core-worktree", ensure_core_worktree, 0}, {"relative-path", resolve_relative_path, 0}, - {"resolve-relative-url", resolve_relative_url, 0}, {"resolve-relative-url-test", resolve_relative_url_test, 0}, {"foreach", module_foreach, SUPPORT_SUPER_PREFIX}, {"init", module_init, SUPPORT_SUPER_PREFIX}, From de0fcbe0f4987dcaa238daa0d0f9cce17afe0495 Mon Sep 17 00:00:00 2001 From: Atharva Raykar Date: Tue, 10 Aug 2021 17:16:41 +0530 Subject: [PATCH 9/9] submodule--helper: rename compute_submodule_clone_url() Let's rename 'compute_submodule_clone_url()' to 'resolve_relative_url()' to make it clear that this internal helper need not be used exclusively for computing submodule clone URLs. Since the original 'resolve-relative-url' subcommand and its C entry point has been removed in c461095ae3 (submodule--helper: remove resolve-relative-url subcommand, 2021-07-02), this rename can be done without causing any confusion about which function it actually binds to. Signed-off-by: Atharva Raykar Mentored-by: Christian Couder Mentored-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 9d450b5860..c9ec03964b 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -199,7 +199,7 @@ static char *relative_url(const char *remote_url, return strbuf_detach(&sb, NULL); } -static char *compute_submodule_clone_url(const char *rel_url, const char *up_path, int quiet) +static char *resolve_relative_url(const char *rel_url, const char *up_path, int quiet) { char *remoteurl, *resolved_url; char *remote = get_default_remote(); @@ -634,7 +634,7 @@ static void init_submodule(const char *path, const char *prefix, if (starts_with_dot_dot_slash(url) || starts_with_dot_slash(url)) { char *oldurl = url; - url = compute_submodule_clone_url(oldurl, NULL, 0); + url = resolve_relative_url(oldurl, NULL, 0); free(oldurl); } @@ -1355,8 +1355,8 @@ static void sync_submodule(const char *path, const char *prefix, if (starts_with_dot_dot_slash(sub->url) || starts_with_dot_slash(sub->url)) { char *up_path = get_up_path(path); - sub_origin_url = compute_submodule_clone_url(sub->url, up_path, 1); - super_config_url = compute_submodule_clone_url(sub->url, NULL, 1); + sub_origin_url = resolve_relative_url(sub->url, up_path, 1); + super_config_url = resolve_relative_url(sub->url, NULL, 1); free(up_path); } else { sub_origin_url = xstrdup(sub->url); @@ -2098,7 +2098,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, if (repo_config_get_string_tmp(the_repository, sb.buf, &url)) { if (starts_with_dot_slash(sub->url) || starts_with_dot_dot_slash(sub->url)) { - url = compute_submodule_clone_url(sub->url, NULL, 0); + url = resolve_relative_url(sub->url, NULL, 0); need_free_url = 1; } else url = sub->url; @@ -3026,7 +3026,7 @@ static int module_add(int argc, const char **argv, const char *prefix) "of the working tree")); /* dereference source url relative to parent's url */ - add_data.realrepo = compute_submodule_clone_url(add_data.repo, NULL, 1); + add_data.realrepo = resolve_relative_url(add_data.repo, NULL, 1); } else if (is_dir_sep(add_data.repo[0]) || strchr(add_data.repo, ':')) { add_data.realrepo = add_data.repo; } else {