submodule--helper: introduce add-config subcommand

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@gmail.com> Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Based-on-patch-by: Shourya Shukla <periperidip@gmail.com> Based-on-patch-by: Prathamesh Chavan <pc44800@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Atharva Raykar committed Aug 6, 2021 at 19:34 UTC a452128a36cb73f5366c23eabe93c7edfa227866
3 files changed +135 -27
builtin/submodule--helper.c
+129
@@ -2935,6 +2935,134 @@ static int add_clone(int argc, const char **argv, const char *prefix)
2935 return 0;
2936 }
2937
2938 +static int config_submodule_in_gitmodules(const char *name, const char *var, const char *value)
2939 +{
2940 + char *key;
2941 + int ret;
2942 +
2943 + if (!is_writing_gitmodules_ok())
2944 + die(_("please make sure that the .gitmodules file is in the working tree"));
2945 +
2946 + key = xstrfmt("submodule.%s.%s", name, var);
2947 + ret = config_set_in_gitmodules_file_gently(key, value);
2948 + free(key);
2949 +
2950 + return ret;
2951 +}
2952 +
2953 +static void configure_added_submodule(struct add_data *add_data)
2954 +{
2955 + char *key;
2956 + char *val = NULL;
2957 + struct child_process add_submod = CHILD_PROCESS_INIT;
2958 + struct child_process add_gitmodules = CHILD_PROCESS_INIT;
2959 +
2960 + key = xstrfmt("submodule.%s.url", add_data->sm_name);
2961 + git_config_set_gently(key, add_data->realrepo);
2962 + free(key);
2963 +
2964 + add_submod.git_cmd = 1;
2965 + strvec_pushl(&add_submod.args, "add",
2966 + "--no-warn-embedded-repo", NULL);
2967 + if (add_data->force)
2968 + strvec_push(&add_submod.args, "--force");
2969 + strvec_pushl(&add_submod.args, "--", add_data->sm_path, NULL);
2970 +
2971 + if (run_command(&add_submod))
2972 + die(_("Failed to add submodule '%s'"), add_data->sm_path);
2973 +
2974 + if (config_submodule_in_gitmodules(add_data->sm_name, "path", add_data->sm_path) ||
2975 + config_submodule_in_gitmodules(add_data->sm_name, "url", add_data->repo))
2976 + die(_("Failed to register submodule '%s'"), add_data->sm_path);
2977 +
2978 + if (add_data->branch) {
2979 + if (config_submodule_in_gitmodules(add_data->sm_name,
2980 + "branch", add_data->branch))
2981 + die(_("Failed to register submodule '%s'"), add_data->sm_path);
2982 + }
2983 +
2984 + add_gitmodules.git_cmd = 1;
2985 + strvec_pushl(&add_gitmodules.args,
2986 + "add", "--force", "--", ".gitmodules", NULL);
2987 +
2988 + if (run_command(&add_gitmodules))
2989 + die(_("Failed to register submodule '%s'"), add_data->sm_path);
2990 +
2991 + /*
2992 + * NEEDSWORK: In a multi-working-tree world this needs to be
2993 + * set in the per-worktree config.
2994 + */
2995 + /*
2996 + * NEEDSWORK: In the longer run, we need to get rid of this
2997 + * pattern of querying "submodule.active" before calling
2998 + * is_submodule_active(), since that function needs to find
2999 + * out the value of "submodule.active" again anyway.
3000 + */
3001 + if (!git_config_get_string("submodule.active", &val) && val) {
3002 + /*
3003 + * If the submodule being added isn't already covered by the
3004 + * current configured pathspec, set the submodule's active flag
3005 + */
3006 + if (!is_submodule_active(the_repository, add_data->sm_path)) {
3007 + key = xstrfmt("submodule.%s.active", add_data->sm_name);
3008 + git_config_set_gently(key, "true");
3009 + free(key);
3010 + }
3011 + } else {
3012 + key = xstrfmt("submodule.%s.active", add_data->sm_name);
3013 + git_config_set_gently(key, "true");
3014 + free(key);
3015 + }
3016 +}
3017 +
3018 +static int add_config(int argc, const char **argv, const char *prefix)
3019 +{
3020 + int force = 0;
3021 + struct add_data add_data = ADD_DATA_INIT;
3022 +
3023 + struct option options[] = {
3024 + OPT_STRING('b', "branch", &add_data.branch,
3025 + N_("branch"),
3026 + N_("branch of repository to store in "
3027 + "the submodule configuration")),
3028 + OPT_STRING(0, "url", &add_data.repo,
3029 + N_("string"),
3030 + N_("url to clone submodule from")),
3031 + OPT_STRING(0, "resolved-url", &add_data.realrepo,
3032 + N_("string"),
3033 + N_("url to clone the submodule from, after it has "
3034 + "been dereferenced relative to parent's url, "
3035 + "in the case where <url> is a relative url")),
3036 + OPT_STRING(0, "path", &add_data.sm_path,
3037 + N_("path"),
3038 + N_("where the new submodule will be cloned to")),
3039 + OPT_STRING(0, "name", &add_data.sm_name,
3040 + N_("string"),
3041 + N_("name of the new submodule")),
3042 + OPT__FORCE(&force, N_("allow adding an otherwise ignored submodule path"),
3043 + PARSE_OPT_NOCOMPLETE),
3044 + OPT_END()
3045 + };
3046 +
3047 + const char *const usage[] = {
3048 + N_("git submodule--helper add-config "
3049 + "[--force|-f] [--branch|-b <branch>] "
3050 + "--url <url> --resolved-url <resolved-url> "
3051 + "--path <path> --name <name>"),
3052 + NULL
3053 + };
3054 +
3055 + argc = parse_options(argc, argv, prefix, options, usage, 0);
3056 +
3057 + if (argc)
3058 + usage_with_options(usage, options);
3059 +
3060 + add_data.force = !!force;
3061 + configure_added_submodule(&add_data);
3062 +
3063 + return 0;
3064 +}
3065 +
3066 #define SUPPORT_SUPER_PREFIX (1<<0)
3067
3068 struct cmd_struct {
@@ -2948,6 +3076,7 @@ static struct cmd_struct commands[] = {
3076 {"name", module_name, 0},
3077 {"clone", module_clone, 0},
3078 {"add-clone", add_clone, 0},
3079 + {"add-config", add_config, 0},
3080 {"update-module-mode", module_update_module_mode, 0},
3081 {"update-clone", update_clone, 0},
3082 {"ensure-core-worktree", ensure_core_worktree, 0},
git-submodule.sh
+1 -27
@@ -242,33 +242,7 @@ cmd_add()
242 fi
243
244 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
245 - git config submodule."$sm_name".url "$realrepo"
246 -
247 - git add --no-warn-embedded-repo $force "$sm_path" ||
248 - die "fatal: $(eval_gettext "Failed to add submodule '\$sm_path'")"
249 -
250 - git submodule--helper config submodule."$sm_name".path "$sm_path" &&
251 - git submodule--helper config submodule."$sm_name".url "$repo" &&
252 - if test -n "$branch"
253 - then
254 - git submodule--helper config submodule."$sm_name".branch "$branch"
255 - fi &&
256 - git add --force .gitmodules ||
257 - die "fatal: $(eval_gettext "Failed to register submodule '\$sm_path'")"
258 -
259 - # NEEDSWORK: In a multi-working-tree world, this needs to be
260 - # set in the per-worktree config.
261 - if git config --get submodule.active >/dev/null
262 - then
263 - # If the submodule being adding isn't already covered by the
264 - # current configured pathspec, set the submodule's active flag
265 - if ! git submodule--helper is-active "$sm_path"
266 - then
267 - git config submodule."$sm_name".active "true"
268 - fi
269 - else
270 - git config submodule."$sm_name".active "true"
271 - fi
245 + git submodule--helper add-config ${force:+--force} ${branch:+--branch "$branch"} --url "$repo" --resolved-url "$realrepo" --path "$sm_path" --name "$sm_name"
246 }
247
248 #
submodule.c
+5
@@ -237,6 +237,11 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
237 /*
238 * Determine if a submodule has been initialized at a given 'path'
239 */
240 +/*
241 + * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
242 + * ie, the config looks like: "[submodule] active\n".
243 + * Since that is an invalid pathspec, we should inform the user.
244 + */
245 int is_submodule_active(struct repository *repo, const char *path)
246 {
247 int ret = 0;