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: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <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>
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>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/submodule--helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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,
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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&&
This is dead code - it has not been used since c51f8f94e5
(submodule--helper: run update procedures from C, 2021-08-24).
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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()
Introduce a function, update_submodule2(), that performs an update for
one submodule. This function will implement the functionality of
run-update-procedure and its surrounding shell code in submodule.sh.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
@@ -2978,6 +2979,15 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+/* NEEDSWORK: this is a temporary name until we delete update_submodule() */+staticintupdate_submodule2(structupdate_data*update_data)+{+if(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)+returndo_run_update_procedure(update_data);++return3;+}+structadd_data{constchar*prefix;constchar*branch;
Teach run-update-procedure to handle --remote instead of parsing
--remote in git-submodule.sh. As a result, "git submodule--helper
print-default-remote" has no more callers, so remove it.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 39 ++++++++++++++++++++++---------------
git-submodule.sh | 30 +---------------------------
2 files changed, 24 insertions(+), 45 deletions(-)
@@ -2585,6 +2571,8 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)OPT_CALLBACK_F(0,"oid",&opt.oid,N_("sha1"),N_("SHA1 expected by superproject"),PARSE_OPT_NONEG,parse_opt_object_id),+OPT_BOOL(0,"remote",&opt.remote,+N_("use SHA-1 of submodule's remote tracking branch")),OPT_END()};
@@ -2988,6 +2976,25 @@ static int update_submodule2(struct update_data *update_data)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)returndo_run_update_procedure(update_data);
@@ -3389,10 +3396,10 @@ static struct cmd_struct commands[] = {{"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},+/* NEEDSWORK: remote-branch is also obsolete */{"remote-branch",resolve_remote_submodule_branch,0},{"push-check",push_check,0},{"absorb-git-dirs",absorb_git_dirs,SUPPORT_SUPER_PREFIX},
@@ -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#
@@ -396,21 +382,6 @@ cmd_update()just_cloned=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}\
@@ -2746,17 +2746,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);
@@ -2967,6 +2959,8 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)/* NEEDSWORK: this is a temporary name until we delete update_submodule() */staticintupdate_submodule2(structupdate_data*update_data){+ensure_core_worktree(update_data->sm_path);+/* NEEDSWORK: fix the style issues e.g. braces */if(update_data->just_cloned){oidcpy(&update_data->suboid,null_oid());
Teach run-update-procedure to determine the oid of the submodule's HEAD
instead of doing it in git-subomdule.sh.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 12 +++++++++---
git-submodule.sh | 8 +-------
2 files changed, 10 insertions(+), 10 deletions(-)
@@ -2585,9 +2585,6 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)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()};
@@ -2982,6 +2979,15 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)/* NEEDSWORK: this is a temporary name until we delete update_submodule() */staticintupdate_submodule2(structupdate_data*update_data){+/* NEEDSWORK: fix the style issues e.g. braces */+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(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)returndo_run_update_procedure(update_data);
@@ -391,14 +391,9 @@ cmd_update()displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")-iftest$just_cloned-eq1+iftest$just_cloned-eq0then-subsha1=-elsejust_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"fiiftest-n"$remote"
Teach "git submodule--helper update-clone" the --init flag and remove
the corresponding shell code.
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. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 26 ++++++++++++++++++++++++++
git-submodule.sh | 9 +++------
2 files changed, 29 insertions(+), 6 deletions(-)
@@ -2483,6 +2484,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;structoptionmodule_update_clone_options[]={+OPT_BOOL(0,"init",&opt.init,+N_("initialize uninitialized submodules before update")),OPT_STRING(0,"prefix",&prefix,N_("path"),N_("path into the working tree")),
The next commit will change the internals of several functions and
arrange them in a more logical manner. Move these functions to their
final positions so that the diff is smaller.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 228 ++++++++++++++++++------------------
1 file changed, 114 insertions(+), 114 deletions(-)
@@ -2451,120 +2451,6 @@ static void update_submodule(struct update_clone_data *ucd)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_BOOL(0,"init",&opt.init,-N_("initialize uninitialized submodules before update")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"recursive-prefix",&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;--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);-}-/**NEEDSWORK:Useaforwarddeclarationtoavoidmoving*run_update_procedure()(whichwillberemovedsoon).
@@ -3021,6 +2907,120 @@ static int update_submodule2(struct update_data *update_data)return3;}+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_BOOL(0,"init",&opt.init,+N_("initialize uninitialized submodules before update")),+OPT_STRING(0,"prefix",&prefix,+N_("path"),+N_("path into the working tree")),+OPT_STRING(0,"recursive-prefix",&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;++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;
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.
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>
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 360 ++++++++++++++++++++++--------------
git-submodule.sh | 102 +---------
2 files changed, 227 insertions(+), 235 deletions(-)
@@ -1977,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;
@@ -2316,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;++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(ud->update_strategy.type){+switch(strategy.type){caseSM_UPDATE_CHECKOUT:cp.git_cmd=1;strvec_pushl(&cp.args,"checkout","-q",NULL);
@@ -2340,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);
@@ -2403,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;
@@ -2443,89 +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);-}--/*-*NEEDSWORK:Useaforwarddeclarationtoavoidmoving-*run_update_procedure()(whichwillberemovedsoon).-*/-staticintupdate_submodule2(structupdate_data*update_data);-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_BOOL(0,"remote",&opt.remote,-N_("use SHA-1 of submodule's remote tracking branch")),-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);-returnupdate_submodule2(&opt);-}--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;
@@ -2868,11 +2830,66 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}-/* NEEDSWORK: this is a temporary name until we delete update_submodule() */-staticintupdate_submodule2(structupdate_data*update_data)+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);+/* NEEDSWORK: fix the style issues e.g. braces */if(update_data->just_cloned){oidcpy(&update_data->suboid,null_oid());
@@ -2902,18 +2919,55 @@ static int update_submodule2(struct update_data *update_data)}if(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)-returndo_run_update_procedure(update_data);+if(run_update_procedure(update_data))+return1;-return3;+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(structsubmodule_update_clone*suc)+staticintupdate_submodules(structupdate_data*update_data){-inti;+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;-run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,+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",+update_clone_task_finished,&suc,"submodule","parallel/update");/*
@@ -2924,25 +2978,45 @@ static int update_submodules(struct submodule_update_clone *suc)*checkoutinvolvemorestraightforwardsequentialI/O.*-thelistenercanavoiddoinganyworkiffetchingfailed.*/-if(suc->quickstop)-return1;+if(suc.quickstop){+res=1;+gotocleanup;+}-for(i=0;i<suc->update_clone_nr;i++)-update_submodule(&suc->update_clone[i]);+for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];-return0;+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;}-staticintupdate_clone(intargc,constchar**argv,constchar*prefix)+staticintmodule_update(intargc,constchar**argv,constchar*prefix){constchar*update=NULL;structpathspecpathspec;-structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;+structupdate_dataopt=UPDATE_DATA_INIT;+/* NEEDSWORK: update names and strings */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_STRING(0,"prefix",&prefix,+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,
@@ -2953,21 +3027,21 @@ static int update_clone(int argc, const char **argv, const char *prefix)N_("string"),N_("rebase, merge, checkout or none")),OPT_STRING_LIST(0,"reference",&opt.references,N_("repo"),-N_("reference repository")),+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")),+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")),+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")),+N_("force cloning progress")),OPT_BOOL(0,"require-init",&opt.require_init,-N_("disallow cloning into non-empty directory")),+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()
@@ -2977,16 +3051,18 @@ static int update_clone(int argc, const char **argv, const char *prefix)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);+oidcpy(&opt.oid,null_oid());+oidcpy(&opt.suboid,null_oid());if(update)-if(parse_submodule_update_strategy(update,&opt.update)<0)+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)
@@ -347,108 +347,26 @@ cmd_update()shiftdone-{-git${wt_prefix:+-C "$wt_prefix"}submodule--helperupdate-clone\+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+${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"--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq0-then-just_cloned=-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"}\-${remote:+--remote}\-"--"\-"$sm_path")--# exit codes for run-update-procedure:-# 0: update was successful, say command output-# 1: update procedure failed, but should not die-# 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-}+"$@"}#
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 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: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <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: Æ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'"),
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>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/submodule--helper.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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()
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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,
From: Ævar Arnfjörð Bjarmason <redacted>
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>
Signed-off-by: Junio C Hamano <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&&
This is dead code - it has not been used since c51f8f94e5
(submodule--helper: run update procedures from C, 2021-08-24).
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
Teach run-update-procedure to determine the oid of the submodule's HEAD
instead of doing it in git-subomdule.sh.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 12 +++++++++---
git-submodule.sh | 8 +-------
2 files changed, 10 insertions(+), 10 deletions(-)
@@ -2585,9 +2585,6 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)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()};
@@ -2982,6 +2979,15 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)/* NEEDSWORK: this is a temporary name until we delete update_submodule() */staticintupdate_submodule2(structupdate_data*update_data){+/* NEEDSWORK: fix the style issues e.g. braces */+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(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)returndo_run_update_procedure(update_data);
@@ -391,14 +391,9 @@ cmd_update()displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")-iftest$just_cloned-eq1+iftest$just_cloned-eq0then-subsha1=-elsejust_cloned=-subsha1=$(sanitize_submodule_env;cd"$sm_path"&&-gitrev-parse--verifyHEAD)||-die"fatal: $(eval_gettext"Unable to find current revision in submodule path '\$displaypath'")"fiiftest-n"$remote"
Introduce a function, update_submodule2(), that performs an update for
one submodule. This function will implement the functionality of
run-update-procedure and its surrounding shell code in submodule.sh.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
@@ -2978,6 +2979,15 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}+/* NEEDSWORK: this is a temporary name until we delete update_submodule() */+staticintupdate_submodule2(structupdate_data*update_data)+{+if(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)+returndo_run_update_procedure(update_data);++return3;+}+structadd_data{constchar*prefix;constchar*branch;
Teach run-update-procedure to handle --remote instead of parsing
--remote in git-submodule.sh. As a result, "git submodule--helper
print-default-remote" has no more callers, so remove it.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 39 ++++++++++++++++++++++---------------
git-submodule.sh | 30 +---------------------------
2 files changed, 24 insertions(+), 45 deletions(-)
@@ -2585,6 +2571,8 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)OPT_CALLBACK_F(0,"oid",&opt.oid,N_("sha1"),N_("SHA1 expected by superproject"),PARSE_OPT_NONEG,parse_opt_object_id),+OPT_BOOL(0,"remote",&opt.remote,+N_("use SHA-1 of submodule's remote tracking branch")),OPT_END()};
@@ -2988,6 +2976,25 @@ static int update_submodule2(struct update_data *update_data)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)returndo_run_update_procedure(update_data);
@@ -3389,10 +3396,10 @@ static struct cmd_struct commands[] = {{"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},+/* NEEDSWORK: remote-branch is also obsolete */{"remote-branch",resolve_remote_submodule_branch,0},{"push-check",push_check,0},{"absorb-git-dirs",absorb_git_dirs,SUPPORT_SUPER_PREFIX},
@@ -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#
@@ -396,21 +382,6 @@ cmd_update()just_cloned=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}\
@@ -2746,17 +2746,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);
@@ -2967,6 +2959,8 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)/* NEEDSWORK: this is a temporary name until we delete update_submodule() */staticintupdate_submodule2(structupdate_data*update_data){+ensure_core_worktree(update_data->sm_path);+/* NEEDSWORK: fix the style issues e.g. braces */if(update_data->just_cloned){oidcpy(&update_data->suboid,null_oid());
Teach "git submodule--helper update-clone" the --init flag and remove
the corresponding shell code.
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. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.
[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 26 ++++++++++++++++++++++++++
git-submodule.sh | 9 +++------
2 files changed, 29 insertions(+), 6 deletions(-)
@@ -2483,6 +2484,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;structoptionmodule_update_clone_options[]={+OPT_BOOL(0,"init",&opt.init,+N_("initialize uninitialized submodules before update")),OPT_STRING(0,"prefix",&prefix,N_("path"),N_("path into the working tree")),
A subsequent commit will change the internals of several functions and
arrange them in a more logical manner. Move these functions to their
final positions so that the diff is smaller.
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 228 ++++++++++++++++++------------------
1 file changed, 114 insertions(+), 114 deletions(-)
@@ -2451,120 +2451,6 @@ static void update_submodule(struct update_clone_data *ucd)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_BOOL(0,"init",&opt.init,-N_("initialize uninitialized submodules before update")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"recursive-prefix",&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;--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);-}-/**NEEDSWORK:Useaforwarddeclarationtoavoidmoving*run_update_procedure()(whichwillberemovedsoon).
@@ -3021,6 +2907,120 @@ static int update_submodule2(struct update_data *update_data)return3;}+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_BOOL(0,"init",&opt.init,+N_("initialize uninitialized submodules before update")),+OPT_STRING(0,"prefix",&prefix,+N_("path"),+N_("path into the working tree")),+OPT_STRING(0,"recursive-prefix",&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;++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;
A later commit will combine the "update-clone" and
"run-update-procedure" commands, so run_update_procedure() will be
removed. Prepare for this by moving as much logic as possible out of
run_update_procedure() and into update_submodule2().
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
@@ -2471,10 +2472,10 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)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,+OPT_STRING(0,"prefix",&opt.prefix,N_("path"),N_("path into the working tree")),-OPT_STRING(0,"update",&update,+OPT_STRING(0,"update",&opt.update_default,N_("string"),N_("rebase, merge, checkout or none")),OPT_STRING(0,"recursive-prefix",&opt.recursive_prefix,N_("path"),
@@ -2871,8 +2860,24 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)/* NEEDSWORK: this is a temporary name until we delete update_submodule() */staticintupdate_submodule2(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);++determine_submodule_update_strategy(the_repository,update_data->just_cloned,+update_data->sm_path,update_data->update_default,+&update_data->update_strategy);+/* NEEDSWORK: fix the style issues e.g. braces */if(update_data->just_cloned){oidcpy(&update_data->suboid,null_oid());
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.
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>
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 312 ++++++++++++++++++++++--------------
git-submodule.sh | 105 ++----------
2 files changed, 201 insertions(+), 216 deletions(-)
@@ -1977,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;
@@ -2356,40 +2394,35 @@ static int run_update_command(struct update_data *ud, int subforce)if(run_command(&cp)){switch(ud->update_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);+die_message(_("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);+die_message(_("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);+die_message(_("Execution of '%s %s' failed in submodule path '%s'"),+ud->update_strategy.command,oid,ud->displaypath);break;default:BUG("unexpected update strategy type: %s",submodule_strategy_to_string(&ud->update_strategy));}-/*-*NEEDSWORK:Wearecurrentlyprintingtostdoutwitherror-*returnsothattheshellcallerhandlestheerroroutput-*properly.Oncewestarthandlingtheerrormessageswithin-*C,weshouldusedie()instead.-*/if(must_die_on_failure)-return2;-/*-*Thissignifiestothecallerinshellthatthecommand-*failedwithoutdying-*/+exit(128);++/* the command failed, but update must continue */return1;}+if(ud->quiet)+return0;+switch(ud->update_strategy.type){caseSM_UPDATE_CHECKOUT:printf(_("Submodule path '%s': checked out '%s'\n"),
@@ -2415,7 +2448,7 @@ static int run_update_command(struct update_data *ud, int subforce)return0;}-staticintdo_run_update_procedure(structupdate_data*ud)+staticintrun_update_procedure(structupdate_data*ud){intsubforce=is_null_oid(&ud->suboid)||ud->force;
@@ -2445,76 +2478,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);-}--/*-*NEEDSWORK:Useaforwarddeclarationtoavoidmoving-*run_update_procedure()(whichwillberemovedsoon).-*/-staticintupdate_submodule2(structupdate_data*update_data);-staticintrun_update_procedure(intargc,constchar**argv,constchar*prefix)-{-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",&opt.prefix,-N_("path"),-N_("path into the working tree")),-OPT_STRING(0,"update",&opt.update_default,-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_BOOL(0,"remote",&opt.remote,-N_("use SHA-1 of submodule's remote tracking branch")),-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];--returnupdate_submodule2(&opt);-}--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;
@@ -2857,8 +2820,53 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)return!!ret;}-/* NEEDSWORK: this is a temporary name until we delete update_submodule() */-staticintupdate_submodule2(structupdate_data*update_data)+staticvoidupdate_data_to_args(structupdate_data*update_data,structstrvec*args)+{+strvec_pushl(args,"submodule--helper","update","--recursive",NULL);+strvec_pushf(args,"--jobs=%d",update_data->max_jobs);+/*+*NEEDSWORK:theequivalentcodeingit-submodule.shdoesnot+*pass--prefix,sothisshouldn'teither+*/+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_data->update_default)+strvec_pushl(args,"--update",update_data->update_default,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;
@@ -2907,18 +2915,55 @@ static int update_submodule2(struct update_data *update_data)}if(!oideq(&update_data->oid,&update_data->suboid)||update_data->force)-returndo_run_update_procedure(update_data);+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);-return3;+/* 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(structsubmodule_update_clone*suc)+staticintupdate_submodules(structupdate_data*update_data){-inti;+inti,res=0;+structsubmodule_update_clonesuc=SUBMODULE_UPDATE_CLONE_INIT;-run_processes_parallel_tr2(suc->max_jobs,update_clone_get_next_task,+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",+update_clone_task_finished,&suc,"submodule","parallel/update");/*
@@ -2929,50 +2974,69 @@ static int update_submodules(struct submodule_update_clone *suc)*checkoutinvolvemorestraightforwardsequentialI/O.*-thelistenercanavoiddoinganyworkiffetchingfailed.*/-if(suc->quickstop)-return1;+if(suc.quickstop){+res=1;+gotocleanup;+}-for(i=0;i<suc->update_clone_nr;i++)-update_submodule(&suc->update_clone[i]);+for(i=0;i<suc.update_clone_nr;i++){+structupdate_clone_dataucd=suc.update_clone[i];-return0;+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;}-staticintupdate_clone(intargc,constchar**argv,constchar*prefix)+staticintmodule_update(intargc,constchar**argv,constchar*prefix){-constchar*update=NULL;structpathspecpathspec;-structsubmodule_update_cloneopt=SUBMODULE_UPDATE_CLONE_INIT;+structupdate_dataopt=UPDATE_DATA_INIT;+/* NEEDSWORK: update names and strings */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_STRING(0,"prefix",&prefix,+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,+OPT_STRING(0,"update",&opt.update_default,N_("string"),N_("rebase, merge, checkout or none")),OPT_STRING_LIST(0,"reference",&opt.references,N_("repo"),-N_("reference repository")),+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")),+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")),+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")),+N_("force cloning progress")),OPT_BOOL(0,"require-init",&opt.require_init,-N_("disallow cloning into non-empty directory")),+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()
@@ -2982,16 +3046,18 @@ static int update_clone(int argc, const char **argv, const char *prefix)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);+oidcpy(&opt.oid,null_oid());+oidcpy(&opt.suboid,null_oid());-if(update)-if(parse_submodule_update_strategy(update,&opt.update)<0)+if(opt.update_default)+if(parse_submodule_update_strategy(opt.update_default,+&opt.update_strategy)<0)die(_("bad value for update parameter"));if(module_list_compute(argc,argv,prefix,&pathspec,&opt.list)<0)
@@ -50,6 +50,7 @@ single_branch=jobs=recommend_shallow=+# NEEDSWORK this is now unused die_if_unmatched(){iftest"$1"="#unmatched"
@@ -347,108 +348,28 @@ cmd_update()shiftdone-{-git${wt_prefix:+-C "$wt_prefix"}submodule--helperupdate-clone\+# NEEDSWORK --super-prefix isn't actually supported by this+# command - we just pass the $prefix to --recursive-prefix.+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\${GIT_QUIET:+--quiet}\-${progress:+"--progress"}\+${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"--displaypath=$(gitsubmodule--helperrelative-path"$prefix$sm_path""$wt_prefix")--iftest$just_cloned-eq0-then-just_cloned=-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"}\-${remote:+--remote}\-"--"\-"$sm_path")--# exit codes for run-update-procedure:-# 0: update was successful, say command output-# 1: update procedure failed, but should not die-# 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-}+"$@"}#
@@ -2886,14 +2886,11 @@ static int update_submodule(struct update_data *update_data)update_data->sm_path,update_data->update_default,&update_data->update_strategy);-/* NEEDSWORK: fix the style issues e.g. braces */-if(update_data->just_cloned){+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);-}+elseif(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);
@@ -50,15 +50,6 @@ single_branch=jobs=recommend_shallow=-# NEEDSWORK this is now unused-die_if_unmatched()-{-iftest"$1"="#unmatched"-then-exit${2:-1}-fi-}- isnumber(){n=$(($1+0))2>/dev/null&&test"$n"="$1"
@@ -348,9 +339,7 @@ cmd_update()shiftdone-# NEEDSWORK --super-prefix isn't actually supported by this-# command - we just pass the $prefix to --recursive-prefix.-git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\+git${wt_prefix:+-C "$wt_prefix"}submodule--helperupdate\${GIT_QUIET:+--quiet}\${force:+--force}\${progress:+--progress}\
@@ -2355,15 +2356,8 @@ 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;-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){+switch(ud->update_strategy.type){caseSM_UPDATE_CHECKOUT:cp.git_cmd=1;strvec_pushl(&cp.args,"checkout","-q",NULL);
@@ -2386,45 +2380,41 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:cp.use_shell=1;-strvec_push(&cp.args,strategy.command);+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(&strategy));+submodule_strategy_to_string(&ud->update_strategy));}strvec_push(&cp.args,oid);cp.dir=xstrdup(ud->sm_path);prepare_submodule_repo_env(&cp.env_array);if(run_command(&cp)){-switch(strategy.type){+switch(ud->update_strategy.type){caseSM_UPDATE_CHECKOUT:die_message(_("Unable to checkout '%s' in submodule path '%s'"),oid,ud->displaypath);break;caseSM_UPDATE_REBASE:-if(!must_die_on_failure)-break;-die(_("Unable to rebase '%s' in submodule path '%s'"),+die_message(_("Unable to rebase '%s' in submodule path '%s'"),oid,ud->displaypath);break;caseSM_UPDATE_MERGE:-if(!must_die_on_failure)-break;-die(_("Unable to merge '%s' in submodule path '%s'"),+die_message(_("Unable to merge '%s' in submodule path '%s'"),oid,ud->displaypath);break;caseSM_UPDATE_COMMAND:-if(!must_die_on_failure)-break;-die(_("Execution of '%s %s' failed in submodule path '%s'"),-strategy.command,oid,ud->displaypath);+die_message(_("Execution of '%s %s' failed in submodule path '%s'"),+ud->update_strategy.command,oid,ud->displaypath);break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&strategy));+submodule_strategy_to_string(&ud->update_strategy));}+if(must_die_on_failure)+exit(128);/* the command failed, but update must continue */return1;
@@ -2433,7 +2423,7 @@ static int run_update_command(struct update_data *ud, int subforce)if(ud->quiet)return0;-switch(strategy.type){+switch(ud->update_strategy.type){caseSM_UPDATE_CHECKOUT:printf(_("Submodule path '%s': checked out '%s'\n"),ud->displaypath,oid);
@@ -2448,11 +2438,11 @@ static int run_update_command(struct update_data *ud, int subforce)break;caseSM_UPDATE_COMMAND:printf(_("Submodule path '%s': '%s %s'\n"),-ud->displaypath,strategy.command,oid);+ud->displaypath,ud->update_strategy.command,oid);break;default:BUG("unexpected update strategy type: %s",-submodule_strategy_to_string(&strategy));+submodule_strategy_to_string(&ud->update_strategy));}return0;
@@ -2890,6 +2882,11 @@ static int update_submodule(struct update_data *update_data)update_data->prefix);free(prefixed_path);+determine_submodule_update_strategy(the_repository,update_data->just_cloned,+update_data->sm_path,update_data->update_default,+&update_data->update_strategy);++/* NEEDSWORK: fix the style issues e.g. braces */if(update_data->just_cloned){oidcpy(&update_data->suboid,null_oid());}else{
@@ -3000,10 +2997,10 @@ static int update_submodules(struct update_data *update_data)staticintmodule_update(intargc,constchar**argv,constchar*prefix){-constchar*update=NULL;structpathspecpathspec;structupdate_dataopt=UPDATE_DATA_INIT;+/* NEEDSWORK: update names and strings */structoptionmodule_update_clone_options[]={OPT__FORCE(&opt.force,N_("force checkout updates"),0),OPT_BOOL(0,"init",&opt.init,
@@ -3021,7 +3018,7 @@ static int module_update(int argc, const char **argv, const char *prefix)N_("path"),N_("path into the working tree, across nested ""submodule boundaries")),-OPT_STRING(0,"update",&update,+OPT_STRING(0,"update",&opt.update_default,N_("string"),N_("rebase, merge, checkout or none")),OPT_STRING_LIST(0,"reference",&opt.references,N_("repo"),
@@ -3058,8 +3055,8 @@ static int module_update(int argc, const char **argv, const char *prefix)oidcpy(&opt.oid,null_oid());oidcpy(&opt.suboid,null_oid());-if(update)-if(parse_submodule_update_strategy(update,+if(opt.update_default)+if(parse_submodule_update_strategy(opt.update_default,&opt.update_strategy)<0)die(_("bad value for update parameter"));
@@ -3490,6 +3487,7 @@ static struct cmd_struct commands[] = {{"sync",module_sync,SUPPORT_SUPER_PREFIX},{"deinit",module_deinit,0},{"summary",module_summary,SUPPORT_SUPER_PREFIX},+/* NEEDSWORK: remote-branch is also obsolete */{"remote-branch",resolve_remote_submodule_branch,0},{"push-check",push_check,0},{"absorb-git-dirs",absorb_git_dirs,SUPPORT_SUPER_PREFIX},
@@ -50,6 +50,7 @@ single_branch=jobs=recommend_shallow=+# NEEDSWORK this is now unused die_if_unmatched(){iftest"$1"="#unmatched"
@@ -347,6 +348,8 @@ cmd_update()shiftdone+# NEEDSWORK --super-prefix isn't actually supported by this+# command - we just pass the $prefix to --recursive-prefix.git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperupdate\${GIT_QUIET:+--quiet}\${force:+--force}\
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)
Nit: overly long line (also in my v5, but since we're applying some
final polishing touches...)
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: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <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);
Note/nit on existing (pre this series) code, I wonder why we ended up
with this init_submodule() v.s. init_submodule_cb() indirection,
v.s. just doing:
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'"),
If this series is re-arranged so that this comes first, this compiles
just fine & passes all tests.
So I wonder if we can peel this and perhaps other such easy to review
prep changes off into its own series, since this one has grown to 20
patches.
We could then hopefully fast-track those easy-to-review prep changes,
which would make the "real" series to follow smaller and easier to
review/grok.
This is dead code - it has not been used since c51f8f94e5
(submodule--helper: run update procedures from C, 2021-08-24).
Signed-off-by: Glen Choo <redacted>
---
builtin/submodule--helper.c | 24 ------------------------
1 file changed, 24 deletions(-)
Nice catch!
Re my comment on 04/20 in [off-list ref]
at least 04..09/20 could be split into such a "trivial refactors for
later changes" series, and it would make sense to lead with this (and
any other deletions of already-dead code).
A subsequent commit will change the internals of several functions and
arrange them in a more logical manner. Move these functions to their
final positions so that the diff is smaller.
Shouldn't we do this earlier & avoid the FIXME comment in 10/20? (maybe
a subsequent fixup addresses it, haven't checked...)
@@ -2886,14 +2886,11 @@ static int update_submodule(struct update_data *update_data)update_data->sm_path,update_data->update_default,&update_data->update_strategy);-/* NEEDSWORK: fix the style issues e.g. braces */-if(update_data->just_cloned){+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);-}+elseif(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);
This fixup looks good, let's apply this fix-up to the relevant preceding
commit.
This reroll contains another 'easy' preparatory patch and the fixups I
alluded to in v6 [1]. This isn't the split-up I described in the
footnote of v6, but it gets the big patch (patch 17) to what I think is
a reviewable state.
The diff between v7 and v5 is no longer just NEEDSWORK comments, but I
think it is easier to reason about. Patch 17 resembles v5 the most (I
will include a diff in a reply to that patch); everything after patch 17
is fixups (I did not squash them in because they would grow the diff
even more).
I will also leave a review on patch 17 since the changes were not
originally authored by me.
[1] https://lore.kernel.org/git/20220208083952.35036-1-chooglen@google.com
Changes in v7:
- Split the last patch of v6 (the big one) into patches 16-17.
- Patch 16 moves logic out of run_update_procedure() (because the
command is going away), removing some noise from patch 17. This makes
the update_strategy parsing easier to reason about, but at the cost of
growing the diff vis-a-vis v5
- Patches 18-20 are fixups that address NEEDSWORK comments from earlier
patches. Once maintaining a small diff vis-a-vis v5 stops making
sense, I will squash them in.
Atharva Raykar (6):
submodule--helper: get remote names from any repository
submodule--helper: refactor get_submodule_displaypath()
submodule--helper: allow setting superprefix for init_submodule()
submodule--helper: run update using child process struct
builtin/submodule--helper.c: reformat designated initializers
submodule: move core cmd_update() logic to C
Glen Choo (11):
submodule--helper: remove update-module-mode
submodule--helper: reorganize code for sh to C conversion
submodule--helper run-update-procedure: remove --suboid
submodule--helper run-update-procedure: learn --remote
submodule--helper: remove ensure-core-worktree
submodule--helper update-clone: learn --init
submodule--helper: move functions around
submodule--helper: reduce logic in run_update_procedure()
fixup! submodule--helper run-update-procedure: remove --suboid
fixup! submodule--helper run-update-procedure: learn --remote
fixup! submodule: move core cmd_update() logic to C
Ævar Arnfjörð Bjarmason (3):
builtin/submodule--helper.c: rename option variables to "opt"
submodule--helper: don't use bitfield indirection for parse_options()
submodule tests: test for init and update failure output
Thanks a lot for picking this up! This split-up is much easier to read
than my v5, particularly with the end diff-stat of the "main" patch
being:
2 files changed, 201 insertions(+), 216 deletions(-)
Instead of:
2 files changed, 356 insertions(+), 388 deletions(-)
I think sending a version of this with the fixups squashed in as a v8
would be good, and perhaps addressing some of my comments.
I don't know if my suggested split-up of "prep fixes" into another
series would be a good thing to pursue overall, perhaps Junio will chime
in on how he'd be most comfortable in merging this down. I'd think
splitting such trivial fixes into their own series be easier to review,
but perhaps not.
For the Signed-off-by question on v6, I think you should add your SOB to
all the patches you submit. See this in SubmittingPatches:
Notice that you can place your own `Signed-off-by` trailer when
forwarding somebody else's patch with the above rules for
D-C-O. Indeed you are encouraged to do so.
Just running "git rebase -i -x 'git commit --amend --no-edit -s'" should
do it.
Atharva Raykar (6):
submodule--helper: get remote names from any repository
submodule--helper: refactor get_submodule_displaypath()
submodule--helper: allow setting superprefix for init_submodule()
submodule--helper: run update using child process struct
builtin/submodule--helper.c: reformat designated initializers
submodule: move core cmd_update() logic to C
Glen Choo (11):
submodule--helper: remove update-module-mode
submodule--helper: reorganize code for sh to C conversion
submodule--helper run-update-procedure: remove --suboid
submodule--helper run-update-procedure: learn --remote
submodule--helper: remove ensure-core-worktree
submodule--helper update-clone: learn --init
submodule--helper: move functions around
submodule--helper: reduce logic in run_update_procedure()
fixup! submodule--helper run-update-procedure: remove --suboid
fixup! submodule--helper run-update-procedure: learn --remote
fixup! submodule: move core cmd_update() logic to C
Ævar Arnfjörð Bjarmason (3):
builtin/submodule--helper.c: rename option variables to "opt"
submodule--helper: don't use bitfield indirection for parse_options()
submodule tests: test for init and update failure output
I think sending a version of this with the fixups squashed in as a v8
would be good, and perhaps addressing some of my comments.
I don't know if my suggested split-up of "prep fixes" into another
series would be a good thing to pursue overall, perhaps Junio will chime
in on how he'd be most comfortable in merging this down. I'd think
splitting such trivial fixes into their own series be easier to review,
but perhaps not.
Combing through the patches again, I couldn't really convince myself
that the patch 4..9 prep fixes make sense as obvious standalone fixes,
except maybe:
- patch 4 submodule--helper: run update using child process struct
- patch 8 submodule tests: test for init and update failure output
- patch 9: 087bf43aba submodule--helper: remove update-module-mode
But, since the 'final' patch (ignoring the fixup!-s) is consuming a huge
chunk of the work anyway, here's an alternative patch organization with
the fixup!-s squashed:
= Move 'easy' and 'obviously correct' code from sh->C
- patches 8-9 Cleanup and introduce tests
- patches 1-4 Refactor existing functions, which enables..
- patches 10-14 Move 'obviously correct' pieces of logic from sh-> C
= Finalize move from sh->C
i.e. combine "run-update-procedure" and "update-clone" into "update"
- patches 5,7 Cleanup and prep
- patches 6,15-16 Shrinking the diff
- patch 17 Implement "git submodule--helper update"
I'll send this if there are no objections :)
Atharva Raykar (6):
submodule--helper: get remote names from any repository
submodule--helper: refactor get_submodule_displaypath()
submodule--helper: allow setting superprefix for init_submodule()
submodule--helper: run update using child process struct
builtin/submodule--helper.c: reformat designated initializers
submodule: move core cmd_update() logic to C
Glen Choo (11):
submodule--helper: remove update-module-mode
submodule--helper: reorganize code for sh to C conversion
submodule--helper run-update-procedure: remove --suboid
submodule--helper run-update-procedure: learn --remote
submodule--helper: remove ensure-core-worktree
submodule--helper update-clone: learn --init
submodule--helper: move functions around
submodule--helper: reduce logic in run_update_procedure()
fixup! submodule--helper run-update-procedure: remove --suboid
fixup! submodule--helper run-update-procedure: learn --remote
fixup! submodule: move core cmd_update() logic to C
Ævar Arnfjörð Bjarmason (3):
builtin/submodule--helper.c: rename option variables to "opt"
submodule--helper: don't use bitfield indirection for parse_options()
submodule tests: test for init and update failure output
I think sending a version of this with the fixups squashed in as a v8
would be good, and perhaps addressing some of my comments.
I don't know if my suggested split-up of "prep fixes" into another
series would be a good thing to pursue overall, perhaps Junio will chime
in on how he'd be most comfortable in merging this down. I'd think
splitting such trivial fixes into their own series be easier to review,
but perhaps not.
Combing through the patches again, I couldn't really convince myself
that the patch 4..9 prep fixes make sense as obvious standalone fixes,
except maybe:
- patch 4 submodule--helper: run update using child process struct
- patch 8 submodule tests: test for init and update failure output
- patch 9: 087bf43aba submodule--helper: remove update-module-mode
But, since the 'final' patch (ignoring the fixup!-s) is consuming a huge
chunk of the work anyway, here's an alternative patch organization with
the fixup!-s squashed:
= Move 'easy' and 'obviously correct' code from sh->C
- patches 8-9 Cleanup and introduce tests
- patches 1-4 Refactor existing functions, which enables..
- patches 10-14 Move 'obviously correct' pieces of logic from sh-> C
= Finalize move from sh->C
i.e. combine "run-update-procedure" and "update-clone" into "update"
- patches 5,7 Cleanup and prep
- patches 6,15-16 Shrinking the diff
- patch 17 Implement "git submodule--helper update"
I'll send this if there are no objections :)
Yes that sounds good, or rather, I haven't re-looked at that in detail,
but I think if you think it makes sense we should go for it.
Or rather, we should really be aiming to produce a patch series that
makes sense in its current iteration, as opposed to optimizing for a
diff against some ad-hoc re-roll I produced a few versions ago :)
Thanks again for working on this & picking this up. It's great to see
progress in this area!
Atharva Raykar (6):
submodule--helper: get remote names from any repository
submodule--helper: refactor get_submodule_displaypath()
submodule--helper: allow setting superprefix for init_submodule()
submodule--helper: run update using child process struct
builtin/submodule--helper.c: reformat designated initializers
submodule: move core cmd_update() logic to C
Glen Choo (11):
submodule--helper: remove update-module-mode
submodule--helper: reorganize code for sh to C conversion
submodule--helper run-update-procedure: remove --suboid
submodule--helper run-update-procedure: learn --remote
submodule--helper: remove ensure-core-worktree
submodule--helper update-clone: learn --init
submodule--helper: move functions around
submodule--helper: reduce logic in run_update_procedure()
fixup! submodule--helper run-update-procedure: remove --suboid
fixup! submodule--helper run-update-procedure: learn --remote
fixup! submodule: move core cmd_update() logic to C
Ævar Arnfjörð Bjarmason (3):
builtin/submodule--helper.c: rename option variables to "opt"
submodule--helper: don't use bitfield indirection for parse_options()
submodule tests: test for init and update failure output
I think sending a version of this with the fixups squashed in as a v8
would be good, and perhaps addressing some of my comments.
I don't know if my suggested split-up of "prep fixes" into another
series would be a good thing to pursue overall, perhaps Junio will chime
in on how he'd be most comfortable in merging this down. I'd think
splitting such trivial fixes into their own series be easier to review,
but perhaps not.
Combing through the patches again, I couldn't really convince myself
that the patch 4..9 prep fixes make sense as obvious standalone fixes,
except maybe:
- patch 4 submodule--helper: run update using child process struct
- patch 8 submodule tests: test for init and update failure output
- patch 9: 087bf43aba submodule--helper: remove update-module-mode
But, since the 'final' patch (ignoring the fixup!-s) is consuming a huge
chunk of the work anyway, here's an alternative patch organization with
the fixup!-s squashed:
= Move 'easy' and 'obviously correct' code from sh->C
- patches 8-9 Cleanup and introduce tests
- patches 1-4 Refactor existing functions, which enables..
- patches 10-14 Move 'obviously correct' pieces of logic from sh-> C
= Finalize move from sh->C
i.e. combine "run-update-procedure" and "update-clone" into "update"
- patches 5,7 Cleanup and prep
- patches 6,15-16 Shrinking the diff
- patch 17 Implement "git submodule--helper update"
I'll send this if there are no objections :)
Yes that sounds good, or rather, I haven't re-looked at that in detail,
but I think if you think it makes sense we should go for it.
Or rather, we should really be aiming to produce a patch series that
makes sense in its current iteration, as opposed to optimizing for a
diff against some ad-hoc re-roll I produced a few versions ago :)
Agreed, makes sense.
Thanks again for working on this & picking this up. It's great to see
progress in this area!
Thanks to you too for getting the ball rolling and lending me your
thoughts :)