NOTE: This series uses the change introduced by 'ar/submodule-add-config'[1]
This series completes the conversion of all the important shell logic in
'submodule add' to C, by wrapping it in a submodule--helper builtin subcommand
called 'add'.
The first 4 patches are preparatory patches. The refactors mostly involve
exposing interfaces to C that were only previously usable as shell subcommands.
Then we have a patch that translates the shell code to C, faithfully reproducing
the behaviour before the conversion.
The last 3 patches are cleanup patches. Our conversions have introduced a lot of
dead code, all of them being 'submodule--helper' subcommands that have no
further use, as we have C interfaces for these already. We remove these
subcommands.
A question about the cache API used in [PATCH 5/8]:
What is the difference between 'read_cache()' and 'read_cache_preload()'? [2]
Which one is more appropriate for use in 'die_on_index_match()'?
Fetch-it-Via:
git fetch https://github.com/tfidfwastaken/git submodule-helper-add-list-1
Footnotes
=========
[1] https://lore.kernel.org/git/20210801063352.50813-1-raykar.ath@gmail.com/
[2] More about this question has been detailed in this section of my blog:
http://atharvaraykar.me/gitnotes/week5#some-challenges-with-the-changes-that-are-cooking
I'll quote it here for convenience:
Before iterating through the cache entries of the index, you need to populate
it.
There’s two functions for this: read_cache() and read_cache_preload(). I have
used the latter in my code. The thing is, when I swap it with the former, I
could not find any change in the behaviour of my code. They appear to function
equivalently.
I understand that the *_preload() variant takes a pathspec which preloads index
contents that match the pathspec in parallel. I don’t know what passing NULL to
it does. Moreover, does this imply that read_cache() loads the cache on-demand,
ie, it does no preloading?
I am not sure about what exactly are their differences, and when is one variant
preferred over the other.
Atharva Raykar (8):
submodule--helper: refactor resolve_relative_url() helper
submodule--helper: remove repeated code in sync_submodule()
dir: libify and export helper functions from clone.c
submodule--helper: remove constness of sm_path
submodule--helper: convert the bulk of cmd_add() to C
submodule--helper: remove add-clone subcommand
submodule--helper: remove add-config subcommand
submodule--helper: remove resolve-relative-url subcommand
builtin/clone.c | 118 +-------------
builtin/submodule--helper.c | 304 +++++++++++++++++++-----------------
dir.c | 114 ++++++++++++++
dir.h | 3 +
git-submodule.sh | 96 +-----------
5 files changed, 278 insertions(+), 357 deletions(-)
--
2.32.0
Refactor the helper function to resolve a relative url, by reusing the
existing `compute_submodule_clone_url()` function.
`compute_submodule_clone_url()` performs the same work that
`resolve_relative_url()` is doing, so we eliminate this code repetition
by moving the former function's definition up, and calling it inside
`resolve_relative_url()`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 61 +++++++++++++++----------------------
1 file changed, 25 insertions(+), 36 deletions(-)
@@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url,returnstrbuf_detach(&sb,NULL);}+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)+{+char*remoteurl,*relurl;+char*remote=get_default_remote();+structstrbufremotesb=STRBUF_INIT;++strbuf_addf(&remotesb,"remote.%s.url",remote);+if(git_config_get_string(remotesb.buf,&remoteurl)){+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);+remoteurl=xgetcwd();+}+relurl=relative_url(remoteurl,rel_url,up_path);++free(remote);+free(remoteurl);+strbuf_release(&remotesb);++returnrelurl;+}+staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix){-char*remoteurl=NULL;-char*remote=get_default_remote();constchar*up_path=NULL;char*res;constchar*url;-structstrbufsb=STRBUF_INIT;if(argc!=2&&argc!=3)die("resolve-relative-url only accepts one or two arguments");url=argv[1];-strbuf_addf(&sb,"remote.%s.url",remote);-free(remote);--if(git_config_get_string(sb.buf,&remoteurl))-/* the repository is its own authoritative upstream */-remoteurl=xgetcwd();-if(argc==3)up_path=argv[2];-res=relative_url(remoteurl,url,up_path);+res=compute_submodule_clone_url(url,up_path,1);puts(res);free(res);-free(remoteurl);return0;}
@@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)-{-char*remoteurl,*relurl;-char*remote=get_default_remote();-structstrbufremotesb=STRBUF_INIT;--strbuf_addf(&remotesb,"remote.%s.url",remote);-if(git_config_get_string(remotesb.buf,&remoteurl)){-if(!quiet)-warning(_("could not look up configuration '%s'. "-"Assuming this repository is its own "-"authoritative upstream."),-remotesb.buf);-remoteurl=xgetcwd();-}-relurl=relative_url(remoteurl,rel_url,up_path);--free(remote);-free(remoteurl);-strbuf_release(&remotesb);--returnrelurl;-}-structinit_cb{constchar*prefix;unsignedintflags;
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
These functions can be useful to other parts of Git. Let's move them to
dir.c, while renaming them to be make their functionality more explicit.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/clone.c | 118 +-----------------------------------------------
dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
dir.h | 3 ++
3 files changed, 119 insertions(+), 116 deletions(-)
@@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle)returncanon;}-staticchar*guess_dir_name(constchar*repo,intis_bundle,intis_bare)-{-constchar*end=repo+strlen(repo),*start,*ptr;-size_tlen;-char*dir;--/*-*Skipscheme.-*/-start=strstr(repo,"://");-if(start==NULL)-start=repo;-else-start+=3;--/*-*Skipauthenticationdata.Thestrippingdoeshappen-*greedily,suchthatwestripuptothelast'@'inside-*thehostpart.-*/-for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){-if(*ptr=='@')-start=ptr+1;-}--/*-*Striptrailingspaces,slashesand/.git-*/-while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))-end--;-if(end-start>5&&is_dir_sep(end[-5])&&-!strncmp(end-4,".git",4)){-end-=5;-while(start<end&&is_dir_sep(end[-1]))-end--;-}--/*-*Striptrailingportnumberifwe'vegotonlya-*hostname(thatis,thereisnodirseparatorbuta-*colon).Thischeckisrequiredsuchthatwedonot-*stripURI'slike'/foo/bar:2222.git',whichshould-*resultinadir'2222'beingguessedduetobackwards-*compatibility.-*/-if(memchr(start,'/',end-start)==NULL-&&memchr(start,':',end-start)!=NULL){-ptr=end;-while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')-ptr--;-if(start<ptr&&ptr[-1]==':')-end=ptr-1;-}--/*-*Findlastcomponent.Toremainbackwardscompatiblewe-*alsoregardcolonsaspathseparators,suchthat-*cloningarepository'foo:bar.git'wouldresultina-*directory'bar'beingguessed.-*/-ptr=end;-while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')-ptr--;-start=ptr;--/*-*Strip.{bundle,git}.-*/-len=end-start;-strip_suffix_mem(start,&len,is_bundle?".bundle":".git");--if(!len||(len==1&&*start=='/'))-die(_("No directory name could be guessed.\n"-"Please specify a directory on the command line"));--if(is_bare)-dir=xstrfmt("%.*s.git",(int)len,start);-else-dir=xstrndup(start,len);-/*-*Replacesequencesof'control'charactersandwhitespace-*withoneasciispace,removeleadingandtrailingspaces.-*/-if(*dir){-char*out=dir;-intprev_space=1/* strip leading whitespace */;-for(end=dir;*end;++end){-charch=*end;-if((unsignedchar)ch<'\x20')-ch='\x20';-if(isspace(ch)){-if(prev_space)-continue;-prev_space=1;-}else-prev_space=0;-*out++=ch;-}-*out='\0';-if(out>dir&&prev_space)-out[-1]='\0';-}-returndir;-}--staticvoidstrip_trailing_slashes(char*dir)-{-char*end=dir+strlen(dir);--while(dir<end-1&&is_dir_sep(end[-1]))-end--;-*end='\0';-}-staticintadd_one_reference(structstring_list_item*item,void*cb_data){structstrbuferr=STRBUF_INIT;
@@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path)returnret;}+char*guess_dir_name_from_git_url(constchar*repo,intis_bundle,intis_bare)+{+constchar*end=repo+strlen(repo),*start,*ptr;+size_tlen;+char*dir;++/*+*Skipscheme.+*/+start=strstr(repo,"://");+if(start==NULL)+start=repo;+else+start+=3;++/*+*Skipauthenticationdata.Thestrippingdoeshappen+*greedily,suchthatwestripuptothelast'@'inside+*thehostpart.+*/+for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){+if(*ptr=='@')+start=ptr+1;+}++/*+*Striptrailingspaces,slashesand/.git+*/+while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))+end--;+if(end-start>5&&is_dir_sep(end[-5])&&+!strncmp(end-4,".git",4)){+end-=5;+while(start<end&&is_dir_sep(end[-1]))+end--;+}++/*+*Striptrailingportnumberifwe'vegotonlya+*hostname(thatis,thereisnodirseparatorbuta+*colon).Thischeckisrequiredsuchthatwedonot+*stripURI'slike'/foo/bar:2222.git',whichshould+*resultinadir'2222'beingguessedduetobackwards+*compatibility.+*/+if(memchr(start,'/',end-start)==NULL+&&memchr(start,':',end-start)!=NULL){+ptr=end;+while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')+ptr--;+if(start<ptr&&ptr[-1]==':')+end=ptr-1;+}++/*+*Findlastcomponent.Toremainbackwardscompatiblewe+*alsoregardcolonsaspathseparators,suchthat+*cloningarepository'foo:bar.git'wouldresultina+*directory'bar'beingguessed.+*/+ptr=end;+while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')+ptr--;+start=ptr;++/*+*Strip.{bundle,git}.+*/+len=end-start;+strip_suffix_mem(start,&len,is_bundle?".bundle":".git");++if(!len||(len==1&&*start=='/'))+die(_("No directory name could be guessed.\n"+"Please specify a directory on the command line"));++if(is_bare)+dir=xstrfmt("%.*s.git",(int)len,start);+else+dir=xstrndup(start,len);+/*+*Replacesequencesof'control'charactersandwhitespace+*withoneasciispace,removeleadingandtrailingspaces.+*/+if(*dir){+char*out=dir;+intprev_space=1/* strip leading whitespace */;+for(end=dir;*end;++end){+charch=*end;+if((unsignedchar)ch<'\x20')+ch='\x20';+if(isspace(ch)){+if(prev_space)+continue;+prev_space=1;+}else+prev_space=0;+*out++=ch;+}+*out='\0';+if(out>dir&&prev_space)+out[-1]='\0';+}+returndir;+}++voidstrip_dir_trailing_slashes(char*dir)+{+char*end=dir+strlen(dir);++while(dir<end-1&&is_dir_sep(end[-1]))+end--;+*end='\0';+}+staticintremove_dir_recurse(structstrbuf*path,intflag,int*kept_up){DIR*dir;
This is needed so that it can be modified by normalize_path_copy() in
the next patch.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 160 ++++++++++++++++++++++++++++++++++++
git-submodule.sh | 96 +---------------------
2 files changed, 162 insertions(+), 94 deletions(-)
@@ -3046,6 +3046,165 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=guess_dir_name_from_git_url(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);++if(prefix&&*prefix&&!is_absolute_path(add_data.sm_path))+add_data.sm_path=xstrfmt("%s%s",prefix,add_data.sm_path);++if(starts_with_dot_dot_slash(add_data.repo)||+starts_with_dot_slash(add_data.repo)){+if(prefix)+die(_("Relative path can only be used from the toplevel "+"of the working tree"));++/* dereference source url relative to parent's url */+add_data.realrepo=compute_submodule_clone_url(add_data.repo,NULL,1);+}elseif(is_dir_sep(add_data.repo[0])||strchr(add_data.repo,':')){+add_data.realrepo=add_data.repo;+}else{+die(_("repo URL: '%s' must be absolute or begin with ./|../"),+add_data.repo);+}++/*+*normalizepath:+*multiple//; leading ./; /./; /../;+*/+normalize_path_copy(add_data.sm_path,add_data.sm_path);+strip_dir_trailing_slashes(add_data.sm_path);++die_on_index_match(add_data.sm_path,force);+die_on_repo_without_commits(add_data.sm_path);++if(!force){+intexit_code=-1;+structstrbufsb=STRBUF_INIT;+structchild_processcp=CHILD_PROCESS_INIT;+cp.git_cmd=1;+cp.no_stdout=1;+strvec_pushl(&cp.args,"add","--dry-run","--ignore-missing",+"--no-warn-embedded-repo",add_data.sm_path,NULL);+if((exit_code=pipe_command(&cp,NULL,0,NULL,0,&sb,0))){+strbuf_complete_line(&sb);+fputs(sb.buf,stderr);+returnexit_code;+}+strbuf_release(&sb);+}++if(!add_data.sm_name)+add_data.sm_name=add_data.sm_path;++if(check_submodule_name(add_data.sm_name))+die(_("'%s' is not a valid submodule name"),add_data.sm_name);++add_data.prefix=prefix;+add_data.force=!!force;+add_data.quiet=!!quiet;+add_data.progress=!!progress;+add_data.dissociate=!!dissociate;++if(add_submodule(&add_data))+return1;+configure_added_submodule(&add_data);+free(add_data.sm_path);++return0;+}+#define SUPPORT_SUPER_PREFIX (1<<0)structcmd_struct{
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
We no longer need this subcommand, as all of its functionality is being
called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 60 -------------------------------------
1 file changed, 60 deletions(-)
@@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data)return0;}-staticintadd_clone(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,dissociate=0,progress=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to checkout on cloning")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("alternative anchor for relative paths")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT_STRING(0,"url",&add_data.realrepo,-N_("string"),-N_("url where to clone the submodule from")),-OPT_STRING(0,"reference",&add_data.reference_path,-N_("repo"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&dissociate,-N_("use --reference only while cloning")),-OPT_INTEGER(0,"depth",&add_data.depth,-N_("depth for shallow clones")),-OPT_BOOL(0,"progress",&progress,-N_("force cloning progress")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT__QUIET(&quiet,"suppress output for cloning a submodule"),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-clone [<options>...] "-"--url <url> --path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc!=0)-usage_with_options(usage,options);--add_data.prefix=prefix;-add_data.progress=!!progress;-add_data.dissociate=!!dissociate;-add_data.force=!!force;-add_data.quiet=!!quiet;--if(add_submodule(&add_data))-return1;--return0;-}-staticintconfig_submodule_in_gitmodules(constchar*name,constchar*var,constchar*value){char*key;
The shell subcommand `resolve-relative-url` is no longer required, as
its last caller has been removed when it was converted to C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 20 --------------------
1 file changed, 20 deletions(-)
@@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_patreturnrelurl;}-staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix)-{-constchar*up_path=NULL;-char*res;-constchar*url;--if(argc!=2&&argc!=3)-die("resolve-relative-url only accepts one or two arguments");--url=argv[1];-if(argc==3)-up_path=argv[2];--res=compute_submodule_clone_url(url,up_path,1);-puts(res);-free(res);-return0;-}-staticintresolve_relative_url_test(intargc,constchar**argv,constchar*prefix){char*remoteurl,*res;
Also no longer needed is this subcommand, as all of its functionality is
being called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 49 -------------------------------------
1 file changed, 49 deletions(-)
@@ -2939,54 +2939,6 @@ static void configure_added_submodule(struct add_data *add_data)}}-staticintadd_config(intargc,constchar**argv,constchar*prefix)-{-intforce=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to store in "-"the submodule configuration")),-OPT_STRING(0,"url",&add_data.repo,-N_("string"),-N_("url to clone submodule from")),-OPT_STRING(0,"resolved-url",&add_data.realrepo,-N_("string"),-N_("url to clone the submodule from, after it has "-"been dereferenced relative to parent's url, "-"in the case where <url> is a relative url")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-config "-"[--force|-f] [--branch|-b <branch>] "-"--url <url> --resolved-url <resolved-url> "-"--path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc)-usage_with_options(usage,options);--add_data.force=!!force;-configure_added_submodule(&add_data);--return0;-}-staticvoiddie_on_index_match(constchar*path,intforce){structpathspecps;
I missed the adding the first patch of this series, so I am re-sending with the
missing commit included.
No change since v1, except for the inclusion of 75edf24186 (submodule--helper:
add options for compute_submodule_clone_url(), 2021-07-06) at the beginning.
I am including the cover letter of v1 as-is, so the discussion can start from
here, with minor edits to reflect the accurate counts of the patches:
---8<------8<------8<------8<------8<---
NOTE: This series uses the change introduced by 'ar/submodule-add-config'[1]
This series completes the conversion of all the important shell logic in
'submodule add' to C, by wrapping it in a submodule--helper builtin subcommand
called 'add'.
The first 5 patches are preparatory patches. The refactors mostly involve
exposing interfaces to C that were only previously usable as shell subcommands.
Then we have a patch that translates the shell code to C, faithfully reproducing
the behaviour before the conversion.
The last 3 patches are cleanup patches. Our conversions have introduced a lot of
dead code, all of them being 'submodule--helper' subcommands that have no
further use, as we have C interfaces for these already. We remove these
subcommands.
A question about the cache API used in [PATCH 5/9]:
What is the difference between 'read_cache()' and 'read_cache_preload()'? [2]
Which one is more appropriate for use in 'die_on_index_match()'?
Fetch-it-Via:
git fetch https://github.com/tfidfwastaken/git submodule-helper-add-list-1
Footnotes
=========
[1] https://lore.kernel.org/git/20210801063352.50813-1-raykar.ath@gmail.com/
[2] More about this question has been detailed in this section of my blog:
http://atharvaraykar.me/gitnotes/week5#some-challenges-with-the-changes-that-are-cooking
I'll quote it here for convenience:
Before iterating through the cache entries of the index, you need to populate
it.
There’s two functions for this: read_cache() and read_cache_preload(). I have
used the latter in my code. The thing is, when I swap it with the former, I
could not find any change in the behaviour of my code. They appear to function
equivalently.
I understand that the *_preload() variant takes a pathspec which preloads index
contents that match the pathspec in parallel. I don’t know what passing NULL to
it does. Moreover, does this imply that read_cache() loads the cache on-demand,
ie, it does no preloading?
I am not sure about what exactly are their differences, and when is one variant
preferred over the other.
--->8------>8------>8------>8------>8---
Atharva Raykar (9):
submodule--helper: add options for compute_submodule_clone_url()
submodule--helper: refactor resolve_relative_url() helper
submodule--helper: remove repeated code in sync_submodule()
dir: libify and export helper functions from clone.c
submodule--helper: remove constness of sm_path
submodule--helper: convert the bulk of cmd_add() to C
submodule--helper: remove add-clone subcommand
submodule--helper: remove add-config subcommand
submodule--helper: remove resolve-relative-url subcommand
builtin/clone.c | 118 +-------------
builtin/submodule--helper.c | 304 +++++++++++++++++++-----------------
dir.c | 114 ++++++++++++++
dir.h | 3 +
git-submodule.sh | 96 +-----------
5 files changed, 280 insertions(+), 355 deletions(-)
--
2.32.0
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, to suppress that warning when not needed, and then refactor
`resolve_relative_url()` by using this function, something we will do in
the next patch.
Having this functionality factored out will be useful for converting the
rest of `submodule add` in subsequent patches.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url)strbuf_addf(&remotesb,"remote.%s.url",remote);if(git_config_get_string(remotesb.buf,&remoteurl)){-warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."),remotesb.buf);+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);remoteurl=xgetcwd();}-relurl=relative_url(remoteurl,rel_url,NULL);+relurl=relative_url(remoteurl,rel_url,up_path);free(remote);free(remoteurl);
Refactor the helper function to resolve a relative url, by reusing the
existing `compute_submodule_clone_url()` function.
`compute_submodule_clone_url()` performs the same work that
`resolve_relative_url()` is doing, so we eliminate this code repetition
by moving the former function's definition up, and calling it inside
`resolve_relative_url()`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 61 +++++++++++++++----------------------
1 file changed, 25 insertions(+), 36 deletions(-)
@@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url,returnstrbuf_detach(&sb,NULL);}+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)+{+char*remoteurl,*relurl;+char*remote=get_default_remote();+structstrbufremotesb=STRBUF_INIT;++strbuf_addf(&remotesb,"remote.%s.url",remote);+if(git_config_get_string(remotesb.buf,&remoteurl)){+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);+remoteurl=xgetcwd();+}+relurl=relative_url(remoteurl,rel_url,up_path);++free(remote);+free(remoteurl);+strbuf_release(&remotesb);++returnrelurl;+}+staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix){-char*remoteurl=NULL;-char*remote=get_default_remote();constchar*up_path=NULL;char*res;constchar*url;-structstrbufsb=STRBUF_INIT;if(argc!=2&&argc!=3)die("resolve-relative-url only accepts one or two arguments");url=argv[1];-strbuf_addf(&sb,"remote.%s.url",remote);-free(remote);--if(git_config_get_string(sb.buf,&remoteurl))-/* the repository is its own authoritative upstream */-remoteurl=xgetcwd();-if(argc==3)up_path=argv[2];-res=relative_url(remoteurl,url,up_path);+res=compute_submodule_clone_url(url,up_path,1);puts(res);free(res);-free(remoteurl);return0;}
@@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)-{-char*remoteurl,*relurl;-char*remote=get_default_remote();-structstrbufremotesb=STRBUF_INIT;--strbuf_addf(&remotesb,"remote.%s.url",remote);-if(git_config_get_string(remotesb.buf,&remoteurl)){-if(!quiet)-warning(_("could not look up configuration '%s'. "-"Assuming this repository is its own "-"authoritative upstream."),-remotesb.buf);-remoteurl=xgetcwd();-}-relurl=relative_url(remoteurl,rel_url,up_path);--free(remote);-free(remoteurl);-strbuf_release(&remotesb);--returnrelurl;-}-structinit_cb{constchar*prefix;unsignedintflags;
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
These functions can be useful to other parts of Git. Let's move them to
dir.c, while renaming them to be make their functionality more explicit.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/clone.c | 118 +-----------------------------------------------
dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
dir.h | 3 ++
3 files changed, 119 insertions(+), 116 deletions(-)
@@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle)returncanon;}-staticchar*guess_dir_name(constchar*repo,intis_bundle,intis_bare)-{-constchar*end=repo+strlen(repo),*start,*ptr;-size_tlen;-char*dir;--/*-*Skipscheme.-*/-start=strstr(repo,"://");-if(start==NULL)-start=repo;-else-start+=3;--/*-*Skipauthenticationdata.Thestrippingdoeshappen-*greedily,suchthatwestripuptothelast'@'inside-*thehostpart.-*/-for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){-if(*ptr=='@')-start=ptr+1;-}--/*-*Striptrailingspaces,slashesand/.git-*/-while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))-end--;-if(end-start>5&&is_dir_sep(end[-5])&&-!strncmp(end-4,".git",4)){-end-=5;-while(start<end&&is_dir_sep(end[-1]))-end--;-}--/*-*Striptrailingportnumberifwe'vegotonlya-*hostname(thatis,thereisnodirseparatorbuta-*colon).Thischeckisrequiredsuchthatwedonot-*stripURI'slike'/foo/bar:2222.git',whichshould-*resultinadir'2222'beingguessedduetobackwards-*compatibility.-*/-if(memchr(start,'/',end-start)==NULL-&&memchr(start,':',end-start)!=NULL){-ptr=end;-while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')-ptr--;-if(start<ptr&&ptr[-1]==':')-end=ptr-1;-}--/*-*Findlastcomponent.Toremainbackwardscompatiblewe-*alsoregardcolonsaspathseparators,suchthat-*cloningarepository'foo:bar.git'wouldresultina-*directory'bar'beingguessed.-*/-ptr=end;-while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')-ptr--;-start=ptr;--/*-*Strip.{bundle,git}.-*/-len=end-start;-strip_suffix_mem(start,&len,is_bundle?".bundle":".git");--if(!len||(len==1&&*start=='/'))-die(_("No directory name could be guessed.\n"-"Please specify a directory on the command line"));--if(is_bare)-dir=xstrfmt("%.*s.git",(int)len,start);-else-dir=xstrndup(start,len);-/*-*Replacesequencesof'control'charactersandwhitespace-*withoneasciispace,removeleadingandtrailingspaces.-*/-if(*dir){-char*out=dir;-intprev_space=1/* strip leading whitespace */;-for(end=dir;*end;++end){-charch=*end;-if((unsignedchar)ch<'\x20')-ch='\x20';-if(isspace(ch)){-if(prev_space)-continue;-prev_space=1;-}else-prev_space=0;-*out++=ch;-}-*out='\0';-if(out>dir&&prev_space)-out[-1]='\0';-}-returndir;-}--staticvoidstrip_trailing_slashes(char*dir)-{-char*end=dir+strlen(dir);--while(dir<end-1&&is_dir_sep(end[-1]))-end--;-*end='\0';-}-staticintadd_one_reference(structstring_list_item*item,void*cb_data){structstrbuferr=STRBUF_INIT;
@@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path)returnret;}+char*guess_dir_name_from_git_url(constchar*repo,intis_bundle,intis_bare)+{+constchar*end=repo+strlen(repo),*start,*ptr;+size_tlen;+char*dir;++/*+*Skipscheme.+*/+start=strstr(repo,"://");+if(start==NULL)+start=repo;+else+start+=3;++/*+*Skipauthenticationdata.Thestrippingdoeshappen+*greedily,suchthatwestripuptothelast'@'inside+*thehostpart.+*/+for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){+if(*ptr=='@')+start=ptr+1;+}++/*+*Striptrailingspaces,slashesand/.git+*/+while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))+end--;+if(end-start>5&&is_dir_sep(end[-5])&&+!strncmp(end-4,".git",4)){+end-=5;+while(start<end&&is_dir_sep(end[-1]))+end--;+}++/*+*Striptrailingportnumberifwe'vegotonlya+*hostname(thatis,thereisnodirseparatorbuta+*colon).Thischeckisrequiredsuchthatwedonot+*stripURI'slike'/foo/bar:2222.git',whichshould+*resultinadir'2222'beingguessedduetobackwards+*compatibility.+*/+if(memchr(start,'/',end-start)==NULL+&&memchr(start,':',end-start)!=NULL){+ptr=end;+while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')+ptr--;+if(start<ptr&&ptr[-1]==':')+end=ptr-1;+}++/*+*Findlastcomponent.Toremainbackwardscompatiblewe+*alsoregardcolonsaspathseparators,suchthat+*cloningarepository'foo:bar.git'wouldresultina+*directory'bar'beingguessed.+*/+ptr=end;+while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')+ptr--;+start=ptr;++/*+*Strip.{bundle,git}.+*/+len=end-start;+strip_suffix_mem(start,&len,is_bundle?".bundle":".git");++if(!len||(len==1&&*start=='/'))+die(_("No directory name could be guessed.\n"+"Please specify a directory on the command line"));++if(is_bare)+dir=xstrfmt("%.*s.git",(int)len,start);+else+dir=xstrndup(start,len);+/*+*Replacesequencesof'control'charactersandwhitespace+*withoneasciispace,removeleadingandtrailingspaces.+*/+if(*dir){+char*out=dir;+intprev_space=1/* strip leading whitespace */;+for(end=dir;*end;++end){+charch=*end;+if((unsignedchar)ch<'\x20')+ch='\x20';+if(isspace(ch)){+if(prev_space)+continue;+prev_space=1;+}else+prev_space=0;+*out++=ch;+}+*out='\0';+if(out>dir&&prev_space)+out[-1]='\0';+}+returndir;+}++voidstrip_dir_trailing_slashes(char*dir)+{+char*end=dir+strlen(dir);++while(dir<end-1&&is_dir_sep(end[-1]))+end--;+*end='\0';+}+staticintremove_dir_recurse(structstrbuf*path,intflag,int*kept_up){DIR*dir;
This is needed so that it can be modified by normalize_path_copy() in
the next patch.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 160 ++++++++++++++++++++++++++++++++++++
git-submodule.sh | 96 +---------------------
2 files changed, 162 insertions(+), 94 deletions(-)
@@ -3046,6 +3046,165 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=guess_dir_name_from_git_url(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);++if(prefix&&*prefix&&!is_absolute_path(add_data.sm_path))+add_data.sm_path=xstrfmt("%s%s",prefix,add_data.sm_path);++if(starts_with_dot_dot_slash(add_data.repo)||+starts_with_dot_slash(add_data.repo)){+if(prefix)+die(_("Relative path can only be used from the toplevel "+"of the working tree"));++/* dereference source url relative to parent's url */+add_data.realrepo=compute_submodule_clone_url(add_data.repo,NULL,1);+}elseif(is_dir_sep(add_data.repo[0])||strchr(add_data.repo,':')){+add_data.realrepo=add_data.repo;+}else{+die(_("repo URL: '%s' must be absolute or begin with ./|../"),+add_data.repo);+}++/*+*normalizepath:+*multiple//; leading ./; /./; /../;+*/+normalize_path_copy(add_data.sm_path,add_data.sm_path);+strip_dir_trailing_slashes(add_data.sm_path);++die_on_index_match(add_data.sm_path,force);+die_on_repo_without_commits(add_data.sm_path);++if(!force){+intexit_code=-1;+structstrbufsb=STRBUF_INIT;+structchild_processcp=CHILD_PROCESS_INIT;+cp.git_cmd=1;+cp.no_stdout=1;+strvec_pushl(&cp.args,"add","--dry-run","--ignore-missing",+"--no-warn-embedded-repo",add_data.sm_path,NULL);+if((exit_code=pipe_command(&cp,NULL,0,NULL,0,&sb,0))){+strbuf_complete_line(&sb);+fputs(sb.buf,stderr);+returnexit_code;+}+strbuf_release(&sb);+}++if(!add_data.sm_name)+add_data.sm_name=add_data.sm_path;++if(check_submodule_name(add_data.sm_name))+die(_("'%s' is not a valid submodule name"),add_data.sm_name);++add_data.prefix=prefix;+add_data.force=!!force;+add_data.quiet=!!quiet;+add_data.progress=!!progress;+add_data.dissociate=!!dissociate;++if(add_submodule(&add_data))+return1;+configure_added_submodule(&add_data);+free(add_data.sm_path);++return0;+}+#define SUPPORT_SUPER_PREFIX (1<<0)structcmd_struct{
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
We no longer need this subcommand, as all of its functionality is being
called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 60 -------------------------------------
1 file changed, 60 deletions(-)
@@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data)return0;}-staticintadd_clone(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,dissociate=0,progress=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to checkout on cloning")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("alternative anchor for relative paths")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT_STRING(0,"url",&add_data.realrepo,-N_("string"),-N_("url where to clone the submodule from")),-OPT_STRING(0,"reference",&add_data.reference_path,-N_("repo"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&dissociate,-N_("use --reference only while cloning")),-OPT_INTEGER(0,"depth",&add_data.depth,-N_("depth for shallow clones")),-OPT_BOOL(0,"progress",&progress,-N_("force cloning progress")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT__QUIET(&quiet,"suppress output for cloning a submodule"),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-clone [<options>...] "-"--url <url> --path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc!=0)-usage_with_options(usage,options);--add_data.prefix=prefix;-add_data.progress=!!progress;-add_data.dissociate=!!dissociate;-add_data.force=!!force;-add_data.quiet=!!quiet;--if(add_submodule(&add_data))-return1;--return0;-}-staticintconfig_submodule_in_gitmodules(constchar*name,constchar*var,constchar*value){char*key;
Also no longer needed is this subcommand, as all of its functionality is
being called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 49 -------------------------------------
1 file changed, 49 deletions(-)
@@ -2939,54 +2939,6 @@ static void configure_added_submodule(struct add_data *add_data)}}-staticintadd_config(intargc,constchar**argv,constchar*prefix)-{-intforce=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to store in "-"the submodule configuration")),-OPT_STRING(0,"url",&add_data.repo,-N_("string"),-N_("url to clone submodule from")),-OPT_STRING(0,"resolved-url",&add_data.realrepo,-N_("string"),-N_("url to clone the submodule from, after it has "-"been dereferenced relative to parent's url, "-"in the case where <url> is a relative url")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-config "-"[--force|-f] [--branch|-b <branch>] "-"--url <url> --resolved-url <resolved-url> "-"--path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc)-usage_with_options(usage,options);--add_data.force=!!force;-configure_added_submodule(&add_data);--return0;-}-staticvoiddie_on_index_match(constchar*path,intforce){structpathspecps;
The shell subcommand `resolve-relative-url` is no longer required, as
its last caller has been removed when it was converted to C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 20 --------------------
1 file changed, 20 deletions(-)
@@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_patreturnrelurl;}-staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix)-{-constchar*up_path=NULL;-char*res;-constchar*url;--if(argc!=2&&argc!=3)-die("resolve-relative-url only accepts one or two arguments");--url=argv[1];-if(argc==3)-up_path=argv[2];--res=compute_submodule_clone_url(url,up_path,1);-puts(res);-free(res);-return0;-}-staticintresolve_relative_url_test(intargc,constchar**argv,constchar*prefix){char*remoteurl,*res;
From: Đoàn Trần Công Danh <hidden> Date: 2021-08-06 00:54:19
On 2021-08-05 12:49:11+0530, Atharva Raykar [off-list ref] wrote:
quoted hunk
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
While previous patch is definitely a refactoring, this patch add small
overhead to the system, the new code will query (then free())
git_config_get_string() and/or xgetcwd() one more time in the second
compute_submodule_clone_url()
I think the abstraction overhead is not that big, though.
From: Đoàn Trần Công Danh <hidden> Date: 2021-08-06 01:15:00
On 2021-08-05 13:10:51+0530, Atharva Raykar [off-list ref] wrote:
quoted hunk
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 160 ++++++++++++++++++++++++++++++++++++
git-submodule.sh | 96 +---------------------
2 files changed, 162 insertions(+), 94 deletions(-)
@@ -3046,6 +3046,165 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=guess_dir_name_from_git_url(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);
add_data.sm_path is allocated in this block (regardless of legs).
+ if (prefix && *prefix && !is_absolute_path(add_data.sm_path))
+ add_data.sm_path = xstrfmt("%s%s", prefix, add_data.sm_path);
+
+ if (starts_with_dot_dot_slash(add_data.repo) ||
+ starts_with_dot_slash(add_data.repo)) {
+ if (prefix)
+ die(_("Relative path can only be used from the toplevel "
+ "of the working tree"));
+
+ /* dereference source url relative to parent's url */
+ add_data.realrepo = compute_submodule_clone_url(add_data.repo, NULL, 1);
+ } else if (is_dir_sep(add_data.repo[0]) || strchr(add_data.repo, ':')) {
+ add_data.realrepo = add_data.repo;
+ } else {
+ die(_("repo URL: '%s' must be absolute or begin with ./|../"),
+ add_data.repo);
+ }
+
+ /*
+ * normalize path:
+ * multiple //; leading ./; /./; /../;
+ */
+ normalize_path_copy(add_data.sm_path, add_data.sm_path);
+ strip_dir_trailing_slashes(add_data.sm_path);
+
+ die_on_index_match(add_data.sm_path, force);
+ die_on_repo_without_commits(add_data.sm_path);
+
+ if (!force) {
+ int exit_code = -1;
+ struct strbuf sb = STRBUF_INIT;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ cp.git_cmd = 1;
+ cp.no_stdout = 1;
+ strvec_pushl(&cp.args, "add", "--dry-run", "--ignore-missing",
+ "--no-warn-embedded-repo", add_data.sm_path, NULL);
+ if ((exit_code = pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0))) {
+ strbuf_complete_line(&sb);
+ fputs(sb.buf, stderr);
+ return exit_code;
However, it will be free()-d here, is it intended?
I think we may use UNLEAK above (for now) because we will exit process
after this function.
However, I anticipated we may need to do more stuffs after this
function in the future.
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
From: Christian Couder <hidden> Date: 2021-08-06 09:06:17
On Fri, Aug 6, 2021 at 2:54 AM Đoàn Trần Công Danh [off-list ref] wrote:
While previous patch is definitely a refactoring, this patch add small
overhead to the system, the new code will query (then free())
git_config_get_string() and/or xgetcwd() one more time in the second
compute_submodule_clone_url()
I think the abstraction overhead is not that big, though.
Yeah, Junio made basically the same comment. So it would be nice if
the commit message could mention we are adding a very small overhead
in exchange for code simplification (10 lines removed).
On Fri, Aug 6, 2021 at 2:54 AM Đoàn Trần Công Danh [off-list ref] wrote:
quoted
While previous patch is definitely a refactoring, this patch add small
overhead to the system, the new code will query (then free())
git_config_get_string() and/or xgetcwd() one more time in the second
compute_submodule_clone_url()
I think the abstraction overhead is not that big, though.
Yeah, Junio made basically the same comment. So it would be nice if
the commit message could mention we are adding a very small overhead
in exchange for code simplification (10 lines removed).
However, it will be free()-d here, is it intended?
Yeah I meant to have it free()'d wherever possible, although I suppose
it isn't strictly necessary since we exit.
I think we may use UNLEAK above (for now) because we will exit process
after this function.
However, I anticipated we may need to do more stuffs after this
function in the future.
Right. So it's better I ensure that it's freed properly everywhere.
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, to suppress that warning when not needed, and then refactor
`resolve_relative_url()` by using this function, something we will do in
the next patch.
Having this functionality factored out will be useful for converting the
rest of `submodule add` in subsequent patches.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url)strbuf_addf(&remotesb,"remote.%s.url",remote);if(git_config_get_string(remotesb.buf,&remoteurl)){-warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."),remotesb.buf);+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);remoteurl=xgetcwd();}-relurl=relative_url(remoteurl,rel_url,NULL);+relurl=relative_url(remoteurl,rel_url,up_path);free(remote);free(remoteurl);
Refactor the helper function to resolve a relative url, by reusing the
existing `compute_submodule_clone_url()` function.
`compute_submodule_clone_url()` performs the same work that
`resolve_relative_url()` is doing, so we eliminate this code repetition
by moving the former function's definition up, and calling it inside
`resolve_relative_url()`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 61 +++++++++++++++----------------------
1 file changed, 25 insertions(+), 36 deletions(-)
@@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url,returnstrbuf_detach(&sb,NULL);}+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)+{+char*remoteurl,*relurl;+char*remote=get_default_remote();+structstrbufremotesb=STRBUF_INIT;++strbuf_addf(&remotesb,"remote.%s.url",remote);+if(git_config_get_string(remotesb.buf,&remoteurl)){+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);+remoteurl=xgetcwd();+}+relurl=relative_url(remoteurl,rel_url,up_path);++free(remote);+free(remoteurl);+strbuf_release(&remotesb);++returnrelurl;+}+staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix){-char*remoteurl=NULL;-char*remote=get_default_remote();constchar*up_path=NULL;char*res;constchar*url;-structstrbufsb=STRBUF_INIT;if(argc!=2&&argc!=3)die("resolve-relative-url only accepts one or two arguments");url=argv[1];-strbuf_addf(&sb,"remote.%s.url",remote);-free(remote);--if(git_config_get_string(sb.buf,&remoteurl))-/* the repository is its own authoritative upstream */-remoteurl=xgetcwd();-if(argc==3)up_path=argv[2];-res=relative_url(remoteurl,url,up_path);+res=compute_submodule_clone_url(url,up_path,1);puts(res);free(res);-free(remoteurl);return0;}
@@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)-{-char*remoteurl,*relurl;-char*remote=get_default_remote();-structstrbufremotesb=STRBUF_INIT;--strbuf_addf(&remotesb,"remote.%s.url",remote);-if(git_config_get_string(remotesb.buf,&remoteurl)){-if(!quiet)-warning(_("could not look up configuration '%s'. "-"Assuming this repository is its own "-"authoritative upstream."),-remotesb.buf);-remoteurl=xgetcwd();-}-relurl=relative_url(remoteurl,rel_url,up_path);--free(remote);-free(remoteurl);-strbuf_release(&remotesb);--returnrelurl;-}-structinit_cb{constchar*prefix;unsignedintflags;
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Note that this change adds a small overhead where we allocate and free
the 'remote' twice, but that is a small price to pay for the higher
level of abstraction we get.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
These functions can be useful to other parts of Git. Let's move them to
dir.c, while renaming them to be make their functionality more explicit.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/clone.c | 118 +-----------------------------------------------
dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
dir.h | 11 +++++
3 files changed, 127 insertions(+), 116 deletions(-)
@@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle)returncanon;}-staticchar*guess_dir_name(constchar*repo,intis_bundle,intis_bare)-{-constchar*end=repo+strlen(repo),*start,*ptr;-size_tlen;-char*dir;--/*-*Skipscheme.-*/-start=strstr(repo,"://");-if(start==NULL)-start=repo;-else-start+=3;--/*-*Skipauthenticationdata.Thestrippingdoeshappen-*greedily,suchthatwestripuptothelast'@'inside-*thehostpart.-*/-for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){-if(*ptr=='@')-start=ptr+1;-}--/*-*Striptrailingspaces,slashesand/.git-*/-while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))-end--;-if(end-start>5&&is_dir_sep(end[-5])&&-!strncmp(end-4,".git",4)){-end-=5;-while(start<end&&is_dir_sep(end[-1]))-end--;-}--/*-*Striptrailingportnumberifwe'vegotonlya-*hostname(thatis,thereisnodirseparatorbuta-*colon).Thischeckisrequiredsuchthatwedonot-*stripURI'slike'/foo/bar:2222.git',whichshould-*resultinadir'2222'beingguessedduetobackwards-*compatibility.-*/-if(memchr(start,'/',end-start)==NULL-&&memchr(start,':',end-start)!=NULL){-ptr=end;-while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')-ptr--;-if(start<ptr&&ptr[-1]==':')-end=ptr-1;-}--/*-*Findlastcomponent.Toremainbackwardscompatiblewe-*alsoregardcolonsaspathseparators,suchthat-*cloningarepository'foo:bar.git'wouldresultina-*directory'bar'beingguessed.-*/-ptr=end;-while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')-ptr--;-start=ptr;--/*-*Strip.{bundle,git}.-*/-len=end-start;-strip_suffix_mem(start,&len,is_bundle?".bundle":".git");--if(!len||(len==1&&*start=='/'))-die(_("No directory name could be guessed.\n"-"Please specify a directory on the command line"));--if(is_bare)-dir=xstrfmt("%.*s.git",(int)len,start);-else-dir=xstrndup(start,len);-/*-*Replacesequencesof'control'charactersandwhitespace-*withoneasciispace,removeleadingandtrailingspaces.-*/-if(*dir){-char*out=dir;-intprev_space=1/* strip leading whitespace */;-for(end=dir;*end;++end){-charch=*end;-if((unsignedchar)ch<'\x20')-ch='\x20';-if(isspace(ch)){-if(prev_space)-continue;-prev_space=1;-}else-prev_space=0;-*out++=ch;-}-*out='\0';-if(out>dir&&prev_space)-out[-1]='\0';-}-returndir;-}--staticvoidstrip_trailing_slashes(char*dir)-{-char*end=dir+strlen(dir);--while(dir<end-1&&is_dir_sep(end[-1]))-end--;-*end='\0';-}-staticintadd_one_reference(structstring_list_item*item,void*cb_data){structstrbuferr=STRBUF_INIT;
@@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path)returnret;}+char*guess_target_dir_from_git_url(constchar*repo,intis_bundle,intis_bare)+{+constchar*end=repo+strlen(repo),*start,*ptr;+size_tlen;+char*dir;++/*+*Skipscheme.+*/+start=strstr(repo,"://");+if(start==NULL)+start=repo;+else+start+=3;++/*+*Skipauthenticationdata.Thestrippingdoeshappen+*greedily,suchthatwestripuptothelast'@'inside+*thehostpart.+*/+for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){+if(*ptr=='@')+start=ptr+1;+}++/*+*Striptrailingspaces,slashesand/.git+*/+while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))+end--;+if(end-start>5&&is_dir_sep(end[-5])&&+!strncmp(end-4,".git",4)){+end-=5;+while(start<end&&is_dir_sep(end[-1]))+end--;+}++/*+*Striptrailingportnumberifwe'vegotonlya+*hostname(thatis,thereisnodirseparatorbuta+*colon).Thischeckisrequiredsuchthatwedonot+*stripURI'slike'/foo/bar:2222.git',whichshould+*resultinadir'2222'beingguessedduetobackwards+*compatibility.+*/+if(memchr(start,'/',end-start)==NULL+&&memchr(start,':',end-start)!=NULL){+ptr=end;+while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')+ptr--;+if(start<ptr&&ptr[-1]==':')+end=ptr-1;+}++/*+*Findlastcomponent.Toremainbackwardscompatiblewe+*alsoregardcolonsaspathseparators,suchthat+*cloningarepository'foo:bar.git'wouldresultina+*directory'bar'beingguessed.+*/+ptr=end;+while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')+ptr--;+start=ptr;++/*+*Strip.{bundle,git}.+*/+len=end-start;+strip_suffix_mem(start,&len,is_bundle?".bundle":".git");++if(!len||(len==1&&*start=='/'))+die(_("No directory name could be guessed.\n"+"Please specify a directory on the command line"));++if(is_bare)+dir=xstrfmt("%.*s.git",(int)len,start);+else+dir=xstrndup(start,len);+/*+*Replacesequencesof'control'charactersandwhitespace+*withoneasciispace,removeleadingandtrailingspaces.+*/+if(*dir){+char*out=dir;+intprev_space=1/* strip leading whitespace */;+for(end=dir;*end;++end){+charch=*end;+if((unsignedchar)ch<'\x20')+ch='\x20';+if(isspace(ch)){+if(prev_space)+continue;+prev_space=1;+}else+prev_space=0;+*out++=ch;+}+*out='\0';+if(out>dir&&prev_space)+out[-1]='\0';+}+returndir;+}++voidstrip_dir_trailing_slashes(char*dir)+{+char*end=dir+strlen(dir);++while(dir<end-1&&is_dir_sep(end[-1]))+end--;+*end='\0';+}+staticintremove_dir_recurse(structstrbuf*path,intflag,int*kept_up){DIR*dir;
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
We also remove the constness of the sm_path field of the `add_data`
struct. This is needed so that it can be modified by
normalize_path_copy().
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 165 +++++++++++++++++++++++++++++++++++-
git-submodule.sh | 96 +--------------------
2 files changed, 166 insertions(+), 95 deletions(-)
@@ -3046,6 +3046,168 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=guess_target_dir_from_git_url(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);++if(prefix&&*prefix&&!is_absolute_path(add_data.sm_path))+add_data.sm_path=xstrfmt("%s%s",prefix,add_data.sm_path);++if(starts_with_dot_dot_slash(add_data.repo)||+starts_with_dot_slash(add_data.repo)){+if(prefix)+die(_("Relative path can only be used from the toplevel "+"of the working tree"));++/* dereference source url relative to parent's url */+add_data.realrepo=compute_submodule_clone_url(add_data.repo,NULL,1);+}elseif(is_dir_sep(add_data.repo[0])||strchr(add_data.repo,':')){+add_data.realrepo=add_data.repo;+}else{+die(_("repo URL: '%s' must be absolute or begin with ./|../"),+add_data.repo);+}++/*+*normalizepath:+*multiple//; leading ./; /./; /../;+*/+normalize_path_copy(add_data.sm_path,add_data.sm_path);+strip_dir_trailing_slashes(add_data.sm_path);++die_on_index_match(add_data.sm_path,force);+die_on_repo_without_commits(add_data.sm_path);++if(!force){+intexit_code=-1;+structstrbufsb=STRBUF_INIT;+structchild_processcp=CHILD_PROCESS_INIT;+cp.git_cmd=1;+cp.no_stdout=1;+strvec_pushl(&cp.args,"add","--dry-run","--ignore-missing",+"--no-warn-embedded-repo",add_data.sm_path,NULL);+if((exit_code=pipe_command(&cp,NULL,0,NULL,0,&sb,0))){+strbuf_complete_line(&sb);+fputs(sb.buf,stderr);+free(add_data.sm_path);+returnexit_code;+}+strbuf_release(&sb);+}++if(!add_data.sm_name)+add_data.sm_name=add_data.sm_path;++if(check_submodule_name(add_data.sm_name))+die(_("'%s' is not a valid submodule name"),add_data.sm_name);++add_data.prefix=prefix;+add_data.force=!!force;+add_data.quiet=!!quiet;+add_data.progress=!!progress;+add_data.dissociate=!!dissociate;++if(add_submodule(&add_data)){+free(add_data.sm_path);+return1;+}+configure_added_submodule(&add_data);+free(add_data.sm_path);++return0;+}+#define SUPPORT_SUPER_PREFIX (1<<0)structcmd_struct{
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
We no longer need this subcommand, as all of its functionality is being
called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 60 -------------------------------------
1 file changed, 60 deletions(-)
@@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data)return0;}-staticintadd_clone(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,dissociate=0,progress=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to checkout on cloning")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("alternative anchor for relative paths")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT_STRING(0,"url",&add_data.realrepo,-N_("string"),-N_("url where to clone the submodule from")),-OPT_STRING(0,"reference",&add_data.reference_path,-N_("repo"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&dissociate,-N_("use --reference only while cloning")),-OPT_INTEGER(0,"depth",&add_data.depth,-N_("depth for shallow clones")),-OPT_BOOL(0,"progress",&progress,-N_("force cloning progress")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT__QUIET(&quiet,"suppress output for cloning a submodule"),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-clone [<options>...] "-"--url <url> --path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc!=0)-usage_with_options(usage,options);--add_data.prefix=prefix;-add_data.progress=!!progress;-add_data.dissociate=!!dissociate;-add_data.force=!!force;-add_data.quiet=!!quiet;--if(add_submodule(&add_data))-return1;--return0;-}-staticintconfig_submodule_in_gitmodules(constchar*name,constchar*var,constchar*value){char*key;
Also no longer needed is this subcommand, as all of its functionality is
being called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 49 -------------------------------------
1 file changed, 49 deletions(-)
@@ -2939,54 +2939,6 @@ static void configure_added_submodule(struct add_data *add_data)}}-staticintadd_config(intargc,constchar**argv,constchar*prefix)-{-intforce=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to store in "-"the submodule configuration")),-OPT_STRING(0,"url",&add_data.repo,-N_("string"),-N_("url to clone submodule from")),-OPT_STRING(0,"resolved-url",&add_data.realrepo,-N_("string"),-N_("url to clone the submodule from, after it has "-"been dereferenced relative to parent's url, "-"in the case where <url> is a relative url")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-config "-"[--force|-f] [--branch|-b <branch>] "-"--url <url> --resolved-url <resolved-url> "-"--path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc)-usage_with_options(usage,options);--add_data.force=!!force;-configure_added_submodule(&add_data);--return0;-}-staticvoiddie_on_index_match(constchar*path,intforce){structpathspecps;
The shell subcommand `resolve-relative-url` is no longer required, as
its last caller has been removed when it was converted to C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 20 --------------------
1 file changed, 20 deletions(-)
@@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_patreturnrelurl;}-staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix)-{-constchar*up_path=NULL;-char*res;-constchar*url;--if(argc!=2&&argc!=3)-die("resolve-relative-url only accepts one or two arguments");--url=argv[1];-if(argc==3)-up_path=argv[2];--res=compute_submodule_clone_url(url,up_path,1);-puts(res);-free(res);-return0;-}-staticintresolve_relative_url_test(intargc,constchar**argv,constchar*prefix){char*remoteurl,*res;
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, to suppress that warning when not needed, and then refactor
`resolve_relative_url()` by using this function, something we will do in
the next patch.
Having this functionality factored out will be useful for converting the
rest of `submodule add` in subsequent patches.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url)strbuf_addf(&remotesb,"remote.%s.url",remote);if(git_config_get_string(remotesb.buf,&remoteurl)){-warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."),remotesb.buf);+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);remoteurl=xgetcwd();}-relurl=relative_url(remoteurl,rel_url,NULL);+relurl=relative_url(remoteurl,rel_url,up_path);free(remote);free(remoteurl);
Refactor the helper function to resolve a relative url, by reusing the
existing `compute_submodule_clone_url()` function.
`compute_submodule_clone_url()` performs the same work that
`resolve_relative_url()` is doing, so we eliminate this code repetition
by moving the former function's definition up, and calling it inside
`resolve_relative_url()`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 61 +++++++++++++++----------------------
1 file changed, 25 insertions(+), 36 deletions(-)
@@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url,returnstrbuf_detach(&sb,NULL);}+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)+{+char*remoteurl,*relurl;+char*remote=get_default_remote();+structstrbufremotesb=STRBUF_INIT;++strbuf_addf(&remotesb,"remote.%s.url",remote);+if(git_config_get_string(remotesb.buf,&remoteurl)){+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);+remoteurl=xgetcwd();+}+relurl=relative_url(remoteurl,rel_url,up_path);++free(remote);+free(remoteurl);+strbuf_release(&remotesb);++returnrelurl;+}+staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix){-char*remoteurl=NULL;-char*remote=get_default_remote();constchar*up_path=NULL;char*res;constchar*url;-structstrbufsb=STRBUF_INIT;if(argc!=2&&argc!=3)die("resolve-relative-url only accepts one or two arguments");url=argv[1];-strbuf_addf(&sb,"remote.%s.url",remote);-free(remote);--if(git_config_get_string(sb.buf,&remoteurl))-/* the repository is its own authoritative upstream */-remoteurl=xgetcwd();-if(argc==3)up_path=argv[2];-res=relative_url(remoteurl,url,up_path);+res=compute_submodule_clone_url(url,up_path,1);puts(res);free(res);-free(remoteurl);return0;}
@@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)-{-char*remoteurl,*relurl;-char*remote=get_default_remote();-structstrbufremotesb=STRBUF_INIT;--strbuf_addf(&remotesb,"remote.%s.url",remote);-if(git_config_get_string(remotesb.buf,&remoteurl)){-if(!quiet)-warning(_("could not look up configuration '%s'. "-"Assuming this repository is its own "-"authoritative upstream."),-remotesb.buf);-remoteurl=xgetcwd();-}-relurl=relative_url(remoteurl,rel_url,up_path);--free(remote);-free(remoteurl);-strbuf_release(&remotesb);--returnrelurl;-}-structinit_cb{constchar*prefix;unsignedintflags;
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Note that this change adds a small overhead where we allocate and free
the 'remote' twice, but that is a small price to pay for the higher
level of abstraction we get.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
These functions can be useful to other parts of Git. Let's move them to
dir.c, while renaming them to be make their functionality more explicit.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/clone.c | 118 +-----------------------------------------------
dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
dir.h | 10 ++++
3 files changed, 126 insertions(+), 116 deletions(-)
@@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle)returncanon;}-staticchar*guess_dir_name(constchar*repo,intis_bundle,intis_bare)-{-constchar*end=repo+strlen(repo),*start,*ptr;-size_tlen;-char*dir;--/*-*Skipscheme.-*/-start=strstr(repo,"://");-if(start==NULL)-start=repo;-else-start+=3;--/*-*Skipauthenticationdata.Thestrippingdoeshappen-*greedily,suchthatwestripuptothelast'@'inside-*thehostpart.-*/-for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){-if(*ptr=='@')-start=ptr+1;-}--/*-*Striptrailingspaces,slashesand/.git-*/-while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))-end--;-if(end-start>5&&is_dir_sep(end[-5])&&-!strncmp(end-4,".git",4)){-end-=5;-while(start<end&&is_dir_sep(end[-1]))-end--;-}--/*-*Striptrailingportnumberifwe'vegotonlya-*hostname(thatis,thereisnodirseparatorbuta-*colon).Thischeckisrequiredsuchthatwedonot-*stripURI'slike'/foo/bar:2222.git',whichshould-*resultinadir'2222'beingguessedduetobackwards-*compatibility.-*/-if(memchr(start,'/',end-start)==NULL-&&memchr(start,':',end-start)!=NULL){-ptr=end;-while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')-ptr--;-if(start<ptr&&ptr[-1]==':')-end=ptr-1;-}--/*-*Findlastcomponent.Toremainbackwardscompatiblewe-*alsoregardcolonsaspathseparators,suchthat-*cloningarepository'foo:bar.git'wouldresultina-*directory'bar'beingguessed.-*/-ptr=end;-while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')-ptr--;-start=ptr;--/*-*Strip.{bundle,git}.-*/-len=end-start;-strip_suffix_mem(start,&len,is_bundle?".bundle":".git");--if(!len||(len==1&&*start=='/'))-die(_("No directory name could be guessed.\n"-"Please specify a directory on the command line"));--if(is_bare)-dir=xstrfmt("%.*s.git",(int)len,start);-else-dir=xstrndup(start,len);-/*-*Replacesequencesof'control'charactersandwhitespace-*withoneasciispace,removeleadingandtrailingspaces.-*/-if(*dir){-char*out=dir;-intprev_space=1/* strip leading whitespace */;-for(end=dir;*end;++end){-charch=*end;-if((unsignedchar)ch<'\x20')-ch='\x20';-if(isspace(ch)){-if(prev_space)-continue;-prev_space=1;-}else-prev_space=0;-*out++=ch;-}-*out='\0';-if(out>dir&&prev_space)-out[-1]='\0';-}-returndir;-}--staticvoidstrip_trailing_slashes(char*dir)-{-char*end=dir+strlen(dir);--while(dir<end-1&&is_dir_sep(end[-1]))-end--;-*end='\0';-}-staticintadd_one_reference(structstring_list_item*item,void*cb_data){structstrbuferr=STRBUF_INIT;
@@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path)returnret;}+char*git_url_basename(constchar*repo,intis_bundle,intis_bare)+{+constchar*end=repo+strlen(repo),*start,*ptr;+size_tlen;+char*dir;++/*+*Skipscheme.+*/+start=strstr(repo,"://");+if(start==NULL)+start=repo;+else+start+=3;++/*+*Skipauthenticationdata.Thestrippingdoeshappen+*greedily,suchthatwestripuptothelast'@'inside+*thehostpart.+*/+for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){+if(*ptr=='@')+start=ptr+1;+}++/*+*Striptrailingspaces,slashesand/.git+*/+while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))+end--;+if(end-start>5&&is_dir_sep(end[-5])&&+!strncmp(end-4,".git",4)){+end-=5;+while(start<end&&is_dir_sep(end[-1]))+end--;+}++/*+*Striptrailingportnumberifwe'vegotonlya+*hostname(thatis,thereisnodirseparatorbuta+*colon).Thischeckisrequiredsuchthatwedonot+*stripURI'slike'/foo/bar:2222.git',whichshould+*resultinadir'2222'beingguessedduetobackwards+*compatibility.+*/+if(memchr(start,'/',end-start)==NULL+&&memchr(start,':',end-start)!=NULL){+ptr=end;+while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')+ptr--;+if(start<ptr&&ptr[-1]==':')+end=ptr-1;+}++/*+*Findlastcomponent.Toremainbackwardscompatiblewe+*alsoregardcolonsaspathseparators,suchthat+*cloningarepository'foo:bar.git'wouldresultina+*directory'bar'beingguessed.+*/+ptr=end;+while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')+ptr--;+start=ptr;++/*+*Strip.{bundle,git}.+*/+len=end-start;+strip_suffix_mem(start,&len,is_bundle?".bundle":".git");++if(!len||(len==1&&*start=='/'))+die(_("No directory name could be guessed.\n"+"Please specify a directory on the command line"));++if(is_bare)+dir=xstrfmt("%.*s.git",(int)len,start);+else+dir=xstrndup(start,len);+/*+*Replacesequencesof'control'charactersandwhitespace+*withoneasciispace,removeleadingandtrailingspaces.+*/+if(*dir){+char*out=dir;+intprev_space=1/* strip leading whitespace */;+for(end=dir;*end;++end){+charch=*end;+if((unsignedchar)ch<'\x20')+ch='\x20';+if(isspace(ch)){+if(prev_space)+continue;+prev_space=1;+}else+prev_space=0;+*out++=ch;+}+*out='\0';+if(out>dir&&prev_space)+out[-1]='\0';+}+returndir;+}++voidstrip_dir_trailing_slashes(char*dir)+{+char*end=dir+strlen(dir);++while(dir<end-1&&is_dir_sep(end[-1]))+end--;+*end='\0';+}+staticintremove_dir_recurse(structstrbuf*path,intflag,int*kept_up){DIR*dir;
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
We also remove the constness of the sm_path field of the `add_data`
struct. This is needed so that it can be modified by
normalize_path_copy().
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 165 +++++++++++++++++++++++++++++++++++-
git-submodule.sh | 96 +--------------------
2 files changed, 166 insertions(+), 95 deletions(-)
@@ -3046,6 +3046,168 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=git_url_basename(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);++if(prefix&&*prefix&&!is_absolute_path(add_data.sm_path))+add_data.sm_path=xstrfmt("%s%s",prefix,add_data.sm_path);++if(starts_with_dot_dot_slash(add_data.repo)||+starts_with_dot_slash(add_data.repo)){+if(prefix)+die(_("Relative path can only be used from the toplevel "+"of the working tree"));++/* dereference source url relative to parent's url */+add_data.realrepo=compute_submodule_clone_url(add_data.repo,NULL,1);+}elseif(is_dir_sep(add_data.repo[0])||strchr(add_data.repo,':')){+add_data.realrepo=add_data.repo;+}else{+die(_("repo URL: '%s' must be absolute or begin with ./|../"),+add_data.repo);+}++/*+*normalizepath:+*multiple//; leading ./; /./; /../;+*/+normalize_path_copy(add_data.sm_path,add_data.sm_path);+strip_dir_trailing_slashes(add_data.sm_path);++die_on_index_match(add_data.sm_path,force);+die_on_repo_without_commits(add_data.sm_path);++if(!force){+intexit_code=-1;+structstrbufsb=STRBUF_INIT;+structchild_processcp=CHILD_PROCESS_INIT;+cp.git_cmd=1;+cp.no_stdout=1;+strvec_pushl(&cp.args,"add","--dry-run","--ignore-missing",+"--no-warn-embedded-repo",add_data.sm_path,NULL);+if((exit_code=pipe_command(&cp,NULL,0,NULL,0,&sb,0))){+strbuf_complete_line(&sb);+fputs(sb.buf,stderr);+free(add_data.sm_path);+returnexit_code;+}+strbuf_release(&sb);+}++if(!add_data.sm_name)+add_data.sm_name=add_data.sm_path;++if(check_submodule_name(add_data.sm_name))+die(_("'%s' is not a valid submodule name"),add_data.sm_name);++add_data.prefix=prefix;+add_data.force=!!force;+add_data.quiet=!!quiet;+add_data.progress=!!progress;+add_data.dissociate=!!dissociate;++if(add_submodule(&add_data)){+free(add_data.sm_path);+return1;+}+configure_added_submodule(&add_data);+free(add_data.sm_path);++return0;+}+#define SUPPORT_SUPER_PREFIX (1<<0)structcmd_struct{
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
We no longer need this subcommand, as all of its functionality is being
called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 60 -------------------------------------
1 file changed, 60 deletions(-)
@@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data)return0;}-staticintadd_clone(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,dissociate=0,progress=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to checkout on cloning")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("alternative anchor for relative paths")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT_STRING(0,"url",&add_data.realrepo,-N_("string"),-N_("url where to clone the submodule from")),-OPT_STRING(0,"reference",&add_data.reference_path,-N_("repo"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&dissociate,-N_("use --reference only while cloning")),-OPT_INTEGER(0,"depth",&add_data.depth,-N_("depth for shallow clones")),-OPT_BOOL(0,"progress",&progress,-N_("force cloning progress")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT__QUIET(&quiet,"suppress output for cloning a submodule"),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-clone [<options>...] "-"--url <url> --path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc!=0)-usage_with_options(usage,options);--add_data.prefix=prefix;-add_data.progress=!!progress;-add_data.dissociate=!!dissociate;-add_data.force=!!force;-add_data.quiet=!!quiet;--if(add_submodule(&add_data))-return1;--return0;-}-staticintconfig_submodule_in_gitmodules(constchar*name,constchar*var,constchar*value){char*key;
Also no longer needed is this subcommand, as all of its functionality is
being called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 49 -------------------------------------
1 file changed, 49 deletions(-)
@@ -2939,54 +2939,6 @@ static void configure_added_submodule(struct add_data *add_data)}}-staticintadd_config(intargc,constchar**argv,constchar*prefix)-{-intforce=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to store in "-"the submodule configuration")),-OPT_STRING(0,"url",&add_data.repo,-N_("string"),-N_("url to clone submodule from")),-OPT_STRING(0,"resolved-url",&add_data.realrepo,-N_("string"),-N_("url to clone the submodule from, after it has "-"been dereferenced relative to parent's url, "-"in the case where <url> is a relative url")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-config "-"[--force|-f] [--branch|-b <branch>] "-"--url <url> --resolved-url <resolved-url> "-"--path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc)-usage_with_options(usage,options);--add_data.force=!!force;-configure_added_submodule(&add_data);--return0;-}-staticvoiddie_on_index_match(constchar*path,intforce){structpathspecps;
The shell subcommand `resolve-relative-url` is no longer required, as
its last caller has been removed when it was converted to C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 20 --------------------
1 file changed, 20 deletions(-)
@@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_patreturnrelurl;}-staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix)-{-constchar*up_path=NULL;-char*res;-constchar*url;--if(argc!=2&&argc!=3)-die("resolve-relative-url only accepts one or two arguments");--url=argv[1];-if(argc==3)-up_path=argv[2];--res=compute_submodule_clone_url(url,up_path,1);-puts(res);-free(res);-return0;-}-staticintresolve_relative_url_test(intargc,constchar**argv,constchar*prefix){char*remoteurl,*res;
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, ...
It took me a while to figure what "it" meant in the above sentence. Does it
refer to `compute_submodule_clone_url` or `resolve_relative_url`. After one
sees the patch and takes a look at `resolve_relative_url`, it's clear the "it"
indeed does refer to `resolve_relative_url`. But it might worth clarifying this
in the commit message itself.
Certainly not worth a re-roll on its own. May be Junio could amend this while queing ?
... to suppress that warning when not needed, and then refactor
`resolve_relative_url()` by using this function, something we will do in
the next patch.
Having this functionality factored out will be useful for converting the
rest of `submodule add` in subsequent patches.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Changes since v3:
* Rename the libified dir helper and update the docstring.
Just a note. I'm not really sure about this yet, the fact
that this series depends on the change introduced by
'ar/submodule-add-config'[1] might be worth mentioning in
re-rolls too. This could help the maintainer to easily identify
the topic dependency :-)
[1]: https://lore.kernel.org/git/20210801063352.50813-1-raykar.ath@gmail.com/
And ...
Atharva Raykar (8):
submodule--helper: add options for compute_submodule_clone_url()
submodule--helper: refactor resolve_relative_url() helper
submodule--helper: remove repeated code in sync_submodule()
dir: libify and export helper functions from clone.c
submodule--helper: convert the bulk of cmd_add() to C
submodule--helper: remove add-clone subcommand
submodule--helper: remove add-config subcommand
submodule--helper: remove resolve-relative-url subcommand
builtin/clone.c | 118 +-------------
builtin/submodule--helper.c | 307 +++++++++++++++++++-----------------
dir.c | 114 +++++++++++++
dir.h | 10 ++
git-submodule.sh | 96 +----------
5 files changed, 290 insertions(+), 355 deletions(-)
Range-diff against v3:
Thanks for consistently including a useful range-diff!
--
Sivaraam
-: ---------- > 1: 75edf24186 submodule--helper: add options for compute_submodule_clone_url()
-: ---------- > 2: 8e7a3e727a submodule--helper: refactor resolve_relative_url() helper
-: ---------- > 3: 82961ddd02 submodule--helper: remove repeated code in sync_submodule()
1: 99d139375d ! 4: fa97d6801e dir: libify and export helper functions from clone.c
@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
else
- dir = guess_dir_name(repo_name, is_bundle, option_bare);
- strip_trailing_slashes(dir);
-+ dir = guess_target_dir_from_git_url(repo_name, is_bundle, option_bare);
++ dir = git_url_basename(repo_name, is_bundle, option_bare);
+ strip_dir_trailing_slashes(dir);
dest_exists = path_exists(dir);
@@ dir.c: int is_empty_dir(const char *path)
return ret;
}
-+char *guess_target_dir_from_git_url(const char *repo, int is_bundle, int is_bare)
++char *git_url_basename(const char *repo, int is_bundle, int is_bare)
+{
+ const char *end = repo + strlen(repo), *start, *ptr;
+ size_t len;
@@ dir.h: static inline int is_dot_or_dotdot(const char *name)
int is_empty_dir(const char *dir);
+/*
-+ * Retrieve a target directory name by reading "humanish" part of the
-+ * given Git URL.
++ * Retrieve the "humanish" basename of the given Git URL.
+ *
+ * For example:
+ * /path/to/repo.git => "repo"
+ * host.xz.foo/.git => "foo"
+ */
-+char *guess_target_dir_from_git_url(const char *repo, int is_bundle, int is_bare);
++char *git_url_basename(const char *repo, int is_bundle, int is_bare);
+void strip_dir_trailing_slashes(char *dir);
+
void setup_standard_excludes(struct dir_struct *dir);
2: 11eea777ba ! 5: a3aa25518d submodule--helper: convert the bulk of cmd_add() to C
@@ builtin/submodule--helper.c: static int add_config(int argc, const char **argv,
+
+ add_data.repo = argv[0];
+ if (argc == 1)
-+ add_data.sm_path = guess_target_dir_from_git_url(add_data.repo, 0, 0);
++ add_data.sm_path = git_url_basename(add_data.repo, 0, 0);
+ else
+ add_data.sm_path = xstrdup(argv[1]);
+
3: 51393cd99b = 6: 9667159d4b submodule--helper: remove add-clone subcommand
4: 50cedcd8a8 = 7: dc87b5627a submodule--helper: remove add-config subcommand
5: 02558da532 = 8: ea08e4fbad submodule--helper: remove resolve-relative-url subcommand
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, ...
It took me a while to figure what "it" meant in the above sentence. Does it
refer to `compute_submodule_clone_url` or `resolve_relative_url`. After one
sees the patch and takes a look at `resolve_relative_url`, it's clear the "it"
indeed does refer to `resolve_relative_url`. But it might worth clarifying this
in the commit message itself.
Certainly not worth a re-roll on its own. May be Junio could amend this while queing ?
Actually, I just noticed two other things which might be re-roll worthy. Read on ...
I know this isn't new code. But there's already an argument names
'rel_url'. So, a variable named 'relurl' in the same scope is making it
hard to distinguish between these two. Could you also try distinguishing
these better by renaming 'relurl' to 'res' or something else?
quoted hunk
char *remote = get_default_remote();
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url) strbuf_addf(&remotesb, "remote.%s.url", remote); if (git_config_get_string(remotesb.buf, &remoteurl)) {- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);+ if (!quiet)+ warning(_("could not look up configuration '%s'. "+ "Assuming this repository is its own "+ "authoritative upstream."),+ remotesb.buf); remoteurl = xgetcwd(); }- relurl = relative_url(remoteurl, rel_url, NULL);+ relurl = relative_url(remoteurl, rel_url, up_path);
After reading 2/8 of the series, I just noticed that 'remoteurl' is always
initialized in 'resolve_realtive_url'. It is either initialized to the return
value of 'xgetcwd' or retains its assigned value of 'NULL'. But it looks
like that's not the case here. 'remoteurl' could be used uninitialized
when the above if block does not get executed which in turn could result in
weird behaviour in case 'remoteurl' gets a value of anything other than 'NULL'
at runtime.
This again has nothing to do with the change done in this patch. Regardless, it
looks like something worth correcting. Thus, I thought of pointing it out.
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Nice to see more code redundancy being removed. Now that we're using
'compute_submodule_clone_url' in multiple places, I'm starting to
wonder if the name still suits the helper. Yeah, I just started yet
another naming discussion ;-) I guess this one wouldn't be tough though.
It feels to me like 'resolve_relative_url' is a good enough name that
doesn't mislead readers by having 'clone_url' in its name. In case anyone
else has better name suggestions, they are indeed very welcome to suggest
those :-)
Once there's agreement on a particular name, I think the helper function
could be renamed. Possibly in a new patch next to this one.
quoted hunk
Note that this change adds a small overhead where we allocate and free
the 'remote' twice, but that is a small price to pay for the higher
level of abstraction we get.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
@@ -453,6 +453,16 @@ static inline int is_dot_or_dotdot(const char *name)intis_empty_dir(constchar*dir);+/*+*Retrievethe"humanish"basenameofthegivenGitURL.+*+*Forexample:+*/path/to/repo.git=>"repo"+*host.xz.foo/.git=>"foo"+*/
Are you sure about the examples here? I just tried and ...
- '/path/to/repo.git' gave me 'repo' like you said
.. but ..
- 'host.xz.foo/.git' gives me 'host.xz.foo' instead of 'foo'.
I think you meant to have 'host.xz/foo.git' in the example.
Also, here's another example that might be useful to mention in the docstring:
- 'http://example.com/user/bar.baz' => 'bar.baz'
+char *git_url_basename(const char *repo, int is_bundle, int is_bare);
+void strip_dir_trailing_slashes(char *dir);
+
void setup_standard_excludes(struct dir_struct *dir);
char *get_sparse_checkout_filename(void);
It took me a while to figure what "it" meant in the above sentence. Does it
refer to `compute_submodule_clone_url` or `resolve_relative_url`. After one
sees the patch and takes a look at `resolve_relative_url`, it's clear the "it"
indeed does refer to `resolve_relative_url`. But it might worth clarifying this
in the commit message itself.
Certainly not worth a re-roll on its own. May be Junio could amend this while
queing ?
Actually, I just noticed two other things which might be re-roll worthy. Read on ...
I'll keep re-rolling till the code is good, it's never a problem ;-)
I know this isn't new code. But there's already an argument names
'rel_url'. So, a variable named 'relurl' in the same scope is making it
hard to distinguish between these two. Could you also try distinguishing
these better by renaming 'relurl' to 'res' or something else?
Okay.
quoted
char *remote = get_default_remote();
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url) strbuf_addf(&remotesb, "remote.%s.url", remote); if (git_config_get_string(remotesb.buf, &remoteurl)) {- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);+ if (!quiet)+ warning(_("could not look up configuration '%s'. "+ "Assuming this repository is its own "+ "authoritative upstream."),+ remotesb.buf); remoteurl = xgetcwd(); }- relurl = relative_url(remoteurl, rel_url, NULL);+ relurl = relative_url(remoteurl, rel_url, up_path);
After reading 2/8 of the series, I just noticed that 'remoteurl' is always
initialized in 'resolve_realtive_url'. It is either initialized to the return
value of 'xgetcwd' or retains its assigned value of 'NULL'. But it looks
like that's not the case here. 'remoteurl' could be used uninitialized
when the above if block does not get executed which in turn could result in
weird behaviour in case 'remoteurl' gets a value of anything other than 'NULL'
at runtime.
This again has nothing to do with the change done in this patch. Regardless, it
looks like something worth correcting. Thus, I thought of pointing it out.
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Nice to see more code redundancy being removed. Now that we're using
'compute_submodule_clone_url' in multiple places, I'm starting to
wonder if the name still suits the helper. Yeah, I just started yet
another naming discussion ;-) I guess this one wouldn't be tough though.
It feels to me like 'resolve_relative_url' is a good enough name that
doesn't mislead readers by having 'clone_url' in its name. In case anyone
else has better name suggestions, they are indeed very welcome to suggest
those :-)
Once there's agreement on a particular name, I think the helper function
could be renamed. Possibly in a new patch next to this one.
I don't mind the rename back to resolve_relative_url(), although it
definitely has to come in a separate patch. I didn't want to create a
situation where readers will be confused about what actually happened
with that function. I felt a thing might happen if I repurpose it from
subcommand to internal helper in the same patch.
quoted
Note that this change adds a small overhead where we allocate and free
the 'remote' twice, but that is a small price to pay for the higher
level of abstraction we get.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
@@ -453,6 +453,16 @@ static inline int is_dot_or_dotdot(const char *name)intis_empty_dir(constchar*dir);+/*+*Retrievethe"humanish"basenameofthegivenGitURL.+*+*Forexample:+*/path/to/repo.git=>"repo"+*host.xz.foo/.git=>"foo"+*/
Are you sure about the examples here? I just tried and ...
- '/path/to/repo.git' gave me 'repo' like you said
.. but ..
- 'host.xz.foo/.git' gives me 'host.xz.foo' instead of 'foo'.
I think you meant to have 'host.xz/foo.git' in the example.
Yikes! I meant 'host.xz:foo/.git'. That should give us 'foo'. Thanks for
the correction.
Also, here's another example that might be useful to mention in the docstring:
- 'http://example.com/user/bar.baz' => 'bar.baz'
Yeah, especially since that's probably the most familiar example for
most end users.
quoted
+char *git_url_basename(const char *repo, int is_bundle, int is_bare);
+void strip_dir_trailing_slashes(char *dir);
+
void setup_standard_excludes(struct dir_struct *dir);
char *get_sparse_checkout_filename(void);
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url) strbuf_addf(&remotesb, "remote.%s.url", remote); if (git_config_get_string(remotesb.buf, &remoteurl)) {- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);+ if (!quiet)+ warning(_("could not look up configuration '%s'. "+ "Assuming this repository is its own "+ "authoritative upstream."),+ remotesb.buf); remoteurl = xgetcwd(); }- relurl = relative_url(remoteurl, rel_url, NULL);+ relurl = relative_url(remoteurl, rel_url, up_path);
After reading 2/8 of the series, I just noticed that 'remoteurl' is always
initialized in 'resolve_realtive_url'. It is either initialized to the return
value of 'xgetcwd' or retains its assigned value of 'NULL'. But it looks
like that's not the case here. 'remoteurl' could be used uninitialized
when the above if block does not get executed which in turn could result in
weird behaviour in case 'remoteurl' gets a value of anything other than 'NULL'
at runtime.
This again has nothing to do with the change done in this patch. Regardless, it
looks like something worth correcting. Thus, I thought of pointing it out.
Right. I agree it should be corrected.
Actually on having another look, I'm not sure if we need to assign NULL
to 'remoteurl' at all.
The 'if (git_config_get_string(...))' on success will allocate
'remoteurl'. If it fails, it will be given the return value of
'xgetcwd()'. There is nothing in the config API docs that suggest a
success mode for the git_config_get_*() functions that will assign
nothing to the buffer we give it. Therefore, by the time we get to the
variable's first use in the 'relative_url()' function, we are guaranteed
to have a well-defined value.
It seems to me that the original 'resolve_relative_url()' had an
unnecessary NULL initialization.
Let's modify the interface to `compute_submodule_clone_url()` function
by adding two more arguments, so that we can reuse this in various parts
of `submodule--helper.c` that follow a common pattern, which is--read
the remote url configuration of the superproject and then call
`relative_url()`.
This function is nearly identical to `resolve_relative_url()`, the only
difference being the extra warning message. We can add a quiet flag to
it, to suppress that warning when not needed, and then refactor
`resolve_relative_url()` by using this function, something we will do in
the next patch.
We also rename the local variable 'relurl' to avoid potential confusion
with the 'rel_url' parameter while we are at it.
Having this functionality factored out will be useful for converting the
rest of `submodule add` in subsequent patches.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
@@ -590,24 +590,28 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url)+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet){-char*remoteurl,*relurl;+char*remoteurl,*resolved_url;char*remote=get_default_remote();structstrbufremotesb=STRBUF_INIT;strbuf_addf(&remotesb,"remote.%s.url",remote);if(git_config_get_string(remotesb.buf,&remoteurl)){-warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."),remotesb.buf);+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);remoteurl=xgetcwd();}-relurl=relative_url(remoteurl,rel_url,NULL);+resolved_url=relative_url(remoteurl,rel_url,up_path);free(remote);free(remoteurl);strbuf_release(&remotesb);-returnrelurl;+returnresolved_url;}structinit_cb{
This part of `sync_submodule()` is doing the same thing that
`compute_submodule_clone_url()` is doing. Let's reuse that helper here.
Note that this change adds a small overhead where we allocate and free
the 'remote' twice, but that is a small price to pay for the higher
level of abstraction we get.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
Refactor the helper function to resolve a relative url, by reusing the
existing `compute_submodule_clone_url()` function.
`compute_submodule_clone_url()` performs the same work that
`resolve_relative_url()` is doing, so we eliminate this code repetition
by moving the former function's definition up, and calling it inside
`resolve_relative_url()`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 61 +++++++++++++++----------------------
1 file changed, 25 insertions(+), 36 deletions(-)
@@ -199,33 +199,46 @@ static char *relative_url(const char *remote_url,returnstrbuf_detach(&sb,NULL);}+staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)+{+char*remoteurl,*resolved_url;+char*remote=get_default_remote();+structstrbufremotesb=STRBUF_INIT;++strbuf_addf(&remotesb,"remote.%s.url",remote);+if(git_config_get_string(remotesb.buf,&remoteurl)){+if(!quiet)+warning(_("could not look up configuration '%s'. "+"Assuming this repository is its own "+"authoritative upstream."),+remotesb.buf);+remoteurl=xgetcwd();+}+resolved_url=relative_url(remoteurl,rel_url,up_path);++free(remote);+free(remoteurl);+strbuf_release(&remotesb);++returnresolved_url;+}+staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix){-char*remoteurl=NULL;-char*remote=get_default_remote();constchar*up_path=NULL;char*res;constchar*url;-structstrbufsb=STRBUF_INIT;if(argc!=2&&argc!=3)die("resolve-relative-url only accepts one or two arguments");url=argv[1];-strbuf_addf(&sb,"remote.%s.url",remote);-free(remote);--if(git_config_get_string(sb.buf,&remoteurl))-/* the repository is its own authoritative upstream */-remoteurl=xgetcwd();-if(argc==3)up_path=argv[2];-res=relative_url(remoteurl,url,up_path);+res=compute_submodule_clone_url(url,up_path,1);puts(res);free(res);-free(remoteurl);return0;}
@@ -590,30 +603,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix)return0;}-staticchar*compute_submodule_clone_url(constchar*rel_url,constchar*up_path,intquiet)-{-char*remoteurl,*resolved_url;-char*remote=get_default_remote();-structstrbufremotesb=STRBUF_INIT;--strbuf_addf(&remotesb,"remote.%s.url",remote);-if(git_config_get_string(remotesb.buf,&remoteurl)){-if(!quiet)-warning(_("could not look up configuration '%s'. "-"Assuming this repository is its own "-"authoritative upstream."),-remotesb.buf);-remoteurl=xgetcwd();-}-resolved_url=relative_url(remoteurl,rel_url,up_path);--free(remote);-free(remoteurl);-strbuf_release(&remotesb);--returnresolved_url;-}-structinit_cb{constchar*prefix;unsignedintflags;
These functions can be useful to other parts of Git. Let's move them to
dir.c, while renaming them to be make their functionality more explicit.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/clone.c | 118 +-----------------------------------------------
dir.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
dir.h | 11 +++++
3 files changed, 127 insertions(+), 116 deletions(-)
@@ -217,120 +217,6 @@ static char *get_repo_path(const char *repo, int *is_bundle)returncanon;}-staticchar*guess_dir_name(constchar*repo,intis_bundle,intis_bare)-{-constchar*end=repo+strlen(repo),*start,*ptr;-size_tlen;-char*dir;--/*-*Skipscheme.-*/-start=strstr(repo,"://");-if(start==NULL)-start=repo;-else-start+=3;--/*-*Skipauthenticationdata.Thestrippingdoeshappen-*greedily,suchthatwestripuptothelast'@'inside-*thehostpart.-*/-for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){-if(*ptr=='@')-start=ptr+1;-}--/*-*Striptrailingspaces,slashesand/.git-*/-while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))-end--;-if(end-start>5&&is_dir_sep(end[-5])&&-!strncmp(end-4,".git",4)){-end-=5;-while(start<end&&is_dir_sep(end[-1]))-end--;-}--/*-*Striptrailingportnumberifwe'vegotonlya-*hostname(thatis,thereisnodirseparatorbuta-*colon).Thischeckisrequiredsuchthatwedonot-*stripURI'slike'/foo/bar:2222.git',whichshould-*resultinadir'2222'beingguessedduetobackwards-*compatibility.-*/-if(memchr(start,'/',end-start)==NULL-&&memchr(start,':',end-start)!=NULL){-ptr=end;-while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')-ptr--;-if(start<ptr&&ptr[-1]==':')-end=ptr-1;-}--/*-*Findlastcomponent.Toremainbackwardscompatiblewe-*alsoregardcolonsaspathseparators,suchthat-*cloningarepository'foo:bar.git'wouldresultina-*directory'bar'beingguessed.-*/-ptr=end;-while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')-ptr--;-start=ptr;--/*-*Strip.{bundle,git}.-*/-len=end-start;-strip_suffix_mem(start,&len,is_bundle?".bundle":".git");--if(!len||(len==1&&*start=='/'))-die(_("No directory name could be guessed.\n"-"Please specify a directory on the command line"));--if(is_bare)-dir=xstrfmt("%.*s.git",(int)len,start);-else-dir=xstrndup(start,len);-/*-*Replacesequencesof'control'charactersandwhitespace-*withoneasciispace,removeleadingandtrailingspaces.-*/-if(*dir){-char*out=dir;-intprev_space=1/* strip leading whitespace */;-for(end=dir;*end;++end){-charch=*end;-if((unsignedchar)ch<'\x20')-ch='\x20';-if(isspace(ch)){-if(prev_space)-continue;-prev_space=1;-}else-prev_space=0;-*out++=ch;-}-*out='\0';-if(out>dir&&prev_space)-out[-1]='\0';-}-returndir;-}--staticvoidstrip_trailing_slashes(char*dir)-{-char*end=dir+strlen(dir);--while(dir<end-1&&is_dir_sep(end[-1]))-end--;-*end='\0';-}-staticintadd_one_reference(structstring_list_item*item,void*cb_data){structstrbuferr=STRBUF_INIT;
@@ -2970,6 +2970,120 @@ int is_empty_dir(const char *path)returnret;}+char*git_url_basename(constchar*repo,intis_bundle,intis_bare)+{+constchar*end=repo+strlen(repo),*start,*ptr;+size_tlen;+char*dir;++/*+*Skipscheme.+*/+start=strstr(repo,"://");+if(start==NULL)+start=repo;+else+start+=3;++/*+*Skipauthenticationdata.Thestrippingdoeshappen+*greedily,suchthatwestripuptothelast'@'inside+*thehostpart.+*/+for(ptr=start;ptr<end&&!is_dir_sep(*ptr);ptr++){+if(*ptr=='@')+start=ptr+1;+}++/*+*Striptrailingspaces,slashesand/.git+*/+while(start<end&&(is_dir_sep(end[-1])||isspace(end[-1])))+end--;+if(end-start>5&&is_dir_sep(end[-5])&&+!strncmp(end-4,".git",4)){+end-=5;+while(start<end&&is_dir_sep(end[-1]))+end--;+}++/*+*Striptrailingportnumberifwe'vegotonlya+*hostname(thatis,thereisnodirseparatorbuta+*colon).Thischeckisrequiredsuchthatwedonot+*stripURI'slike'/foo/bar:2222.git',whichshould+*resultinadir'2222'beingguessedduetobackwards+*compatibility.+*/+if(memchr(start,'/',end-start)==NULL+&&memchr(start,':',end-start)!=NULL){+ptr=end;+while(start<ptr&&isdigit(ptr[-1])&&ptr[-1]!=':')+ptr--;+if(start<ptr&&ptr[-1]==':')+end=ptr-1;+}++/*+*Findlastcomponent.Toremainbackwardscompatiblewe+*alsoregardcolonsaspathseparators,suchthat+*cloningarepository'foo:bar.git'wouldresultina+*directory'bar'beingguessed.+*/+ptr=end;+while(start<ptr&&!is_dir_sep(ptr[-1])&&ptr[-1]!=':')+ptr--;+start=ptr;++/*+*Strip.{bundle,git}.+*/+len=end-start;+strip_suffix_mem(start,&len,is_bundle?".bundle":".git");++if(!len||(len==1&&*start=='/'))+die(_("No directory name could be guessed.\n"+"Please specify a directory on the command line"));++if(is_bare)+dir=xstrfmt("%.*s.git",(int)len,start);+else+dir=xstrndup(start,len);+/*+*Replacesequencesof'control'charactersandwhitespace+*withoneasciispace,removeleadingandtrailingspaces.+*/+if(*dir){+char*out=dir;+intprev_space=1/* strip leading whitespace */;+for(end=dir;*end;++end){+charch=*end;+if((unsignedchar)ch<'\x20')+ch='\x20';+if(isspace(ch)){+if(prev_space)+continue;+prev_space=1;+}else+prev_space=0;+*out++=ch;+}+*out='\0';+if(out>dir&&prev_space)+out[-1]='\0';+}+returndir;+}++voidstrip_dir_trailing_slashes(char*dir)+{+char*end=dir+strlen(dir);++while(dir<end-1&&is_dir_sep(end[-1]))+end--;+*end='\0';+}+staticintremove_dir_recurse(structstrbuf*path,intflag,int*kept_up){DIR*dir;
Introduce the 'add' subcommand to `submodule--helper.c` that does all
the work 'submodule add' past the parsing of flags.
We also remove the constness of the sm_path field of the `add_data`
struct. This is needed so that it can be modified by
normalize_path_copy().
As with the previous conversions, this is meant to be a faithful
conversion with no modification to the behaviour of `submodule add`.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Helped-by: Kaartic Sivaraam [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Shourya Shukla [off-list ref]
Based-on-patch-by: Prathamesh Chavan [off-list ref]
---
builtin/submodule--helper.c | 165 +++++++++++++++++++++++++++++++++++-
git-submodule.sh | 96 +--------------------
2 files changed, 166 insertions(+), 95 deletions(-)
@@ -3046,6 +3046,168 @@ static int add_config(int argc, const char **argv, const char *prefix)return0;}+staticvoiddie_on_index_match(constchar*path,intforce)+{+structpathspecps;+constchar*args[]={path,NULL};+parse_pathspec(&ps,0,PATHSPEC_PREFER_CWD,NULL,args);++if(read_cache_preload(NULL)<0)+die(_("index file corrupt"));++if(ps.nr){+inti;+char*ps_matched=xcalloc(ps.nr,1);++/* TODO: audit for interaction with sparse-index. */+ensure_full_index(&the_index);++/*+*Sincethereisonlyonepathspec,wejustneed+*needtocheckps_matched[0]toknowifacache+*entrymatched.+*/+for(i=0;i<active_nr;i++){+ce_path_match(&the_index,active_cache[i],&ps,+ps_matched);++if(ps_matched[0]){+if(!force)+die(_("'%s' already exists in the index"),+path);+if(!S_ISGITLINK(active_cache[i]->ce_mode))+die(_("'%s' already exists in the index "+"and is not a submodule"),path);+break;+}+}+free(ps_matched);+}+}++staticvoiddie_on_repo_without_commits(constchar*path)+{+structstrbufsb=STRBUF_INIT;+strbuf_addstr(&sb,path);+if(is_nonbare_repository_dir(&sb)){+structobject_idoid;+if(resolve_gitlink_ref(path,"HEAD",&oid)<0)+die(_("'%s' does not have a commit checked out"),path);+}+}++staticintmodule_add(intargc,constchar**argv,constchar*prefix)+{+intforce=0,quiet=0,progress=0,dissociate=0;+structadd_dataadd_data=ADD_DATA_INIT;++structoptionoptions[]={+OPT_STRING('b',"branch",&add_data.branch,N_("branch"),+N_("branch of repository to add as submodule")),+OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),+PARSE_OPT_NOCOMPLETE),+OPT__QUIET(&quiet,N_("print only error messages")),+OPT_BOOL(0,"progress",&progress,N_("force cloning progress")),+OPT_STRING(0,"reference",&add_data.reference_path,N_("repository"),+N_("reference repository")),+OPT_BOOL(0,"dissociate",&dissociate,N_("borrow the objects from reference repositories")),+OPT_STRING(0,"name",&add_data.sm_name,N_("name"),+N_("sets the submodule’s name to the given string "+"instead of defaulting to its path")),+OPT_INTEGER(0,"depth",&add_data.depth,N_("depth for shallow clones")),+OPT_END()+};++constchar*constusage[]={+N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),+NULL+};++argc=parse_options(argc,argv,prefix,options,usage,0);++if(!is_writing_gitmodules_ok())+die(_("please make sure that the .gitmodules file is in the working tree"));++if(prefix&&*prefix&&+add_data.reference_path&&!is_absolute_path(add_data.reference_path))+add_data.reference_path=xstrfmt("%s%s",prefix,add_data.reference_path);++if(argc==0||argc>2)+usage_with_options(usage,options);++add_data.repo=argv[0];+if(argc==1)+add_data.sm_path=git_url_basename(add_data.repo,0,0);+else+add_data.sm_path=xstrdup(argv[1]);++if(prefix&&*prefix&&!is_absolute_path(add_data.sm_path))+add_data.sm_path=xstrfmt("%s%s",prefix,add_data.sm_path);++if(starts_with_dot_dot_slash(add_data.repo)||+starts_with_dot_slash(add_data.repo)){+if(prefix)+die(_("Relative path can only be used from the toplevel "+"of the working tree"));++/* dereference source url relative to parent's url */+add_data.realrepo=compute_submodule_clone_url(add_data.repo,NULL,1);+}elseif(is_dir_sep(add_data.repo[0])||strchr(add_data.repo,':')){+add_data.realrepo=add_data.repo;+}else{+die(_("repo URL: '%s' must be absolute or begin with ./|../"),+add_data.repo);+}++/*+*normalizepath:+*multiple//; leading ./; /./; /../;+*/+normalize_path_copy(add_data.sm_path,add_data.sm_path);+strip_dir_trailing_slashes(add_data.sm_path);++die_on_index_match(add_data.sm_path,force);+die_on_repo_without_commits(add_data.sm_path);++if(!force){+intexit_code=-1;+structstrbufsb=STRBUF_INIT;+structchild_processcp=CHILD_PROCESS_INIT;+cp.git_cmd=1;+cp.no_stdout=1;+strvec_pushl(&cp.args,"add","--dry-run","--ignore-missing",+"--no-warn-embedded-repo",add_data.sm_path,NULL);+if((exit_code=pipe_command(&cp,NULL,0,NULL,0,&sb,0))){+strbuf_complete_line(&sb);+fputs(sb.buf,stderr);+free(add_data.sm_path);+returnexit_code;+}+strbuf_release(&sb);+}++if(!add_data.sm_name)+add_data.sm_name=add_data.sm_path;++if(check_submodule_name(add_data.sm_name))+die(_("'%s' is not a valid submodule name"),add_data.sm_name);++add_data.prefix=prefix;+add_data.force=!!force;+add_data.quiet=!!quiet;+add_data.progress=!!progress;+add_data.dissociate=!!dissociate;++if(add_submodule(&add_data)){+free(add_data.sm_path);+return1;+}+configure_added_submodule(&add_data);+free(add_data.sm_path);++return0;+}+#define SUPPORT_SUPER_PREFIX (1<<0)structcmd_struct{
@@ -145,104 +145,12 @@ cmd_add()shiftdone-if!gitsubmodule--helperconfig--check-writeable>/dev/null2>&1+iftest-z"$1"then-die"fatal: $(eval_gettext"please make sure that the .gitmodules file is in the working tree")"-fi--iftest-n"$reference_path"-then-is_absolute_path"$reference_path"||-reference_path="$wt_prefix$reference_path"--reference="--reference=$reference_path"-fi--repo=$1-sm_path=$2--iftest-z"$sm_path";then-sm_path=$(printf'%s\n'"$repo"|-sed-e's|/$||'-e's|:*/*\.git$||'-e's|.*[/:]||g')-fi--iftest-z"$repo"||test-z"$sm_path";thenusagefi-is_absolute_path"$sm_path"||sm_path="$wt_prefix$sm_path"--# assure repo is absolute or relative to parent-case"$repo"in-./*|../*)-test-z"$wt_prefix"||-die"fatal: $(gettext"Relative path can only be used from the toplevel of the working tree")"--# dereference source url relative to parent's url-realrepo=$(gitsubmodule--helperresolve-relative-url"$repo")||exit-;;-*:*|/*)-# absolute url-realrepo=$repo-;;-*)-die"fatal: $(eval_gettext"repo URL: '\$repo' must be absolute or begin with ./|../")"-;;-esac--# normalize path:-# multiple //; leading ./; /./; /../; trailing /-sm_path=$(printf'%s/\n'"$sm_path"|-sed-e'-s|//*|/|g-s|^\(\./\)*||-s|/\(\./\)*|/|g-:start-s|\([^/]*\)/\.\./||-tstart-s|/*$||-')-iftest-z"$force"-then-gitls-files--error-unmatch"$sm_path">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index")"-else-gitls-files-s"$sm_path"|sane_grep-v"^160000">/dev/null2>&1&&-die"fatal: $(eval_gettext"'\$sm_path' already exists in the index and is not a submodule")"-fi--iftest-d"$sm_path"&&-test-z$(git-C"$sm_path"rev-parse--show-cdup2>/dev/null)-then-git-C"$sm_path"rev-parse--verify-qHEAD>/dev/null||-die"fatal: $(eval_gettext"'\$sm_path' does not have a commit checked out")"-fi--iftest-z"$force"-then-dryerr=$(gitadd--dry-run--ignore-missing--no-warn-embedded-repo"$sm_path"2>&1>/dev/null)-res=$?-iftest$res-ne0-then-echo>&2"$dryerr"-exit$res-fi-fi--iftest-n"$custom_name"-then-sm_name="$custom_name"-else-sm_name="$sm_path"-fi--if!gitsubmodule--helpercheck-name"$sm_name"-then-die"fatal: $(eval_gettext"'$sm_name' is not a valid submodule name")"-fi--gitsubmodule--helperadd-clone${GIT_QUIET:+--quiet}${force:+"--force"}${progress:+"--progress"}${branch:+--branch "$branch"}--prefix"$wt_prefix"--path"$sm_path"--name"$sm_name"--url"$realrepo"${reference:+"$reference"}${dissociate:+"--dissociate"}${depth:+"$depth"}||exit-gitsubmodule--helperadd-config${force:+--force}${branch:+--branch "$branch"}--url"$repo"--resolved-url"$realrepo"--path"$sm_path"--name"$sm_name"+git${wt_prefix:+-C "$wt_prefix"}${prefix:+--super-prefix "$prefix"}submodule--helperadd${GIT_QUIET:+--quiet}${force:+--force}${progress:+"--progress"}${branch:+--branch "$branch"}${reference_path:+--reference "$reference_path"}${dissociate:+--dissociate}${custom_name:+--name "$custom_name"}${depth:+"$depth"}--"$@"}#
We no longer need this subcommand, as all of its functionality is being
called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 60 -------------------------------------
1 file changed, 60 deletions(-)
@@ -2860,65 +2860,6 @@ static int add_submodule(const struct add_data *add_data)return0;}-staticintadd_clone(intargc,constchar**argv,constchar*prefix)-{-intforce=0,quiet=0,dissociate=0,progress=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to checkout on cloning")),-OPT_STRING(0,"prefix",&prefix,-N_("path"),-N_("alternative anchor for relative paths")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT_STRING(0,"url",&add_data.realrepo,-N_("string"),-N_("url where to clone the submodule from")),-OPT_STRING(0,"reference",&add_data.reference_path,-N_("repo"),-N_("reference repository")),-OPT_BOOL(0,"dissociate",&dissociate,-N_("use --reference only while cloning")),-OPT_INTEGER(0,"depth",&add_data.depth,-N_("depth for shallow clones")),-OPT_BOOL(0,"progress",&progress,-N_("force cloning progress")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT__QUIET(&quiet,"suppress output for cloning a submodule"),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-clone [<options>...] "-"--url <url> --path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc!=0)-usage_with_options(usage,options);--add_data.prefix=prefix;-add_data.progress=!!progress;-add_data.dissociate=!!dissociate;-add_data.force=!!force;-add_data.quiet=!!quiet;--if(add_submodule(&add_data))-return1;--return0;-}-staticintconfig_submodule_in_gitmodules(constchar*name,constchar*var,constchar*value){char*key;
Let's rename 'compute_submodule_clone_url()' to 'resolve_relative_url()'
to make it clear that this internal helper need not be used exclusively
for computing submodule clone URLs.
Since the original 'resolve-relative-url' subcommand and its C entry
point has been removed in c461095ae3 (submodule--helper: remove
resolve-relative-url subcommand, 2021-07-02), this rename can be done
without causing any confusion about which function it actually binds to.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
The shell subcommand `resolve-relative-url` is no longer required, as
its last caller has been removed when it was converted to C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 20 --------------------
1 file changed, 20 deletions(-)
@@ -223,25 +223,6 @@ static char *compute_submodule_clone_url(const char *rel_url, const char *up_patreturnresolved_url;}-staticintresolve_relative_url(intargc,constchar**argv,constchar*prefix)-{-constchar*up_path=NULL;-char*res;-constchar*url;--if(argc!=2&&argc!=3)-die("resolve-relative-url only accepts one or two arguments");--url=argv[1];-if(argc==3)-up_path=argv[2];--res=compute_submodule_clone_url(url,up_path,1);-puts(res);-free(res);-return0;-}-staticintresolve_relative_url_test(intargc,constchar**argv,constchar*prefix){char*remoteurl,*res;
Also no longer needed is this subcommand, as all of its functionality is
being called by the newly-introduced `module_add()` directly within C.
Signed-off-by: Atharva Raykar <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Shourya Shukla [off-list ref]
---
builtin/submodule--helper.c | 49 -------------------------------------
1 file changed, 49 deletions(-)
@@ -2939,54 +2939,6 @@ static void configure_added_submodule(struct add_data *add_data)}}-staticintadd_config(intargc,constchar**argv,constchar*prefix)-{-intforce=0;-structadd_dataadd_data=ADD_DATA_INIT;--structoptionoptions[]={-OPT_STRING('b',"branch",&add_data.branch,-N_("branch"),-N_("branch of repository to store in "-"the submodule configuration")),-OPT_STRING(0,"url",&add_data.repo,-N_("string"),-N_("url to clone submodule from")),-OPT_STRING(0,"resolved-url",&add_data.realrepo,-N_("string"),-N_("url to clone the submodule from, after it has "-"been dereferenced relative to parent's url, "-"in the case where <url> is a relative url")),-OPT_STRING(0,"path",&add_data.sm_path,-N_("path"),-N_("where the new submodule will be cloned to")),-OPT_STRING(0,"name",&add_data.sm_name,-N_("string"),-N_("name of the new submodule")),-OPT__FORCE(&force,N_("allow adding an otherwise ignored submodule path"),-PARSE_OPT_NOCOMPLETE),-OPT_END()-};--constchar*constusage[]={-N_("git submodule--helper add-config "-"[--force|-f] [--branch|-b <branch>] "-"--url <url> --resolved-url <resolved-url> "-"--path <path> --name <name>"),-NULL-};--argc=parse_options(argc,argv,prefix,options,usage,0);--if(argc)-usage_with_options(usage,options);--add_data.force=!!force;-configure_added_submodule(&add_data);--return0;-}-staticvoiddie_on_index_match(constchar*path,intforce){structpathspecps;
@@ -598,10 +598,14 @@ static char *compute_submodule_clone_url(const char *rel_url) strbuf_addf(&remotesb, "remote.%s.url", remote); if (git_config_get_string(remotesb.buf, &remoteurl)) {- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);+ if (!quiet)+ warning(_("could not look up configuration '%s'. "+ "Assuming this repository is its own "+ "authoritative upstream."),+ remotesb.buf); remoteurl = xgetcwd(); }- relurl = relative_url(remoteurl, rel_url, NULL);+ relurl = relative_url(remoteurl, rel_url, up_path);
After reading 2/8 of the series, I just noticed that 'remoteurl' is always
initialized in 'resolve_realtive_url'. It is either initialized to the return
value of 'xgetcwd' or retains its assigned value of 'NULL'. But it looks
like that's not the case here. 'remoteurl' could be used uninitialized
when the above if block does not get executed which in turn could result in
weird behaviour in case 'remoteurl' gets a value of anything other than 'NULL'
at runtime.
This again has nothing to do with the change done in this patch. Regardless, it
looks like something worth correcting. Thus, I thought of pointing it out.
Right. I agree it should be corrected.
Actually on having another look, I'm not sure if we need to assign NULL
to 'remoteurl' at all.
The 'if (git_config_get_string(...))' on success will allocate
'remoteurl'. If it fails, it will be given the return value of
'xgetcwd()'. There is nothing in the config API docs that suggest a
success mode for the git_config_get_*() functions that will assign
nothing to the buffer we give it. Therefore, by the time we get to the
variable's first use in the 'relative_url()' function, we are guaranteed
to have a well-defined value.
Ah ha! That explains why we haven't got any reports about weird behaviours
when using the likes of `git submodule init` so far ;-)
Thanks for digging this and sorry about the false flag!
It seems to me that the original 'resolve_relative_url()' had an
unnecessary NULL initialization.
Makes sense. I guess I feel into the trap of blindly trusting that the original
code was written correctly x-<
--
Sivaraam
@@ -453,6 +453,16 @@ static inline int is_dot_or_dotdot(const char *name)intis_empty_dir(constchar*dir);+/*+*Retrievethe"humanish"basenameofthegivenGitURL.+*+*Forexample:+*/path/to/repo.git=>"repo"+*host.xz.foo/.git=>"foo"+*/
Are you sure about the examples here? I just tried and ...
- '/path/to/repo.git' gave me 'repo' like you said
.. but ..
- 'host.xz.foo/.git' gives me 'host.xz.foo' instead of 'foo'.
I think you meant to have 'host.xz/foo.git' in the example.
Yikes! I meant 'host.xz:foo/.git'. That should give us 'foo'. Thanks for
the correction.
Interesting. I've usually seen host.xz:foo like syntax in HTTP URLs. For instance,
http://host.xz:4000/bar.baz.git
`git_url_basename` returns `bar.baz` for the above.
I wonder what real-world URL has a syntax like 'host.xz:foo/.git' for which
'foo' would be an appropriate basename to return. Does a real-world URL of
this form exist? Or is this just cooked up to demonstrate the basename that
would be returned for a hypothetical URL like this?
--
Sivaraam
if (git_config_get_string(remotesb.buf, &remoteurl)) {
- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
+ if (!quiet)
+ warning(_("could not look up configuration '%s'. "
+ "Assuming this repository is its own "
+ "authoritative upstream."),
+ remotesb.buf);
remoteurl = xgetcwd();
}
Why did you split warning message? We could keep that in one line.
--
An old man doll... just what I always wanted! - Clara
Yikes! I meant 'host.xz:foo/.git'. That should give us 'foo'. Thanks for
the correction.
Interesting. I've usually seen host.xz:foo like syntax in HTTP URLs. For instance,
http://host.xz:4000/bar.baz.git
`git_url_basename` returns `bar.baz` for the above.
I wonder what real-world URL has a syntax like 'host.xz:foo/.git' for which
'foo' would be an appropriate basename to return. Does a real-world URL of
this form exist? Or is this just cooked up to demonstrate the basename that
would be returned for a hypothetical URL like this?
if (git_config_get_string(remotesb.buf, &remoteurl)) {
- warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
+ if (!quiet)
+ warning(_("could not look up configuration '%s'. "
+ "Assuming this repository is its own "
+ "authoritative upstream."),
+ remotesb.buf);
remoteurl = xgetcwd();
}
Why did you split warning message? We could keep that in one line.