It looks like I forgot send this as a reply to my v3, so I'll link that series here instead: https://lore.kernel.org/git/20210801063352.50813-1-raykar.ath@xxxxxxxxx/ And to fetch these changes: git fetch https://github.com/tfidfwastaken/git.git submodule-helper-add-config-4 Atharva Raykar <raykar.ath@xxxxxxxxx> writes: > Add a new "add-config" subcommand to `git submodule--helper` with the > goal of converting part of the shell code in git-submodule.sh related to > `git submodule add` into C code. This new subcommand sets the > configuration variables of a newly added submodule, by registering the > url in local git config, as well as the submodule name and path in the > .gitmodules file. It also sets 'submodule.<name>.active' to "true" if > the submodule path has not already been covered by any pathspec > specified in 'submodule.active'. > > This is meant to be a faithful conversion from shell to C, although we > add comments to areas that could be improved in future patches, after > the conversion has settled. > > Signed-off-by: Atharva Raykar <raykar.ath@xxxxxxxxx> > Mentored-by: Christian Couder <christian.couder@xxxxxxxxx> > Mentored-by: Shourya Shukla <periperidip@xxxxxxxxx> > Based-on-patch-by: Shourya Shukla <periperidip@xxxxxxxxx> > Based-on-patch-by: Prathamesh Chavan <pc44800@xxxxxxxxx> > --- > > Changes since v3: > Address style nit. > > Range-diff against v3: > 1: be520ad028 ! 1: b3df2a5e6c submodule--helper: introduce add-config subcommand > @@ builtin/submodule--helper.c: static int add_clone(int argc, const char **argv, c > + config_submodule_in_gitmodules(add_data->sm_name, "url", add_data->repo)) > + die(_("Failed to register submodule '%s'"), add_data->sm_path); > + > -+ if (add_data->branch) > ++ if (add_data->branch) { > + if (config_submodule_in_gitmodules(add_data->sm_name, > + "branch", add_data->branch)) > + die(_("Failed to register submodule '%s'"), add_data->sm_path); > ++ } > + > + add_gitmodules.git_cmd = 1; > + strvec_pushl(&add_gitmodules.args, > > builtin/submodule--helper.c | 129 ++++++++++++++++++++++++++++++++++++ > git-submodule.sh | 28 +------- > submodule.c | 5 ++ > 3 files changed, 135 insertions(+), 27 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 862053c9f2..abf1ec2000 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -2936,6 +2936,134 @@ static int add_clone(int argc, const char **argv, const char *prefix) > return 0; > } > > +static int config_submodule_in_gitmodules(const char *name, const char *var, const char *value) > +{ > + char *key; > + int ret; > + > + if (!is_writing_gitmodules_ok()) > + die(_("please make sure that the .gitmodules file is in the working tree")); > + > + key = xstrfmt("submodule.%s.%s", name, var); > + ret = config_set_in_gitmodules_file_gently(key, value); > + free(key); > + > + return ret; > +} > + > +static void configure_added_submodule(struct add_data *add_data) > +{ > + char *key; > + char *val = NULL; > + struct child_process add_submod = CHILD_PROCESS_INIT; > + struct child_process add_gitmodules = CHILD_PROCESS_INIT; > + > + key = xstrfmt("submodule.%s.url", add_data->sm_name); > + git_config_set_gently(key, add_data->realrepo); > + free(key); > + > + add_submod.git_cmd = 1; > + strvec_pushl(&add_submod.args, "add", > + "--no-warn-embedded-repo", NULL); > + if (add_data->force) > + strvec_push(&add_submod.args, "--force"); > + strvec_pushl(&add_submod.args, "--", add_data->sm_path, NULL); > + > + if (run_command(&add_submod)) > + die(_("Failed to add submodule '%s'"), add_data->sm_path); > + > + if (config_submodule_in_gitmodules(add_data->sm_name, "path", add_data->sm_path) || > + config_submodule_in_gitmodules(add_data->sm_name, "url", add_data->repo)) > + die(_("Failed to register submodule '%s'"), add_data->sm_path); > + > + if (add_data->branch) { > + if (config_submodule_in_gitmodules(add_data->sm_name, > + "branch", add_data->branch)) > + die(_("Failed to register submodule '%s'"), add_data->sm_path); > + } > + > + add_gitmodules.git_cmd = 1; > + strvec_pushl(&add_gitmodules.args, > + "add", "--force", "--", ".gitmodules", NULL); > + > + if (run_command(&add_gitmodules)) > + die(_("Failed to register submodule '%s'"), add_data->sm_path); > + > + /* > + * NEEDSWORK: In a multi-working-tree world this needs to be > + * set in the per-worktree config. > + */ > + /* > + * NEEDSWORK: In the longer run, we need to get rid of this > + * pattern of querying "submodule.active" before calling > + * is_submodule_active(), since that function needs to find > + * out the value of "submodule.active" again anyway. > + */ > + if (!git_config_get_string("submodule.active", &val) && val) { > + /* > + * If the submodule being added isn't already covered by the > + * current configured pathspec, set the submodule's active flag > + */ > + if (!is_submodule_active(the_repository, add_data->sm_path)) { > + key = xstrfmt("submodule.%s.active", add_data->sm_name); > + git_config_set_gently(key, "true"); > + free(key); > + } > + } else { > + key = xstrfmt("submodule.%s.active", add_data->sm_name); > + git_config_set_gently(key, "true"); > + free(key); > + } > +} > + > +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 <url> 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 <branch>] " > + "--url <url> --resolved-url <resolved-url> " > + "--path <path> --name <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; > +} > + > #define SUPPORT_SUPER_PREFIX (1<<0) > > struct cmd_struct { > @@ -2949,6 +3077,7 @@ static struct cmd_struct commands[] = { > {"name", module_name, 0}, > {"clone", module_clone, 0}, > {"add-clone", add_clone, 0}, > + {"add-config", add_config, 0}, > {"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 053daf3724..f713cb113c 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -242,33 +242,7 @@ cmd_add() > 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 config submodule."$sm_name".url "$realrepo" > - > - git add --no-warn-embedded-repo $force "$sm_path" || > - die "fatal: $(eval_gettext "Failed to add submodule '\$sm_path'")" > - > - git submodule--helper config submodule."$sm_name".path "$sm_path" && > - git submodule--helper config submodule."$sm_name".url "$repo" && > - if test -n "$branch" > - then > - git submodule--helper config submodule."$sm_name".branch "$branch" > - fi && > - git add --force .gitmodules || > - die "fatal: $(eval_gettext "Failed to register submodule '\$sm_path'")" > - > - # NEEDSWORK: In a multi-working-tree world, this needs to be > - # set in the per-worktree config. > - if git config --get submodule.active >/dev/null > - then > - # If the submodule being adding isn't already covered by the > - # current configured pathspec, set the submodule's active flag > - if ! git submodule--helper is-active "$sm_path" > - then > - git config submodule."$sm_name".active "true" > - fi > - else > - git config submodule."$sm_name".active "true" > - fi > + git submodule--helper add-config ${force:+--force} ${branch:+--branch "$branch"} --url "$repo" --resolved-url "$realrepo" --path "$sm_path" --name "$sm_name" > } > > # > diff --git a/submodule.c b/submodule.c > index 0b1d9c1dde..8577667773 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -237,6 +237,11 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt, > /* > * Determine if a submodule has been initialized at a given 'path' > */ > +/* > + * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless, > + * ie, the config looks like: "[submodule] active\n". > + * Since that is an invalid pathspec, we should inform the user. > + */ > int is_submodule_active(struct repository *repo, const char *path) > { > int ret = 0; -- --- Atharva Raykar ಅಥರ್ವ ರಾಯ್ಕರ್ अथर्व रायकर
