NOTE: This series uses ar/submodule-run-update-procedure [1]
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'. Even though this patch series looks long, a lot
of it is preparatory patches and cleanup of unused functions that result from
this conversion. The real action happens at [6/8].
As with the other series, the goal is to be a faithful conversion, with no
change in behaviour.
This would be the last command whose logic would be moved into C, other than
'submodule add', whose patches have been sent already.
After this works out, we can invert the shell-C relationship and make
'submodule' a proper C builtin.
Fetch-it-Via:
git fetch https://github.com/tfidfwastaken/git submodule-update-list-1
[1] https://lore.kernel.org/git/20210824140609.1496-1-raykar.ath@gmail.com/
Atharva Raykar (13):
submodule--helper: split up ensure_core_worktree()
submodule--helper: get remote names from any repository
submodule--helper: introduce get_default_remote_submodule()
submodule--helper: rename helpers for update-clone
submodule--helper: refactor get_submodule_displaypath()
submodule: move core cmd_update() logic to C
submodule: remove fetch_in_submodule shell function
submodule--helper: remove update-clone subcommand
submodule--helper: remove update-module-mode subcommand
submodule--helper: remove shell interface to ensure_core_worktree()
submodule--helper: remove print-default-remote subcommand
submodule--helper: remove relative-path subcommand
submodule--helper: remove run-update-procedure subcommand
builtin/submodule--helper.c | 764 +++++++++++++++++++++---------------
git-submodule.sh | 145 +------
2 files changed, 455 insertions(+), 454 deletions(-)
--
2.32.0
Let's split up `ensure_core_worktree()` so that we can call it from C
code without needing to deal with command line arguments.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Before 8ef1d2b549 (submodule--helper: get remote names from any
repository, 2021-07-20), it was not possible to directly retrieve a
submodule's remote name within the same process, because
`get_default_remote()` used only knew about the current repository.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
Let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
@@ -1423,14 +1435,9 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+strbuf_addstr(&sb,get_default_remote_submodule(path));+if(!sb.buf)die(_("failed to get the default remote for submodule '%s'"),path);
The `update-clone` subcommand helpers that perform the parallel clone
and printing to stdout for shell script consumption, are renamed.
This lets us use the names `update_submodules()` and
`update_submodule()` for the helpers in the next patch, when we create
an `update` subcommand that does a full conversion.
We will get rid of these helpers in a cleanup patch at the end of this
series, when the `update-clone` command is no longer useful to us.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -269,11 +269,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -289,6 +286,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules()` and `update_submodule()` which
are quite similar to `update_clone_submodules()` and
`update_clone_submodule()`, and will supersede them.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule directly.
This introduces another problem, because there is no mechanism to pass
the superproject path prefix (ie, `--super-prefix`) without starting a
new git process. This field is required for obtaining the display path
for that is used by the command's output messages. So let's add a field
into the `init_cb` struct that lets us pass this information to
`init_submodule()`, which will now also take an explicit 'superprefix'
argument.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 502 ++++++++++++++++++++++++++++++------
git-submodule.sh | 131 +---------
2 files changed, 430 insertions(+), 203 deletions(-)
@@ -2039,7 +2043,6 @@ struct submodule_update_clone {constchar*prefix;intsingle_branch;-/* to be consumed by git-submodule.sh */structupdate_clone_data*update_clone;intupdate_clone_nr;intupdate_clone_alloc;
@@ -2369,111 +2416,113 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strreturnrun_command(&cp);}-staticintrun_update_command(structupdate_data*ud,intsubforce)+staticintrun_update_command(structupdate_data*ud,intsubforce,structstring_list*err){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);+structstrbufout=STRBUF_INIT;intmust_die_on_failure=0;-intgit_cmd;+structsubmodule_update_strategystrategy=SUBMODULE_UPDATE_STRATEGY_INIT;-switch(ud->update_strategy.type){+if(ud->update_strategy.type==SM_UPDATE_UNSPECIFIED||ud->just_cloned)+determine_submodule_update_strategy(the_repository,ud->just_cloned,+ud->sm_path,NULL,&strategy);+else+strategy=ud->update_strategy;++cp.dir=xstrdup(ud->sm_path);+switch(strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_UPDATE_COMMAND:-git_cmd=0;-strvec_push(&args,ud->update_strategy.command);+cp.git_cmd=0;+cp.use_shell=1;+strvec_push(&cp.args,strategy.command);must_die_on_failure=1;break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&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)){-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Unable to checkout '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_REBASE:-printf(_("Unable to rebase '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_MERGE:-printf(_("Unable to merge '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_COMMAND:-printf(_("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));+prepare_submodule_repo_env(&cp.env_array);+if(capture_command(&cp,&out,0)){+if(must_die_on_failure){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+die(_("Unable to checkout '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_REBASE:+die(_("Unable to rebase '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_MERGE:+die(_("Unable to merge '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_COMMAND:+die(_("Execution of '%s %s' failed in submodule path '%s'"),+strategy.command,oid,ud->displaypath);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/-if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/++/* the command failed, but update must continue */+string_list_append(err,out.buf);return1;}-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Submodule path '%s': checked out '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_REBASE:-printf(_("Submodule path '%s': rebased into '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_MERGE:-printf(_("Submodule path '%s': merged in '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_COMMAND:-printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,ud->update_strategy.command,oid);-break;-default:-BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+if(!ud->quiet){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+printf(_("Submodule path '%s': checked out '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_REBASE:+printf(_("Submodule path '%s': rebased into '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_MERGE:+printf(_("Submodule path '%s': merged in '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_COMMAND:+printf(_("Submodule path '%s': '%s %s'\n"),+ud->displaypath,strategy.command,oid);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintdo_run_update_procedure(structupdate_data*ud,structstring_list*err){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2500,7 +2549,7 @@ static int do_run_update_procedure(struct update_data *ud)ud->displaypath,oid_to_hex(&ud->oid));}-returnrun_update_command(ud,subforce);+returnrun_update_command(ud,subforce,err);}staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)
@@ -2605,6 +2654,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)intforce=0,quiet=0,nofetch=0,just_cloned=0;char*prefixed_path,*update=NULL;structupdate_dataupdate_data=UPDATE_DATA_INIT;+structstring_listerr=STRING_LIST_INIT_DUP;structoptionoptions[]={OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),
@@ -3038,6 +3088,288 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+constchar*update=submodule_strategy_to_string(&update_data->update_strategy);++strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+if(update_data->prefix)+strvec_pushl(args,"--prefix",update_data->prefix,NULL);+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)+strvec_pushl(args,"--update",update,NULL);+if(update_data->references.nr){+structstring_list_item*item;+for_each_string_list_item(item,&update_data->references)+strvec_pushl(args,"--reference",item->string,NULL);+}+if(update_data->recommend_shallow==0)+strvec_push(args,"--no-recommend-shallow");+elseif(update_data->recommend_shallow==1)+strvec_push(args,"--recommend-shallow");+if(update_data->single_branch>=0)+strvec_push(args,"--single-branch");+}++staticintupdate_submodule(structupdate_data*update_data)+{+char*prefixed_path;+structstring_listerr=STRING_LIST_INIT_DUP;++do_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);++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);+constchar*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(do_run_update_procedure(update_data,&err))+return1;++if(update_data->recursive){+intres;+structchild_processcp=CHILD_PROCESS_INIT;+structupdate_datanext=*update_data;+char*die_msg=xstrfmt(_("Failed to recurse into submodule path '%s'"),+update_data->displaypath);++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 */+if((res=run_command(&cp))==128)+die("%s",die_msg);+if(res)+string_list_append(&err,die_msg);++free(die_msg);+}++if(err.nr){+structstring_list_item*item;+for_each_string_list_item(item,&err)+fputs(item->string,stderr);+return1;+}++return0;+}++staticintupdate_submodules(structupdate_data*update_data)+{+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;++update_clone_from_update_data(&suc,update_data);+run_processes_parallel_tr2(suc.max_jobs,update_clone_get_next_task,+update_clone_start_failure,+update_clone_task_finished,&suc,"submodule",+"parallel/update");++/*+*Wesavedtheoutputandputitoutallatoncenow.+*Thatmeans:+*-thelistenerdoesnothavetointerleavetheir(checkout)+*workwithourfetching.Thewritesinvolvedina+*checkoutinvolvemorestraightforwardsequentialI/O.+*-thelistenercanavoiddoinganyworkiffetchingfailed.+*/+if(suc.quickstop)+return1;++for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];++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;+}++returnres;+}++staticintmodule_update(intargc,constchar**argv,constchar*prefix)+{+intinit=0,force=0,quiet=0,nofetch=0;+intremote=0,recursive=0,dissociate=0;+intprogress=0,require_init=0;+constchar*update=NULL;+structpathspecpathspec;+structupdate_dataupdate_data=UPDATE_DATA_INIT;++structoptionmodule_update_clone_options[]={+OPT__FORCE(&force,N_("force checkout updates"),0),+OPT_BOOL(0,"init",&init,+N_("initialize uninitialized submodules before update")),+OPT_BOOL(0,"remote",&remote,+N_("use SHA-1 of submodule's remote tracking branch")),+OPT_BOOL(0,"recursive",&recursive,+N_("traverse submodules recursively")),+OPT_BOOL('N',"no-fetch",&nofetch,+N_("don't fetch new objects from the remote site")),+OPT_STRING(0,"prefix",&prefix,+N_("path"),+N_("path into the working tree")),+OPT_STRING(0,"recursive-prefix",&update_data.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",&update_data.references,N_("repo"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,+N_("use --reference only while cloning")),+OPT_INTEGER(0,"depth",&update_data.depth,+N_("create a shallow clone truncated to the "+"specified number of revisions")),+OPT_INTEGER('j',"jobs",&update_data.max_jobs,+N_("parallel jobs")),+OPT_BOOL(0,"recommend-shallow",&update_data.recommend_shallow,+N_("whether the initial clone should follow the shallow recommendation")),+OPT__QUIET(&quiet,N_("don't print cloning progress")),+OPT_BOOL(0,"progress",&progress,+N_("force cloning progress")),+OPT_BOOL(0,"require-init",&require_init,+N_("disallow cloning into non-empty directory")),+OPT_BOOL(0,"single-branch",&update_data.single_branch,+N_("clone only one branch, HEAD or --branch")),+OPT_END()+};++constchar*constgit_submodule_helper_usage[]={+N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),+NULL+};++update_clone_config_from_gitmodules(&update_data.max_jobs);+git_config(git_update_clone_config,&update_data.max_jobs);++argc=parse_options(argc,argv,prefix,module_update_clone_options,+git_submodule_helper_usage,0);+update_data.prefix=prefix;++update_data.force=!!force;+update_data.quiet=!!quiet;+update_data.nofetch=!!nofetch;+update_data.init=!!init;+update_data.require_init=!!require_init;+update_data.remote=!!remote;+update_data.recursive=!!recursive;+update_data.progress=!!progress;+update_data.dissociate=!!dissociate;+oidcpy(&update_data.oid,null_oid());+oidcpy(&update_data.suboid,null_oid());++if(update)+if(parse_submodule_update_strategy(update,+&update_data.update_strategy)<0)+die(_("bad value for update parameter"));++if(module_list_compute(argc,argv,prefix,&pathspec,&update_data.list)<0)+return1;++if(pathspec.nr)+update_data.warn_if_uninitialized=1;++if(update_data.init){+structmodule_listlist=MODULE_LIST_INIT;+structinit_cbinfo=INIT_CB_INIT;++if(module_list_compute(argc,argv,update_data.prefix,+&pathspec,&list)<0)+return1;++/*+*Iftherearenopathargsandsubmodule.activeissetthen,+*bydefault,onlyinitialize'active'modules.+*/+if(!argc&&git_config_get_value_multi("submodule.active"))+module_list_active(&list);++info.prefix=update_data.prefix;+info.superprefix=update_data.recursive_prefix;+if(update_data.quiet)+info.flags|=OPT_QUIET;++for_each_listed_submodule(&list,init_submodule_cb,&info);+}++returnupdate_submodules(&update_data);+}+structadd_data{constchar*prefix;constchar*branch;
@@ -484,133 +484,26 @@ cmd_update()shiftdone-iftest-n"$init"-then-cmd_init"--""$@"||return-fi--{-gitsubmodule--helperupdate-clone${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+${GIT_QUIET:+--quiet}\+${force:+--force}\+${progress:+--progress}\+${dissociate:+--dissociate}\+${remote:+--remote}\+${recursive:+--recursive}\+${init:+--init}\+${require_init:+--require-init}\+${nofetch:+--no-fetch}\${wt_prefix:+--prefix "$wt_prefix"}\${prefix:+--recursive-prefix "$prefix"}\${update:+--update "$update"}\${reference:+"$reference"}\-${dissociate:+"--dissociate"}\-${depth:+--depth "$depth"}\-${require_init:+--require-init}\+${depth:+"$depth"}\$single_branch\$recommend_shallow\$jobs\--\-"$@"||echo"#unmatched"$?-}|{-err=-whileread-rquickabortsha1just_clonedsm_path-do-die_if_unmatched"$quickabort""$sha1"--gitsubmodule--helperensure-core-worktree"$sm_path"||exit1--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq1-then-subsha1=-else-just_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"-fi--iftest-n"$remote"-then-branch=$(gitsubmodule--helperremote-branch"$sm_path")-iftest-z"$nofetch"-then-# Fetch remote before determining tracking $sha1-fetch_in_submodule"$sm_path"$depth||-die"fatal: $(eval_gettext"Unable to fetch in submodule path '\$sm_path'")"-fi-remote_name=$(sanitize_submodule_env;cd"$sm_path"&&gitsubmodule--helperprint-default-remote)-sha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verify"${remote_name}/${branch}")||-die"fatal: $(eval_gettext"Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"-fi--out=$(gitsubmodule--helperrun-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"}\-${subsha1:+--suboid "$subsha1"}\-"--"\-"$sm_path")--# 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-# 3: no update procedure was run-res="$?"-case$resin-0)-say"$out"-;;-1)-err="${err};fatal: $out"-continue-;;-2|128)-die_with_status$res"fatal: $out"-;;-esac--iftest-n"$recursive"-then-(-prefix=$(gitsubmodule--helperrelative-path"$prefix$sm_path/""$wt_prefix")-wt_prefix=-sanitize_submodule_env-cd"$sm_path"&&-evalcmd_update-)-res=$?-iftest$res-gt0-then-die_msg="fatal: $(eval_gettext"Failed to recurse into submodule path '\$displaypath'")"-iftest$res-ne2-then-err="${err};$die_msg"-continue-else-die_with_status$res"$die_msg"-fi-fi-fi-done--iftest-n"$err"-then-OIFS=$IFS-IFS=';'-forein$err-do-iftest-n"$e"-then-echo>&2"$e"-fi-done-IFS=$OIFS-exit1-fi-}+"$@"}#
This function has no more use in 'git-submodule.sh' after
bd82d7d467 (submodule: move core cmd_update() logic to C, 2021-07-20),
where we moved all of its uses to C, which has its own version for the
same.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
git-submodule.sh | 14 --------------
1 file changed, 14 deletions(-)
@@ -369,20 +369,6 @@ cmd_deinit()git${wt_prefix:+-C "$wt_prefix"}submodule--helperdeinit${GIT_QUIET:+--quiet}${force:+--force}${deinit_all:+--all}--"$@"}-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]-# Because arguments are positional, use an empty string to omit <depth>-# but include <sha1>.-fetch_in_submodule()(-sanitize_submodule_env&&-cd"$1"&&-iftest$#-eq3-then-echo"$3"|gitfetch${GIT_QUIET:+--quiet}--stdin${2:+"$2"}-else-gitfetch${GIT_QUIET:+--quiet}${2:+"$2"}-fi-)-## Update each submodule path to correct revision, using clone and checkout as needed#
We no longer need this subcommand as the shell version calls the
'update' subcommand instead, which does all the cloning within C itself.
We also no longer need the 'update_clone_submodules()' and
'update_clone_submodule()' functions, so we remove those as well.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 98 -------------------------------------
1 file changed, 98 deletions(-)
@@ -2552,103 +2552,6 @@ static int do_run_update_procedure(struct update_data *ud, struct string_list *ereturnrun_update_command(ud,subforce,err);}-staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)-{-fprintf(stdout,"dummy %s %d\t%s\n",-oid_to_hex(&ucd->oid),-ucd->just_cloned,-ucd->sub->path);-}--staticintupdate_clone_submodules(structsubmodule_update_clone*suc)-{-inti;--run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,-update_clone_start_failure,-update_clone_task_finished,suc,"submodule",-"parallel/update");--/*-*Wesavedtheoutputandputitoutallatoncenow.-*Thatmeans:-*-thelistenerdoesnothavetointerleavetheir(checkout)-*workwithourfetching.Thewritesinvolvedina-*checkoutinvolvemorestraightforwardsequentialI/O.-*-thelistenercanavoiddoinganyworkiffetchingfailed.-*/-if(suc->quickstop)-return1;--for(i=0;i<suc->update_clone_nr;i++)-update_clone_submodule(&suc->update_clone[i]);--return0;-}--staticintupdate_clone(intargc,constchar**argv,constchar*prefix)-{-constchar*update=NULL;-structpathspecpathspec;-structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;--structoptionmodule_update_clone_options[]={-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"recursive-prefix",&suc.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"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&suc.dissociate,-N_("use --reference only while cloning")),-OPT_STRING(0,"depth",&suc.depth,"<depth>",-N_("create a shallow clone truncated to the "-"specified number of revisions")),-OPT_INTEGER('j',"jobs",&suc.max_jobs,-N_("parallel jobs")),-OPT_BOOL(0,"recommend-shallow",&suc.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,-N_("force cloning progress")),-OPT_BOOL(0,"require-init",&suc.require_init,-N_("disallow cloning into non-empty directory")),-OPT_BOOL(0,"single-branch",&suc.single_branch,-N_("clone only one branch, HEAD or --branch")),-OPT_END()-};--constchar*constgit_submodule_helper_usage[]={-N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),-NULL-};-suc.prefix=prefix;--update_clone_config_from_gitmodules(&suc.max_jobs);-git_config(git_update_clone_config,&suc.max_jobs);--argc=parse_options(argc,argv,prefix,module_update_clone_options,-git_submodule_helper_usage,0);--if(update)-if(parse_submodule_update_strategy(update,&suc.update)<0)-die(_("bad value for update parameter"));--if(module_list_compute(argc,argv,prefix,&pathspec,&suc.list)<0)-return1;--if(pathspec.nr)-suc.warn_if_uninitialized=1;--returnupdate_clone_submodules(&suc);-}-staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix){intforce=0,quiet=0,nofetch=0,just_cloned=0;
This subcommand was once useful for 'submodule update', but now that we
have converted the shell code to C, it is no longer used.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
The 'ensure-core-worktree' subcommand is no longer needed since the
conversion of the update code from shell to C.
Let's remove the subcommand, and while we are at it, let's rename
'do_ensure_core_worktree()' to 'ensure_core_worktree()' to signal that
it is no longer a utility function meant to be called by another
function.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
This subcommand was once useful for submodule functionality, but after
the various conversions of shell code to C, it is no longer used.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 16 ----------------
1 file changed, 16 deletions(-)
This subcommand was once used extensively for submodule functionality
when it was written in shell, but now that we have converted the shell
code to C, it is no longer used.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 12 ------------
1 file changed, 12 deletions(-)
The subcommand 'submodule--helper run-update-procedure' is no longer
needed after the conversion of the bulk of 'update' to C.
While we are at it, let's rename 'do_run_update_procedure()' to
'run_update_procedure()' to reflect the fact that it is no longer a
utility function meant to be wrapped in another function.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 73 +------------------------------------
1 file changed, 2 insertions(+), 71 deletions(-)
@@ -2484,7 +2484,7 @@ static int run_update_command(struct update_data *ud, int subforce, struct strinreturn0;}-staticintdo_run_update_procedure(structupdate_data*ud,structstring_list*err)+staticintrun_update_procedure(structupdate_data*ud,structstring_list*err){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2514,74 +2514,6 @@ static int do_run_update_procedure(struct update_data *ud, struct string_list *ereturnrun_update_command(ud,subforce,err);}-staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,nofetch=0,just_cloned=0;-char*prefixed_path,*update=NULL;-structupdate_dataupdate_data=UPDATE_DATA_INIT;-structstring_listerr=STRING_LIST_INIT_DUP;--structoptionoptions[]={-OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),-OPT__FORCE(&force,N_("force checkout updates"),0),-OPT_BOOL('N',"no-fetch",&nofetch,-N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&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,-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_CALLBACK_F(0,"suboid",&update_data.suboid,N_("subsha1"),-N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_END()-};--constchar*constusage[]={-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.force=!!force;-update_data.quiet=!!quiet;-update_data.nofetch=!!nofetch;-update_data.just_cloned=!!just_cloned;-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,-&update_data.update_strategy);--free(prefixed_path);--if(!oideq(&update_data.oid,&update_data.suboid)||update_data.force)-returndo_run_update_procedure(&update_data,&err);--return3;-}-staticconstchar*remote_submodule_branch(constchar*path){conststructsubmodule*sub;
@@ -3018,7 +2950,7 @@ static int update_submodule(struct update_data *update_data)}if(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)-if(do_run_update_procedure(update_data,&err))+if(run_update_procedure(update_data,&err))return1;if(update_data->recursive){
NOTE: This series uses ar/submodule-run-update-procedure [1]
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'. Even though this patch series looks long, a lot
of it is preparatory patches and cleanup of unused functions that result from
this conversion. The real action happens at [6/8].
It looks like the 6/x part of that still applies, but not the "/8",
i.e. this is now a 13-part series. Is this summary otherwise still
current with what's being submitted here?
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
I'd find this easier to follow if this were just squashed into the next
commit. Both are rather small, but following the context of first adding
a function, then using it, instead of just adding it, changing the old
users etc. is harder than just having it in one commit.
Since you're doing some cleanup while you're at it, just changing this
in some earlier step to:
define INIT_CB_INIT { 0 }
Is better, i.e. the NULL-ing out is implicit here. I have an unsubmitted
series that does that across the codebase.
+ .references = STRING_LIST_INIT_DUP, \
We do inits here, and append, but it seems nothing clears this string_list.
This function has no more use in 'git-submodule.sh' after
bd82d7d467 (submodule: move core cmd_update() logic to C, 2021-07-20),
where we moved all of its uses to C, which has its own version for the
same.
This commit ID appears to be a reference to your own 06/13, so the OID
won't work once this is merged to git.git.
Perhaps just squash this into 06/13 instead?
We no longer need this subcommand as the shell version calls the
'update' subcommand instead, which does all the cloning within C itself.
We also no longer need the 'update_clone_submodules()' and
'update_clone_submodule()' functions, so we remove those as well.
So in 04/13 update_clone_submodules() was renamed, but now we're getting
rid of it. Maybe there's not an easy way to avoid this churn, but if there is...
This subcommand was once useful for 'submodule update', but now that we
have converted the shell code to C, it is no longer used.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
So in https://lore.kernel.org/git/87sfyglfl9.fsf@evledraar.gmail.com I
suggested squashing the shell removal, but I see now that here later in
09-13/13.
So yeah, having 08/13 stand-alone is easier to read then, but I think
then squashing all of 09-13 together is better. I.e. there's no reason
to remove these one at a time, let's just remove them all at once.
That also makes it clear that it's a remove-only change aside from your
"refactor while at it" of renaming this function:
-static int do_run_update_procedure(struct update_data *ud, struct string_list *err)
+static int run_update_procedure(struct update_data *ud, struct string_list *err)
We could either skip that, or split that later refactoring into another
commit.
Thanks for working on this, I'm exciting to see more of git-submodule.sh
go away. Hopefully these comments I left are useful / will aid future
reviewer readability of this series.
NOTE: This series uses ar/submodule-run-update-procedure [1]
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'. Even though this patch series looks long, a lot
of it is preparatory patches and cleanup of unused functions that result from
this conversion. The real action happens at [6/8].
It looks like the 6/x part of that still applies, but not the "/8",
i.e. this is now a 13-part series. Is this summary otherwise still
current with what's being submitted here?
Sorry, I meant [6/13], this is meant to be 13 parts only. Other than
that typo, the summary is up-to-date with what the series contains (I
wrote all of it today).
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
I'd find this easier to follow if this were just squashed into the next
commit. Both are rather small, but following the context of first adding
a function, then using it, instead of just adding it, changing the old
users etc. is harder than just having it in one commit.
I am in two minds about this. I initially had both these changes in one
commit, but Christian suggested I split the changes into a part that
refactors existing code (this commit), and one that introduces a new
helper (the next commit).
I guess I will squash it for now and see how it is received. Maybe
bringing down the "/13" might help get more reviews ;-)
This subcommand was once useful for 'submodule update', but now that we
have converted the shell code to C, it is no longer used.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
So in https://lore.kernel.org/git/87sfyglfl9.fsf@evledraar.gmail.com I
suggested squashing the shell removal, but I see now that here later in
09-13/13.
So yeah, having 08/13 stand-alone is easier to read then, but I think
then squashing all of 09-13 together is better. I.e. there's no reason
to remove these one at a time, let's just remove them all at once.
Okay, I shall do this.
That also makes it clear that it's a remove-only change aside from your
"refactor while at it" of renaming this function:
-static int do_run_update_procedure(struct update_data *ud, struct string_list *err)
+static int run_update_procedure(struct update_data *ud, struct string_list *err)
We could either skip that, or split that later refactoring into another
commit.
Thanks for working on this, I'm exciting to see more of git-submodule.sh
go away. Hopefully these comments I left are useful / will aid future
reviewer readability of this series.
Thanks for your suggestions. I've gone through all of them, and I'll
reroll soon.
NOTE: This series uses ar/submodule-run-update-procedure, which is now part of
'next'. [1]
Since v1:
I have incorporated Ævar's suggestions, and attempted to make this easier to
review.
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'. Even though this patch series looks a bit long, a
lot of it is preparatory patches and cleanup of unused functions that result
from this conversion. The real action happens at [5/8].
As with the other series, the goal is to be a faithful conversion, with no
change in behaviour.
This would be the last command whose logic would be moved into C, other than
'submodule add', whose patches have been sent already.
After this works out, we can invert the shell-C relationship and make
'submodule' a proper C builtin.
Fetch-it-Via:
git fetch https://github.com/tfidfwastaken/git submodule-update-list-2
[1] https://lore.kernel.org/git/20210824140609.1496-1-raykar.ath@gmail.com/
Atharva Raykar (8):
submodule--helper: split up ensure_core_worktree()
submodule--helper: get remote names from any repository
submodule--helper: rename helpers for update-clone
submodule--helper: refactor get_submodule_displaypath()
submodule: move core cmd_update() logic to C
submodule--helper: remove update-clone subcommand
submodule--helper: remove unused helpers
submodule--helper: rename helper functions
builtin/submodule--helper.c | 767 +++++++++++++++++++++---------------
git-submodule.sh | 145 +------
2 files changed, 458 insertions(+), 454 deletions(-)
Range-diff against v1:
1: 2cfdc0e10a < -: ---------- submodule--helper: get remote names from any repository
-: ---------- > 1: f83a5b7f34 submodule--helper: split up ensure_core_worktree()
2: be83ba7fdb ! 2: 7f4e24ce25 submodule--helper: introduce get_default_remote_submodule()
@@ Metadata
Author: Atharva Raykar [off-list ref]
## Commit message ##
- submodule--helper: introduce get_default_remote_submodule()
+ submodule--helper: get remote names from any repository
- Before 8ef1d2b549 (submodule--helper: get remote names from any
- repository, 2021-07-20), it was not possible to directly retrieve a
- submodule's remote name within the same process, because
- `get_default_remote()` used only knew about the current repository.
+ `get_default_remote()` retrieves the name of a remote by resolving the
+ refs from of the current repository's ref store.
+
+ Thus in order to use it for retrieving the remote name of a submodule,
+ we have to start a new subprocess which runs from the submodule
+ directory.
+
+ Let's instead introduce a function called `repo_get_default_remote()`
+ which takes any repository object and retrieves the remote accordingly.
+
+ `get_default_remote()` is then defined as a call to
+ `repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
- Let's make a function called `get_default_remote_submodule()` which
+ So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
@@ Commit message
Signed-off-by: Atharva Raykar [off-list ref]
## builtin/submodule--helper.c ##
-@@ builtin/submodule--helper.c: static char *repo_get_default_remote(struct repository *repo, const char *refnam
+@@
+ typedef void (*each_submodule_fn)(const struct cache_entry *list_item,
+ void *cb_data);
+
+-static char *get_default_remote(void)
++static char *repo_get_default_remote(struct repository *repo, const char *refname)
+ {
+ char *dest = NULL, *ret;
+ struct strbuf sb = STRBUF_INIT;
+- const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+
+ if (!refname)
+ die(_("No such ref: %s"), "HEAD");
+@@ builtin/submodule--helper.c: static char *get_default_remote(void)
+ die(_("Expecting a full ref name, got %s"), refname);
+
+ strbuf_addf(&sb, "branch.%s.remote", refname);
+- if (git_config_get_string(sb.buf, &dest))
++ if (repo_config_get_string(repo, sb.buf, &dest))
+ ret = xstrdup("origin");
+ else
+ ret = dest;
+@@ builtin/submodule--helper.c: static char *get_default_remote(void)
return ret;
}
@@ builtin/submodule--helper.c: static char *repo_get_default_remote(struct reposit
+ return repo_get_default_remote(&subrepo, refname);
+}
+
- static char *get_default_remote(void)
++static char *get_default_remote(void)
++{
++ const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
++ return repo_get_default_remote(the_repository, refname);
++}
++
+ static int print_default_remote(int argc, const char **argv, const char *prefix)
{
- const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+ char *remote;
@@ builtin/submodule--helper.c: static void sync_submodule(const char *path, const char *prefix,
char *remote_key = NULL;
char *sub_origin_url, *super_config_url, *displaypath;
3: 8981b2c4c5 = 3: 390596ee7c submodule--helper: rename helpers for update-clone
4: 420d792f99 = 4: 2a2ddcac91 submodule--helper: refactor get_submodule_displaypath()
5: 6c4696b70a ! 5: 832020a290 submodule: move core cmd_update() logic to C
@@ Commit message
`init_submodule()`, which will now also take an explicit 'superprefix'
argument.
+ While we are at it, we also remove the fetch_in_submodule() shell
+ function since it is no longer used anywhere.
+
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
@@ builtin/submodule--helper.c: static char *compute_submodule_clone_url(const char
unsigned int flags;
};
-#define INIT_CB_INIT { NULL, 0 }
-+#define INIT_CB_INIT { NULL, NULL, 0 }
++#define INIT_CB_INIT { 0 }
static void init_submodule(const char *path, const char *prefix,
- unsigned int flags)
@@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char *
+ * checkout involve more straightforward sequential I/O.
+ * - the listener can avoid doing any work if fetching failed.
+ */
-+ if (suc.quickstop)
++ if (suc.quickstop) {
++ string_list_clear(&update_data->references, 0);
+ return 1;
++ }
+
+ for (i = 0; i < suc.update_clone_nr; i++) {
+ struct update_clone_data ucd = suc.update_clone[i];
@@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char *
+ res = 1;
+ }
+
++ string_list_clear(&update_data->references, 0);
+ return res;
+}
+
@@ builtin/submodule--helper.c: static struct cmd_struct commands[] = {
{"run-update-procedure", run_update_procedure, 0},
## git-submodule.sh ##
+@@ git-submodule.sh: cmd_deinit()
+ git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${force:+--force} ${deinit_all:+--all} -- "$@"
+ }
+
+-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]
+-# Because arguments are positional, use an empty string to omit <depth>
+-# but include <sha1>.
+-fetch_in_submodule () (
+- sanitize_submodule_env &&
+- cd "$1" &&
+- if test $# -eq 3
+- then
+- echo "$3" | git fetch ${GIT_QUIET:+--quiet} --stdin ${2:+"$2"}
+- else
+- git fetch ${GIT_QUIET:+--quiet} ${2:+"$2"}
+- fi
+-)
+-
+ #
+ # Update each submodule path to correct revision, using clone and checkout as needed
+ #
@@ git-submodule.sh: cmd_update()
shift
done
6: 524ae77c3f < -: ---------- submodule: remove fetch_in_submodule shell function
7: ea56f7319a = 6: fb3fa8174a submodule--helper: remove update-clone subcommand
8: 10a62172a2 < -: ---------- submodule--helper: remove update-module-mode subcommand
9: dbbe5d3f53 < -: ---------- submodule--helper: remove shell interface to ensure_core_worktree()
10: a015af3a16 < -: ---------- submodule--helper: remove print-default-remote subcommand
11: f5a7ba1405 < -: ---------- submodule--helper: remove relative-path subcommand
12: 9f54eb5972 ! 7: 364f72f870 submodule--helper: remove run-update-procedure subcommand
@@ Metadata
Author: Atharva Raykar [off-list ref]
## Commit message ##
- submodule--helper: remove run-update-procedure subcommand
+ submodule--helper: remove unused helpers
- The subcommand 'submodule--helper run-update-procedure' is no longer
- needed after the conversion of the bulk of 'update' to C.
-
- While we are at it, let's rename 'do_run_update_procedure()' to
- 'run_update_procedure()' to reflect the fact that it is no longer a
- utility function meant to be wrapped in another function.
+ These helpers were useful back when 'submodule update' had most of its
+ logic in shell. Now that they will never be invoked, let us remove them.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar [off-list ref]
## builtin/submodule--helper.c ##
-@@ builtin/submodule--helper.c: static int run_update_command(struct update_data *ud, int subforce, struct strin
- return 0;
+@@ builtin/submodule--helper.c: static char *get_default_remote(void)
+ return repo_get_default_remote(the_repository, refname);
}
--static int do_run_update_procedure(struct update_data *ud, struct string_list *err)
-+static int run_update_procedure(struct update_data *ud, struct string_list *err)
+-static int print_default_remote(int argc, const char **argv, const char *prefix)
+-{
+- char *remote;
+-
+- if (argc != 1)
+- die(_("submodule--helper print-default-remote takes no arguments"));
+-
+- remote = get_default_remote();
+- if (remote)
+- printf("%s\n", remote);
+-
+- free(remote);
+- return 0;
+-}
+-
+ static int starts_with_dot_slash(const char *str)
{
- int subforce = is_null_oid(&ud->suboid) || ud->force;
+ return str[0] == '.' && is_dir_sep(str[1]);
+@@ builtin/submodule--helper.c: static void determine_submodule_update_strategy(struct repository *r,
+ free(key);
+ }
+-static int module_update_module_mode(int argc, const char **argv, const char *prefix)
+-{
+- const char *path, *update = NULL;
+- int just_cloned;
+- struct submodule_update_strategy update_strategy = { .type = SM_UPDATE_CHECKOUT };
+-
+- if (argc < 3 || argc > 4)
+- die("submodule--helper update-module-clone expects <just-cloned> <path> [<update>]");
+-
+- just_cloned = git_config_int("just_cloned", argv[1]);
+- path = argv[2];
+-
+- if (argc == 4)
+- update = argv[3];
+-
+- determine_submodule_update_strategy(the_repository,
+- just_cloned, path, update,
+- &update_strategy);
+- fputs(submodule_strategy_to_string(&update_strategy), stdout);
+-
+- return 0;
+-}
+-
+ struct update_clone_data {
+ const struct submodule *sub;
+ struct object_id oid;
@@ builtin/submodule--helper.c: static int do_run_update_procedure(struct update_data *ud, struct string_list *e
return run_update_command(ud, subforce, err);
}
@@ builtin/submodule--helper.c: static int do_run_update_procedure(struct update_da
-
- return 3;
-}
+-
+-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;
-@@ builtin/submodule--helper.c: static int update_submodule(struct update_data *update_data)
+@@ builtin/submodule--helper.c: static void do_ensure_core_worktree(const char *path)
}
+ }
- if (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)
-- if (do_run_update_procedure(update_data, &err))
-+ if (run_update_procedure(update_data, &err))
- return 1;
-
- if (update_data->recursive) {
+-static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
+-{
+- const char *path;
+-
+- if (argc != 2)
+- BUG("submodule--helper ensure-core-worktree <path>");
+-
+- path = argv[1];
+- do_ensure_core_worktree(path);
+-
+- return 0;
+-}
+-
+ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
+ {
+ int i;
@@ builtin/submodule--helper.c: static struct cmd_struct commands[] = {
{"clone", module_clone, 0},
{"add-clone", add_clone, 0},
{"update", module_update, 0},
+- {"update-module-mode", module_update_module_mode, 0},
- {"run-update-procedure", run_update_procedure, 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},
+ {"status", module_status, SUPPORT_SUPER_PREFIX},
+- {"print-default-remote", print_default_remote, 0},
+ {"sync", module_sync, SUPPORT_SUPER_PREFIX},
+ {"deinit", module_deinit, 0},
+ {"summary", module_summary, SUPPORT_SUPER_PREFIX},
-: ---------- > 8: ca48dd452c submodule--helper: rename helper functions
--
2.32.0
Let's split up `ensure_core_worktree()` so that we can call it from C
code without needing to deal with command line arguments.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
@@ -1418,14 +1435,9 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+strbuf_addstr(&sb,get_default_remote_submodule(path));+if(!sb.buf)die(_("failed to get the default remote for submodule '%s'"),path);
The `update-clone` subcommand helpers that perform the parallel clone
and printing to stdout for shell script consumption, are renamed.
This lets us use the names `update_submodules()` and
`update_submodule()` for the helpers in the next patch, when we create
an `update` subcommand that does a full conversion.
We will get rid of these helpers in a cleanup patch at the end of this
series, when the `update-clone` command is no longer useful to us.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -269,11 +269,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -289,6 +286,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules()` and `update_submodule()` which
are quite similar to `update_clone_submodules()` and
`update_clone_submodule()`, and will supersede them.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule directly.
This introduces another problem, because there is no mechanism to pass
the superproject path prefix (ie, `--super-prefix`) without starting a
new git process. This field is required for obtaining the display path
for that is used by the command's output messages. So let's add a field
into the `init_cb` struct that lets us pass this information to
`init_submodule()`, which will now also take an explicit 'superprefix'
argument.
While we are at it, we also remove the fetch_in_submodule() shell
function since it is no longer used anywhere.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 505 ++++++++++++++++++++++++++++++------
git-submodule.sh | 145 +----------
2 files changed, 433 insertions(+), 217 deletions(-)
@@ -634,18 +634,22 @@ static char *compute_submodule_clone_url(const char *rel_url)structinit_cb{constchar*prefix;+constchar*superprefix;unsignedintflags;};-#define INIT_CB_INIT { NULL, 0 }+#define INIT_CB_INIT { 0 }staticvoidinit_submodule(constchar*path,constchar*prefix,-unsignedintflags)+constchar*superprefix,unsignedintflags){conststructsubmodule*sub;structstrbufsb=STRBUF_INIT;char*upd=NULL,*url=NULL,*displaypath;-displaypath=get_submodule_displaypath(path,prefix);+/* try superprefix from the environment, if it is not passed explicitly */+if(!superprefix)+superprefix=get_super_prefix();+displaypath=do_get_submodule_displaypath(path,prefix,superprefix);sub=submodule_from_path(the_repository,null_oid(),path);
@@ -2039,7 +2043,6 @@ struct submodule_update_clone {constchar*prefix;intsingle_branch;-/* to be consumed by git-submodule.sh */structupdate_clone_data*update_clone;intupdate_clone_nr;intupdate_clone_alloc;
@@ -2369,111 +2416,113 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strreturnrun_command(&cp);}-staticintrun_update_command(structupdate_data*ud,intsubforce)+staticintrun_update_command(structupdate_data*ud,intsubforce,structstring_list*err){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);+structstrbufout=STRBUF_INIT;intmust_die_on_failure=0;-intgit_cmd;+structsubmodule_update_strategystrategy=SUBMODULE_UPDATE_STRATEGY_INIT;-switch(ud->update_strategy.type){+if(ud->update_strategy.type==SM_UPDATE_UNSPECIFIED||ud->just_cloned)+determine_submodule_update_strategy(the_repository,ud->just_cloned,+ud->sm_path,NULL,&strategy);+else+strategy=ud->update_strategy;++cp.dir=xstrdup(ud->sm_path);+switch(strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_UPDATE_COMMAND:-git_cmd=0;-strvec_push(&args,ud->update_strategy.command);+cp.git_cmd=0;+cp.use_shell=1;+strvec_push(&cp.args,strategy.command);must_die_on_failure=1;break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&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)){-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Unable to checkout '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_REBASE:-printf(_("Unable to rebase '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_MERGE:-printf(_("Unable to merge '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_COMMAND:-printf(_("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));+prepare_submodule_repo_env(&cp.env_array);+if(capture_command(&cp,&out,0)){+if(must_die_on_failure){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+die(_("Unable to checkout '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_REBASE:+die(_("Unable to rebase '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_MERGE:+die(_("Unable to merge '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_COMMAND:+die(_("Execution of '%s %s' failed in submodule path '%s'"),+strategy.command,oid,ud->displaypath);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/-if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/++/* the command failed, but update must continue */+string_list_append(err,out.buf);return1;}-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Submodule path '%s': checked out '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_REBASE:-printf(_("Submodule path '%s': rebased into '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_MERGE:-printf(_("Submodule path '%s': merged in '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_COMMAND:-printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,ud->update_strategy.command,oid);-break;-default:-BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+if(!ud->quiet){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+printf(_("Submodule path '%s': checked out '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_REBASE:+printf(_("Submodule path '%s': rebased into '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_MERGE:+printf(_("Submodule path '%s': merged in '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_COMMAND:+printf(_("Submodule path '%s': '%s %s'\n"),+ud->displaypath,strategy.command,oid);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintdo_run_update_procedure(structupdate_data*ud,structstring_list*err){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2500,7 +2549,7 @@ static int do_run_update_procedure(struct update_data *ud)ud->displaypath,oid_to_hex(&ud->oid));}-returnrun_update_command(ud,subforce);+returnrun_update_command(ud,subforce,err);}staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)
@@ -2605,6 +2654,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)intforce=0,quiet=0,nofetch=0,just_cloned=0;char*prefixed_path,*update=NULL;structupdate_dataupdate_data=UPDATE_DATA_INIT;+structstring_listerr=STRING_LIST_INIT_DUP;structoptionoptions[]={OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),
@@ -3038,6 +3088,291 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+constchar*update=submodule_strategy_to_string(&update_data->update_strategy);++strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+if(update_data->prefix)+strvec_pushl(args,"--prefix",update_data->prefix,NULL);+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)+strvec_pushl(args,"--update",update,NULL);+if(update_data->references.nr){+structstring_list_item*item;+for_each_string_list_item(item,&update_data->references)+strvec_pushl(args,"--reference",item->string,NULL);+}+if(update_data->recommend_shallow==0)+strvec_push(args,"--no-recommend-shallow");+elseif(update_data->recommend_shallow==1)+strvec_push(args,"--recommend-shallow");+if(update_data->single_branch>=0)+strvec_push(args,"--single-branch");+}++staticintupdate_submodule(structupdate_data*update_data)+{+char*prefixed_path;+structstring_listerr=STRING_LIST_INIT_DUP;++do_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);++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);+constchar*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(do_run_update_procedure(update_data,&err))+return1;++if(update_data->recursive){+intres;+structchild_processcp=CHILD_PROCESS_INIT;+structupdate_datanext=*update_data;+char*die_msg=xstrfmt(_("Failed to recurse into submodule path '%s'"),+update_data->displaypath);++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 */+if((res=run_command(&cp))==128)+die("%s",die_msg);+if(res)+string_list_append(&err,die_msg);++free(die_msg);+}++if(err.nr){+structstring_list_item*item;+for_each_string_list_item(item,&err)+fputs(item->string,stderr);+return1;+}++return0;+}++staticintupdate_submodules(structupdate_data*update_data)+{+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;++update_clone_from_update_data(&suc,update_data);+run_processes_parallel_tr2(suc.max_jobs,update_clone_get_next_task,+update_clone_start_failure,+update_clone_task_finished,&suc,"submodule",+"parallel/update");++/*+*Wesavedtheoutputandputitoutallatoncenow.+*Thatmeans:+*-thelistenerdoesnothavetointerleavetheir(checkout)+*workwithourfetching.Thewritesinvolvedina+*checkoutinvolvemorestraightforwardsequentialI/O.+*-thelistenercanavoiddoinganyworkiffetchingfailed.+*/+if(suc.quickstop){+string_list_clear(&update_data->references,0);+return1;+}++for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];++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;+}++string_list_clear(&update_data->references,0);+returnres;+}++staticintmodule_update(intargc,constchar**argv,constchar*prefix)+{+intinit=0,force=0,quiet=0,nofetch=0;+intremote=0,recursive=0,dissociate=0;+intprogress=0,require_init=0;+constchar*update=NULL;+structpathspecpathspec;+structupdate_dataupdate_data=UPDATE_DATA_INIT;++structoptionmodule_update_clone_options[]={+OPT__FORCE(&force,N_("force checkout updates"),0),+OPT_BOOL(0,"init",&init,+N_("initialize uninitialized submodules before update")),+OPT_BOOL(0,"remote",&remote,+N_("use SHA-1 of submodule's remote tracking branch")),+OPT_BOOL(0,"recursive",&recursive,+N_("traverse submodules recursively")),+OPT_BOOL('N',"no-fetch",&nofetch,+N_("don't fetch new objects from the remote site")),+OPT_STRING(0,"prefix",&prefix,+N_("path"),+N_("path into the working tree")),+OPT_STRING(0,"recursive-prefix",&update_data.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",&update_data.references,N_("repo"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,+N_("use --reference only while cloning")),+OPT_INTEGER(0,"depth",&update_data.depth,+N_("create a shallow clone truncated to the "+"specified number of revisions")),+OPT_INTEGER('j',"jobs",&update_data.max_jobs,+N_("parallel jobs")),+OPT_BOOL(0,"recommend-shallow",&update_data.recommend_shallow,+N_("whether the initial clone should follow the shallow recommendation")),+OPT__QUIET(&quiet,N_("don't print cloning progress")),+OPT_BOOL(0,"progress",&progress,+N_("force cloning progress")),+OPT_BOOL(0,"require-init",&require_init,+N_("disallow cloning into non-empty directory")),+OPT_BOOL(0,"single-branch",&update_data.single_branch,+N_("clone only one branch, HEAD or --branch")),+OPT_END()+};++constchar*constgit_submodule_helper_usage[]={+N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),+NULL+};++update_clone_config_from_gitmodules(&update_data.max_jobs);+git_config(git_update_clone_config,&update_data.max_jobs);++argc=parse_options(argc,argv,prefix,module_update_clone_options,+git_submodule_helper_usage,0);+update_data.prefix=prefix;++update_data.force=!!force;+update_data.quiet=!!quiet;+update_data.nofetch=!!nofetch;+update_data.init=!!init;+update_data.require_init=!!require_init;+update_data.remote=!!remote;+update_data.recursive=!!recursive;+update_data.progress=!!progress;+update_data.dissociate=!!dissociate;+oidcpy(&update_data.oid,null_oid());+oidcpy(&update_data.suboid,null_oid());++if(update)+if(parse_submodule_update_strategy(update,+&update_data.update_strategy)<0)+die(_("bad value for update parameter"));++if(module_list_compute(argc,argv,prefix,&pathspec,&update_data.list)<0)+return1;++if(pathspec.nr)+update_data.warn_if_uninitialized=1;++if(update_data.init){+structmodule_listlist=MODULE_LIST_INIT;+structinit_cbinfo=INIT_CB_INIT;++if(module_list_compute(argc,argv,update_data.prefix,+&pathspec,&list)<0)+return1;++/*+*Iftherearenopathargsandsubmodule.activeissetthen,+*bydefault,onlyinitialize'active'modules.+*/+if(!argc&&git_config_get_value_multi("submodule.active"))+module_list_active(&list);++info.prefix=update_data.prefix;+info.superprefix=update_data.recursive_prefix;+if(update_data.quiet)+info.flags|=OPT_QUIET;++for_each_listed_submodule(&list,init_submodule_cb,&info);+}++returnupdate_submodules(&update_data);+}+structadd_data{constchar*prefix;constchar*branch;
@@ -369,20 +369,6 @@ cmd_deinit()git${wt_prefix:+-C "$wt_prefix"}submodule--helperdeinit${GIT_QUIET:+--quiet}${force:+--force}${deinit_all:+--all}--"$@"}-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]-# Because arguments are positional, use an empty string to omit <depth>-# but include <sha1>.-fetch_in_submodule()(-sanitize_submodule_env&&-cd"$1"&&-iftest$#-eq3-then-echo"$3"|gitfetch${GIT_QUIET:+--quiet}--stdin${2:+"$2"}-else-gitfetch${GIT_QUIET:+--quiet}${2:+"$2"}-fi-)-## Update each submodule path to correct revision, using clone and checkout as needed#
@@ -484,133 +470,26 @@ cmd_update()shiftdone-iftest-n"$init"-then-cmd_init"--""$@"||return-fi--{-gitsubmodule--helperupdate-clone${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+${GIT_QUIET:+--quiet}\+${force:+--force}\+${progress:+--progress}\+${dissociate:+--dissociate}\+${remote:+--remote}\+${recursive:+--recursive}\+${init:+--init}\+${require_init:+--require-init}\+${nofetch:+--no-fetch}\${wt_prefix:+--prefix "$wt_prefix"}\${prefix:+--recursive-prefix "$prefix"}\${update:+--update "$update"}\${reference:+"$reference"}\-${dissociate:+"--dissociate"}\-${depth:+--depth "$depth"}\-${require_init:+--require-init}\+${depth:+"$depth"}\$single_branch\$recommend_shallow\$jobs\--\-"$@"||echo"#unmatched"$?-}|{-err=-whileread-rquickabortsha1just_clonedsm_path-do-die_if_unmatched"$quickabort""$sha1"--gitsubmodule--helperensure-core-worktree"$sm_path"||exit1--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq1-then-subsha1=-else-just_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"-fi--iftest-n"$remote"-then-branch=$(gitsubmodule--helperremote-branch"$sm_path")-iftest-z"$nofetch"-then-# Fetch remote before determining tracking $sha1-fetch_in_submodule"$sm_path"$depth||-die"fatal: $(eval_gettext"Unable to fetch in submodule path '\$sm_path'")"-fi-remote_name=$(sanitize_submodule_env;cd"$sm_path"&&gitsubmodule--helperprint-default-remote)-sha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verify"${remote_name}/${branch}")||-die"fatal: $(eval_gettext"Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"-fi--out=$(gitsubmodule--helperrun-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"}\-${subsha1:+--suboid "$subsha1"}\-"--"\-"$sm_path")--# 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-# 3: no update procedure was run-res="$?"-case$resin-0)-say"$out"-;;-1)-err="${err};fatal: $out"-continue-;;-2|128)-die_with_status$res"fatal: $out"-;;-esac--iftest-n"$recursive"-then-(-prefix=$(gitsubmodule--helperrelative-path"$prefix$sm_path/""$wt_prefix")-wt_prefix=-sanitize_submodule_env-cd"$sm_path"&&-evalcmd_update-)-res=$?-iftest$res-gt0-then-die_msg="fatal: $(eval_gettext"Failed to recurse into submodule path '\$displaypath'")"-iftest$res-ne2-then-err="${err};$die_msg"-continue-else-die_with_status$res"$die_msg"-fi-fi-fi-done--iftest-n"$err"-then-OIFS=$IFS-IFS=';'-forein$err-do-iftest-n"$e"-then-echo>&2"$e"-fi-done-IFS=$OIFS-exit1-fi-}+"$@"}#
We no longer need this subcommand as the shell version calls the
'update' subcommand instead, which does all the cloning within C itself.
We also no longer need the 'update_clone_submodules()' and
'update_clone_submodule()' functions, so we remove those as well.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 98 -------------------------------------
1 file changed, 98 deletions(-)
@@ -2552,103 +2552,6 @@ static int do_run_update_procedure(struct update_data *ud, struct string_list *ereturnrun_update_command(ud,subforce,err);}-staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)-{-fprintf(stdout,"dummy %s %d\t%s\n",-oid_to_hex(&ucd->oid),-ucd->just_cloned,-ucd->sub->path);-}--staticintupdate_clone_submodules(structsubmodule_update_clone*suc)-{-inti;--run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,-update_clone_start_failure,-update_clone_task_finished,suc,"submodule",-"parallel/update");--/*-*Wesavedtheoutputandputitoutallatoncenow.-*Thatmeans:-*-thelistenerdoesnothavetointerleavetheir(checkout)-*workwithourfetching.Thewritesinvolvedina-*checkoutinvolvemorestraightforwardsequentialI/O.-*-thelistenercanavoiddoinganyworkiffetchingfailed.-*/-if(suc->quickstop)-return1;--for(i=0;i<suc->update_clone_nr;i++)-update_clone_submodule(&suc->update_clone[i]);--return0;-}--staticintupdate_clone(intargc,constchar**argv,constchar*prefix)-{-constchar*update=NULL;-structpathspecpathspec;-structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;--structoptionmodule_update_clone_options[]={-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"recursive-prefix",&suc.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"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&suc.dissociate,-N_("use --reference only while cloning")),-OPT_STRING(0,"depth",&suc.depth,"<depth>",-N_("create a shallow clone truncated to the "-"specified number of revisions")),-OPT_INTEGER('j',"jobs",&suc.max_jobs,-N_("parallel jobs")),-OPT_BOOL(0,"recommend-shallow",&suc.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,-N_("force cloning progress")),-OPT_BOOL(0,"require-init",&suc.require_init,-N_("disallow cloning into non-empty directory")),-OPT_BOOL(0,"single-branch",&suc.single_branch,-N_("clone only one branch, HEAD or --branch")),-OPT_END()-};--constchar*constgit_submodule_helper_usage[]={-N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),-NULL-};-suc.prefix=prefix;--update_clone_config_from_gitmodules(&suc.max_jobs);-git_config(git_update_clone_config,&suc.max_jobs);--argc=parse_options(argc,argv,prefix,module_update_clone_options,-git_submodule_helper_usage,0);--if(update)-if(parse_submodule_update_strategy(update,&suc.update)<0)-die(_("bad value for update parameter"));--if(module_list_compute(argc,argv,prefix,&pathspec,&suc.list)<0)-return1;--if(pathspec.nr)-suc.warn_if_uninitialized=1;--returnupdate_clone_submodules(&suc);-}-staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix){intforce=0,quiet=0,nofetch=0,just_cloned=0;
These helpers were useful back when 'submodule update' had most of its
logic in shell. Now that they will never be invoked, let us remove them.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 135 ------------------------------------
1 file changed, 135 deletions(-)
@@ -2552,85 +2514,6 @@ static int do_run_update_procedure(struct update_data *ud, struct string_list *ereturnrun_update_command(ud,subforce,err);}-staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,nofetch=0,just_cloned=0;-char*prefixed_path,*update=NULL;-structupdate_dataupdate_data=UPDATE_DATA_INIT;-structstring_listerr=STRING_LIST_INIT_DUP;--structoptionoptions[]={-OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),-OPT__FORCE(&force,N_("force checkout updates"),0),-OPT_BOOL('N',"no-fetch",&nofetch,-N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&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,-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_CALLBACK_F(0,"suboid",&update_data.suboid,N_("subsha1"),-N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_END()-};--constchar*constusage[]={-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.force=!!force;-update_data.quiet=!!quiet;-update_data.nofetch=!!nofetch;-update_data.just_cloned=!!just_cloned;-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,-&update_data.update_strategy);--free(prefixed_path);--if(!oideq(&update_data.oid,&update_data.suboid)||update_data.force)-returndo_run_update_procedure(&update_data,&err);--return3;-}--staticintresolve_relative_path(intargc,constchar**argv,constchar*prefix)-{-structstrbufsb=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);-return0;-}-staticconstchar*remote_submodule_branch(constchar*path){conststructsubmodule*sub;
These two functions were prefixed with 'do' before the shell-to-C
conversion because they were utility functions meant to be called by
their non-prefixed counterpart.
Since those callers don't exist anymore, and these functions can now be
used directly, let's rename them to signal this fact.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: Junio C Hamano <hidden> Date: 2021-09-20 21:30:07
Atharva Raykar [off-list ref] writes:
+static int update_submodules(struct update_data *update_data)
+{
+ int i, res = 0;
+ struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
+
+ update_clone_from_update_data(&suc, update_data);
+ run_processes_parallel_tr2(suc.max_jobs, update_clone_get_next_task,
+ update_clone_start_failure,
+ update_clone_task_finished, &suc, "submodule",
+ "parallel/update");
+ ...
As ab/config-based-hooks-base topic from Ævar changes the way this
helper function gets called at 73367f2f (run-command: add stdin
callback for parallelization, 2021-09-02) and then again in 2aba2f5f
(run-command: allow capturing of collated output, 2021-09-02), this
part needs to be adjusted when the topics collide in 'seen'.
I _think_ I've resolved conflict correctly, but please double-check
the result when today's integration result is pushed out later, both
of you.
Thanks.
This call (and I think there is another call in this file) to
repo_submodule_init() is affected by what Jonathan's 8eb8dcf9
(repository: support unabsorbed in repo_submodule_init, 2021-09-09)
wants to do, namely to lose "struct submodule sub" as a parameter
and instead take the path to the module and the treeish name as
parameters to repo_submodule_init().
I _think_ I resolved the conflict correctly, but please double check
the result when it is pushed out later today, both of you.
Thanks.
This call (and I think there is another call in this file) to
repo_submodule_init() is affected by what Jonathan's 8eb8dcf9
(repository: support unabsorbed in repo_submodule_init, 2021-09-09)
wants to do, namely to lose "struct submodule sub" as a parameter
and instead take the path to the module and the treeish name as
parameters to repo_submodule_init().
I _think_ I resolved the conflict correctly, but please double check
the result when it is pushed out later today, both of you.
Thanks.
Thanks for calling this out. There are indeed 2 such calls in 33cfc43433
("Merge branch 'ar/submodule-update' into seen", 2021-09-20) and both
look correct.
Let's split up `ensure_core_worktree()` so that we can call it from C
code without needing to deal with command line arguments.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
@@ -2764,17 +2764,11 @@ static int push_check(int argc, const char **argv, const char *prefix)return0;}-staticintensure_core_worktree(intargc,constchar**argv,constchar*prefix)+staticvoiddo_ensure_core_worktree(constchar*path){-constchar*path;constchar*cw;structrepositorysubrepo;-if(argc!=2)-BUG("submodule--helper ensure-core-worktree <path>");--path=argv[1];-if(repo_submodule_init(&subrepo,the_repository,path,null_oid()))die(_("could not get a repository handle for submodule '%s'"),path);
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 39 +++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
@@ -1382,21 +1397,15 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+default_remote=get_default_remote_submodule(path);+if(!default_remote)die(_("failed to get the default remote for submodule '%s'"),path);-strbuf_strip_suffix(&sb,"\n");-remote_key=xstrfmt("remote.%s.url",sb.buf);+remote_key=xstrfmt("remote.%s.url",default_remote);+free(default_remote);-strbuf_reset(&sb);submodule_to_gitdir(&sb,path);strbuf_addstr(&sb,"/config");
The `update-clone` subcommand helpers that perform the parallel clone
and printing to stdout for shell script consumption, are renamed.
This lets us use the names `update_submodules()` and
`update_submodule()` for the helpers in the next patch, when we create
an `update` subcommand that does a full conversion.
We will get rid of these helpers in a cleanup patch at the end of this
series, when the `update-clone` command is no longer useful to us.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -261,11 +261,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -281,6 +278,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
These helpers were useful back when 'submodule update' had most of its
logic in shell. Now that they will never be invoked, let us remove them.
We also no longer need the 'update_clone_submodules()' and
'update_clone_submodule()' functions, so we remove those as well.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 233 ------------------------------------
1 file changed, 233 deletions(-)
@@ -2518,182 +2480,6 @@ static int do_run_update_procedure(struct update_data *ud, struct string_list *ereturnrun_update_command(ud,subforce,err);}-staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)-{-fprintf(stdout,"dummy %s %d\t%s\n",-oid_to_hex(&ucd->oid),-ucd->just_cloned,-ucd->sub->path);-}--staticintupdate_clone_submodules(structsubmodule_update_clone*suc)-{-inti;--run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,-update_clone_start_failure,-update_clone_task_finished,suc,"submodule",-"parallel/update");--/*-*Wesavedtheoutputandputitoutallatoncenow.-*Thatmeans:-*-thelistenerdoesnothavetointerleavetheir(checkout)-*workwithourfetching.Thewritesinvolvedina-*checkoutinvolvemorestraightforwardsequentialI/O.-*-thelistenercanavoiddoinganyworkiffetchingfailed.-*/-if(suc->quickstop)-return1;--for(i=0;i<suc->update_clone_nr;i++)-update_clone_submodule(&suc->update_clone[i]);--return0;-}--staticintupdate_clone(intargc,constchar**argv,constchar*prefix)-{-constchar*update=NULL;-structpathspecpathspec;-structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;--structoptionmodule_update_clone_options[]={-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"recursive-prefix",&suc.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"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&suc.dissociate,-N_("use --reference only while cloning")),-OPT_STRING(0,"depth",&suc.depth,"<depth>",-N_("create a shallow clone truncated to the "-"specified number of revisions")),-OPT_INTEGER('j',"jobs",&suc.max_jobs,-N_("parallel jobs")),-OPT_BOOL(0,"recommend-shallow",&suc.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,-N_("force cloning progress")),-OPT_BOOL(0,"require-init",&suc.require_init,-N_("disallow cloning into non-empty directory")),-OPT_BOOL(0,"single-branch",&suc.single_branch,-N_("clone only one branch, HEAD or --branch")),-OPT_END()-};--constchar*constgit_submodule_helper_usage[]={-N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),-NULL-};-suc.prefix=prefix;--update_clone_config_from_gitmodules(&suc.max_jobs);-git_config(git_update_clone_config,&suc.max_jobs);--argc=parse_options(argc,argv,prefix,module_update_clone_options,-git_submodule_helper_usage,0);--if(update)-if(parse_submodule_update_strategy(update,&suc.update)<0)-die(_("bad value for update parameter"));--if(module_list_compute(argc,argv,prefix,&pathspec,&suc.list)<0)-return1;--if(pathspec.nr)-suc.warn_if_uninitialized=1;--returnupdate_clone_submodules(&suc);-}--staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,nofetch=0,just_cloned=0;-char*prefixed_path,*update=NULL;-structupdate_dataupdate_data=UPDATE_DATA_INIT;-structstring_listerr=STRING_LIST_INIT_DUP;--structoptionoptions[]={-OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),-OPT__FORCE(&force,N_("force checkout updates"),0),-OPT_BOOL('N',"no-fetch",&nofetch,-N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&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,-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_CALLBACK_F(0,"suboid",&update_data.suboid,N_("subsha1"),-N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_END()-};--constchar*constusage[]={-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.force=!!force;-update_data.quiet=!!quiet;-update_data.nofetch=!!nofetch;-update_data.just_cloned=!!just_cloned;-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,-&update_data.update_strategy);--free(prefixed_path);--if(!oideq(&update_data.oid,&update_data.suboid)||update_data.force)-returndo_run_update_procedure(&update_data,&err);--return3;-}--staticintresolve_relative_path(intargc,constchar**argv,constchar*prefix)-{-structstrbufsb=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);-return0;-}-staticconstchar*remote_submodule_branch(constchar*path){conststructsubmodule*sub;
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 [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
@@ -2342,47 +2342,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strstaticintrun_update_command(structupdate_data*ud,intsubforce){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);intmust_die_on_failure=0;-intgit_cmd;switch(ud->update_strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_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){caseSM_UPDATE_CHECKOUT:printf(_("Unable to checkout '%s' in submodule path '%s'"),
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules()` and `update_submodule()` which
are quite similar to `update_clone_submodules()` and
`update_clone_submodule()`, and will supersede them.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
While we are at it, we also remove the fetch_in_submodule() shell
function since it is no longer used anywhere.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 462 +++++++++++++++++++++++++++++++-----
git-submodule.sh | 145 +----------
2 files changed, 410 insertions(+), 197 deletions(-)
@@ -2010,7 +2010,6 @@ struct submodule_update_clone {constchar*prefix;intsingle_branch;-/* to be consumed by git-submodule.sh */structupdate_clone_data*update_clone;intupdate_clone_nr;intupdate_clone_alloc;
@@ -2340,13 +2383,21 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strreturnrun_command(&cp);}-staticintrun_update_command(structupdate_data*ud,intsubforce)+staticintrun_update_command(structupdate_data*ud,intsubforce,structstring_list*err){structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);+structstrbufout=STRBUF_INIT;intmust_die_on_failure=0;+structsubmodule_update_strategystrategy=SUBMODULE_UPDATE_STRATEGY_INIT;-switch(ud->update_strategy.type){+if(ud->update_strategy.type==SM_UPDATE_UNSPECIFIED||ud->just_cloned)+determine_submodule_update_strategy(the_repository,ud->just_cloned,+ud->sm_path,NULL,&strategy);+else+strategy=ud->update_strategy;++switch(strategy.type){caseSM_UPDATE_CHECKOUT:cp.git_cmd=1;strvec_pushl(&cp.args,"checkout","-q",NULL);
@@ -2369,80 +2420,75 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:cp.use_shell=1;-strvec_push(&cp.args,ud->update_strategy.command);+strvec_push(&cp.args,strategy.command);must_die_on_failure=1;break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}strvec_push(&cp.args,oid);cp.dir=xstrdup(ud->sm_path);prepare_submodule_repo_env(&cp.env_array);-if(run_command(&cp)){-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Unable to checkout '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_REBASE:-printf(_("Unable to rebase '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_MERGE:-printf(_("Unable to merge '%s' in submodule path '%s'"),-oid,ud->displaypath);-break;-caseSM_UPDATE_COMMAND:-printf(_("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));+if(capture_command(&cp,&out,0)){+if(must_die_on_failure){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+die(_("Unable to checkout '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_REBASE:+die(_("Unable to rebase '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_MERGE:+die(_("Unable to merge '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_COMMAND:+die(_("Execution of '%s %s' failed in submodule path '%s'"),+strategy.command,oid,ud->displaypath);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/-if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/++/* the command failed, but update must continue */+string_list_append(err,out.buf);return1;}-switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Submodule path '%s': checked out '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_REBASE:-printf(_("Submodule path '%s': rebased into '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_MERGE:-printf(_("Submodule path '%s': merged in '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_COMMAND:-printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,ud->update_strategy.command,oid);-break;-default:-BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+if(!ud->quiet){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+printf(_("Submodule path '%s': checked out '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_REBASE:+printf(_("Submodule path '%s': rebased into '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_MERGE:+printf(_("Submodule path '%s': merged in '%s'\n"),+ud->displaypath,oid);+break;+caseSM_UPDATE_COMMAND:+printf(_("Submodule path '%s': '%s %s'\n"),+ud->displaypath,strategy.command,oid);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}}return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintdo_run_update_procedure(structupdate_data*ud,structstring_list*err){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2469,7 +2515,7 @@ static int do_run_update_procedure(struct update_data *ud)ud->displaypath,oid_to_hex(&ud->oid));}-returnrun_update_command(ud,subforce);+returnrun_update_command(ud,subforce,err);}staticvoidupdate_clone_submodule(structupdate_clone_data*ucd)
@@ -2574,6 +2620,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)intforce=0,quiet=0,nofetch=0,just_cloned=0;char*prefixed_path,*update=NULL;structupdate_dataupdate_data=UPDATE_DATA_INIT;+structstring_listerr=STRING_LIST_INIT_DUP;structoptionoptions[]={OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),
@@ -3002,6 +3049,291 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+constchar*update=submodule_strategy_to_string(&update_data->update_strategy);++strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+if(update_data->prefix)+strvec_pushl(args,"--prefix",update_data->prefix,NULL);+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)+strvec_pushl(args,"--update",update,NULL);+if(update_data->references.nr){+structstring_list_item*item;+for_each_string_list_item(item,&update_data->references)+strvec_pushl(args,"--reference",item->string,NULL);+}+if(update_data->recommend_shallow==0)+strvec_push(args,"--no-recommend-shallow");+elseif(update_data->recommend_shallow==1)+strvec_push(args,"--recommend-shallow");+if(update_data->single_branch>=0)+strvec_push(args,"--single-branch");+}++staticintupdate_submodule(structupdate_data*update_data)+{+char*prefixed_path;+structstring_listerr=STRING_LIST_INIT_DUP;++do_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);++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);+constchar*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(do_run_update_procedure(update_data,&err))+return1;++if(update_data->recursive){+intres;+structchild_processcp=CHILD_PROCESS_INIT;+structupdate_datanext=*update_data;+char*die_msg=xstrfmt(_("Failed to recurse into submodule path '%s'"),+update_data->displaypath);++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 */+if((res=run_command(&cp))==128)+die("%s",die_msg);+if(res)+string_list_append(&err,die_msg);++free(die_msg);+}++if(err.nr){+structstring_list_item*item;+for_each_string_list_item(item,&err)+fputs(item->string,stderr);+return1;+}++return0;+}++staticintupdate_submodules(structupdate_data*update_data)+{+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;++update_clone_from_update_data(&suc,update_data);+run_processes_parallel_tr2(suc.max_jobs,update_clone_get_next_task,+update_clone_start_failure,+update_clone_task_finished,&suc,"submodule",+"parallel/update");++/*+*Wesavedtheoutputandputitoutallatoncenow.+*Thatmeans:+*-thelistenerdoesnothavetointerleavetheir(checkout)+*workwithourfetching.Thewritesinvolvedina+*checkoutinvolvemorestraightforwardsequentialI/O.+*-thelistenercanavoiddoinganyworkiffetchingfailed.+*/+if(suc.quickstop){+string_list_clear(&update_data->references,0);+return1;+}++for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];++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;+}++string_list_clear(&update_data->references,0);+returnres;+}++staticintmodule_update(intargc,constchar**argv,constchar*prefix)+{+intinit=0,force=0,quiet=0,nofetch=0;+intremote=0,recursive=0,dissociate=0;+intprogress=0,require_init=0;+constchar*update=NULL;+structpathspecpathspec;+structupdate_dataupdate_data=UPDATE_DATA_INIT;++structoptionmodule_update_clone_options[]={+OPT__FORCE(&force,N_("force checkout updates"),0),+OPT_BOOL(0,"init",&init,+N_("initialize uninitialized submodules before update")),+OPT_BOOL(0,"remote",&remote,+N_("use SHA-1 of submodule's remote tracking branch")),+OPT_BOOL(0,"recursive",&recursive,+N_("traverse submodules recursively")),+OPT_BOOL('N',"no-fetch",&nofetch,+N_("don't fetch new objects from the remote site")),+OPT_STRING(0,"prefix",&prefix,+N_("path"),+N_("path into the working tree")),+OPT_STRING(0,"recursive-prefix",&update_data.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",&update_data.references,N_("repo"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,+N_("use --reference only while cloning")),+OPT_INTEGER(0,"depth",&update_data.depth,+N_("create a shallow clone truncated to the "+"specified number of revisions")),+OPT_INTEGER('j',"jobs",&update_data.max_jobs,+N_("parallel jobs")),+OPT_BOOL(0,"recommend-shallow",&update_data.recommend_shallow,+N_("whether the initial clone should follow the shallow recommendation")),+OPT__QUIET(&quiet,N_("don't print cloning progress")),+OPT_BOOL(0,"progress",&progress,+N_("force cloning progress")),+OPT_BOOL(0,"require-init",&require_init,+N_("disallow cloning into non-empty directory")),+OPT_BOOL(0,"single-branch",&update_data.single_branch,+N_("clone only one branch, HEAD or --branch")),+OPT_END()+};++constchar*constgit_submodule_helper_usage[]={+N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),+NULL+};++update_clone_config_from_gitmodules(&update_data.max_jobs);+git_config(git_update_clone_config,&update_data.max_jobs);++argc=parse_options(argc,argv,prefix,module_update_clone_options,+git_submodule_helper_usage,0);+update_data.prefix=prefix;++update_data.force=!!force;+update_data.quiet=!!quiet;+update_data.nofetch=!!nofetch;+update_data.init=!!init;+update_data.require_init=!!require_init;+update_data.remote=!!remote;+update_data.recursive=!!recursive;+update_data.progress=!!progress;+update_data.dissociate=!!dissociate;+oidcpy(&update_data.oid,null_oid());+oidcpy(&update_data.suboid,null_oid());++if(update)+if(parse_submodule_update_strategy(update,+&update_data.update_strategy)<0)+die(_("bad value for update parameter"));++if(module_list_compute(argc,argv,prefix,&pathspec,&update_data.list)<0)+return1;++if(pathspec.nr)+update_data.warn_if_uninitialized=1;++if(update_data.init){+structmodule_listlist=MODULE_LIST_INIT;+structinit_cbinfo=INIT_CB_INIT;++if(module_list_compute(argc,argv,update_data.prefix,+&pathspec,&list)<0)+return1;++/*+*Iftherearenopathargsandsubmodule.activeissetthen,+*bydefault,onlyinitialize'active'modules.+*/+if(!argc&&git_config_get_value_multi("submodule.active"))+module_list_active(&list);++info.prefix=update_data.prefix;+info.superprefix=update_data.recursive_prefix;+if(update_data.quiet)+info.flags|=OPT_QUIET;++for_each_listed_submodule(&list,init_submodule_cb,&info);+}++returnupdate_submodules(&update_data);+}+structadd_data{constchar*prefix;constchar*branch;
@@ -246,20 +246,6 @@ cmd_deinit()git${wt_prefix:+-C "$wt_prefix"}submodule--helperdeinit${GIT_QUIET:+--quiet}${force:+--force}${deinit_all:+--all}--"$@"}-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]-# Because arguments are positional, use an empty string to omit <depth>-# but include <sha1>.-fetch_in_submodule()(-sanitize_submodule_env&&-cd"$1"&&-iftest$#-eq3-then-echo"$3"|gitfetch${GIT_QUIET:+--quiet}--stdin${2:+"$2"}-else-gitfetch${GIT_QUIET:+--quiet}${2:+"$2"}-fi-)-## Update each submodule path to correct revision, using clone and checkout as needed#
@@ -361,133 +347,26 @@ cmd_update()shiftdone-iftest-n"$init"-then-cmd_init"--""$@"||return-fi--{-gitsubmodule--helperupdate-clone${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+${GIT_QUIET:+--quiet}\+${force:+--force}\+${progress:+--progress}\+${dissociate:+--dissociate}\+${remote:+--remote}\+${recursive:+--recursive}\+${init:+--init}\+${require_init:+--require-init}\+${nofetch:+--no-fetch}\${wt_prefix:+--prefix "$wt_prefix"}\${prefix:+--recursive-prefix "$prefix"}\${update:+--update "$update"}\${reference:+"$reference"}\-${dissociate:+"--dissociate"}\-${depth:+--depth "$depth"}\-${require_init:+--require-init}\+${depth:+"$depth"}\$single_branch\$recommend_shallow\$jobs\--\-"$@"||echo"#unmatched"$?-}|{-err=-whileread-rquickabortsha1just_clonedsm_path-do-die_if_unmatched"$quickabort""$sha1"--gitsubmodule--helperensure-core-worktree"$sm_path"||exit1--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq1-then-subsha1=-else-just_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"-fi--iftest-n"$remote"-then-branch=$(gitsubmodule--helperremote-branch"$sm_path")-iftest-z"$nofetch"-then-# Fetch remote before determining tracking $sha1-fetch_in_submodule"$sm_path"$depth||-die"fatal: $(eval_gettext"Unable to fetch in submodule path '\$sm_path'")"-fi-remote_name=$(sanitize_submodule_env;cd"$sm_path"&&gitsubmodule--helperprint-default-remote)-sha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verify"${remote_name}/${branch}")||-die"fatal: $(eval_gettext"Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"-fi--out=$(gitsubmodule--helperrun-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"}\-${subsha1:+--suboid "$subsha1"}\-"--"\-"$sm_path")--# 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-# 3: no update procedure was run-res="$?"-case$resin-0)-say"$out"-;;-1)-err="${err};fatal: $out"-continue-;;-2|128)-die_with_status$res"fatal: $out"-;;-esac--iftest-n"$recursive"-then-(-prefix=$(gitsubmodule--helperrelative-path"$prefix$sm_path/""$wt_prefix")-wt_prefix=-sanitize_submodule_env-cd"$sm_path"&&-evalcmd_update-)-res=$?-iftest$res-gt0-then-die_msg="fatal: $(eval_gettext"Failed to recurse into submodule path '\$displaypath'")"-iftest$res-ne2-then-err="${err};$die_msg"-continue-else-die_with_status$res"$die_msg"-fi-fi-fi-done--iftest-n"$err"-then-OIFS=$IFS-IFS=';'-forein$err-do-iftest-n"$e"-then-echo>&2"$e"-fi-done-IFS=$OIFS-exit1-fi-}+"$@"}#
We allow callers of the `init_submodule()` function to optionally
override the superprefix from the environment.
We need to enable this option because in our conversion of the update
command that will follow, the '--init' option will be handled through
this API. We will need to change the superprefix at that time to ensure
the display paths show correctly in the output messages.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -606,18 +606,22 @@ static int module_foreach(int argc, const char **argv, const char *prefix)structinit_cb{constchar*prefix;+constchar*superprefix;unsignedintflags;};-#define INIT_CB_INIT { NULL, 0 }+#define INIT_CB_INIT { 0 }staticvoidinit_submodule(constchar*path,constchar*prefix,-unsignedintflags)+constchar*superprefix,unsignedintflags){conststructsubmodule*sub;structstrbufsb=STRBUF_INIT;char*upd=NULL,*url=NULL,*displaypath;-displaypath=get_submodule_displaypath(path,prefix);+/* try superprefix from the environment, if it is not passed explicitly */+if(!superprefix)+superprefix=get_super_prefix();+displaypath=do_get_submodule_displaypath(path,prefix,superprefix);sub=submodule_from_path(the_repository,null_oid(),path);
These two functions were prefixed with 'do' before the shell-to-C
conversion because they were utility functions meant to be called by
their non-prefixed counterpart.
Since those callers don't exist anymore, and these functions can now be
used directly, let's rename them to signal this fact.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
---
builtin/submodule--helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'.
Hey Atharva! I'm working on a series that will teach "git branch" how to
handle "--recurse-submodules". I plan to do this in-process because I
think this will take less overall effort than using child processes, and
to make it happen, I'm planning to add a helper function like
"for_each_submodule()", which would call a C callback function on each
submodule.
This is conceptually similar to "git submodule foreach" and
for_each_listed_submodule() (though not exactly equivalent), so I'm
reaching out to you in case this work is already on your radar. If so,
and if it is coming soon, it might be easier to for me to base my work
off yours instead of duplicating our efforts :)
This series builds upon the previous conversion work on 'submodule update' and
moves out all of that shell logic in 'git-submodule.sh' into
'builtin/submodule--helper.c'.
Hey Atharva! I'm working on a series that will teach "git branch" how to
handle "--recurse-submodules". I plan to do this in-process because I
think this will take less overall effort than using child processes, and
to make it happen, I'm planning to add a helper function like
"for_each_submodule()", which would call a C callback function on each
submodule.
This is conceptually similar to "git submodule foreach" and
for_each_listed_submodule() (though not exactly equivalent), so I'm
reaching out to you in case this work is already on your radar. If so,
and if it is coming soon, it might be easier to for me to base my work
off yours instead of duplicating our efforts :)
Thanks for reaching out. I don't have anything like this on my radar, so
feel free to go ahead with your plan :)
From: Atharva Raykar <redacted>
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 41 +++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 15 deletions(-)
@@ -1382,21 +1399,15 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+default_remote=get_default_remote_submodule(path);+if(!default_remote)die(_("failed to get the default remote for submodule '%s'"),path);-strbuf_strip_suffix(&sb,"\n");-remote_key=xstrfmt("remote.%s.url",sb.buf);+remote_key=xstrfmt("remote.%s.url",default_remote);+free(default_remote);-strbuf_reset(&sb);submodule_to_gitdir(&sb,path);strbuf_addstr(&sb,"/config");
From: Atharva Raykar <redacted>
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -263,11 +263,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -283,6 +280,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
From: Atharva Raykar <redacted>
We allow callers of the `init_submodule()` function to optionally
override the superprefix from the environment.
We need to enable this option because in our conversion of the update
command that will follow, the '--init' option will be handled through
this API. We will need to change the superprefix at that time to ensure
the display paths show correctly in the output messages.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
@@ -608,18 +608,22 @@ static int module_foreach(int argc, const char **argv, const char *prefix)structinit_cb{constchar*prefix;+constchar*superprefix;unsignedintflags;};#define INIT_CB_INIT { 0 }staticvoidinit_submodule(constchar*path,constchar*prefix,-unsignedintflags)+constchar*superprefix,unsignedintflags){conststructsubmodule*sub;structstrbufsb=STRBUF_INIT;char*upd=NULL,*url=NULL,*displaypath;-displaypath=get_submodule_displaypath(path,prefix);+/* try superprefix from the environment, if it is not passed explicitly */+if(!superprefix)+superprefix=get_super_prefix();+displaypath=do_get_submodule_displaypath(path,prefix,superprefix);sub=submodule_from_path(the_repository,null_oid(),path);
From: Atharva Raykar <redacted>
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 [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
@@ -2346,47 +2346,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strstaticintrun_update_command(structupdate_data*ud,intsubforce){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);intmust_die_on_failure=0;-intgit_cmd;switch(ud->update_strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_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){caseSM_UPDATE_CHECKOUT:printf(_("Unable to checkout '%s' in submodule path '%s'"),
From: Atharva Raykar <redacted>
The second hunk here will make a subsequent commit's diff smaller, and
let's do the first and third hunks while we're at it so that we
consistently format all of these.
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
Rename the "suc" variable in "builtin/submodule--helper.c" to
"opt". The only reason for this change is to make the subsequent
commit's diff smaller, by doing this rename we can "anchor" the diff
better, as it "borrow" most of the options declared here as-is as far
as the diff rename detection is concerned.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
@@ -2519,36 +2519,36 @@ static int update_clone(int argc, const char **argv, const char *prefix){constchar*update=NULL;structpathspecpathspec;-structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;+structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;structoptionmodule_update_clone_options[]={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_END()};
@@ -2557,25 +2557,25 @@ static int update_clone(int argc, const char **argv, const char *prefix)N_("git submodule--helper update-clone [--prefix=<path>] [<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);argc=parse_options(argc,argv,prefix,module_update_clone_options,git_submodule_helper_usage,0);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)return1;if(pathspec.nr)-suc.warn_if_uninitialized=1;+opt.warn_if_uninitialized=1;-returnupdate_submodules(&suc);+returnupdate_submodules(&opt);}staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)
From: Atharva Raykar <redacted>
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules2()` and `update_submodule2()`
which will supersede `update_submodules()` and `update_submodule()`.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
While we are at it, we also remove the fetch_in_submodule() shell
function since it is no longer used anywhere.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 662 ++++++++++++++++++++----------------
git-submodule.sh | 145 +-------
2 files changed, 389 insertions(+), 418 deletions(-)
@@ -2017,7 +1979,6 @@ struct submodule_update_clone {constchar*prefix;intsingle_branch;-/* to be consumed by git-submodule.sh */structupdate_clone_data*update_clone;intupdate_clone_nr;intupdate_clone_alloc;
@@ -2349,13 +2352,21 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strreturnrun_command(&cp);}-staticintrun_update_command(structupdate_data*ud,intsubforce)+staticintrun_update_command(structupdate_data*ud,intsubforce,structstring_list*err){structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);+structstrbufout=STRBUF_INIT;intmust_die_on_failure=0;+structsubmodule_update_strategystrategy=SUBMODULE_UPDATE_STRATEGY_INIT;-switch(ud->update_strategy.type){+if(ud->update_strategy.type==SM_UPDATE_UNSPECIFIED||ud->just_cloned)+determine_submodule_update_strategy(the_repository,ud->just_cloned,+ud->sm_path,NULL,&strategy);+else+strategy=ud->update_strategy;++switch(strategy.type){caseSM_UPDATE_CHECKOUT:cp.git_cmd=1;strvec_pushl(&cp.args,"checkout","-q",NULL);
@@ -2378,80 +2389,76 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:cp.use_shell=1;-strvec_push(&cp.args,ud->update_strategy.command);+strvec_push(&cp.args,strategy.command);must_die_on_failure=1;break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}strvec_push(&cp.args,oid);cp.dir=xstrdup(ud->sm_path);prepare_submodule_repo_env(&cp.env_array);-if(run_command(&cp)){-switch(ud->update_strategy.type){+if(capture_command(&cp,&out,0)){+if(must_die_on_failure){+switch(strategy.type){+caseSM_UPDATE_CHECKOUT:+die(_("Unable to checkout '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_REBASE:+die(_("Unable to rebase '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_MERGE:+die(_("Unable to merge '%s' in submodule path '%s'"),+oid,ud->displaypath);+break;+caseSM_UPDATE_COMMAND:+die(_("Execution of '%s %s' failed in submodule path '%s'"),+strategy.command,oid,ud->displaypath);+break;+default:+BUG("unexpected update strategy type: %s",+submodule_strategy_to_string(&strategy));+}+}++/* the command failed, but update must continue */+string_list_append_nodup(err,strbuf_detach(&out,NULL));+return1;+}++if(!ud->quiet){+switch(strategy.type){caseSM_UPDATE_CHECKOUT:-printf(_("Unable to checkout '%s' in submodule path '%s'"),-oid,ud->displaypath);+printf(_("Submodule path '%s': checked out '%s'\n"),+ud->displaypath,oid);break;caseSM_UPDATE_REBASE:-printf(_("Unable to rebase '%s' in submodule path '%s'"),-oid,ud->displaypath);+printf(_("Submodule path '%s': rebased into '%s'\n"),+ud->displaypath,oid);break;caseSM_UPDATE_MERGE:-printf(_("Unable to merge '%s' in submodule path '%s'"),-oid,ud->displaypath);+printf(_("Submodule path '%s': merged in '%s'\n"),+ud->displaypath,oid);break;caseSM_UPDATE_COMMAND:-printf(_("Execution of '%s %s' failed in submodule path '%s'"),-ud->update_strategy.command,oid,ud->displaypath);+printf(_("Submodule path '%s': '%s %s'\n"),+ud->displaypath,strategy.command,oid);break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/-if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/-return1;-}--switch(ud->update_strategy.type){-caseSM_UPDATE_CHECKOUT:-printf(_("Submodule path '%s': checked out '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_REBASE:-printf(_("Submodule path '%s': rebased into '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_MERGE:-printf(_("Submodule path '%s': merged in '%s'\n"),-ud->displaypath,oid);-break;-caseSM_UPDATE_COMMAND:-printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,ud->update_strategy.command,oid);-break;-default:-BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));}+strbuf_release(&out);return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintrun_update_procedure(structupdate_data*ud,structstring_list*err){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2478,182 +2485,7 @@ static int do_run_update_procedure(struct update_data *ud)ud->displaypath,oid_to_hex(&ud->oid));}-returnrun_update_command(ud,subforce);-}--staticvoidupdate_submodule(structupdate_clone_data*ucd)-{-fprintf(stdout,"dummy %s %d\t%s\n",-oid_to_hex(&ucd->oid),-ucd->just_cloned,-ucd->sub->path);-}--staticintupdate_submodules(structsubmodule_update_clone*suc)-{-inti;--run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,-update_clone_start_failure,-update_clone_task_finished,suc,"submodule",-"parallel/update");--/*-*Wesavedtheoutputandputitoutallatoncenow.-*Thatmeans:-*-thelistenerdoesnothavetointerleavetheir(checkout)-*workwithourfetching.Thewritesinvolvedina-*checkoutinvolvemorestraightforwardsequentialI/O.-*-thelistenercanavoiddoinganyworkiffetchingfailed.-*/-if(suc->quickstop)-return1;--for(i=0;i<suc->update_clone_nr;i++)-update_submodule(&suc->update_clone[i]);--return0;-}--staticintupdate_clone(intargc,constchar**argv,constchar*prefix)-{-constchar*update=NULL;-structpathspecpathspec;-structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;--structoptionmodule_update_clone_options[]={-OPT_STRING(0,"prefix",&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,-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>",-N_("create a shallow clone truncated to the "-"specified number of revisions")),-OPT_INTEGER('j',"jobs",&opt.max_jobs,-N_("parallel jobs")),-OPT_BOOL(0,"recommend-shallow",&opt.recommend_shallow,-N_("whether the initial clone should follow the shallow recommendation")),-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",&opt.require_init,-N_("disallow cloning into non-empty directory")),-OPT_BOOL(0,"single-branch",&opt.single_branch,-N_("clone only one branch, HEAD or --branch")),-OPT_END()-};--constchar*constgit_submodule_helper_usage[]={-N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),-NULL-};-opt.prefix=prefix;--update_clone_config_from_gitmodules(&opt.max_jobs);-git_config(git_update_clone_config,&opt.max_jobs);--argc=parse_options(argc,argv,prefix,module_update_clone_options,-git_submodule_helper_usage,0);--if(update)-if(parse_submodule_update_strategy(update,&opt.update)<0)-die(_("bad value for update parameter"));--if(module_list_compute(argc,argv,prefix,&pathspec,&opt.list)<0)-return1;--if(pathspec.nr)-opt.warn_if_uninitialized=1;--returnupdate_submodules(&opt);-}--staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,nofetch=0,just_cloned=0;-char*prefixed_path,*update=NULL;-structupdate_dataupdate_data=UPDATE_DATA_INIT;--structoptionoptions[]={-OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),-OPT__FORCE(&force,N_("force checkout updates"),0),-OPT_BOOL('N',"no-fetch",&nofetch,-N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&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,-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_CALLBACK_F(0,"suboid",&update_data.suboid,N_("subsha1"),-N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_END()-};--constchar*constusage[]={-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.force=!!force;-update_data.quiet=!!quiet;-update_data.nofetch=!!nofetch;-update_data.just_cloned=!!just_cloned;-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,-&update_data.update_strategy);--free(prefixed_path);--if(!oideq(&update_data.oid,&update_data.suboid)||update_data.force)-returndo_run_update_procedure(&update_data);--return3;-}--staticintresolve_relative_path(intargc,constchar**argv,constchar*prefix)-{-structstrbufsb=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);-return0;+returnrun_update_command(ud,subforce,err);}staticconstchar*remote_submodule_branch(constchar*path)
@@ -2788,17 +2620,11 @@ static int push_check(int argc, const char **argv, const char *prefix)return0;}-staticintensure_core_worktree(intargc,constchar**argv,constchar*prefix)+staticvoidensure_core_worktree(constchar*path){-constchar*path;constchar*cw;structrepositorysubrepo;-if(argc!=2)-BUG("submodule--helper ensure-core-worktree <path>");--path=argv[1];-if(repo_submodule_init(&subrepo,the_repository,path,null_oid()))die(_("could not get a repository handle for submodule '%s'"),path);
@@ -3006,6 +2830,279 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+constchar*update=submodule_strategy_to_string(&update_data->update_strategy);++strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+if(update_data->prefix)+strvec_pushl(args,"--prefix",update_data->prefix,NULL);+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)+strvec_pushl(args,"--update",update,NULL);+if(update_data->references.nr){+structstring_list_item*item;+for_each_string_list_item(item,&update_data->references)+strvec_pushl(args,"--reference",item->string,NULL);+}+if(update_data->recommend_shallow==0)+strvec_push(args,"--no-recommend-shallow");+elseif(update_data->recommend_shallow==1)+strvec_push(args,"--recommend-shallow");+if(update_data->single_branch>=0)+strvec_push(args,"--single-branch");+}++staticintupdate_submodule(structupdate_data*update_data)+{+char*prefixed_path;+structstring_listerr=STRING_LIST_INIT_DUP;++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);++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);+constchar*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,&err))+return1;++if(update_data->recursive){+intres;+structchild_processcp=CHILD_PROCESS_INIT;+structupdate_datanext=*update_data;+char*die_msg=xstrfmt(_("Failed to recurse into submodule path '%s'"),+update_data->displaypath);++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 */+if((res=run_command(&cp))==128)+die("%s",die_msg);+if(res)+string_list_append(&err,die_msg);++free(die_msg);+}++if(err.nr){+structstring_list_item*item;+for_each_string_list_item(item,&err)+fputs(item->string,stderr);+string_list_clear(&err,0);+return1;+}++return0;+}++staticintupdate_submodules(structupdate_data*update_data)+{+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;++update_clone_from_update_data(&suc,update_data);+run_processes_parallel_tr2(suc.max_jobs,update_clone_get_next_task,+update_clone_start_failure,+update_clone_task_finished,&suc,"submodule",+"parallel/update");++/*+*Wesavedtheoutputandputitoutallatoncenow.+*Thatmeans:+*-thelistenerdoesnothavetointerleavetheir(checkout)+*workwithourfetching.Thewritesinvolvedina+*checkoutinvolvemorestraightforwardsequentialI/O.+*-thelistenercanavoiddoinganyworkiffetchingfailed.+*/+if(suc.quickstop){+res=1;+gotocleanup;+}++for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];++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);+returnres;+}++staticintmodule_update(intargc,constchar**argv,constchar*prefix)+{+constchar*update=NULL;+structpathspecpathspec;+structupdate_dataopt=UPDATE_DATA_INIT;++structoptionmodule_update_clone_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")),+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",&opt.references,N_("repo"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&opt.dissociate,+N_("use --reference only while cloning")),+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,+N_("parallel jobs")),+OPT_BOOL(0,"recommend-shallow",&opt.recommend_shallow,+N_("whether the initial clone should follow the shallow recommendation")),+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",&opt.require_init,+N_("disallow cloning into non-empty directory")),+OPT_BOOL(0,"single-branch",&opt.single_branch,+N_("clone only one branch, HEAD or --branch")),+OPT_END()+};++constchar*constgit_submodule_helper_usage[]={+N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),+NULL+};++update_clone_config_from_gitmodules(&opt.max_jobs);+git_config(git_update_clone_config,&opt.max_jobs);++argc=parse_options(argc,argv,prefix,module_update_clone_options,+git_submodule_helper_usage,0);+oidcpy(&opt.oid,null_oid());+oidcpy(&opt.suboid,null_oid());++if(update)+if(parse_submodule_update_strategy(update,+&opt.update_strategy)<0)+die(_("bad value for update parameter"));++if(module_list_compute(argc,argv,prefix,&pathspec,&opt.list)<0)+return1;++if(pathspec.nr)+opt.warn_if_uninitialized=1;++if(opt.init){+structmodule_listlist=MODULE_LIST_INIT;+structinit_cbinfo=INIT_CB_INIT;++if(module_list_compute(argc,argv,opt.prefix,+&pathspec,&list)<0)+return1;++/*+*Iftherearenopathargsandsubmodule.activeissetthen,+*bydefault,onlyinitialize'active'modules.+*/+if(!argc&&git_config_get_value_multi("submodule.active"))+module_list_active(&list);++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);+}++returnupdate_submodules(&opt);+}+structadd_data{constchar*prefix;constchar*branch;
@@ -246,20 +246,6 @@ cmd_deinit()git${wt_prefix:+-C "$wt_prefix"}submodule--helperdeinit${GIT_QUIET:+--quiet}${force:+--force}${deinit_all:+--all}--"$@"}-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]-# Because arguments are positional, use an empty string to omit <depth>-# but include <sha1>.-fetch_in_submodule()(-sanitize_submodule_env&&-cd"$1"&&-iftest$#-eq3-then-echo"$3"|gitfetch${GIT_QUIET:+--quiet}--stdin${2:+"$2"}-else-gitfetch${GIT_QUIET:+--quiet}${2:+"$2"}-fi-)-## Update each submodule path to correct revision, using clone and checkout as needed#
@@ -361,133 +347,26 @@ cmd_update()shiftdone-iftest-n"$init"-then-cmd_init"--""$@"||return-fi--{-gitsubmodule--helperupdate-clone${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+${GIT_QUIET:+--quiet}\+${force:+--force}\+${progress:+--progress}\+${dissociate:+--dissociate}\+${remote:+--remote}\+${recursive:+--recursive}\+${init:+--init}\+${require_init:+--require-init}\+${nofetch:+--no-fetch}\${wt_prefix:+--prefix "$wt_prefix"}\${prefix:+--recursive-prefix "$prefix"}\${update:+--update "$update"}\${reference:+"$reference"}\-${dissociate:+"--dissociate"}\-${depth:+--depth "$depth"}\-${require_init:+--require-init}\+${depth:+"$depth"}\$single_branch\$recommend_shallow\$jobs\--\-"$@"||echo"#unmatched"$?-}|{-err=-whileread-rquickabortsha1just_clonedsm_path-do-die_if_unmatched"$quickabort""$sha1"--gitsubmodule--helperensure-core-worktree"$sm_path"||exit1--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq1-then-subsha1=-else-just_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"-fi--iftest-n"$remote"-then-branch=$(gitsubmodule--helperremote-branch"$sm_path")-iftest-z"$nofetch"-then-# Fetch remote before determining tracking $sha1-fetch_in_submodule"$sm_path"$depth||-die"fatal: $(eval_gettext"Unable to fetch in submodule path '\$sm_path'")"-fi-remote_name=$(sanitize_submodule_env;cd"$sm_path"&&gitsubmodule--helperprint-default-remote)-sha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verify"${remote_name}/${branch}")||-die"fatal: $(eval_gettext"Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"-fi--out=$(gitsubmodule--helperrun-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"}\-${subsha1:+--suboid "$subsha1"}\-"--"\-"$sm_path")--# 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-# 3: no update procedure was run-res="$?"-case$resin-0)-say"$out"-;;-1)-err="${err};fatal: $out"-continue-;;-2|128)-die_with_status$res"fatal: $out"-;;-esac--iftest-n"$recursive"-then-(-prefix=$(gitsubmodule--helperrelative-path"$prefix$sm_path/""$wt_prefix")-wt_prefix=-sanitize_submodule_env-cd"$sm_path"&&-evalcmd_update-)-res=$?-iftest$res-gt0-then-die_msg="fatal: $(eval_gettext"Failed to recurse into submodule path '\$displaypath'")"-iftest$res-ne2-then-err="${err};$die_msg"-continue-else-die_with_status$res"$die_msg"-fi-fi-fi-done--iftest-n"$err"-then-OIFS=$IFS-IFS=';'-forein$err-do-iftest-n"$e"-then-echo>&2"$e"-fi-done-IFS=$OIFS-exit1-fi-}+"$@"}#
Ævar Arnfjörð Bjarmason [off-list ref] writes:
This wasn't introduced by you (it was introduced in v1 [1]), but I think
it's worth pointing out.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
We say this, suggesting that repo_get_default_remote()'s signature is
just get_default_remote()'s plus a "struct repository *" (like most
repo_*). But..
repo_get_default_remote() actually take yet another argument - refname.
It looks to me that repo_get_default_remote() shouldn't take the
refname argument at all and that we should be using
refs_resolve_ref_unsafe() instead, like:
+static char *repo_get_default_remote(struct repository *repo)
{
char *dest = NULL, *ret;
struct strbuf sb = STRBUF_INIT;
- const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+ const char *refname = refs_resolve_ref_unsafe(
+ get_main_ref_store(repo), "HEAD", 0, NULL, NULL /*, errno? */);
this makes the rest of the code a lot cleaner..
+static char *get_default_remote_submodule(const char *module_path)
+{
+ struct repository subrepo;
+
+ repo_submodule_init(&subrepo, the_repository, module_path, null_oid());
+ return repo_get_default_remote(&subrepo);
+}
+
+static char *get_default_remote(void)
+{
+ return repo_get_default_remote(the_repository);
+}
And because it's quite idiomatic to initialize the subrepo struct in
order to can call repo_* functions, we could even drop
get_default_remote_submodule() altogether.
As for why this wasn't the original approach, the only reason I can
think of is that we didn't realize get_main_ref_store(subrepo) was an
option.
[1] https://lore.kernel.org/git/20210907115932.36068-3-raykar.ath@gmail.com/
From: Atharva Raykar <redacted>
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules2()` and `update_submodule2()`
which will supersede `update_submodules()` and `update_submodule()`.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
While we are at it, we also remove the fetch_in_submodule() shell
function since it is no longer used anywhere.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
I've read through all of the patches besides this one - I hope to get
through this one soon.
The diff is quite large, but I can't think of any way to shrink it down
at the moment.
From: Atharva Raykar <redacted>
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Helped-by: Glen Choo [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 39 +++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
@@ -1382,21 +1397,15 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+default_remote=get_default_remote_submodule(path);+if(!default_remote)die(_("failed to get the default remote for submodule '%s'"),path);-strbuf_strip_suffix(&sb,"\n");-remote_key=xstrfmt("remote.%s.url",sb.buf);+remote_key=xstrfmt("remote.%s.url",default_remote);+free(default_remote);-strbuf_reset(&sb);submodule_to_gitdir(&sb,path);strbuf_addstr(&sb,"/config");
From: Atharva Raykar <redacted>
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -261,11 +261,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -281,6 +278,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
From: Atharva Raykar <redacted>
We allow callers of the `init_submodule()` function to optionally
override the superprefix from the environment.
We need to enable this option because in our conversion of the update
command that will follow, the '--init' option will be handled through
this API. We will need to change the superprefix at that time to ensure
the display paths show correctly in the output messages.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
@@ -606,18 +606,22 @@ static int module_foreach(int argc, const char **argv, const char *prefix)structinit_cb{constchar*prefix;+constchar*superprefix;unsignedintflags;};#define INIT_CB_INIT { 0 }staticvoidinit_submodule(constchar*path,constchar*prefix,-unsignedintflags)+constchar*superprefix,unsignedintflags){conststructsubmodule*sub;structstrbufsb=STRBUF_INIT;char*upd=NULL,*url=NULL,*displaypath;-displaypath=get_submodule_displaypath(path,prefix);+/* try superprefix from the environment, if it is not passed explicitly */+if(!superprefix)+superprefix=get_super_prefix();+displaypath=do_get_submodule_displaypath(path,prefix,superprefix);sub=submodule_from_path(the_repository,null_oid(),path);
From: Atharva Raykar <redacted>
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 [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
@@ -2344,47 +2344,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strstaticintrun_update_command(structupdate_data*ud,intsubforce){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);intmust_die_on_failure=0;-intgit_cmd;switch(ud->update_strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_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){caseSM_UPDATE_CHECKOUT:printf(_("Unable to checkout '%s' in submodule path '%s'"),
From: Atharva Raykar <redacted>
The second hunk here will make a subsequent commit's diff smaller, and
let's do the first and third hunks while we're at it so that we
consistently format all of these.
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
Rename the "suc" variable in update_clone() to "opt", and do the same
for the "update_data" variable in run_update_procedure().
The only reason for this change is to make the subsequent commit's
diff smaller, by doing this rename we can "anchor" the diff better, as
it "borrow" most of the options declared here as-is as far as the diff
rename detection is concerned.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 74 ++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 37 deletions(-)
@@ -2517,36 +2517,36 @@ static int update_clone(int argc, const char **argv, const char *prefix){constchar*update=NULL;structpathspecpathspec;-structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;+structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;structoptionmodule_update_clone_options[]={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_END()};
@@ -2555,32 +2555,32 @@ static int update_clone(int argc, const char **argv, const char *prefix)N_("git submodule--helper update-clone [--prefix=<path>] [<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);argc=parse_options(argc,argv,prefix,module_update_clone_options,git_submodule_helper_usage,0);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)return1;if(pathspec.nr)-suc.warn_if_uninitialized=1;+opt.warn_if_uninitialized=1;-returnupdate_submodules(&suc);+returnupdate_submodules(&opt);}staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix){intforce=0,quiet=0,nofetch=0,just_cloned=0;char*prefixed_path,*update=NULL;-structupdate_dataupdate_data=UPDATE_DATA_INIT;+structupdate_dataopt=UPDATE_DATA_INIT;structoptionoptions[]={OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),
@@ -2589,20 +2589,20 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)N_("don't fetch new objects from the remote site")),OPT_BOOL(0,"just-cloned",&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_INTEGER(0,"depth",&opt.depth,N_("depth for shallow fetch")),OPT_STRING(0,"prefix",&prefix,N_("path"),N_("path into the working tree")),OPT_STRING(0,"update",&update,N_("string"),N_("rebase, merge, checkout or none")),-OPT_STRING(0,"recursive-prefix",&update_data.recursive_prefix,N_("path"),+OPT_STRING(0,"recursive-prefix",&opt.recursive_prefix,N_("path"),N_("path into the working tree, across nested ""submodule boundaries")),-OPT_CALLBACK_F(0,"oid",&update_data.oid,N_("sha1"),+OPT_CALLBACK_F(0,"oid",&opt.oid,N_("sha1"),N_("SHA1 expected by superproject"),PARSE_OPT_NONEG,parse_opt_object_id),-OPT_CALLBACK_F(0,"suboid",&update_data.suboid,N_("subsha1"),+OPT_CALLBACK_F(0,"suboid",&opt.suboid,N_("subsha1"),N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,parse_opt_object_id),OPT_END()
Do away with the indirection of local variables added in
c51f8f94e5b (submodule--helper: run update procedures from C,
2021-08-24).
These were only needed because in C you can't get a pointer to a
single bit, so we were using intermediate variables instead.
This will also make a subsequent large commit's diff smaller.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
@@ -2578,16 +2578,17 @@ static int update_clone(int argc, const char **argv, const char *prefix)staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix){-intforce=0,quiet=0,nofetch=0,just_cloned=0;char*prefixed_path,*update=NULL;structupdate_dataopt=UPDATE_DATA_INIT;structoptionoptions[]={-OPT__QUIET(&quiet,N_("suppress output for update by rebase or merge")),-OPT__FORCE(&force,N_("force checkout updates"),0),-OPT_BOOL('N',"no-fetch",&nofetch,+OPT__QUIET(&opt.quiet,+N_("suppress output for update by rebase or merge")),+OPT__FORCE(&opt.force,N_("force checkout updates"),+0),+OPT_BOOL('N',"no-fetch",&opt.nofetch,N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&just_cloned,+OPT_BOOL(0,"just-cloned",&opt.just_cloned,N_("overrides update mode in case the repository is a fresh clone")),OPT_INTEGER(0,"depth",&opt.depth,N_("depth for shallow fetch")),OPT_STRING(0,"prefix",&prefix,
Amend some submodule tests to test for the failure output of "git
submodule [update|init]". The lack of such tests hid a regression in
an earlier version of a subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
t/t7406-submodule-update.sh | 14 ++++++++++++--
t/t7408-submodule-reference.sh | 14 +++++++++++++-
2 files changed, 25 insertions(+), 3 deletions(-)
@@ -205,8 +205,18 @@ test_expect_success 'submodule update should fail due to local changes' '(cdsubmodule&&compare_head)&&-test_must_failgitsubmoduleupdatesubmodule-)+test_must_failgitsubmoduleupdatesubmodule2>../actual.raw+)&&+sed"s/^> //">expect<<-\EOF&&+>error:Yourlocalchangestothefollowingfileswouldbeoverwrittenbycheckout:+>file+>Pleasecommityourchangesorstashthembeforeyouswitchbranches.+>Aborting+>fatal:UnabletocheckoutOIDinsubmodulepath'\''submodule'\''+EOF+sed-e"s/checkout $SQ[^$SQ]*$SQ/checkout OID/"<actual.raw>actual&&+test_cmpexpectactual+' test_expect_success'submodule update should throw away changes with --force ''(cdsuper&&
From: Atharva Raykar <redacted>
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.
We add more fields to the `struct update_data` that are required by
`struct submodule_update_clone` to be able to perform a clone, when that
is needed to be done.
Recursing on a submodule is done by calling a subprocess that launches
`submodule--helper update`, with a modified `--recursive-prefix` and
`--prefix` parameter.
We also introduce `update_submodules2()` and `update_submodule2()`
which will supersede `update_submodules()` and `update_submodule()`.
When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call[1]. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
While we are at it, we also remove the fetch_in_submodule() shell
function since it is no longer used anywhere.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/submodule--helper.c | 599 +++++++++++++++++++++---------------
git-submodule.sh | 145 +--------
2 files changed, 356 insertions(+), 388 deletions(-)
@@ -2015,7 +1977,6 @@ struct submodule_update_clone {constchar*prefix;intsingle_branch;-/* to be consumed by git-submodule.sh */structupdate_clone_data*update_clone;intupdate_clone_nr;intupdate_clone_alloc;
@@ -2352,8 +2355,15 @@ static int run_update_command(struct update_data *ud, int subforce)structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);intmust_die_on_failure=0;+structsubmodule_update_strategystrategy=SUBMODULE_UPDATE_STRATEGY_INIT;-switch(ud->update_strategy.type){+if(ud->update_strategy.type==SM_UPDATE_UNSPECIFIED||ud->just_cloned)+determine_submodule_update_strategy(the_repository,ud->just_cloned,+ud->sm_path,NULL,&strategy);+else+strategy=ud->update_strategy;++switch(strategy.type){caseSM_UPDATE_CHECKOUT:cp.git_cmd=1;strvec_pushl(&cp.args,"checkout","-q",NULL);
@@ -2376,55 +2386,54 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:cp.use_shell=1;-strvec_push(&cp.args,ud->update_strategy.command);+strvec_push(&cp.args,strategy.command);must_die_on_failure=1;break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}strvec_push(&cp.args,oid);cp.dir=xstrdup(ud->sm_path);prepare_submodule_repo_env(&cp.env_array);if(run_command(&cp)){-switch(ud->update_strategy.type){+switch(strategy.type){caseSM_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;caseSM_UPDATE_REBASE:-printf(_("Unable to rebase '%s' in submodule path '%s'"),-oid,ud->displaypath);+if(!must_die_on_failure)+break;+die(_("Unable to rebase '%s' in submodule path '%s'"),+oid,ud->displaypath);break;caseSM_UPDATE_MERGE:-printf(_("Unable to merge '%s' in submodule path '%s'"),-oid,ud->displaypath);+if(!must_die_on_failure)+break;+die(_("Unable to merge '%s' in submodule path '%s'"),+oid,ud->displaypath);break;caseSM_UPDATE_COMMAND:-printf(_("Execution of '%s %s' failed in submodule path '%s'"),-ud->update_strategy.command,oid,ud->displaypath);+if(!must_die_on_failure)+break;+die(_("Execution of '%s %s' failed in submodule path '%s'"),+strategy.command,oid,ud->displaypath);break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/-if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/++/* the command failed, but update must continue */return1;}-switch(ud->update_strategy.type){+if(ud->quiet)+return0;++switch(strategy.type){caseSM_UPDATE_CHECKOUT:printf(_("Submodule path '%s': checked out '%s'\n"),ud->displaypath,oid);
@@ -2439,17 +2448,17 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,ud->update_strategy.command,oid);+ud->displaypath,strategy.command,oid);break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&ud->update_strategy));+submodule_strategy_to_string(&strategy));}return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintrun_update_procedure(structupdate_data*ud){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2479,178 +2488,6 @@ static int do_run_update_procedure(struct update_data *ud)returnrun_update_command(ud,subforce);}-staticvoidupdate_submodule(structupdate_clone_data*ucd)-{-fprintf(stdout,"dummy %s %d\t%s\n",-oid_to_hex(&ucd->oid),-ucd->just_cloned,-ucd->sub->path);-}--staticintupdate_submodules(structsubmodule_update_clone*suc)-{-inti;--run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,-update_clone_start_failure,-update_clone_task_finished,suc,"submodule",-"parallel/update");--/*-*Wesavedtheoutputandputitoutallatoncenow.-*Thatmeans:-*-thelistenerdoesnothavetointerleavetheir(checkout)-*workwithourfetching.Thewritesinvolvedina-*checkoutinvolvemorestraightforwardsequentialI/O.-*-thelistenercanavoiddoinganyworkiffetchingfailed.-*/-if(suc->quickstop)-return1;--for(i=0;i<suc->update_clone_nr;i++)-update_submodule(&suc->update_clone[i]);--return0;-}--staticintupdate_clone(intargc,constchar**argv,constchar*prefix)-{-constchar*update=NULL;-structpathspecpathspec;-structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;--structoptionmodule_update_clone_options[]={-OPT_STRING(0,"prefix",&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,-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>",-N_("create a shallow clone truncated to the "-"specified number of revisions")),-OPT_INTEGER('j',"jobs",&opt.max_jobs,-N_("parallel jobs")),-OPT_BOOL(0,"recommend-shallow",&opt.recommend_shallow,-N_("whether the initial clone should follow the shallow recommendation")),-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",&opt.require_init,-N_("disallow cloning into non-empty directory")),-OPT_BOOL(0,"single-branch",&opt.single_branch,-N_("clone only one branch, HEAD or --branch")),-OPT_END()-};--constchar*constgit_submodule_helper_usage[]={-N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),-NULL-};-opt.prefix=prefix;--update_clone_config_from_gitmodules(&opt.max_jobs);-git_config(git_update_clone_config,&opt.max_jobs);--argc=parse_options(argc,argv,prefix,module_update_clone_options,-git_submodule_helper_usage,0);--if(update)-if(parse_submodule_update_strategy(update,&opt.update)<0)-die(_("bad value for update parameter"));--if(module_list_compute(argc,argv,prefix,&pathspec,&opt.list)<0)-return1;--if(pathspec.nr)-opt.warn_if_uninitialized=1;--returnupdate_submodules(&opt);-}--staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-char*prefixed_path,*update=NULL;-structupdate_dataopt=UPDATE_DATA_INIT;--structoptionoptions[]={-OPT__QUIET(&opt.quiet,-N_("suppress output for update by rebase or merge")),-OPT__FORCE(&opt.force,N_("force checkout updates"),-0),-OPT_BOOL('N',"no-fetch",&opt.nofetch,-N_("don't fetch new objects from the remote site")),-OPT_BOOL(0,"just-cloned",&opt.just_cloned,-N_("overrides update mode in case the repository is a fresh clone")),-OPT_INTEGER(0,"depth",&opt.depth,N_("depth for shallow fetch")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"update",&update,-N_("string"),-N_("rebase, merge, checkout or none")),-OPT_STRING(0,"recursive-prefix",&opt.recursive_prefix,N_("path"),-N_("path into the working tree, across nested "-"submodule boundaries")),-OPT_CALLBACK_F(0,"oid",&opt.oid,N_("sha1"),-N_("SHA1 expected by superproject"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_CALLBACK_F(0,"suboid",&opt.suboid,N_("subsha1"),-N_("SHA1 of submodule's HEAD"),PARSE_OPT_NONEG,-parse_opt_object_id),-OPT_END()-};--constchar*constusage[]={-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);--opt.sm_path=argv[0];--if(opt.recursive_prefix)-prefixed_path=xstrfmt("%s%s",opt.recursive_prefix,opt.sm_path);-else-prefixed_path=xstrdup(opt.sm_path);--opt.displaypath=get_submodule_displaypath(prefixed_path,prefix);--determine_submodule_update_strategy(the_repository,opt.just_cloned,-opt.sm_path,update,-&opt.update_strategy);--free(prefixed_path);--if(!oideq(&opt.oid,&opt.suboid)||opt.force)-returndo_run_update_procedure(&opt);--return3;-}--staticintresolve_relative_path(intargc,constchar**argv,constchar*prefix)-{-structstrbufsb=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);-return0;-}-staticconstchar*remote_submodule_branch(constchar*path){conststructsubmodule*sub;
@@ -2783,17 +2620,11 @@ static int push_check(int argc, const char **argv, const char *prefix)return0;}-staticintensure_core_worktree(intargc,constchar**argv,constchar*prefix)+staticvoidensure_core_worktree(constchar*path){-constchar*path;constchar*cw;structrepositorysubrepo;-if(argc!=2)-BUG("submodule--helper ensure-core-worktree <path>");--path=argv[1];-if(repo_submodule_init(&subrepo,the_repository,path,null_oid()))die(_("could not get a repository handle for submodule '%s'"),path);
@@ -3001,6 +2830,271 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+constchar*update=submodule_strategy_to_string(&update_data->update_strategy);++strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+if(update_data->prefix)+strvec_pushl(args,"--prefix",update_data->prefix,NULL);+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)+strvec_pushl(args,"--update",update,NULL);+if(update_data->references.nr){+structstring_list_item*item;+for_each_string_list_item(item,&update_data->references)+strvec_pushl(args,"--reference",item->string,NULL);+}+if(update_data->recommend_shallow==0)+strvec_push(args,"--no-recommend-shallow");+elseif(update_data->recommend_shallow==1)+strvec_push(args,"--recommend-shallow");+if(update_data->single_branch>=0)+strvec_push(args,"--single-branch");+}++staticintupdate_submodule(structupdate_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);++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);+constchar*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))+return1;++if(update_data->recursive){+structchild_processcp=CHILD_PROCESS_INIT;+structupdate_datanext=*update_data;+intres;++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)+return0;+die_message(_("Failed to recurse into submodule path '%s'"),+update_data->displaypath);+if(res==128)+exit(res);+elseif(res)+return1;+}++return0;+}++staticintupdate_submodules(structupdate_data*update_data)+{+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;++update_clone_from_update_data(&suc,update_data);+run_processes_parallel_tr2(suc.max_jobs,update_clone_get_next_task,+update_clone_start_failure,+update_clone_task_finished,&suc,"submodule",+"parallel/update");++/*+*Wesavedtheoutputandputitoutallatoncenow.+*Thatmeans:+*-thelistenerdoesnothavetointerleavetheir(checkout)+*workwithourfetching.Thewritesinvolvedina+*checkoutinvolvemorestraightforwardsequentialI/O.+*-thelistenercanavoiddoinganyworkiffetchingfailed.+*/+if(suc.quickstop){+res=1;+gotocleanup;+}++for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];++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);+returnres;+}++staticintmodule_update(intargc,constchar**argv,constchar*prefix)+{+constchar*update=NULL;+structpathspecpathspec;+structupdate_dataopt=UPDATE_DATA_INIT;++structoptionmodule_update_clone_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")),+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",&opt.references,N_("repo"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&opt.dissociate,+N_("use --reference only while cloning")),+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,+N_("parallel jobs")),+OPT_BOOL(0,"recommend-shallow",&opt.recommend_shallow,+N_("whether the initial clone should follow the shallow recommendation")),+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",&opt.require_init,+N_("disallow cloning into non-empty directory")),+OPT_BOOL(0,"single-branch",&opt.single_branch,+N_("clone only one branch, HEAD or --branch")),+OPT_END()+};++constchar*constgit_submodule_helper_usage[]={+N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),+NULL+};++update_clone_config_from_gitmodules(&opt.max_jobs);+git_config(git_update_clone_config,&opt.max_jobs);++argc=parse_options(argc,argv,prefix,module_update_clone_options,+git_submodule_helper_usage,0);+oidcpy(&opt.oid,null_oid());+oidcpy(&opt.suboid,null_oid());++if(update)+if(parse_submodule_update_strategy(update,+&opt.update_strategy)<0)+die(_("bad value for update parameter"));++if(module_list_compute(argc,argv,prefix,&pathspec,&opt.list)<0)+return1;++if(pathspec.nr)+opt.warn_if_uninitialized=1;++if(opt.init){+structmodule_listlist=MODULE_LIST_INIT;+structinit_cbinfo=INIT_CB_INIT;++if(module_list_compute(argc,argv,opt.prefix,+&pathspec,&list)<0)+return1;++/*+*Iftherearenopathargsandsubmodule.activeissetthen,+*bydefault,onlyinitialize'active'modules.+*/+if(!argc&&git_config_get_value_multi("submodule.active"))+module_list_active(&list);++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);+}++returnupdate_submodules(&opt);+}+structadd_data{constchar*prefix;constchar*branch;
@@ -246,20 +246,6 @@ cmd_deinit()git${wt_prefix:+-C "$wt_prefix"}submodule--helperdeinit${GIT_QUIET:+--quiet}${force:+--force}${deinit_all:+--all}--"$@"}-# usage: fetch_in_submodule <module_path> [<depth>] [<sha1>]-# Because arguments are positional, use an empty string to omit <depth>-# but include <sha1>.-fetch_in_submodule()(-sanitize_submodule_env&&-cd"$1"&&-iftest$#-eq3-then-echo"$3"|gitfetch${GIT_QUIET:+--quiet}--stdin${2:+"$2"}-else-gitfetch${GIT_QUIET:+--quiet}${2:+"$2"}-fi-)-## Update each submodule path to correct revision, using clone and checkout as needed#
@@ -361,133 +347,26 @@ cmd_update()shiftdone-iftest-n"$init"-then-cmd_init"--""$@"||return-fi--{-gitsubmodule--helperupdate-clone${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+${GIT_QUIET:+--quiet}\+${force:+--force}\+${progress:+--progress}\+${dissociate:+--dissociate}\+${remote:+--remote}\+${recursive:+--recursive}\+${init:+--init}\+${require_init:+--require-init}\+${nofetch:+--no-fetch}\${wt_prefix:+--prefix "$wt_prefix"}\${prefix:+--recursive-prefix "$prefix"}\${update:+--update "$update"}\${reference:+"$reference"}\-${dissociate:+"--dissociate"}\-${depth:+--depth "$depth"}\-${require_init:+--require-init}\+${depth:+"$depth"}\$single_branch\$recommend_shallow\$jobs\--\-"$@"||echo"#unmatched"$?-}|{-err=-whileread-rquickabortsha1just_clonedsm_path-do-die_if_unmatched"$quickabort""$sha1"--gitsubmodule--helperensure-core-worktree"$sm_path"||exit1--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq1-then-subsha1=-else-just_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"-fi--iftest-n"$remote"-then-branch=$(gitsubmodule--helperremote-branch"$sm_path")-iftest-z"$nofetch"-then-# Fetch remote before determining tracking $sha1-fetch_in_submodule"$sm_path"$depth||-die"fatal: $(eval_gettext"Unable to fetch in submodule path '\$sm_path'")"-fi-remote_name=$(sanitize_submodule_env;cd"$sm_path"&&gitsubmodule--helperprint-default-remote)-sha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verify"${remote_name}/${branch}")||-die"fatal: $(eval_gettext"Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"-fi--out=$(gitsubmodule--helperrun-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"}\-${subsha1:+--suboid "$subsha1"}\-"--"\-"$sm_path")--# 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-# 3: no update procedure was run-res="$?"-case$resin-0)-say"$out"-;;-1)-err="${err};fatal: $out"-continue-;;-2|128)-die_with_status$res"fatal: $out"-;;-esac--iftest-n"$recursive"-then-(-prefix=$(gitsubmodule--helperrelative-path"$prefix$sm_path/""$wt_prefix")-wt_prefix=-sanitize_submodule_env-cd"$sm_path"&&-evalcmd_update-)-res=$?-iftest$res-gt0-then-die_msg="fatal: $(eval_gettext"Failed to recurse into submodule path '\$displaypath'")"-iftest$res-ne2-then-err="${err};$die_msg"-continue-else-die_with_status$res"$die_msg"-fi-fi-fi-done--iftest-n"$err"-then-OIFS=$IFS-IFS=';'-forein$err-do-iftest-n"$e"-then-echo>&2"$e"-fi-done-IFS=$OIFS-exit1-fi-}+"$@"}#
I mentioned (out-of-mailing-list) that I was still looking at this, but
that the big sh -> c conversion is quite challenging for me to parse
personally. I'm still looking at it, but it will take some time..
I'm of the opinion that this patch would be a lot easier to review if it
were broken up into more patches, but it has always looked like this
[1]. The only real difference from [1] to this version is that this also
removes all of the dead code, which doesn't really hinder reviewability.
I don't think you have to go through the effort of splitting it up -
after all you were able to review [1], so given enough time I should
be able to read through this patch too :) That said, I wish that we
already had a split up patch for at least 2 other reasons:
- Junio pointed out that this conflicts with
es/superproject-aware-submodules [2]. I'm not sure which should be
based on which. If this does end up being based on
es/superproject-aware-submodules, it would probably be easier to
rebase as a series of smaller patches. Atharva noted that the
conflicts are mild though, so maybe it's not so bad.
- Besides making sure that the sh -> c is faithful, a thorough review
should hopefully catch unintentional mistakes. The size of this patch
makes such mistakes difficult to spot. For instance, here's something
I spotted only after trying to split the patch myself..
> +static int module_update(int argc, const char **argv, const char *prefix)
> +{
> + const char *update = NULL;
> + struct pathspec pathspec;
> + struct update_data opt = UPDATE_DATA_INIT;
> +
> + struct option module_update_clone_options[] = {
[...]
> + };
> +
> + const char *const git_submodule_helper_usage[] = {
> + N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),
> + NULL
> + };
> +
> + update_clone_config_from_gitmodules(&opt.max_jobs);
> + git_config(git_update_clone_config, &opt.max_jobs);
Notice that we copy-pasted the option parsing from update-clone into
module_update() but forgot to update the names.
My ideal patch organization would be something like:
- wrap some existing command in "git submodule--helper update" (probably
run-update-procedure)
- absorb the surrounding sh code into "git submodule--helper
update" one command at-a-time i.e. deprecating and removing the
commands one at a time - instead of deprecating and removing them all
at once (like this patch), or deprecating all at once and removing
them one at a time (like v1).
I don't know if it's feasible or not; Atharva noted upthread that there
are some technical reasons why some things can be done in-process and
some cannot, but it might be a useful exercise.
Here's what I propose:
- If you think this alternative organization would be helpful for you
too, I will attempt it. This will take a while, but by the end you and
I will have effectively reviewed all of the code, so it should be easy
to finish up the review.
- Otherwise e.g. maybe this is a huge waste of time, or you're already
really confident in the correctness of the sh -> c when you reviewed
the original patch, etc, I'll just review this patch as-is. I'd
appreciate any tips and tricks that might help :)
- Orthogonal to patch organization, I'm still not sure if this will be
rebased on es/superproject-aware-submodules or vice-versa, and I don't
want either of us to sink too much effort before knowing the answer.
[1] https://lore.kernel.org/git/20210907115932.36068-7-raykar.ath@gmail.com/
[2] https://lore.kernel.org/git/YWiXL+plA7GHfuVv@google.com/
- Junio pointed out that this conflicts with
es/superproject-aware-submodules [2]. I'm not sure which should be
based on which. If this does end up being based on
es/superproject-aware-submodules, it would probably be easier to
rebase as a series of smaller patches. Atharva noted that the
conflicts are mild though, so maybe it's not so bad.
I think it makes sense to get this series through first, i.e. the
(supposedly) no-behavior-changing one, and then one that introduces new
submodule behavior.
Particularly because for es/superproject-aware-submodules the main
selling point is a performance improvement, which as I noted in the
review for it I've been unable to observe once the C<->sh layer goes
away.
I'm not saying it's not there, just that I don't think it's been shown
so far, IIRC there was some reference to some Google-internal network FS
that might or might not be helped by it...
- Besides making sure that the sh -> c is faithful, a thorough review
should hopefully catch unintentional mistakes. The size of this patch
makes such mistakes difficult to spot. For instance, here's something
I spotted only after trying to split the patch myself..
> +static int module_update(int argc, const char **argv, const char *prefix)
> +{
> + const char *update = NULL;
> + struct pathspec pathspec;
> + struct update_data opt = UPDATE_DATA_INIT;
> +
> + struct option module_update_clone_options[] = {
[...]
> + };
> +
> + const char *const git_submodule_helper_usage[] = {
> + N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),
> + NULL
> + };
> +
> + update_clone_config_from_gitmodules(&opt.max_jobs);
> + git_config(git_update_clone_config, &opt.max_jobs);
Notice that we copy-pasted the option parsing from update-clone into
module_update() but forgot to update the names.
My ideal patch organization would be something like:
- wrap some existing command in "git submodule--helper update" (probably
run-update-procedure)
- absorb the surrounding sh code into "git submodule--helper
update" one command at-a-time i.e. deprecating and removing the
commands one at a time - instead of deprecating and removing them all
at once (like this patch), or deprecating all at once and removing
them one at a time (like v1).
I do think atomic changes that don't leave dead code for removal later
are easier to read & reason about, whatever else is reorganized.
I.e. not to have something where we replace all the running code, and
then remove already-unused code later.
On that topic, I noticed this series could/should have [1] fixed up into
it.
- If you think this alternative organization would be helpful for you
too, I will attempt it. This will take a while, but by the end you and
I will have effectively reviewed all of the code, so it should be easy
to finish up the review.
I think it might, but I really don't know. We'll just have to see, so if
you want to take a stab at it that would be great.
Maybe it's a good way forward. E.g. as af first small step we could turn:
while read -r quickabort sha1 just_cloned sm_path
[...]
die_if_unmatched "$quickabort" "$sha1"
into version where we fold that die_if_unmatched() logic into the C
code, and then ensure-core-worktree etc.
- Otherwise e.g. maybe this is a huge waste of time, or you're already
really confident in the correctness of the sh -> c when you reviewed
the original patch, etc, I'll just review this patch as-is. I'd
appreciate any tips and tricks that might help :)
I'm not really confident in it.
I've read it, tested it as well as I could manage etc. but it's still a
very large change.
1.
@@ -286,13 +286,6 @@ get_author_ident_from_commit () {parse_ident_from_commitauthorAUTHOR}-# Clear repo-local GIT_* environment variables. Useful when switching to-# another repository (e.g. when entering a submodule). See also the env-# list in git_connect()-clear_local_git_env(){-unset$(gitrev-parse--local-env-vars)-}-# Generate a virtual base file for a two-file merge. Uses git apply to# remove lines from $1 that are not in $2, leaving only common lines. create_virtual_base(){
@@ -50,30 +50,11 @@ single_branch=jobs=recommend_shallow=-die_if_unmatched()-{-iftest"$1"="#unmatched"-then-exit${2:-1}-fi-}- isnumber(){n=$(($1+0))2>/dev/null&&test"$n"="$1"}-# Sanitize the local git environment for use within a submodule. We-# can't simply use clear_local_git_env since we want to preserve some-# of the settings from GIT_CONFIG_PARAMETERS.-sanitize_submodule_env()-{-save_config=$GIT_CONFIG_PARAMETERS-clear_local_git_env-GIT_CONFIG_PARAMETERS=$save_config-exportGIT_CONFIG_PARAMETERS-}-## Add a new submodule to the working tree, .gitmodules and the index#
On Thu, Feb 03 2022, Ævar Arnfjörð Bjarmason wrote:
On Wed, Feb 02 2022, Glen Choo wrote:
quoted
- Junio pointed out that this conflicts with
es/superproject-aware-submodules [2]. I'm not sure which should be
based on which. If this does end up being based on
es/superproject-aware-submodules, it would probably be easier to
rebase as a series of smaller patches. Atharva noted that the
conflicts are mild though, so maybe it's not so bad.
I think it makes sense to get this series through first, i.e. the
(supposedly) no-behavior-changing one, and then one that introduces new
submodule behavior.
Particularly because for es/superproject-aware-submodules the main
selling point is a performance improvement, which as I noted in the
review for it I've been unable to observe once the C<->sh layer goes
away.
I'm not saying it's not there, just that I don't think it's been shown
so far, IIRC there was some reference to some Google-internal network FS
that might or might not be helped by it...
quoted
- Besides making sure that the sh -> c is faithful, a thorough review
should hopefully catch unintentional mistakes. The size of this patch
makes such mistakes difficult to spot. For instance, here's something
I spotted only after trying to split the patch myself..
> +static int module_update(int argc, const char **argv, const char *prefix)
> +{
> + const char *update = NULL;
> + struct pathspec pathspec;
> + struct update_data opt = UPDATE_DATA_INIT;
> +
> + struct option module_update_clone_options[] = {
[...]
> + };
> +
> + const char *const git_submodule_helper_usage[] = {
> + N_("git submodule--helper update-clone [--prefix=<path>] [<path>...]"),
> + NULL
> + };
> +
> + update_clone_config_from_gitmodules(&opt.max_jobs);
> + git_config(git_update_clone_config, &opt.max_jobs);
Notice that we copy-pasted the option parsing from update-clone into
module_update() but forgot to update the names.
My ideal patch organization would be something like:
- wrap some existing command in "git submodule--helper update" (probably
run-update-procedure)
- absorb the surrounding sh code into "git submodule--helper
update" one command at-a-time i.e. deprecating and removing the
commands one at a time - instead of deprecating and removing them all
at once (like this patch), or deprecating all at once and removing
them one at a time (like v1).
I do think atomic changes that don't leave dead code for removal later
are easier to read & reason about, whatever else is reorganized.
I.e. not to have something where we replace all the running code, and
then remove already-unused code later.
On that topic, I noticed this series could/should have [1] fixed up into
it.
quoted
- If you think this alternative organization would be helpful for you
too, I will attempt it. This will take a while, but by the end you and
I will have effectively reviewed all of the code, so it should be easy
to finish up the review.
I think it might, but I really don't know. We'll just have to see, so if
you want to take a stab at it that would be great.
Maybe it's a good way forward. E.g. as af first small step we could turn:
while read -r quickabort sha1 just_cloned sm_path
[...]
die_if_unmatched "$quickabort" "$sha1"
into version where we fold that die_if_unmatched() logic into the C
code, and then ensure-core-worktree etc.
Sorry, that one makes no sense since it's an artifact of the shellscript
implementation.
But I tested the below on top of master, and it passes all tests, which
isn't very promising...
@@ -2783,40 +2783,6 @@ static int push_check(int argc, const char **argv, const char *prefix)return0;}-staticintensure_core_worktree(intargc,constchar**argv,constchar*prefix)-{-constchar*path;-constchar*cw;-structrepositorysubrepo;--if(argc!=2)-BUG("submodule--helper ensure-core-worktree <path>");--path=argv[1];--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;-constchar*rel_path;-structstrbufsb=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);-}--return0;-}-staticintabsorb_git_dirs(intargc,constchar**argv,constchar*prefix){inti;
On Thu, Feb 03 2022, Ævar Arnfjörð Bjarmason wrote:
quoted
On Wed, Feb 02 2022, Glen Choo wrote:
quoted
- Junio pointed out that this conflicts with
es/superproject-aware-submodules [2]. I'm not sure which should be
based on which. If this does end up being based on
es/superproject-aware-submodules, it would probably be easier to
rebase as a series of smaller patches. Atharva noted that the
conflicts are mild though, so maybe it's not so bad.
I think it makes sense to get this series through first, i.e. the
(supposedly) no-behavior-changing one, and then one that introduces new
submodule behavior.
Particularly because for es/superproject-aware-submodules the main
selling point is a performance improvement, which as I noted in the
review for it I've been unable to observe once the C<->sh layer goes
away.
I'm not saying it's not there, just that I don't think it's been shown
so far, IIRC there was some reference to some Google-internal network FS
that might or might not be helped by it...
I'll let the experts chime in, I don't think I can add anything useful
to the discussion.
quoted
quoted
My ideal patch organization would be something like:
- wrap some existing command in "git submodule--helper update" (probably
run-update-procedure)
- absorb the surrounding sh code into "git submodule--helper
update" one command at-a-time i.e. deprecating and removing the
commands one at a time - instead of deprecating and removing them all
at once (like this patch), or deprecating all at once and removing
them one at a time (like v1).
I do think atomic changes that don't leave dead code for removal later
are easier to read & reason about, whatever else is reorganized.
I.e. not to have something where we replace all the running code, and
then remove already-unused code later.
I agree - otherwise patches aren't self-contianed and harder to merge.
quoted
quoted
- If you think this alternative organization would be helpful for you
too, I will attempt it. This will take a while, but by the end you and
I will have effectively reviewed all of the code, so it should be easy
to finish up the review.
I think it might, but I really don't know. We'll just have to see, so if
you want to take a stab at it that would be great.
Maybe it's a good way forward. E.g. as af first small step we could turn:
while read -r quickabort sha1 just_cloned sm_path
[...]
die_if_unmatched "$quickabort" "$sha1"
into version where we fold that die_if_unmatched() logic into the C
code, and then ensure-core-worktree etc.
Sorry, that one makes no sense since it's an artifact of the shellscript
implementation.
Whether or not it makes sense, I think it gets the point across i.e.
that we think folding into C can be done incrementally.
quoted
quoted
- Otherwise e.g. maybe this is a huge waste of time, or you're already
really confident in the correctness of the sh -> c when you reviewed
the original patch, etc, I'll just review this patch as-is. I'd
appreciate any tips and tricks that might help :)
I'm not really confident in it.
I've read it, tested it as well as I could manage etc. but it's still a
very large change.
[...]
But I tested the below on top of master, and it passes all tests, which
isn't very promising...
A good enough (i.e. extremely comprehensive) test suite will all but
guarantee that no behavior has changed. Our test suite is nowhere near
that level and probably never will be, so we can't trust that things are
correct even if it passes tests.
So, for the sake of reviewability, I'll take a stab at reorganizing.
I'll be taking a long flight anyway, so I'll have big chunk of
non-company time to spend on this ;)
From: Atharva Raykar <redacted>
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.
Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.
Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.
`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.
Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.
So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.
We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in the next patch, which will require this
functionality.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Helped-by: Glen Choo [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/submodule--helper.c | 39 +++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
@@ -1382,21 +1397,15 @@ static void sync_submodule(const char *path, const char *prefix,if(!is_submodule_populated_gently(path,NULL))gotocleanup;-prepare_submodule_repo_env(&cp.env_array);-cp.git_cmd=1;-cp.dir=path;-strvec_pushl(&cp.args,"submodule--helper",-"print-default-remote",NULL);-strbuf_reset(&sb);-if(capture_command(&cp,&sb,0))+default_remote=get_default_remote_submodule(path);+if(!default_remote)die(_("failed to get the default remote for submodule '%s'"),path);-strbuf_strip_suffix(&sb,"\n");-remote_key=xstrfmt("remote.%s.url",sb.buf);+remote_key=xstrfmt("remote.%s.url",default_remote);+free(default_remote);-strbuf_reset(&sb);submodule_to_gitdir(&sb,path);strbuf_addstr(&sb,"/config");
From: Atharva Raykar <redacted>
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.
We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/submodule--helper.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -261,11 +261,8 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *prreturn0;}-/* the result should be freed by the caller. */-staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+staticchar*do_get_submodule_displaypath(constchar*path,constchar*prefix,constchar*super_prefix){-constchar*super_prefix=get_super_prefix();-if(prefix&&super_prefix){BUG("cannot have prefix '%s' and superprefix '%s'",prefix,super_prefix);
@@ -281,6 +278,13 @@ static char *get_submodule_displaypath(const char *path, const char *prefix)}}+/* the result should be freed by the caller. */+staticchar*get_submodule_displaypath(constchar*path,constchar*prefix)+{+constchar*super_prefix=get_super_prefix();+returndo_get_submodule_displaypath(path,prefix,super_prefix);+}+staticchar*compute_rev_name(constchar*sub_path,constchar*object_id){structstrbufsb=STRBUF_INIT;
From: Atharva Raykar <redacted>
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 [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Signed-off-by: Atharva Raykar <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/submodule--helper.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
@@ -2344,47 +2344,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, strstaticintrun_update_command(structupdate_data*ud,intsubforce){-structstrvecargs=STRVEC_INIT;-structstrvecchild_env=STRVEC_INIT;+structchild_processcp=CHILD_PROCESS_INIT;char*oid=oid_to_hex(&ud->oid);intmust_die_on_failure=0;-intgit_cmd;switch(ud->update_strategy.type){caseSM_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;caseSM_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;caseSM_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;caseSM_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){caseSM_UPDATE_CHECKOUT:printf(_("Unable to checkout '%s' in submodule path '%s'"),