* [PATCH 1/7] submodule--helper: run update using child process struct
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 2/7] submodule update: use die_message() Glen Choo
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason, Christian Couder,
Shourya Shukla
From: Atharva Raykar <raykar.ath@gmail.com>
We switch to using the run-command API function that takes a
'struct child process', since we are using a lot of the options. This
will also make it simple to switch over to using 'capture_command()'
when we start handling the output of the command completely in C.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bef9ab22d4..95ef113d16 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2325,47 +2325,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, str
static int run_update_command(struct update_data *ud, int subforce)
{
- struct strvec args = STRVEC_INIT;
- struct strvec child_env = STRVEC_INIT;
+ struct child_process cp = CHILD_PROCESS_INIT;
char *oid = oid_to_hex(&ud->oid);
int must_die_on_failure = 0;
- int git_cmd;
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
- git_cmd = 1;
- strvec_pushl(&args, "checkout", "-q", NULL);
+ cp.git_cmd = 1;
+ strvec_pushl(&cp.args, "checkout", "-q", NULL);
if (subforce)
- strvec_push(&args, "-f");
+ strvec_push(&cp.args, "-f");
break;
case SM_UPDATE_REBASE:
- git_cmd = 1;
- strvec_push(&args, "rebase");
+ cp.git_cmd = 1;
+ strvec_push(&cp.args, "rebase");
if (ud->quiet)
- strvec_push(&args, "--quiet");
+ strvec_push(&cp.args, "--quiet");
must_die_on_failure = 1;
break;
case SM_UPDATE_MERGE:
- git_cmd = 1;
- strvec_push(&args, "merge");
+ cp.git_cmd = 1;
+ strvec_push(&cp.args, "merge");
if (ud->quiet)
- strvec_push(&args, "--quiet");
+ strvec_push(&cp.args, "--quiet");
must_die_on_failure = 1;
break;
case SM_UPDATE_COMMAND:
- git_cmd = 0;
- strvec_push(&args, ud->update_strategy.command);
+ cp.use_shell = 1;
+ strvec_push(&cp.args, ud->update_strategy.command);
must_die_on_failure = 1;
break;
default:
BUG("unexpected update strategy type: %s",
submodule_strategy_to_string(&ud->update_strategy));
}
- strvec_push(&args, oid);
+ strvec_push(&cp.args, oid);
- prepare_submodule_repo_env(&child_env);
- if (run_command_v_opt_cd_env(args.v, git_cmd ? RUN_GIT_CMD : RUN_USING_SHELL,
- ud->sm_path, child_env.v)) {
+ cp.dir = xstrdup(ud->sm_path);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp)) {
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
printf(_("Unable to checkout '%s' in submodule path '%s'"),
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/7] submodule update: use die_message()
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
2022-03-15 21:09 ` [PATCH 1/7] submodule--helper: run update using child process struct Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 3/7] builtin/submodule--helper.c: rename option struct to "opt" Glen Choo
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason
Use die_message() to print the "fatal: " prefix instead of doing it in
git-submodule.sh and remove a now-unnecessary exit code from "git
submodule--helper run-update-procedure".
Also, since die_message() adds the newline for us, replace an invocation
of die_with_status() with printf + exit invocations that do not add a
newline, but are otherwise identical to die_with_status().
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Glen Choo <chooglen@google.com>
---
This was spun out of
https://lore.kernel.org/git/patch-v5-9.9-e8e57606ee9-20220128T125206Z-avarab@gmail.com
builtin/submodule--helper.c | 33 ++++++++++++++-------------------
git-submodule.sh | 9 +++++----
2 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 95ef113d16..af1d90af7c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2366,40 +2366,35 @@ static int run_update_command(struct update_data *ud, int subforce)
if (run_command(&cp)) {
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
- printf(_("Unable to checkout '%s' in submodule path '%s'"),
- oid, ud->displaypath);
+ die_message(_("Unable to checkout '%s' in submodule path '%s'"),
+ oid, ud->displaypath);
break;
case SM_UPDATE_REBASE:
- printf(_("Unable to rebase '%s' in submodule path '%s'"),
- oid, ud->displaypath);
+ die_message(_("Unable to rebase '%s' in submodule path '%s'"),
+ oid, ud->displaypath);
break;
case SM_UPDATE_MERGE:
- printf(_("Unable to merge '%s' in submodule path '%s'"),
- oid, ud->displaypath);
+ die_message(_("Unable to merge '%s' in submodule path '%s'"),
+ oid, ud->displaypath);
break;
case SM_UPDATE_COMMAND:
- printf(_("Execution of '%s %s' failed in submodule path '%s'"),
- ud->update_strategy.command, oid, ud->displaypath);
+ die_message(_("Execution of '%s %s' failed in submodule path '%s'"),
+ ud->update_strategy.command, oid, ud->displaypath);
break;
default:
BUG("unexpected update strategy type: %s",
submodule_strategy_to_string(&ud->update_strategy));
}
- /*
- * NEEDSWORK: We are currently printing to stdout with error
- * return so that the shell caller handles the error output
- * properly. Once we start handling the error messages within
- * C, we should use die() instead.
- */
if (must_die_on_failure)
- return 2;
- /*
- * This signifies to the caller in shell that the command
- * failed without dying
- */
+ exit(128);
+
+ /* the command failed, but update must continue */
return 1;
}
+ if (ud->quiet)
+ return 0;
+
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
printf(_("Submodule path '%s': checked out '%s'\n"),
diff --git a/git-submodule.sh b/git-submodule.sh
index aa8bdfca9d..a84143daab 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -404,7 +404,7 @@ cmd_update()
# exit codes for run-update-procedure:
# 0: update was successful, say command output
# 1: update procedure failed, but should not die
- # 2 or 128: subcommand died during execution
+ # 128: subcommand died during execution
# 3: no update procedure was run
res="$?"
case $res in
@@ -412,11 +412,12 @@ cmd_update()
say "$out"
;;
1)
- err="${err};fatal: $out"
+ err="${err};$out"
continue
;;
- 2|128)
- die_with_status $res "fatal: $out"
+ 128)
+ printf >&2 "$out"
+ exit $res
;;
esac
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/7] builtin/submodule--helper.c: rename option struct to "opt"
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
2022-03-15 21:09 ` [PATCH 1/7] submodule--helper: run update using child process struct Glen Choo
2022-03-15 21:09 ` [PATCH 2/7] submodule update: use die_message() Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 4/7] submodule--helper: teach update_data more options Glen Choo
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
In a later commit, update_clone()'s options will be stored in a struct
update_data instead of submodule_update_clone. Preemptively rename the
options struct to "opt" to shrink that commit's diff.
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Glen Choo <chooglen@google.com>
---
This is the successor to
https://lore.kernel.org/git/20220210092833.55360-7-chooglen@google.com.
In that patch, we noted that renaming both functions' variables anchors
the final diff better, but patch 6 in this series shows that
run_update_procedure() is altogether deleted, so there's no point
changing that function.
builtin/submodule--helper.c | 52 ++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index af1d90af7c..0e3cafb500 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2498,40 +2498,40 @@ static int update_clone(int argc, const char **argv, const char *prefix)
{
const char *update = NULL;
struct pathspec pathspec;
- struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
+ struct submodule_update_clone opt = SUBMODULE_UPDATE_CLONE_INIT;
struct list_objects_filter_options filter_options;
int ret;
struct option module_update_clone_options[] = {
- OPT_BOOL(0, "init", &suc.init,
+ OPT_BOOL(0, "init", &opt.init,
N_("initialize uninitialized submodules before update")),
OPT_STRING(0, "prefix", &prefix,
N_("path"),
N_("path into the working tree")),
- OPT_STRING(0, "recursive-prefix", &suc.recursive_prefix,
+ OPT_STRING(0, "recursive-prefix", &opt.recursive_prefix,
N_("path"),
N_("path into the working tree, across nested "
"submodule boundaries")),
OPT_STRING(0, "update", &update,
N_("string"),
N_("rebase, merge, checkout or none")),
- OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
+ OPT_STRING_LIST(0, "reference", &opt.references, N_("repo"),
N_("reference repository")),
- OPT_BOOL(0, "dissociate", &suc.dissociate,
+ OPT_BOOL(0, "dissociate", &opt.dissociate,
N_("use --reference only while cloning")),
- OPT_STRING(0, "depth", &suc.depth, "<depth>",
+ OPT_STRING(0, "depth", &opt.depth, "<depth>",
N_("create a shallow clone truncated to the "
"specified number of revisions")),
- OPT_INTEGER('j', "jobs", &suc.max_jobs,
+ OPT_INTEGER('j', "jobs", &opt.max_jobs,
N_("parallel jobs")),
- OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
+ OPT_BOOL(0, "recommend-shallow", &opt.recommend_shallow,
N_("whether the initial clone should follow the shallow recommendation")),
- OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
- OPT_BOOL(0, "progress", &suc.progress,
+ OPT__QUIET(&opt.quiet, N_("don't print cloning progress")),
+ OPT_BOOL(0, "progress", &opt.progress,
N_("force cloning progress")),
- OPT_BOOL(0, "require-init", &suc.require_init,
+ OPT_BOOL(0, "require-init", &opt.require_init,
N_("disallow cloning into non-empty directory")),
- OPT_BOOL(0, "single-branch", &suc.single_branch,
+ OPT_BOOL(0, "single-branch", &opt.single_branch,
N_("clone only one branch, HEAD or --branch")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
OPT_END()
@@ -2546,16 +2546,16 @@ static int update_clone(int argc, const char **argv, const char *prefix)
" [--recursive] [--[no-]single-branch] [--] [<path>...]"),
NULL
};
- suc.prefix = prefix;
+ opt.prefix = prefix;
- update_clone_config_from_gitmodules(&suc.max_jobs);
- git_config(git_update_clone_config, &suc.max_jobs);
+ update_clone_config_from_gitmodules(&opt.max_jobs);
+ git_config(git_update_clone_config, &opt.max_jobs);
memset(&filter_options, 0, sizeof(filter_options));
argc = parse_options(argc, argv, prefix, module_update_clone_options,
git_submodule_helper_usage, 0);
- if (filter_options.choice && !suc.init) {
+ if (filter_options.choice && !opt.init) {
/*
* NEEDSWORK: Don't use usage_with_options() because the
* usage string is for "git submodule update", but the
@@ -2567,25 +2567,25 @@ static int update_clone(int argc, const char **argv, const char *prefix)
usage(git_submodule_helper_usage[0]);
}
- suc.filter_options = &filter_options;
+ opt.filter_options = &filter_options;
if (update)
- if (parse_submodule_update_strategy(update, &suc.update) < 0)
+ if (parse_submodule_update_strategy(update, &opt.update) < 0)
die(_("bad value for update parameter"));
- if (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0) {
+ if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
list_objects_filter_release(&filter_options);
return 1;
}
if (pathspec.nr)
- suc.warn_if_uninitialized = 1;
+ opt.warn_if_uninitialized = 1;
- if (suc.init) {
+ if (opt.init) {
struct module_list list = MODULE_LIST_INIT;
struct init_cb info = INIT_CB_INIT;
- if (module_list_compute(argc, argv, suc.prefix,
+ if (module_list_compute(argc, argv, opt.prefix,
&pathspec, &list) < 0)
return 1;
@@ -2596,15 +2596,15 @@ static int update_clone(int argc, const char **argv, const char *prefix)
if (!argc && git_config_get_value_multi("submodule.active"))
module_list_active(&list);
- info.prefix = suc.prefix;
- info.superprefix = suc.recursive_prefix;
- if (suc.quiet)
+ info.prefix = opt.prefix;
+ info.superprefix = opt.recursive_prefix;
+ if (opt.quiet)
info.flags |= OPT_QUIET;
for_each_listed_submodule(&list, init_submodule_cb, &info);
}
- ret = update_submodules(&suc);
+ ret = update_submodules(&opt);
list_objects_filter_release(&filter_options);
return ret;
}
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/7] submodule--helper: teach update_data more options
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
` (2 preceding siblings ...)
2022-03-15 21:09 ` [PATCH 3/7] builtin/submodule--helper.c: rename option struct to "opt" Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 5/7] submodule--helper: reduce logic in run_update_procedure() Glen Choo
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason
Refactor 'struct update_data' to hold the parsed args needed by "git
submodule--helper update" and refactor "update-clone" and
"run-update-procedure" (the functions that will be combined to form
"update") to use these options.
For "run-update-procedure", 'struct update_data' already holds its args,
so only arg parsing code needs to be updated.
For "update-clone", move its args from 'struct submodule_update_clone'
into 'struct update_data', and replace them with a pointer to 'struct
update_data'. Its other members hold the submodule iteration state of
"update-clone", so those are unchanged.
Incidentally, since we reformat the designated initializers of the
affected structs, also reformat MODULE_CLONE_DATA_INIT for consistency.
Signed-off-by: Glen Choo <chooglen@google.com>
---
For reviewers who have also looked at ar/submodule-update, note that
this patch changes how the CLI args are stored. In that series, the
update_clone logic continues to read the CLI args from 'struct
submodule_update_clone' (the CLI args are copied over from 'struct
update_data'). In this series, however, all of these CLI args are moved
from 'struct submodule_update_clone' to 'struct update_data'.
This patch also incorporates some reformatting from
https://lore.kernel.org/git/patch-v5-5.9-fa815e37f9f-20220128T125206Z-avarab@gmail.com,
but that patch is now largely obsolete because the initializers are
quite different now.
builtin/submodule--helper.c | 150 ++++++++++++++++++------------------
git-submodule.sh | 2 +-
2 files changed, 76 insertions(+), 76 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0e3cafb500..04874ce111 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1643,7 +1643,10 @@ struct module_clone_data {
unsigned int require_init: 1;
int single_branch;
};
-#define MODULE_CLONE_DATA_INIT { .reference = STRING_LIST_INIT_NODUP, .single_branch = -1 }
+#define MODULE_CLONE_DATA_INIT { \
+ .reference = STRING_LIST_INIT_NODUP, \
+ .single_branch = -1, \
+}
struct submodule_alternate_setup {
const char *submodule_name;
@@ -1967,26 +1970,11 @@ struct update_clone_data {
};
struct submodule_update_clone {
- /* index into 'list', the list of submodules to look into for cloning */
+ /* index into 'update_data.list', the list of submodules to look into for cloning */
int current;
- struct module_list list;
- unsigned warn_if_uninitialized : 1;
-
- /* update parameter passed via commandline */
- struct submodule_update_strategy update;
/* configuration parameters which are passed on to the children */
- int progress;
- int quiet;
- int recommend_shallow;
- struct string_list references;
- int dissociate;
- unsigned require_init;
- const char *depth;
- const char *recursive_prefix;
- const char *prefix;
- int single_branch;
- struct list_objects_filter_options *filter_options;
+ struct update_data *update_data;
/* to be consumed by git-submodule.sh */
struct update_clone_data *update_clone;
@@ -1998,34 +1986,45 @@ struct submodule_update_clone {
/* failed clones to be retried again */
const struct cache_entry **failed_clones;
int failed_clones_nr, failed_clones_alloc;
-
- int max_jobs;
- unsigned int init;
};
-#define SUBMODULE_UPDATE_CLONE_INIT { \
- .list = MODULE_LIST_INIT, \
- .update = SUBMODULE_UPDATE_STRATEGY_INIT, \
- .recommend_shallow = -1, \
- .references = STRING_LIST_INIT_DUP, \
- .single_branch = -1, \
- .max_jobs = 1, \
-}
+#define SUBMODULE_UPDATE_CLONE_INIT { 0 }
struct update_data {
+ const char *prefix;
const char *recursive_prefix;
const char *sm_path;
const char *displaypath;
+ const char *update_default;
struct object_id oid;
struct object_id suboid;
+ struct string_list references;
struct submodule_update_strategy update_strategy;
+ struct list_objects_filter_options *filter_options;
+ struct module_list list;
int depth;
+ int max_jobs;
+ int single_branch;
+ int recommend_shallow;
+ unsigned int require_init;
unsigned int force;
unsigned int quiet;
unsigned int nofetch;
unsigned int just_cloned;
unsigned int remote;
+ unsigned int progress;
+ unsigned int dissociate;
+ unsigned int init;
+ unsigned int warn_if_uninitialized;
};
-#define UPDATE_DATA_INIT { .update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT }
+#define UPDATE_DATA_INIT { \
+ .update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \
+ .list = MODULE_LIST_INIT, \
+ .recommend_shallow = -1, \
+ .references = STRING_LIST_INIT_DUP, \
+ .single_branch = -1, \
+ .max_jobs = 1, \
+ .warn_if_uninitialized = 1, \
+}
static void next_submodule_warn_missing(struct submodule_update_clone *suc,
struct strbuf *out, const char *displaypath)
@@ -2034,7 +2033,7 @@ static void next_submodule_warn_missing(struct submodule_update_clone *suc,
* Only mention uninitialized submodules when their
* paths have been specified.
*/
- if (suc->warn_if_uninitialized) {
+ if (suc->update_data->warn_if_uninitialized) {
strbuf_addf(out,
_("Submodule path '%s' not initialized"),
displaypath);
@@ -2066,8 +2065,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
int need_free_url = 0;
if (ce_stage(ce)) {
- if (suc->recursive_prefix)
- strbuf_addf(&sb, "%s/%s", suc->recursive_prefix, ce->name);
+ if (suc->update_data->recursive_prefix)
+ strbuf_addf(&sb, "%s/%s", suc->update_data->recursive_prefix, ce->name);
else
strbuf_addstr(&sb, ce->name);
strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
@@ -2077,8 +2076,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
sub = submodule_from_path(the_repository, null_oid(), ce->name);
- if (suc->recursive_prefix)
- displaypath = relative_path(suc->recursive_prefix,
+ if (suc->update_data->recursive_prefix)
+ displaypath = relative_path(suc->update_data->recursive_prefix,
ce->name, &displaypath_sb);
else
displaypath = ce->name;
@@ -2096,8 +2095,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
}
free(key);
- if (suc->update.type == SM_UPDATE_NONE
- || (suc->update.type == SM_UPDATE_UNSPECIFIED
+ if (suc->update_data->update_strategy.type == SM_UPDATE_NONE
+ || (suc->update_data->update_strategy.type == SM_UPDATE_UNSPECIFIED
&& update_type == SM_UPDATE_NONE)) {
strbuf_addf(out, _("Skipping submodule '%s'"), displaypath);
strbuf_addch(out, '\n');
@@ -2141,33 +2140,33 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
child->err = -1;
strvec_push(&child->args, "submodule--helper");
strvec_push(&child->args, "clone");
- if (suc->progress)
+ if (suc->update_data->progress)
strvec_push(&child->args, "--progress");
- if (suc->quiet)
+ if (suc->update_data->quiet)
strvec_push(&child->args, "--quiet");
- if (suc->prefix)
- strvec_pushl(&child->args, "--prefix", suc->prefix, NULL);
- if (suc->recommend_shallow && sub->recommend_shallow == 1)
+ if (suc->update_data->prefix)
+ strvec_pushl(&child->args, "--prefix", suc->update_data->prefix, NULL);
+ if (suc->update_data->recommend_shallow && sub->recommend_shallow == 1)
strvec_push(&child->args, "--depth=1");
- if (suc->filter_options && suc->filter_options->choice)
+ else if (suc->update_data->depth)
+ strvec_pushf(&child->args, "--depth=%d", suc->update_data->depth);
+ if (suc->update_data->filter_options && suc->update_data->filter_options->choice)
strvec_pushf(&child->args, "--filter=%s",
- expand_list_objects_filter_spec(suc->filter_options));
- if (suc->require_init)
+ expand_list_objects_filter_spec(suc->update_data->filter_options));
+ if (suc->update_data->require_init)
strvec_push(&child->args, "--require-init");
strvec_pushl(&child->args, "--path", sub->path, NULL);
strvec_pushl(&child->args, "--name", sub->name, NULL);
strvec_pushl(&child->args, "--url", url, NULL);
- if (suc->references.nr) {
+ if (suc->update_data->references.nr) {
struct string_list_item *item;
- for_each_string_list_item(item, &suc->references)
+ for_each_string_list_item(item, &suc->update_data->references)
strvec_pushl(&child->args, "--reference", item->string, NULL);
}
- if (suc->dissociate)
+ if (suc->update_data->dissociate)
strvec_push(&child->args, "--dissociate");
- if (suc->depth)
- strvec_push(&child->args, suc->depth);
- if (suc->single_branch >= 0)
- strvec_push(&child->args, suc->single_branch ?
+ if (suc->update_data->single_branch >= 0)
+ strvec_push(&child->args, suc->update_data->single_branch ?
"--single-branch" :
"--no-single-branch");
@@ -2189,8 +2188,8 @@ static int update_clone_get_next_task(struct child_process *child,
const struct cache_entry *ce;
int index;
- for (; suc->current < suc->list.nr; suc->current++) {
- ce = suc->list.entries[suc->current];
+ for (; suc->current < suc->update_data->list.nr; suc->current++) {
+ ce = suc->update_data->list.entries[suc->current];
if (prepare_to_clone_next_submodule(ce, child, suc, err)) {
int *p = xmalloc(sizeof(*p));
*p = suc->current;
@@ -2205,7 +2204,7 @@ static int update_clone_get_next_task(struct child_process *child,
* stragglers again, which we can imagine as an extension of the
* entry list.
*/
- index = suc->current - suc->list.nr;
+ index = suc->current - suc->update_data->list.nr;
if (index < suc->failed_clones_nr) {
int *p;
ce = suc->failed_clones[index];
@@ -2250,8 +2249,8 @@ static int update_clone_task_finished(int result,
if (!result)
return 0;
- if (idx < suc->list.nr) {
- ce = suc->list.entries[idx];
+ if (idx < suc->update_data->list.nr) {
+ ce = suc->update_data->list.entries[idx];
strbuf_addf(err, _("Failed to clone '%s'. Retry scheduled"),
ce->name);
strbuf_addch(err, '\n');
@@ -2261,7 +2260,7 @@ static int update_clone_task_finished(int result,
suc->failed_clones[suc->failed_clones_nr++] = ce;
return 0;
} else {
- idx -= suc->list.nr;
+ idx -= suc->update_data->list.nr;
ce = suc->failed_clones[idx];
strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
ce->name);
@@ -2468,13 +2467,15 @@ static void update_submodule(struct update_clone_data *ucd)
ucd->sub->path);
}
-static int update_submodules(struct submodule_update_clone *suc)
+static int update_submodules(struct update_data *update_data)
{
int i;
+ struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
- run_processes_parallel_tr2(suc->max_jobs, update_clone_get_next_task,
+ suc.update_data = update_data;
+ run_processes_parallel_tr2(suc.update_data->max_jobs, update_clone_get_next_task,
update_clone_start_failure,
- update_clone_task_finished, suc, "submodule",
+ update_clone_task_finished, &suc, "submodule",
"parallel/update");
/*
@@ -2485,41 +2486,40 @@ static int update_submodules(struct submodule_update_clone *suc)
* checkout involve more straightforward sequential I/O.
* - the listener can avoid doing any work if fetching failed.
*/
- if (suc->quickstop)
+ if (suc.quickstop)
return 1;
- for (i = 0; i < suc->update_clone_nr; i++)
- update_submodule(&suc->update_clone[i]);
+ for (i = 0; i < suc.update_clone_nr; i++)
+ update_submodule(&suc.update_clone[i]);
return 0;
}
static int update_clone(int argc, const char **argv, const char *prefix)
{
- const char *update = NULL;
struct pathspec pathspec;
- struct submodule_update_clone opt = SUBMODULE_UPDATE_CLONE_INIT;
+ struct update_data opt = UPDATE_DATA_INIT;
struct list_objects_filter_options filter_options;
int ret;
struct option module_update_clone_options[] = {
OPT_BOOL(0, "init", &opt.init,
N_("initialize uninitialized submodules before update")),
- OPT_STRING(0, "prefix", &prefix,
+ OPT_STRING(0, "prefix", &opt.prefix,
N_("path"),
N_("path into the working tree")),
OPT_STRING(0, "recursive-prefix", &opt.recursive_prefix,
N_("path"),
N_("path into the working tree, across nested "
"submodule boundaries")),
- OPT_STRING(0, "update", &update,
+ OPT_STRING(0, "update", &opt.update_default,
N_("string"),
N_("rebase, merge, checkout or none")),
OPT_STRING_LIST(0, "reference", &opt.references, N_("repo"),
N_("reference repository")),
OPT_BOOL(0, "dissociate", &opt.dissociate,
N_("use --reference only while cloning")),
- OPT_STRING(0, "depth", &opt.depth, "<depth>",
+ OPT_INTEGER(0, "depth", &opt.depth,
N_("create a shallow clone truncated to the "
"specified number of revisions")),
OPT_INTEGER('j', "jobs", &opt.max_jobs,
@@ -2546,7 +2546,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)
" [--recursive] [--[no-]single-branch] [--] [<path>...]"),
NULL
};
- opt.prefix = prefix;
update_clone_config_from_gitmodules(&opt.max_jobs);
git_config(git_update_clone_config, &opt.max_jobs);
@@ -2569,8 +2568,9 @@ static int update_clone(int argc, const char **argv, const char *prefix)
opt.filter_options = &filter_options;
- if (update)
- if (parse_submodule_update_strategy(update, &opt.update) < 0)
+ if (opt.update_default)
+ if (parse_submodule_update_strategy(opt.update_default,
+ &opt.update_strategy) < 0)
die(_("bad value for update parameter"));
if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
@@ -2611,7 +2611,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)
static int run_update_procedure(int argc, const char **argv, const char *prefix)
{
- char *prefixed_path, *update = NULL;
+ char *prefixed_path;
struct update_data update_data = UPDATE_DATA_INIT;
struct option options[] = {
@@ -2627,7 +2627,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)
OPT_STRING(0, "prefix", &prefix,
N_("path"),
N_("path into the working tree")),
- OPT_STRING(0, "update", &update,
+ OPT_STRING(0, "update", &update_data.update_default,
N_("string"),
N_("rebase, merge, checkout or none")),
OPT_STRING(0, "recursive-prefix", &update_data.recursive_prefix, N_("path"),
@@ -2661,7 +2661,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)
update_data.displaypath = get_submodule_displaypath(prefixed_path, prefix);
determine_submodule_update_strategy(the_repository, update_data.just_cloned,
- update_data.sm_path, update,
+ update_data.sm_path, update_data.update_default,
&update_data.update_strategy);
free(prefixed_path);
diff --git a/git-submodule.sh b/git-submodule.sh
index a84143daab..b63a2aefa7 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -366,7 +366,7 @@ cmd_update()
${update:+--update "$update"} \
${reference:+"$reference"} \
${dissociate:+"--dissociate"} \
- ${depth:+--depth "$depth"} \
+ ${depth:+"$depth"} \
${require_init:+--require-init} \
$single_branch \
$recommend_shallow \
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/7] submodule--helper: reduce logic in run_update_procedure()
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
` (3 preceding siblings ...)
2022-03-15 21:09 ` [PATCH 4/7] submodule--helper: teach update_data more options Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 6/7] submodule: move core cmd_update() logic to C Glen Choo
2022-03-15 21:09 ` [PATCH 7/7] submodule--helper: remove forward declaration Glen Choo
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason
A later commit will combine the "update-clone" and
"run-update-procedure" commands, so run_update_procedure() will be
removed. Prepare for this by moving as much logic as possible out of
run_update_procedure() and into update_submodule2().
Signed-off-by: Glen Choo <chooglen@google.com>
---
builtin/submodule--helper.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 04874ce111..85f37c4458 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2611,7 +2611,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)
static int run_update_procedure(int argc, const char **argv, const char *prefix)
{
- char *prefixed_path;
struct update_data update_data = UPDATE_DATA_INIT;
struct option options[] = {
@@ -2653,18 +2652,6 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)
update_data.sm_path = argv[0];
- if (update_data.recursive_prefix)
- prefixed_path = xstrfmt("%s%s", update_data.recursive_prefix, update_data.sm_path);
- else
- prefixed_path = xstrdup(update_data.sm_path);
-
- update_data.displaypath = get_submodule_displaypath(prefixed_path, prefix);
-
- determine_submodule_update_strategy(the_repository, update_data.just_cloned,
- update_data.sm_path, update_data.update_default,
- &update_data.update_strategy);
-
- free(prefixed_path);
return update_submodule2(&update_data);
}
@@ -3044,7 +3031,24 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)
/* NEEDSWORK: this is a temporary name until we delete update_submodule() */
static int update_submodule2(struct update_data *update_data)
{
+ char *prefixed_path;
+
ensure_core_worktree(update_data->sm_path);
+
+ if (update_data->recursive_prefix)
+ prefixed_path = xstrfmt("%s%s", update_data->recursive_prefix,
+ update_data->sm_path);
+ else
+ prefixed_path = xstrdup(update_data->sm_path);
+
+ update_data->displaypath = get_submodule_displaypath(prefixed_path,
+ update_data->prefix);
+ free(prefixed_path);
+
+ determine_submodule_update_strategy(the_repository, update_data->just_cloned,
+ update_data->sm_path, update_data->update_default,
+ &update_data->update_strategy);
+
if (update_data->just_cloned)
oidcpy(&update_data->suboid, null_oid());
else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/7] submodule: move core cmd_update() logic to C
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
` (4 preceding siblings ...)
2022-03-15 21:09 ` [PATCH 5/7] submodule--helper: reduce logic in run_update_procedure() Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
2022-03-15 21:09 ` [PATCH 7/7] submodule--helper: remove forward declaration Glen Choo
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason, Christian Couder,
Shourya Shukla
From: Atharva Raykar <raykar.ath@gmail.com>
This patch completes the conversion past the flag parsing of
`submodule update` by introducing a helper subcommand called
`submodule--helper update`. The behaviour of `submodule update` should
remain the same after this patch.
Prior to this patch, `submodule update` was implemented by piping the
output of `update-clone` (which clones all missing submodules, then
prints relevant information for all submodules) into
`run-update-procedure` (which reads the information and updates the
submodule tree).
With `submodule--helper update`, we iterate over the submodules and
update the submodule tree in the same process. This reuses most of
existing code structure, except that `update_submodule()` now updates
the submodule tree (instead of printing submodule information to be
consumed by another process).
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Glen Choo <chooglen@google.com>
---
For the curious, the bug that was in ar/submodule-update
(<xmqqr18770pc.fsf@gitster.g>), comes from a typo that caused
`--single-branch` to be set incorrectly in the recursive `update`:
if (update_data->single_branch >= 0)
strvec_push(args, "--single-branch");
This has been corrected to:
if (update_data->single_branch >= 0)
strvec_push(args, update_data->single_branch ?
"--single-branch" :
"--no-single-branch");
(-1 means "--[no-]single-branch" was not specified)
t3207 only failed because of a second-order effect - the test sets up
tracking based on the remote's "fetch" refspec, but `--single-branch`
caused the wrong refspec to be written. This might suggest that more
direct tests of recursive `update` are needed.
builtin/submodule--helper.c | 229 +++++++++++++++++++-----------------
git-submodule.sh | 104 ++--------------
2 files changed, 131 insertions(+), 202 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 85f37c4458..842d9ecaa8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1976,7 +1976,7 @@ struct submodule_update_clone {
/* configuration parameters which are passed on to the children */
struct update_data *update_data;
- /* to be consumed by git-submodule.sh */
+ /* to be consumed by update_submodule() */
struct update_clone_data *update_clone;
int update_clone_nr; int update_clone_alloc;
@@ -1992,10 +1992,8 @@ struct submodule_update_clone {
struct update_data {
const char *prefix;
const char *recursive_prefix;
- const char *sm_path;
const char *displaypath;
const char *update_default;
- struct object_id oid;
struct object_id suboid;
struct string_list references;
struct submodule_update_strategy update_strategy;
@@ -2009,12 +2007,17 @@ struct update_data {
unsigned int force;
unsigned int quiet;
unsigned int nofetch;
- unsigned int just_cloned;
unsigned int remote;
unsigned int progress;
unsigned int dissociate;
unsigned int init;
unsigned int warn_if_uninitialized;
+ unsigned int recursive;
+
+ /* copied over from update_clone_data */
+ struct object_id oid;
+ unsigned int just_cloned;
+ const char *sm_path;
};
#define UPDATE_DATA_INIT { \
.update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \
@@ -2419,7 +2422,7 @@ static int run_update_command(struct update_data *ud, int subforce)
return 0;
}
-static int do_run_update_procedure(struct update_data *ud)
+static int run_update_procedure(struct update_data *ud)
{
int subforce = is_null_oid(&ud->suboid) || ud->force;
@@ -2449,27 +2452,10 @@ static int do_run_update_procedure(struct update_data *ud)
return run_update_command(ud, subforce);
}
-/*
- * NEEDSWORK: As we convert "git submodule update" to C,
- * update_submodule2() will invoke more and more functions, making it
- * difficult to preserve the function ordering without forward
- * declarations.
- *
- * When the conversion is complete, this forward declaration will be
- * unnecessary and should be removed.
- */
-static int update_submodule2(struct update_data *update_data);
-static void update_submodule(struct update_clone_data *ucd)
-{
- fprintf(stdout, "dummy %s %d\t%s\n",
- oid_to_hex(&ucd->oid),
- ucd->just_cloned,
- ucd->sub->path);
-}
-
+static int update_submodule(struct update_data *update_data);
static int update_submodules(struct update_data *update_data)
{
- int i;
+ int i, res = 0;
struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
suc.update_data = update_data;
@@ -2486,25 +2472,44 @@ static int update_submodules(struct update_data *update_data)
* checkout involve more straightforward sequential I/O.
* - the listener can avoid doing any work if fetching failed.
*/
- if (suc.quickstop)
- return 1;
+ if (suc.quickstop) {
+ res = 1;
+ goto cleanup;
+ }
- for (i = 0; i < suc.update_clone_nr; i++)
- update_submodule(&suc.update_clone[i]);
+ for (i = 0; i < suc.update_clone_nr; i++) {
+ struct update_clone_data ucd = suc.update_clone[i];
- return 0;
+ oidcpy(&update_data->oid, &ucd.oid);
+ update_data->just_cloned = ucd.just_cloned;
+ update_data->sm_path = ucd.sub->path;
+
+ if (update_submodule(update_data))
+ res = 1;
+ }
+
+cleanup:
+ string_list_clear(&update_data->references, 0);
+ return res;
}
-static int update_clone(int argc, const char **argv, const char *prefix)
+static int module_update(int argc, const char **argv, const char *prefix)
{
struct pathspec pathspec;
struct update_data opt = UPDATE_DATA_INIT;
struct list_objects_filter_options filter_options;
int ret;
- struct option module_update_clone_options[] = {
+ struct option module_update_options[] = {
+ OPT__FORCE(&opt.force, N_("force checkout updates"), 0),
OPT_BOOL(0, "init", &opt.init,
N_("initialize uninitialized submodules before update")),
+ OPT_BOOL(0, "remote", &opt.remote,
+ N_("use SHA-1 of submodule's remote tracking branch")),
+ OPT_BOOL(0, "recursive", &opt.recursive,
+ N_("traverse submodules recursively")),
+ OPT_BOOL('N', "no-fetch", &opt.nofetch,
+ N_("don't fetch new objects from the remote site")),
OPT_STRING(0, "prefix", &opt.prefix,
N_("path"),
N_("path into the working tree")),
@@ -2551,19 +2556,12 @@ static int update_clone(int argc, const char **argv, const char *prefix)
git_config(git_update_clone_config, &opt.max_jobs);
memset(&filter_options, 0, sizeof(filter_options));
- argc = parse_options(argc, argv, prefix, module_update_clone_options,
+ argc = parse_options(argc, argv, prefix, module_update_options,
git_submodule_helper_usage, 0);
if (filter_options.choice && !opt.init) {
- /*
- * NEEDSWORK: Don't use usage_with_options() because the
- * usage string is for "git submodule update", but the
- * options are for "git submodule--helper update-clone".
- *
- * This will no longer be an issue when "update-clone"
- * is replaced by "git submodule--helper update".
- */
- usage(git_submodule_helper_usage[0]);
+ usage_with_options(git_submodule_helper_usage,
+ module_update_options);
}
opt.filter_options = &filter_options;
@@ -2609,63 +2607,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)
return ret;
}
-static int run_update_procedure(int argc, const char **argv, const char *prefix)
-{
- struct update_data update_data = UPDATE_DATA_INIT;
-
- struct option options[] = {
- OPT__QUIET(&update_data.quiet,
- N_("suppress output for update by rebase or merge")),
- OPT__FORCE(&update_data.force, N_("force checkout updates"),
- 0),
- OPT_BOOL('N', "no-fetch", &update_data.nofetch,
- N_("don't fetch new objects from the remote site")),
- OPT_BOOL(0, "just-cloned", &update_data.just_cloned,
- N_("overrides update mode in case the repository is a fresh clone")),
- OPT_INTEGER(0, "depth", &update_data.depth, N_("depth for shallow fetch")),
- OPT_STRING(0, "prefix", &prefix,
- N_("path"),
- N_("path into the working tree")),
- OPT_STRING(0, "update", &update_data.update_default,
- N_("string"),
- N_("rebase, merge, checkout or none")),
- OPT_STRING(0, "recursive-prefix", &update_data.recursive_prefix, N_("path"),
- N_("path into the working tree, across nested "
- "submodule boundaries")),
- OPT_CALLBACK_F(0, "oid", &update_data.oid, N_("sha1"),
- N_("SHA1 expected by superproject"), PARSE_OPT_NONEG,
- parse_opt_object_id),
- OPT_BOOL(0, "remote", &update_data.remote,
- N_("use SHA-1 of submodule's remote tracking branch")),
- OPT_END()
- };
-
- const char *const usage[] = {
- N_("git submodule--helper run-update-procedure [<options>] <path>"),
- NULL
- };
-
- argc = parse_options(argc, argv, prefix, options, usage, 0);
-
- if (argc != 1)
- usage_with_options(usage, options);
-
- update_data.sm_path = argv[0];
-
- return update_submodule2(&update_data);
-}
-
-static int resolve_relative_path(int argc, const char **argv, const char *prefix)
-{
- struct strbuf sb = STRBUF_INIT;
- if (argc != 3)
- die("submodule--helper relative-path takes exactly 2 arguments, got %d", argc);
-
- printf("%s", relative_path(argv[1], argv[2], &sb));
- strbuf_release(&sb);
- return 0;
-}
-
static const char *remote_submodule_branch(const char *path)
{
const struct submodule *sub;
@@ -3028,8 +2969,53 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)
return 0;
}
-/* NEEDSWORK: this is a temporary name until we delete update_submodule() */
-static int update_submodule2(struct update_data *update_data)
+static void update_data_to_args(struct update_data *update_data, struct strvec *args)
+{
+ strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
+ strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
+ if (update_data->recursive_prefix)
+ strvec_pushl(args, "--recursive-prefix",
+ update_data->recursive_prefix, NULL);
+ if (update_data->quiet)
+ strvec_push(args, "--quiet");
+ if (update_data->force)
+ strvec_push(args, "--force");
+ if (update_data->init)
+ strvec_push(args, "--init");
+ if (update_data->remote)
+ strvec_push(args, "--remote");
+ if (update_data->nofetch)
+ strvec_push(args, "--no-fetch");
+ if (update_data->dissociate)
+ strvec_push(args, "--dissociate");
+ if (update_data->progress)
+ strvec_push(args, "--progress");
+ if (update_data->require_init)
+ strvec_push(args, "--require-init");
+ if (update_data->depth)
+ strvec_pushf(args, "--depth=%d", update_data->depth);
+ if (update_data->update_default)
+ strvec_pushl(args, "--update", update_data->update_default, NULL);
+ if (update_data->references.nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, &update_data->references)
+ strvec_pushl(args, "--reference", item->string, NULL);
+ }
+ if (update_data->filter_options && update_data->filter_options->choice)
+ strvec_pushf(args, "--filter=%s",
+ expand_list_objects_filter_spec(
+ update_data->filter_options));
+ if (update_data->recommend_shallow == 0)
+ strvec_push(args, "--no-recommend-shallow");
+ else if (update_data->recommend_shallow == 1)
+ strvec_push(args, "--recommend-shallow");
+ if (update_data->single_branch >= 0)
+ strvec_push(args, update_data->single_branch ?
+ "--single-branch" :
+ "--no-single-branch");
+}
+
+static int update_submodule(struct update_data *update_data)
{
char *prefixed_path;
@@ -3075,9 +3061,44 @@ static int update_submodule2(struct update_data *update_data)
}
if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)
- return do_run_update_procedure(update_data);
+ if (run_update_procedure(update_data))
+ return 1;
+
+ if (update_data->recursive) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct update_data next = *update_data;
+ int res;
+
+ if (update_data->recursive_prefix)
+ prefixed_path = xstrfmt("%s%s/", update_data->recursive_prefix,
+ update_data->sm_path);
+ else
+ prefixed_path = xstrfmt("%s/", update_data->sm_path);
+
+ next.recursive_prefix = get_submodule_displaypath(prefixed_path,
+ update_data->prefix);
+ next.prefix = NULL;
+ oidcpy(&next.oid, null_oid());
+ oidcpy(&next.suboid, null_oid());
+
+ cp.dir = update_data->sm_path;
+ cp.git_cmd = 1;
+ prepare_submodule_repo_env(&cp.env_array);
+ update_data_to_args(&next, &cp.args);
- return 3;
+ /* die() if child process die()'d */
+ res = run_command(&cp);
+ if (!res)
+ return 0;
+ die_message(_("Failed to recurse into submodule path '%s'"),
+ update_data->displaypath);
+ if (res == 128)
+ exit(res);
+ else if (res)
+ return 1;
+ }
+
+ return 0;
}
struct add_data {
@@ -3465,9 +3486,7 @@ static struct cmd_struct commands[] = {
{"name", module_name, 0},
{"clone", module_clone, 0},
{"add", module_add, SUPPORT_SUPER_PREFIX},
- {"update-clone", update_clone, 0},
- {"run-update-procedure", run_update_procedure, 0},
- {"relative-path", resolve_relative_path, 0},
+ {"update", module_update, 0},
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"foreach", module_foreach, SUPPORT_SUPER_PREFIX},
{"init", module_init, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index b63a2aefa7..fd0b4a2c94 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -51,14 +51,6 @@ jobs=
recommend_shallow=
filter=
-die_if_unmatched ()
-{
- if test "$1" = "#unmatched"
- then
- exit ${2:-1}
- fi
-}
-
isnumber()
{
n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
@@ -356,11 +348,14 @@ cmd_update()
shift
done
- {
- git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update-clone \
+ git ${wt_prefix:+-C "$wt_prefix"} submodule--helper update \
${GIT_QUIET:+--quiet} \
+ ${force:+--force} \
${progress:+"--progress"} \
+ ${remote:+--remote} \
+ ${recursive:+--recursive} \
${init:+--init} \
+ ${nofetch:+--no-fetch} \
${wt_prefix:+--prefix "$wt_prefix"} \
${prefix:+--recursive-prefix "$prefix"} \
${update:+--update "$update"} \
@@ -368,98 +363,13 @@ cmd_update()
${dissociate:+"--dissociate"} \
${depth:+"$depth"} \
${require_init:+--require-init} \
+ ${dissociate:+"--dissociate"} \
$single_branch \
$recommend_shallow \
$jobs \
$filter \
-- \
- "$@" || echo "#unmatched" $?
- } | {
- err=
- while read -r quickabort sha1 just_cloned sm_path
- do
- die_if_unmatched "$quickabort" "$sha1"
-
- displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
-
- if test $just_cloned -eq 0
- then
- just_cloned=
- fi
-
- out=$(git submodule--helper run-update-procedure \
- ${wt_prefix:+--prefix "$wt_prefix"} \
- ${GIT_QUIET:+--quiet} \
- ${force:+--force} \
- ${just_cloned:+--just-cloned} \
- ${nofetch:+--no-fetch} \
- ${depth:+"$depth"} \
- ${update:+--update "$update"} \
- ${prefix:+--recursive-prefix "$prefix"} \
- ${sha1:+--oid "$sha1"} \
- ${remote:+--remote} \
- "--" \
- "$sm_path")
-
- # exit codes for run-update-procedure:
- # 0: update was successful, say command output
- # 1: update procedure failed, but should not die
- # 128: subcommand died during execution
- # 3: no update procedure was run
- res="$?"
- case $res in
- 0)
- say "$out"
- ;;
- 1)
- err="${err};$out"
- continue
- ;;
- 128)
- printf >&2 "$out"
- exit $res
- ;;
- esac
-
- if test -n "$recursive"
- then
- (
- prefix=$(git submodule--helper relative-path "$prefix$sm_path/" "$wt_prefix")
- wt_prefix=
- sanitize_submodule_env
- cd "$sm_path" &&
- eval cmd_update
- )
- res=$?
- if test $res -gt 0
- then
- die_msg="fatal: $(eval_gettext "Failed to recurse into submodule path '\$displaypath'")"
- if test $res -ne 2
- then
- err="${err};$die_msg"
- continue
- else
- die_with_status $res "$die_msg"
- fi
- fi
- fi
- done
-
- if test -n "$err"
- then
- OIFS=$IFS
- IFS=';'
- for e in $err
- do
- if test -n "$e"
- then
- echo >&2 "$e"
- fi
- done
- IFS=$OIFS
- exit 1
- fi
- }
+ "$@"
}
#
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 7/7] submodule--helper: remove forward declaration
2022-03-15 21:09 [PATCH 0/7] Finish converting "submodule update" to C Glen Choo
` (5 preceding siblings ...)
2022-03-15 21:09 ` [PATCH 6/7] submodule: move core cmd_update() logic to C Glen Choo
@ 2022-03-15 21:09 ` Glen Choo
6 siblings, 0 replies; 8+ messages in thread
From: Glen Choo @ 2022-03-15 21:09 UTC (permalink / raw)
To: git
Cc: Glen Choo, Junio C Hamano, Atharva Raykar, Emily Shaffer,
Ævar Arnfjörð Bjarmason
Rearrange functions so that submodule_update() no longer needs to be
forward declared.
Signed-off-by: Glen Choo <chooglen@google.com>
---
builtin/submodule--helper.c | 393 ++++++++++++++++++------------------
1 file changed, 196 insertions(+), 197 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 842d9ecaa8..11289f0ae5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2452,7 +2452,202 @@ static int run_update_procedure(struct update_data *ud)
return run_update_command(ud, subforce);
}
-static int update_submodule(struct update_data *update_data);
+static const char *remote_submodule_branch(const char *path)
+{
+ const struct submodule *sub;
+ const char *branch = NULL;
+ char *key;
+
+ sub = submodule_from_path(the_repository, null_oid(), path);
+ if (!sub)
+ return NULL;
+
+ key = xstrfmt("submodule.%s.branch", sub->name);
+ if (repo_config_get_string_tmp(the_repository, key, &branch))
+ branch = sub->branch;
+ free(key);
+
+ if (!branch)
+ return "HEAD";
+
+ if (!strcmp(branch, ".")) {
+ const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+
+ if (!refname)
+ die(_("No such ref: %s"), "HEAD");
+
+ /* detached HEAD */
+ if (!strcmp(refname, "HEAD"))
+ die(_("Submodule (%s) branch configured to inherit "
+ "branch from superproject, but the superproject "
+ "is not on any branch"), sub->name);
+
+ if (!skip_prefix(refname, "refs/heads/", &refname))
+ die(_("Expecting a full ref name, got %s"), refname);
+ return refname;
+ }
+
+ return branch;
+}
+
+static void ensure_core_worktree(const char *path)
+{
+ const char *cw;
+ struct repository subrepo;
+
+ if (repo_submodule_init(&subrepo, the_repository, path, null_oid()))
+ die(_("could not get a repository handle for submodule '%s'"), path);
+
+ if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) {
+ char *cfg_file, *abs_path;
+ const char *rel_path;
+ struct strbuf sb = STRBUF_INIT;
+
+ cfg_file = repo_git_path(&subrepo, "config");
+
+ abs_path = absolute_pathdup(path);
+ rel_path = relative_path(abs_path, subrepo.gitdir, &sb);
+
+ git_config_set_in_file(cfg_file, "core.worktree", rel_path);
+
+ free(cfg_file);
+ free(abs_path);
+ strbuf_release(&sb);
+ }
+}
+
+static void update_data_to_args(struct update_data *update_data, struct strvec *args)
+{
+ strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
+ strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
+ if (update_data->recursive_prefix)
+ strvec_pushl(args, "--recursive-prefix",
+ update_data->recursive_prefix, NULL);
+ if (update_data->quiet)
+ strvec_push(args, "--quiet");
+ if (update_data->force)
+ strvec_push(args, "--force");
+ if (update_data->init)
+ strvec_push(args, "--init");
+ if (update_data->remote)
+ strvec_push(args, "--remote");
+ if (update_data->nofetch)
+ strvec_push(args, "--no-fetch");
+ if (update_data->dissociate)
+ strvec_push(args, "--dissociate");
+ if (update_data->progress)
+ strvec_push(args, "--progress");
+ if (update_data->require_init)
+ strvec_push(args, "--require-init");
+ if (update_data->depth)
+ strvec_pushf(args, "--depth=%d", update_data->depth);
+ if (update_data->update_default)
+ strvec_pushl(args, "--update", update_data->update_default, NULL);
+ if (update_data->references.nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, &update_data->references)
+ strvec_pushl(args, "--reference", item->string, NULL);
+ }
+ if (update_data->filter_options && update_data->filter_options->choice)
+ strvec_pushf(args, "--filter=%s",
+ expand_list_objects_filter_spec(
+ update_data->filter_options));
+ if (update_data->recommend_shallow == 0)
+ strvec_push(args, "--no-recommend-shallow");
+ else if (update_data->recommend_shallow == 1)
+ strvec_push(args, "--recommend-shallow");
+ if (update_data->single_branch >= 0)
+ strvec_push(args, update_data->single_branch ?
+ "--single-branch" :
+ "--no-single-branch");
+}
+
+static int update_submodule(struct update_data *update_data)
+{
+ char *prefixed_path;
+
+ ensure_core_worktree(update_data->sm_path);
+
+ if (update_data->recursive_prefix)
+ prefixed_path = xstrfmt("%s%s", update_data->recursive_prefix,
+ update_data->sm_path);
+ else
+ prefixed_path = xstrdup(update_data->sm_path);
+
+ update_data->displaypath = get_submodule_displaypath(prefixed_path,
+ update_data->prefix);
+ free(prefixed_path);
+
+ determine_submodule_update_strategy(the_repository, update_data->just_cloned,
+ update_data->sm_path, update_data->update_default,
+ &update_data->update_strategy);
+
+ if (update_data->just_cloned)
+ oidcpy(&update_data->suboid, null_oid());
+ else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
+ die(_("Unable to find current revision in submodule path '%s'"),
+ update_data->displaypath);
+
+ if (update_data->remote) {
+ char *remote_name = get_default_remote_submodule(update_data->sm_path);
+ const char *branch = remote_submodule_branch(update_data->sm_path);
+ char *remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
+
+ if (!update_data->nofetch) {
+ if (fetch_in_submodule(update_data->sm_path, update_data->depth,
+ 0, NULL))
+ die(_("Unable to fetch in submodule path '%s'"),
+ update_data->sm_path);
+ }
+
+ if (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))
+ die(_("Unable to find %s revision in submodule path '%s'"),
+ remote_ref, update_data->sm_path);
+
+ free(remote_ref);
+ }
+
+ if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)
+ if (run_update_procedure(update_data))
+ return 1;
+
+ if (update_data->recursive) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct update_data next = *update_data;
+ int res;
+
+ if (update_data->recursive_prefix)
+ prefixed_path = xstrfmt("%s%s/", update_data->recursive_prefix,
+ update_data->sm_path);
+ else
+ prefixed_path = xstrfmt("%s/", update_data->sm_path);
+
+ next.recursive_prefix = get_submodule_displaypath(prefixed_path,
+ update_data->prefix);
+ next.prefix = NULL;
+ oidcpy(&next.oid, null_oid());
+ oidcpy(&next.suboid, null_oid());
+
+ cp.dir = update_data->sm_path;
+ cp.git_cmd = 1;
+ prepare_submodule_repo_env(&cp.env_array);
+ update_data_to_args(&next, &cp.args);
+
+ /* die() if child process die()'d */
+ res = run_command(&cp);
+ if (!res)
+ return 0;
+ die_message(_("Failed to recurse into submodule path '%s'"),
+ update_data->displaypath);
+ if (res == 128)
+ exit(res);
+ else if (res)
+ return 1;
+ }
+
+ return 0;
+}
+
static int update_submodules(struct update_data *update_data)
{
int i, res = 0;
@@ -2607,44 +2802,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
return ret;
}
-static const char *remote_submodule_branch(const char *path)
-{
- const struct submodule *sub;
- const char *branch = NULL;
- char *key;
-
- sub = submodule_from_path(the_repository, null_oid(), path);
- if (!sub)
- return NULL;
-
- key = xstrfmt("submodule.%s.branch", sub->name);
- if (repo_config_get_string_tmp(the_repository, key, &branch))
- branch = sub->branch;
- free(key);
-
- if (!branch)
- return "HEAD";
-
- if (!strcmp(branch, ".")) {
- const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
-
- if (!refname)
- die(_("No such ref: %s"), "HEAD");
-
- /* detached HEAD */
- if (!strcmp(refname, "HEAD"))
- die(_("Submodule (%s) branch configured to inherit "
- "branch from superproject, but the superproject "
- "is not on any branch"), sub->name);
-
- if (!skip_prefix(refname, "refs/heads/", &refname))
- die(_("Expecting a full ref name, got %s"), refname);
- return refname;
- }
-
- return branch;
-}
-
static int push_check(int argc, const char **argv, const char *prefix)
{
struct remote *remote;
@@ -2722,32 +2879,6 @@ static int push_check(int argc, const char **argv, const char *prefix)
return 0;
}
-static void ensure_core_worktree(const char *path)
-{
- const char *cw;
- struct repository subrepo;
-
- if (repo_submodule_init(&subrepo, the_repository, path, null_oid()))
- die(_("could not get a repository handle for submodule '%s'"), path);
-
- if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) {
- char *cfg_file, *abs_path;
- const char *rel_path;
- struct strbuf sb = STRBUF_INIT;
-
- cfg_file = repo_git_path(&subrepo, "config");
-
- abs_path = absolute_pathdup(path);
- rel_path = relative_path(abs_path, subrepo.gitdir, &sb);
-
- git_config_set_in_file(cfg_file, "core.worktree", rel_path);
-
- free(cfg_file);
- free(abs_path);
- strbuf_release(&sb);
- }
-}
-
static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
{
int i;
@@ -2969,138 +3100,6 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)
return 0;
}
-static void update_data_to_args(struct update_data *update_data, struct strvec *args)
-{
- strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
- strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
- if (update_data->recursive_prefix)
- strvec_pushl(args, "--recursive-prefix",
- update_data->recursive_prefix, NULL);
- if (update_data->quiet)
- strvec_push(args, "--quiet");
- if (update_data->force)
- strvec_push(args, "--force");
- if (update_data->init)
- strvec_push(args, "--init");
- if (update_data->remote)
- strvec_push(args, "--remote");
- if (update_data->nofetch)
- strvec_push(args, "--no-fetch");
- if (update_data->dissociate)
- strvec_push(args, "--dissociate");
- if (update_data->progress)
- strvec_push(args, "--progress");
- if (update_data->require_init)
- strvec_push(args, "--require-init");
- if (update_data->depth)
- strvec_pushf(args, "--depth=%d", update_data->depth);
- if (update_data->update_default)
- strvec_pushl(args, "--update", update_data->update_default, NULL);
- if (update_data->references.nr) {
- struct string_list_item *item;
- for_each_string_list_item(item, &update_data->references)
- strvec_pushl(args, "--reference", item->string, NULL);
- }
- if (update_data->filter_options && update_data->filter_options->choice)
- strvec_pushf(args, "--filter=%s",
- expand_list_objects_filter_spec(
- update_data->filter_options));
- if (update_data->recommend_shallow == 0)
- strvec_push(args, "--no-recommend-shallow");
- else if (update_data->recommend_shallow == 1)
- strvec_push(args, "--recommend-shallow");
- if (update_data->single_branch >= 0)
- strvec_push(args, update_data->single_branch ?
- "--single-branch" :
- "--no-single-branch");
-}
-
-static int update_submodule(struct update_data *update_data)
-{
- char *prefixed_path;
-
- ensure_core_worktree(update_data->sm_path);
-
- if (update_data->recursive_prefix)
- prefixed_path = xstrfmt("%s%s", update_data->recursive_prefix,
- update_data->sm_path);
- else
- prefixed_path = xstrdup(update_data->sm_path);
-
- update_data->displaypath = get_submodule_displaypath(prefixed_path,
- update_data->prefix);
- free(prefixed_path);
-
- determine_submodule_update_strategy(the_repository, update_data->just_cloned,
- update_data->sm_path, update_data->update_default,
- &update_data->update_strategy);
-
- if (update_data->just_cloned)
- oidcpy(&update_data->suboid, null_oid());
- else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
- die(_("Unable to find current revision in submodule path '%s'"),
- update_data->displaypath);
-
- if (update_data->remote) {
- char *remote_name = get_default_remote_submodule(update_data->sm_path);
- const char *branch = remote_submodule_branch(update_data->sm_path);
- char *remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
-
- if (!update_data->nofetch) {
- if (fetch_in_submodule(update_data->sm_path, update_data->depth,
- 0, NULL))
- die(_("Unable to fetch in submodule path '%s'"),
- update_data->sm_path);
- }
-
- if (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))
- die(_("Unable to find %s revision in submodule path '%s'"),
- remote_ref, update_data->sm_path);
-
- free(remote_ref);
- }
-
- if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)
- if (run_update_procedure(update_data))
- return 1;
-
- if (update_data->recursive) {
- struct child_process cp = CHILD_PROCESS_INIT;
- struct update_data next = *update_data;
- int res;
-
- if (update_data->recursive_prefix)
- prefixed_path = xstrfmt("%s%s/", update_data->recursive_prefix,
- update_data->sm_path);
- else
- prefixed_path = xstrfmt("%s/", update_data->sm_path);
-
- next.recursive_prefix = get_submodule_displaypath(prefixed_path,
- update_data->prefix);
- next.prefix = NULL;
- oidcpy(&next.oid, null_oid());
- oidcpy(&next.suboid, null_oid());
-
- cp.dir = update_data->sm_path;
- cp.git_cmd = 1;
- prepare_submodule_repo_env(&cp.env_array);
- update_data_to_args(&next, &cp.args);
-
- /* die() if child process die()'d */
- res = run_command(&cp);
- if (!res)
- return 0;
- die_message(_("Failed to recurse into submodule path '%s'"),
- update_data->displaypath);
- if (res == 128)
- exit(res);
- else if (res)
- return 1;
- }
-
- return 0;
-}
-
struct add_data {
const char *prefix;
const char *branch;
--
2.33.GIT
^ permalink raw reply related [flat|nested] 8+ messages in thread