This version modifies 'module_clone()' and separates the flag parsing from the actual cloning logic. This allows us to use the functionality of 'submodule--helper clone' without needing to push arguments to a strvec from 'add_submodule'. We use a new struct called `module_clone_data` instead. Because this change involves moving the contents of 'module_clone()' to 'clone_submodule()', the whole function had to be relocated further down so that all the helpers it calls are available to it. Other changes include making error output match more closely to the shell version, and better usage of the C API ('is_directory()' -> 'is_nonbare_repo_dir()') Atharva Raykar (3): submodule--helper: refactor module_clone() submodule--helper: introduce add-clone subcommand submodule--helper: introduce add-config subcommand builtin/submodule--helper.c | 536 ++++++++++++++++++++++++++++-------- git-submodule.sh | 66 +---- 2 files changed, 425 insertions(+), 177 deletions(-) Range-diff against v3: -: ---------- > 1: 11d035ce75 submodule--helper: refactor module_clone() 1: 3b5f7bec7c ! 2: c85701b79a submodule--helper: introduce add-clone subcommand @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char * +static int add_submodule(const struct add_data *add_data) +{ + char *submod_gitdir_path; ++ struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT; + + /* perhaps the path already exists and is already a git repo, else clone it */ + if (is_directory(add_data->sm_path)) { ++ struct strbuf sm_path = STRBUF_INIT; ++ strbuf_addstr(&sm_path, add_data->sm_path); + submod_gitdir_path = xstrfmt("%s/.git", add_data->sm_path); -+ if (is_directory(submod_gitdir_path) || file_exists(submod_gitdir_path)) -+ printf(_("Adding existing path at '%s' to index\n"), ++ if (is_nonbare_repository_dir(&sm_path)) ++ printf(_("Adding existing repo at '%s' to the index\n"), + add_data->sm_path); + else + die(_("'%s' already exists and is not a valid git repo"), + add_data->sm_path); ++ strbuf_release(&sm_path); + free(submod_gitdir_path); + } else { -+ struct strvec clone_args = STRVEC_INIT; + struct child_process cp = CHILD_PROCESS_INIT; + submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name); + + if (is_directory(submod_gitdir_path)) { + if (!add_data->force) { -+ error(_("a git directory for '%s' is found " -+ "locally with remote(s):"), add_data->sm_name); ++ fprintf(stderr, _("A git directory for '%s' is found " ++ "locally with remote(s):"), ++ add_data->sm_name); + show_fetch_remotes(stderr, add_data->sm_name, + submod_gitdir_path); -+ fprintf(stderr, -+ _("If you want to reuse this local git " -+ "directory instead of cloning again from\n" -+ " %s\n" -+ "use the '--force' option. If the local git " -+ "directory is not the correct repo\n" -+ "or if you are unsure what this means, choose " -+ "another name with the '--name' option.\n"), -+ add_data->realrepo); + free(submod_gitdir_path); -+ return 1; ++ die(_("If you want to reuse this local git " ++ "directory instead of cloning again from\n" ++ " %s\n" ++ "use the '--force' option. If the local git " ++ "directory is not the correct repo\n" ++ "or if you are unsure what this means, choose " ++ "another name with the '--name' option.\n"), ++ add_data->realrepo); + } else { + printf(_("Reactivating local git directory for " + "submodule '%s'\n"), add_data->sm_name); @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char * + } + free(submod_gitdir_path); + -+ strvec_pushl(&clone_args, "clone", "--path", add_data->sm_path, "--name", -+ add_data->sm_name, "--url", add_data->realrepo, NULL); -+ if (add_data->quiet) -+ strvec_push(&clone_args, "--quiet"); -+ if (add_data->progress) -+ strvec_push(&clone_args, "--progress"); -+ if (add_data->prefix) -+ strvec_pushl(&clone_args, "--prefix", add_data->prefix, NULL); ++ clone_data.prefix = add_data->prefix; ++ clone_data.path = add_data->sm_path; ++ clone_data.name = add_data->sm_name; ++ clone_data.url = add_data->realrepo; ++ clone_data.quiet = add_data->quiet; ++ clone_data.progress = add_data->progress; + if (add_data->reference_path) -+ strvec_pushl(&clone_args, "--reference", -+ add_data->reference_path, NULL); -+ if (add_data->dissociate) -+ strvec_push(&clone_args, "--dissociate"); ++ string_list_append(&clone_data.reference, ++ xstrdup(add_data->reference_path)); ++ clone_data.dissociate = add_data->dissociate; + if (add_data->depth >= 0) -+ strvec_pushf(&clone_args, "--depth=%d", add_data->depth); ++ clone_data.depth = xstrfmt("%d", add_data->depth); + -+ if (module_clone(clone_args.nr, clone_args.v, add_data->prefix)) { -+ strvec_clear(&clone_args); ++ if (clone_submodule(&clone_data)) + return -1; -+ } -+ strvec_clear(&clone_args); + + prepare_submodule_repo_env(&cp.env_array); + cp.git_cmd = 1; 2: a2a6b4d74c = 3: 6532b4ae11 submodule--helper: introduce add-config subcommand -- 2.31.1