Changes from v2 (it's hard to keep track of after the rebase, so I may
be missing something here):
- rebased on top of recent master, incorporate changes in
init_pathspec from jk/pathspec-literal and nd/pathspec-wildcard to
parse_pathspec
- kill strip_trailing_slash_from_submodules and treat_gitlinks
(pretty sure it'll cause conflicts with as/check-ignore)
- kill init_pathspec, match_pathspec, diff_tree_setup_paths and
diff_tree_release_paths
- check points for future pathspec development
As far as I understand the "pathspec unification", I'd say we are
there, with a few exceptions like "mv", external commands.. But those
are pretty much isolated.
I'll send another WIP series implementing :(icase) and :(glob), mainly
to show (me) how future pathspec feature development looks like after
this.
Nguyễn Thái Ngọc Duy (31):
clean: remove unused variable "seen"
Add copy_pathspec
Add parse_pathspec() that converts cmdline args to struct pathspec
parse_pathspec: save original pathspec for reporting
Export parse_pathspec() and convert some get_pathspec() calls
Guard against new pathspec magic in pathspec matching code
clean: convert to use parse_pathspec
parse_pathspec: add PATHSPEC_EMPTY_MATCH_ALL
commit: convert to use parse_pathspec
status: convert to use parse_pathspec
rerere: convert to use parse_pathspec
checkout: convert to use parse_pathspec
rm: convert to use parse_pathspec
parse_pathspec: support stripping submodule trailing slashes
ls-files: convert to use parse_pathspec
archive: convert to use parse_pathspec
parse_pathspec: support stripping/checking submodule paths
add: convert to use parse_pathspec
Convert read_cache_preload() to take struct pathspec
Convert unmerge_cache to take struct pathspec
checkout: convert read_tree_some to take struct pathspec
Convert report_path_error to take struct pathspec
Convert refresh_index to take struct pathspec
Convert {read,fill}_directory to take struct pathspec
Convert add_files_to_cache to take struct pathspec
Convert common_prefix() to use struct pathspec
Remove diff_tree_{setup,release}_paths
Remove init_pathspec() in favor of parse_pathspec()
Remove match_pathspec() in favor of match_pathspec_depth()
tree-diff: remove the use of pathspec's raw[] in follow-rename codepath
Rename field "raw" to "_raw" in struct pathspec
archive.c | 18 +++--
archive.h | 2 +-
builtin/add.c | 155 +++++++++++++++----------------------
builtin/blame.c | 12 +--
builtin/checkout.c | 44 ++++++-----
builtin/clean.c | 21 ++---
builtin/commit.c | 37 +++++----
builtin/diff-files.c | 2 +-
builtin/diff-index.c | 2 +-
builtin/diff.c | 6 +-
builtin/grep.c | 6 +-
builtin/log.c | 2 +-
builtin/ls-files.c | 72 +++++++-----------
builtin/ls-tree.c | 10 ++-
builtin/mv.c | 13 ++--
builtin/rerere.c | 6 +-
builtin/reset.c | 4 +-
builtin/rm.c | 23 +++---
builtin/update-index.c | 3 +-
cache.h | 36 +++++++--
diff-lib.c | 2 +-
diff.h | 2 -
dir.c | 202 ++++++++-----------------------------------------
dir.h | 9 ++-
merge-recursive.c | 2 +-
notes-merge.c | 4 +-
preload-index.c | 20 ++---
read-cache.c | 5 +-
rerere.c | 6 +-
rerere.h | 4 +-
resolve-undo.c | 4 +-
resolve-undo.h | 2 +-
revision.c | 11 +--
setup.c | 149 ++++++++++++++++++++++++++++++------
tree-diff.c | 47 +++++++-----
tree-walk.c | 2 +
tree.c | 4 +-
tree.h | 2 +-
wt-status.c | 17 ++---
wt-status.h | 2 +-
40 files changed, 460 insertions(+), 510 deletions(-)
--
1.8.0.rc2.23.g1fb49df
@@ -81,17 +82,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)if(read_cache()<0)die(_("index file corrupt"));-source=copy_pathspec(prefix,argv,argc,0);+source=internal_copy_pathspec(prefix,argv,argc,0);modes=xcalloc(argc,sizeof(enumupdate_mode));-dest_path=copy_pathspec(prefix,argv+argc,1,0);+dest_path=internal_copy_pathspec(prefix,argv+argc,1,0);if(dest_path[0][0]=='\0')/* special case: "." was normalized to "" */-destination=copy_pathspec(dest_path[0],argv,argc,1);+destination=internal_copy_pathspec(dest_path[0],argv,argc,1);elseif(!lstat(dest_path[0],&st)&&S_ISDIR(st.st_mode)){dest_path[0]=add_slash(dest_path[0]);-destination=copy_pathspec(dest_path[0],argv,argc,1);+destination=internal_copy_pathspec(dest_path[0],argv,argc,1);}else{if(argc!=1)die("destination '%s' is not a directory",dest_path[0]);
Currently to fill a struct pathspec, we do:
const char **paths;
paths = get_pathspec(prefix, argv);
...
init_pathspec(&pathspec, paths);
"paths" can only carry bare strings, which loses information from
command line arguments such as pathspec magic or the prefix part's
length for each argument.
parse_pathspec() is introduced to combine the two calls into one. The
plan is gradually replace all get_pathspec() and init_pathspec() with
parse_pathspec(). get_pathspec() now becomes a thin wrapper of
parse_pathspec().
parse_pathspec() allows the caller to reject the pathspec magics that
it does not support. When a new pathspec magic is introduced, we can
enable it per command after making sure that all underlying code has no
problem with the new magic.
"flags" parameter is currently unused. But it would allow callers to
pass certain instructions to parse_pathspec, for example forcing
literal pathspec when no magic is used.
With the introduction of parse_pathspec, there are now two functions
that can initialize struct pathspec: init_pathspec and
parse_pathspec. Any semantic changes in struct pathspec must be
reflected in both functions. init_pathspec() will be phased out in
favor of parse_pathspec().
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 2 ++
dir.c | 4 +--
dir.h | 2 ++
setup.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
4 files changed, 90 insertions(+), 26 deletions(-)
@@ -241,39 +245,95 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char}if(magic&PATHSPEC_FROMTOP)-returnxstrdup(copyfrom);+match=xstrdup(copyfrom);else-returnprefix_path(prefix,prefixlen,copyfrom);+match=prefix_path(prefix,prefixlen,copyfrom);+*raw=item->match=match;+item->len=strlen(item->match);+item->flags=0;+if(limit_pathspec_to_literal())+item->nowildcard_len=item->len;+else+item->nowildcard_len=simple_length(item->match);+if(item->nowildcard_len<item->len&&+item->match[item->nowildcard_len]=='*'&&+no_wildcard(item->match+item->nowildcard_len+1))+item->flags|=PATHSPEC_ONESTAR;+returnmagic;}-constchar**get_pathspec(constchar*prefix,constchar**pathspec)+staticintpathspec_item_cmp(constvoid*a_,constvoid*b_){-constchar*entry=*pathspec;-constchar**src,**dst;-intprefixlen;+structpathspec_item*a,*b;-if(!prefix&&!entry)-returnNULL;+a=(structpathspec_item*)a_;+b=(structpathspec_item*)b_;+returnstrcmp(a->match,b->match);+}++/*+*Givencommandlineargumentsandaprefix,converttheinputto+*pathspec.die()ifanymagicotherthanonesinmagic_mask.+*/+staticvoidparse_pathspec(structpathspec*pathspec,+unsignedmagic_mask,unsignedflags,+constchar*prefix,constchar**argv)+{+structpathspec_item*item;+constchar*entry=*argv;+inti,n,prefixlen;++memset(pathspec,0,sizeof(*pathspec));++/* No arguments, no prefix -> no pathspec */+if(!entry&&!prefix)+return;+/* No arguments with prefix -> prefix pathspec */if(!entry){-staticconstchar*spec[2];-spec[0]=prefix;-spec[1]=NULL;-returnspec;+staticconstchar*raw[2];++pathspec->items=item=xmalloc(sizeof(*item));+item->match=prefix;+item->nowildcard_len=item->len=strlen(prefix);+raw[0]=prefix;+raw[1]=NULL;+pathspec->nr=1;+pathspec->raw=raw;+return;}-/* Otherwise we have to re-write the entries.. */-src=pathspec;-dst=pathspec;+n=0;+while(argv[n])+n++;++pathspec->nr=n;+pathspec->items=item=xmalloc(sizeof(*item)*n);+pathspec->raw=argv;prefixlen=prefix?strlen(prefix):0;-while(*src){-*(dst++)=prefix_pathspec(prefix,prefixlen,*src);-src++;++for(i=0;i<n;i++){+constchar*arg=argv[i];++item[i].magic=prefix_pathspec(item+i,argv+i,flags,+prefix,prefixlen,arg);+if(item[i].magic&~magic_mask)+die(_("pathspec magic in '%s' is not supported"+" by this command"),arg);+if(item[i].nowildcard_len<item[i].len)+pathspec->has_wildcard=1;+pathspec->magic|=item[i].magic;}-*dst=NULL;-if(!*pathspec)-returnNULL;-returnpathspec;++qsort(pathspec->items,pathspec->nr,+sizeof(structpathspec_item),pathspec_item_cmp);+}++constchar**get_pathspec(constchar*prefix,constchar**pathspec)+{+structpathspecps;+parse_pathspec(&ps,PATHSPEC_FROMTOP,0,prefix,pathspec);+returnps.raw;}/*
We usually use pathspec_item's match field for pathspec error
reporting. However "match" (or "raw") does not show the magic part,
which will play more important role later on. Preserve exact user
input for reporting.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 1 +
dir.c | 1 +
setup.c | 2 ++
3 files changed, 4 insertions(+)
@@ -546,10 +546,9 @@ static int do_reupdate(int ac, const char **av,*/intpos;inthas_head=1;-constchar**paths=get_pathspec(prefix,av+1);structpathspecpathspec;-init_pathspec(&pathspec,paths);+parse_pathspec(&pathspec,PATHSPEC_FROMTOP,0,prefix,av+1);if(read_ref("HEAD",head_sha1))/* If there is no HEAD, that means it is an initial
GUARD_PATHSPEC() marks pathspec-sensitive code (basically anything in
'struct pathspec' except fields "nr" and "original"). GUARD_PATHSPEC()
is not supposed to fail. The steps for a new pathspec magic or
optimization would be:
- update parse_pathspec, add extra information to struct pathspec
- grep GUARD_PATHSPEC() and update all relevant code (or note those
that won't work with your new stuff). Update GUARD_PATHSPEC mask
accordingly.
- update parse_pathspec calls to allow new magic. Make sure
parse_pathspec() catches unsupported syntax early, not until
GUARD_PATHSPEC catches it.
- add tests to verify supported/unsupported commands both work as
expected.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/diff.c | 2 ++
cache.h | 7 +++++++
dir.c | 2 ++
tree-diff.c | 19 +++++++++++++++++++
tree-walk.c | 2 ++
5 files changed, 32 insertions(+)
@@ -199,6 +199,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, coconstchar*paths[1];inti;+/*+*follow-renamecodeisveryspecific,weneedexactlyone+*path.Magicthatmatchesmorethanonepathisnot+*supported.+*/+GUARD_PATHSPEC(&opt->pathspec,PATHSPEC_FROMTOP);+#if 0+/*+*Weshouldrejectwildcardsaswell.Unfortunatelywe+*haven'tgotareliablewaytodetectthat'foo\*bar'in+*facthasnowildcards.nowildcard_lenismerelyahintfor+*optimization.Letitslipfornowuntilwildmatchistaught+*aboutdry-runmodeandreturnswildcardinfo.+*/+if(opt->pathspec.has_wildcard)+die("BUG:%s:%d: wildcards are not supported",+__FILE__,__LINE__);+#endif+/* Remove the file creation entry from the diff queue, and remember it */choice=q->queue[0];q->nr=0;
We have two ways of dealing with empty pathspec:
1. limit it to current prefix
2. match the entire working directory
Some commands go with #1, some with #2. get_pathspec() and
parse_pathspec() only supports #1. Make it support #2 too via
PATHSPEC_EMPTY_MATCH_ALL flag.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 3 +++
setup.c | 3 +++
2 files changed, 6 insertions(+)
This commit introduces a subtle bug:
- when match_pathspec() returns seen[], it follows the order of the
input "const char **pathspec", which is now pathspec.raw[]
- when match_pathspec() returns seen[], it follows the order of
pathspec.items[]
- due to 86e4ca6 (tree_entry_interesting(): fix depth limit with
overlapping pathspecs - 2010-12-15), pathspec.items[] is sorted, but
pathspec.raw[] is NOT.
by converting from match_pathspec() to match_pathspec_depth(), we also
have to switch the original path array. Unfortunately we can't because
this array is processed by report_path_error() and it's also used by
builtin/ls-files.c, which still uses the old indexing.
The bug causes wrong error messages (e.g. if the first pathspec is
faulty, it may report the second..) The bug will be dealt with after
ls-files is converted.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/checkout.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
@@ -256,39 +256,37 @@ static int checkout_paths(const struct checkout_opts *opts,if(opts->patch_mode)returnrun_add_interactive(revision,"--patch=checkout",-opts->pathspec);+opts->pathspec.raw);lock_file=xcalloc(1,sizeof(structlock_file));newfd=hold_locked_index(lock_file,1);-if(read_cache_preload(opts->pathspec)<0)+if(read_cache_preload(opts->pathspec.raw)<0)returnerror(_("corrupt index file"));if(opts->source_tree)-read_tree_some(opts->source_tree,opts->pathspec);+read_tree_some(opts->source_tree,opts->pathspec.raw);-for(pos=0;opts->pathspec[pos];pos++)-;-ps_matched=xcalloc(1,pos);+ps_matched=xcalloc(1,opts->pathspec.nr);for(pos=0;pos<active_nr;pos++){structcache_entry*ce=active_cache[pos];if(opts->source_tree&&!(ce->ce_flags&CE_UPDATE))continue;-match_pathspec(opts->pathspec,ce->name,ce_namelen(ce),0,ps_matched);+match_pathspec_depth(&opts->pathspec,ce->name,ce_namelen(ce),0,ps_matched);}-if(report_path_error(ps_matched,opts->pathspec,opts->prefix))+if(report_path_error(ps_matched,opts->pathspec.raw,opts->prefix))return1;/* "checkout -m path" to recreate conflicted state */if(opts->merge)-unmerge_cache(opts->pathspec);+unmerge_cache(opts->pathspec.raw);/* Any unmerged paths? */for(pos=0;pos<active_nr;pos++){structcache_entry*ce=active_cache[pos];-if(match_pathspec(opts->pathspec,ce->name,ce_namelen(ce),0,NULL)){+if(match_pathspec_depth(&opts->pathspec,ce->name,ce_namelen(ce),0,NULL)){if(!ce_stage(ce))continue;if(opts->force){
@@ -315,7 +313,7 @@ static int checkout_paths(const struct checkout_opts *opts,structcache_entry*ce=active_cache[pos];if(opts->source_tree&&!(ce->ce_flags&CE_UPDATE))continue;-if(match_pathspec(opts->pathspec,ce->name,ce_namelen(ce),0,NULL)){+if(match_pathspec_depth(&opts->pathspec,ce->name,ce_namelen(ce),0,NULL)){if(!ce_stage(ce)){errs|=checkout_entry(ce,&state,NULL);continue;
@@ -960,7 +958,7 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts)staticintcheckout_branch(structcheckout_opts*opts,structbranch_info*new){-if(opts->pathspec)+if(opts->pathspec.nr)die(_("paths cannot be used with switching branches"));if(opts->patch_mode)
@@ -249,31 +249,30 @@ int cmd_rm(int argc, const char **argv, const char *prefix)}}-pathspec=get_pathspec(prefix,argv);-refresh_index(&the_index,REFRESH_QUIET,pathspec,NULL,NULL);+parse_pathspec(&pathspec,PATHSPEC_FROMTOP,0,prefix,argv);+refresh_index(&the_index,REFRESH_QUIET,pathspec.raw,NULL,NULL);seen=NULL;-for(i=0;pathspec[i];i++)-/* nothing */;-seen=xcalloc(i,1);+seen=xcalloc(pathspec.nr,1);for(i=0;i<active_nr;i++){structcache_entry*ce=active_cache[i];-if(!match_pathspec(pathspec,ce->name,ce_namelen(ce),0,seen))+if(!match_pathspec_depth(&pathspec,ce->name,ce_namelen(ce),0,seen))continue;ALLOC_GROW(list.entry,list.nr+1,list.alloc);list.entry[list.nr].name=ce->name;list.entry[list.nr++].is_submodule=S_ISGITLINK(ce->ce_mode);}-if(pathspec){-constchar*match;+if(pathspec.nr){+constchar*original;intseen_any=0;-for(i=0;(match=pathspec[i])!=NULL;i++){+for(i=0;i<pathspec.nr;i++){+original=pathspec.items[i].original;if(!seen[i]){if(!ignore_unmatch){die(_("pathspec '%s' did not match any files"),-match);+original);}}else{
@@ -281,7 +280,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)}if(!recursive&&seen[i]==MATCHED_RECURSIVELY)die(_("not removing '%s' recursively without -r"),-*match?match:".");+*original?original:".");}if(!seen_any)
This flag is equivalent to builtin/ls-files.c:strip_trailing_slashes()
and is intended to replace that function when ls-files is converted to
use parse_pathspec.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 1 +
setup.c | 9 +++++++++
2 files changed, 10 insertions(+)
strip_trailing_slash_from_submodules() modifies pathspec and is moved
to dir.c, close to other pathspec code. It'll be removed later when
parse_pathspec() learns to take over its job.
This commit introduces a subtle bug:
- when match_pathspec() returns seen[], it follows the order of the
input "const char **pathspec", which is now pathspec.raw[]
- when match_pathspec() returns seen[], it follows the order of
pathspec.items[]
- due to 86e4ca6 (tree_entry_interesting(): fix depth limit with
overlapping pathspecs - 2010-12-15), pathspec.items[] is sorted, but
pathspec.raw[] is NOT.
by converting from match_pathspec() to match_pathspec_depth(), we also
have to switch the original path array. The bug causes wrong error
messages (e.g. if the first pathspec is faulty, it may report the
second..). report_path_error will be fixed in a separate patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/ls-files.c | 45 ++++++++++++---------------------------------
1 file changed, 12 insertions(+), 33 deletions(-)
@@ -187,7 +187,7 @@ static void show_ru_info(void)len=strlen(path);if(len<max_prefix_len)continue;/* outside of the prefix */-if(!match_pathspec(pathspec,path,len,max_prefix_len,ps_matched))+if(!match_pathspec_depth(&pathspec,path,len,max_prefix_len,ps_matched))continue;/* uninterested */for(i=0;i<3;i++){if(!ui->mode[i])
@@ -216,7 +216,7 @@ static void show_files(struct dir_struct *dir)/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){-fill_directory(dir,pathspec);+fill_directory(dir,pathspec.raw);if(show_others)show_other_files(dir);if(show_killed)
@@ -549,23 +534,17 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)if(require_work_tree&&!is_inside_work_tree())setup_work_tree();-pathspec=get_pathspec(prefix,argv);--/* be nice with submodule paths ending in a slash */-if(pathspec)-strip_trailing_slash_from_submodules();+parse_pathspec(&pathspec,PATHSPEC_FROMTOP,+PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,+prefix,argv);/* Find common prefix for all pathspec's */-max_prefix=common_prefix(pathspec);+max_prefix=common_prefix(pathspec.raw);max_prefix_len=max_prefix?strlen(max_prefix):0;/* Treat unmatching pathspec elements as errors */-if(pathspec&&error_unmatch){-intnum;-for(num=0;pathspec[num];num++)-;-ps_matched=xcalloc(1,num);-}+if(pathspec.nr&&error_unmatch)+ps_matched=xcalloc(1,pathspec.nr);if((dir.flags&DIR_SHOW_IGNORED)&&!exc_given)die("ls-files --ignored needs some exclude pattern");
@@ -592,7 +571,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)if(ps_matched){intbad;-bad=report_path_error(ps_matched,pathspec,prefix);+bad=report_path_error(ps_matched,pathspec.raw,prefix);if(bad)fprintf(stderr,"Did you forget to 'git add'?\n");
@@ -151,7 +151,6 @@ int write_archive_entries(struct archiver_args *args,structarchiver_contextcontext;structunpack_trees_optionsopts;structtree_desct;-structpathspecpathspec;interr;if(args->baselen>0&&args->base[args->baselen-1]=='/'){
@@ -186,10 +185,8 @@ int write_archive_entries(struct archiver_args *args,git_attr_set_direction(GIT_ATTR_INDEX,&the_index);}-init_pathspec(&pathspec,args->pathspec);-err=read_tree_recursive(args->tree,"",0,0,&pathspec,+err=read_tree_recursive(args->tree,"",0,0,&args->pathspec,write_archive_entry,&context);-free_pathspec(&pathspec);if(err==READ_TREE_RECURSIVE)err=0;returnerr;
@@ -231,8 +228,15 @@ static int path_exists(struct tree *tree, const char *path)staticvoidparse_pathspec_arg(constchar**pathspec,structarchiver_args*ar_args){-ar_args->pathspec=pathspec=get_pathspec("",pathspec);-if(pathspec){+/*+*raw[]isusedtocheckforunusedpathspec.Thisis+*tree_entry_interesting'slimitationbecauseitdoesnot+*mark"used"pathspec.Themagicmaskcannotbelifteduntil+*itdoes.+*/+parse_pathspec(&ar_args->pathspec,PATHSPEC_FROMTOP,0,"",pathspec);+if(ar_args->pathspec.nr){+pathspec=ar_args->pathspec.raw;while(*pathspec){if(!path_exists(ar_args->tree,*pathspec))die("path not found: %s",*pathspec);
PATHSPEC_SYMLINK_LEADING_PATH and _STRIP_SUBMODULE_SLASH_EXPENSIVE are
respectively the alternate implementation of
builtin/add.c:validate_pathspec() and
builtin/add.c:treat_gitlinks(). They are intended to replace those
functions when builtin/add.c is converted to use parse_pathspec.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 2 ++
setup.c | 26 ++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
@@ -259,6 +259,26 @@ static unsigned prefix_pathspec(struct pathspec_item *item,match[item->len]='\0';}+if(flags&PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)+for(i=0;i<active_nr;i++){+structcache_entry*ce=active_cache[i];+intce_len=ce_namelen(ce);++if(!S_ISGITLINK(ce->ce_mode))+continue;++if(item->len<=ce_len||match[ce_len]!='/'||+memcmp(ce->name,match,ce_len))+continue;+if(item->len==ce_len+1){+/* strip trailing slash */+item->len--;+match[item->len]='\0';+}else+die(_("Pathspec '%s' is in submodule '%.*s'"),+elt,ce_len,ce->name);+}+item->flags=0;if(limit_pathspec_to_literal())item->nowildcard_len=item->len;
@@ -333,6 +353,12 @@ void parse_pathspec(struct pathspec *pathspec,if(item[i].magic&~magic_mask)die(_("pathspec magic in '%s' is not supported"" by this command"),arg);++if((flags&PATHSPEC_SYMLINK_LEADING_PATH)&&+has_symlink_leading_path(item[i].match,item[i].len)){+die(_("pathspec '%s' is beyond a symbolic link"),arg);+}+if(item[i].nowildcard_len<item[i].len)pathspec->has_wildcard=1;pathspec->magic|=item[i].magic;
@@ -415,11 +374,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)fprintf(stderr,_("Maybe you wanted to say 'git add .'?\n"));return0;}-pathspec=validate_pathspec(argc,argv,prefix);if(read_cache()<0)die(_("index file corrupt"));-treat_gitlinks(pathspec);++/*+*Checkthe"pathspec '%s' did not match any files"block+*belowbeforeenablingnewmagic.+*/+parse_pathspec(&pathspec,PATHSPEC_FROMTOP,+PATHSPEC_SYMLINK_LEADING_PATH|+PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE,+prefix,argv);if(add_new_files){intbaselen;
@@ -432,33 +398,39 @@ int cmd_add(int argc, const char **argv, const char *prefix)}/* This picks up the paths that are not tracked */-baselen=fill_directory(&dir,pathspec);-if(pathspec)-seen=prune_directory(&dir,pathspec,baselen);+baselen=fill_directory(&dir,pathspec.raw);+if(pathspec.nr)+seen=prune_directory(&dir,pathspec.raw,baselen);}if(refresh_only){-refresh(verbose,pathspec);+refresh(verbose,pathspec.raw);gotofinish;}-if(pathspec){+if(pathspec.nr){inti;structpath_exclude_checkcheck;path_exclude_check_init(&check,&dir);if(!seen)-seen=find_used_pathspec(pathspec);-for(i=0;pathspec[i];i++){-if(!seen[i]&&pathspec[i][0]-&&!file_exists(pathspec[i])){+seen=find_used_pathspec(pathspec.raw);++/*+*file_exists()assumesexactmatch+*/+GUARD_PATHSPEC(&pathspec,PATHSPEC_FROMTOP);++for(i=0;pathspec.raw[i];i++){+if(!seen[i]&&pathspec.raw[i][0]+&&!file_exists(pathspec.raw[i])){if(ignore_missing){intdtype=DT_UNKNOWN;-if(is_path_excluded(&check,pathspec[i],-1,&dtype))-dir_add_ignored(&dir,pathspec[i],strlen(pathspec[i]));+if(is_path_excluded(&check,pathspec.raw[i],-1,&dtype))+dir_add_ignored(&dir,pathspec.raw[i],strlen(pathspec.raw[i]));}elsedie(_("pathspec '%s' did not match any files"),-pathspec[i]);+pathspec.raw[i]);}}free(seen);
@@ -182,6 +182,8 @@ struct cache_entry {#error "CE_EXTENDED_FLAGS out of range"#endif+structpathspec;+/**Copythesha1andstatstateofacacheentryfromoneto*another.Butweneverchangethename,orthehashstate!
@@ -434,7 +436,7 @@ extern int init_db(const char *template_dir, unsigned int flags);/* Initialize and use the cache information */externintread_index(structindex_state*);-externintread_index_preload(structindex_state*,constchar**pathspec);+externintread_index_preload(structindex_state*,conststructpathspec*pathspec);externintread_index_from(structindex_state*,constchar*path);externintis_index_unborn(structindex_state*);externintread_index_unmerged(structindex_state*);
@@ -156,7 +156,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)returnunmerge_index_entry_at(istate,pos);}-voidunmerge_index(structindex_state*istate,constchar**pathspec)+voidunmerge_index(structindex_state*istate,conststructpathspec*pathspec){inti;
@@ -82,12 +82,9 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,return0;}-staticintread_tree_some(structtree*tree,constchar**pathspec)+staticintread_tree_some(structtree*tree,conststructpathspec*pathspec){-structpathspecps;-init_pathspec(&ps,pathspec);-read_tree_recursive(tree,"",0,0,&ps,update_some,NULL);-free_pathspec(&ps);+read_tree_recursive(tree,"",0,0,pathspec,update_some,NULL);/* update the index with the given tree's info*forallargs,expandingwildcards,andexit
@@ -265,7 +262,7 @@ static int checkout_paths(const struct checkout_opts *opts,returnerror(_("corrupt index file"));if(opts->source_tree)-read_tree_some(opts->source_tree,opts->pathspec.raw);+read_tree_some(opts->source_tree,&opts->pathspec);ps_matched=xcalloc(1,opts->pathspec.nr);
This commit fixes a subtle bug in ls-files and commit:
- when match_pathspec() returns seen[], it follows the order of the
input "const char **pathspec", which is now pathspec.raw[]
- when match_pathspec() returns seen[], it follows the order of
pathspec.items[]
- due to 86e4ca6 (tree_entry_interesting(): fix depth limit with
overlapping pathspecs - 2010-12-15), pathspec.items[] is sorted, but
pathspec.raw[] is NOT.
by converting from match_pathspec() to match_pathspec_depth(), we also
have to switch the original path array. We haven't done so because
there are other call sites of report_path_error() that relies on old
order. We now follow pathspec.items[] order.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/checkout.c | 2 +-
builtin/commit.c | 14 ++++++--------
builtin/ls-files.c | 19 +++++++++++--------
cache.h | 2 +-
4 files changed, 19 insertions(+), 18 deletions(-)
@@ -380,9 +384,8 @@ int report_path_error(const char *ps_matched, const char **pathspec, const charif(found_dup)continue;-name=quote_path_relative(pathspec[num],-1,&sb,prefix);error("pathspec '%s' did not match any file(s) known to git.",-name);+pathspec->items[num].original);errors++;}strbuf_release(&sb);
@@ -571,7 +574,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)if(ps_matched){intbad;-bad=report_path_error(ps_matched,pathspec.raw,prefix);+bad=report_path_error(ps_matched,&pathspec,prefix);if(bad)fprintf(stderr,"Did you forget to 'git add'?\n");
@@ -153,19 +153,18 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, intreturnseen;}-staticvoidrefresh(intverbose,constchar**pathspec)+staticvoidrefresh(intverbose,conststructpathspec*pathspec){char*seen;-inti,specs;+inti;-for(specs=0;pathspec[specs];specs++)-/* nothing */;-seen=xcalloc(specs,1);+seen=xcalloc(pathspec->nr,1);refresh_index(&the_index,verbose?REFRESH_IN_PORCELAIN:REFRESH_QUIET,pathspec,seen,_("Unstaged changes after refreshing the index:"));-for(i=0;i<specs;i++){+for(i=0;i<pathspec->nr;i++){if(!seen[i])-die(_("pathspec '%s' did not match any files"),pathspec[i]);+die(_("pathspec '%s' did not match any files"),+pathspec->items[i].match);}free(seen);}
@@ -397,7 +397,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)}/* This picks up the paths that are not tracked */-baselen=fill_directory(&dir,pathspec.raw);+baselen=fill_directory(&dir,&pathspec);if(pathspec.nr)seen=prune_directory(&dir,pathspec.raw,baselen);}
@@ -216,7 +216,7 @@ static void show_files(struct dir_struct *dir)/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){-fill_directory(dir,pathspec.raw);+fill_directory(dir,&pathspec);if(show_others)show_other_files(dir);if(show_killed)
@@ -106,10 +106,10 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)*Calculatecommonprefixforthepathspec,and*usethattooptimizethedirectorywalk*/-len=common_prefix_len(pathspec);+len=common_prefix_len(pathspec->raw);/* Read the directory and prune it */-read_directory(dir,pathspec?*pathspec:"",len,pathspec);+read_directory(dir,pathspec->nr?pathspec->raw[0]:"",len,pathspec);returnlen;}
@@ -1323,14 +1323,20 @@ static int treat_leading_path(struct dir_struct *dir,returnrc;}-intread_directory(structdir_struct*dir,constchar*path,intlen,constchar**pathspec)+intread_directory(structdir_struct*dir,constchar*path,intlen,conststructpathspec*pathspec){structpath_simplify*simplify;+/*+*Checkoutcreate_simplify()+*/+if(pathspec)+GUARD_PATHSPEC(pathspec,PATHSPEC_FROMTOP);+if(has_symlink_leading_path(path,len))returndir->nr;-simplify=create_simplify(pathspec);+simplify=create_simplify(pathspec?pathspec->raw:NULL);if(!len||treat_leading_path(dir,path,len,simplify))read_directory_recursive(dir,path,len,0,simplify);free_simplify(simplify);
@@ -1072,7 +1072,7 @@ static int find_copy_in_parent(struct scoreboard *sb,diff_opts.output_format=DIFF_FORMAT_NO_OUTPUT;paths[0]=NULL;-diff_tree_setup_paths(paths,&diff_opts);+init_pathspec(&diff_opts.pathspec,paths);diff_setup_done(&diff_opts);/* Try "find copies harder" on new path if requested;
@@ -1155,7 +1155,7 @@ static int find_copy_in_parent(struct scoreboard *sb,}reset_scanned_flag(sb);diff_flush(&diff_opts);-diff_tree_release_paths(&diff_opts);+free_pathspec(&diff_opts.pathspec);returnretval;}
@@ -195,7 +195,7 @@ static int read_from_tree(const char *prefix, const char **argv,return1;diffcore_std(&opt);diff_flush(&opt);-diff_tree_release_paths(&opt);+free_pathspec(&opt.pathspec);if(!index_was_discarded)/* The index is still clobbered from do_diff_cache() */
@@ -230,11 +230,11 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, codiff_opts.break_opt=opt->break_opt;diff_opts.rename_score=opt->rename_score;paths[0]=NULL;-diff_tree_setup_paths(paths,&diff_opts);+init_pathspec(&diff_opts.pathspec,paths);diff_setup_done(&diff_opts);diff_tree(t1,t2,base,&diff_opts);diffcore_std(&diff_opts);-diff_tree_release_paths(&diff_opts);+free_pathspec(&diff_opts.pathspec);/* Go through the new set of filepairing, and see if we find a more interesting one */opt->found_follow=0;
@@ -253,9 +253,9 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, cochoice=p;/* Update the path we use from now on.. */-diff_tree_release_paths(opt);+free_pathspec(&opt->pathspec);opt->pathspec.raw[0]=xstrdup(p->one->path);-diff_tree_setup_paths(opt->pathspec.raw,opt);+init_pathspec(&opt->pathspec,opt->pathspec.raw);/**Thecallerexpectsustoreturnasetofvanilla
@@ -542,7 +542,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)prefix,argv);/* Find common prefix for all pathspec's */-max_prefix=common_prefix(pathspec.raw);+max_prefix=common_prefix(&pathspec);max_prefix_len=max_prefix?strlen(max_prefix):0;/* Treat unmatching pathspec elements as errors */
@@ -106,7 +105,7 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)*Calculatecommonprefixforthepathspec,and*usethattooptimizethedirectorywalk*/-len=common_prefix_len(pathspec->raw);+len=common_prefix_len(pathspec);/* Read the directory and prune it */read_directory(dir,pathspec->nr?pathspec->raw[0]:"",len,pathspec);
@@ -1072,7 +1072,7 @@ static int find_copy_in_parent(struct scoreboard *sb,diff_opts.output_format=DIFF_FORMAT_NO_OUTPUT;paths[0]=NULL;-init_pathspec(&diff_opts.pathspec,paths);+parse_pathspec(&diff_opts.pathspec,0,0,"",paths);diff_setup_done(&diff_opts);/* Try "find copies harder" on new path if requested;
@@ -248,14 +245,17 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co*/if((p->status=='R'||p->status=='C')&&!strcmp(p->two->path,opt->pathspec.raw[0])){+constchar*path[2];+/* Switch the file-pairs around */q->queue[i]=choice;choice=p;/* Update the path we use from now on.. */+path[0]=p->one->path;+path[1]=NULL;free_pathspec(&opt->pathspec);-opt->pathspec.raw[0]=xstrdup(p->one->path);-init_pathspec(&opt->pathspec,opt->pathspec.raw);+parse_pathspec(&opt->pathspec,0,0,"",path);/**Thecallerexpectsustoreturnasetofvanilla
@@ -99,7 +99,7 @@ int add_files_to_cache(const char *prefix,return!!data.add_errors;}-staticvoidfill_pathspec_matches(constchar**pathspec,char*seen,intspecs)+staticvoidfill_pathspec_matches(structpathspec*pathspec,char*seen){intnum_unmatched=0,i;
@@ -109,49 +109,43 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)*mistakenlythinkthattheusergaveapathspecthatdidnotmatch*anything.*/-for(i=0;i<specs;i++)+for(i=0;i<pathspec->nr;i++)if(!seen[i])num_unmatched++;if(!num_unmatched)return;for(i=0;i<active_nr;i++){structcache_entry*ce=active_cache[i];-match_pathspec(pathspec,ce->name,ce_namelen(ce),0,seen);+match_pathspec_depth(pathspec,ce->name,ce_namelen(ce),0,seen);}}-staticchar*find_used_pathspec(constchar**pathspec)+staticchar*find_used_pathspec(structpathspec*pathspec){char*seen;-inti;--for(i=0;pathspec[i];i++)-;/* just counting */-seen=xcalloc(i,1);-fill_pathspec_matches(pathspec,seen,i);+seen=xcalloc(pathspec->nr,1);+fill_pathspec_matches(pathspec,seen);returnseen;}-staticchar*prune_directory(structdir_struct*dir,constchar**pathspec,intprefix)+staticchar*prune_directory(structdir_struct*dir,structpathspec*pathspec,intprefix){char*seen;-inti,specs;+inti;structdir_entry**src,**dst;-for(specs=0;pathspec[specs];specs++)-/* nothing */;-seen=xcalloc(specs,1);+seen=xcalloc(pathspec->nr,1);src=dst=dir->entries;i=dir->nr;while(--i>=0){structdir_entry*entry=*src++;-if(match_pathspec(pathspec,entry->name,entry->len,-prefix,seen))+if(match_pathspec_depth(pathspec,entry->name,entry->len,+prefix,seen))*dst++=entry;}dir->nr=dst-dir->entries;-fill_pathspec_matches(pathspec,seen,specs);+fill_pathspec_matches(pathspec,seen);returnseen;}
@@ -401,7 +395,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)/* This picks up the paths that are not tracked */baselen=fill_directory(&dir,&pathspec);if(pathspec.nr)-seen=prune_directory(&dir,pathspec.raw,baselen);+seen=prune_directory(&dir,&pathspec,baselen);}if(refresh_only){
@@ -415,23 +409,23 @@ int cmd_add(int argc, const char **argv, const char *prefix)path_exclude_check_init(&check,&dir);if(!seen)-seen=find_used_pathspec(pathspec.raw);+seen=find_used_pathspec(&pathspec);/**file_exists()assumesexactmatch*/GUARD_PATHSPEC(&pathspec,PATHSPEC_FROMTOP);-for(i=0;pathspec.raw[i];i++){-if(!seen[i]&&pathspec.raw[i][0]-&&!file_exists(pathspec.raw[i])){+for(i=0;i<pathspec.nr;i++){+constchar*path=pathspec.items[i].match;+if(!seen[i]&&!file_exists(path)){if(ignore_missing){intdtype=DT_UNKNOWN;-if(is_path_excluded(&check,pathspec.raw[i],-1,&dtype))-dir_add_ignored(&dir,pathspec.raw[i],strlen(pathspec.raw[i]));+if(is_path_excluded(&check,path,-1,&dtype))+dir_add_ignored(&dir,path,pathspec.items[i].len);}elsedie(_("pathspec '%s' did not match any files"),-pathspec.raw[i]);+pathspec.items[i].original);}}free(seen);
@@ -139,106 +139,6 @@ int within_depth(const char *name, int namelen,**Itreturns0whenthereisnomatch.*/-staticintmatch_one(constchar*match,constchar*name,intnamelen)-{-intmatchlen;-intliteral=limit_pathspec_to_literal();--/* If the match was just the prefix, we matched */-if(!*match)-returnMATCHED_RECURSIVELY;--if(ignore_case){-for(;;){-unsignedcharc1=tolower(*match);-unsignedcharc2=tolower(*name);-if(c1=='\0'||(!literal&&is_glob_special(c1)))-break;-if(c1!=c2)-return0;-match++;-name++;-namelen--;-}-}else{-for(;;){-unsignedcharc1=*match;-unsignedcharc2=*name;-if(c1=='\0'||(!literal&&is_glob_special(c1)))-break;-if(c1!=c2)-return0;-match++;-name++;-namelen--;-}-}--/*-*Ifwedon'tmatchthematchstringexactly,-*weneedtomatchbyfnmatch-*/-matchlen=strlen(match);-if(strncmp_icase(match,name,matchlen)){-if(literal)-return0;-return!fnmatch_icase(match,name,0)?MATCHED_FNMATCH:0;-}--if(namelen==matchlen)-returnMATCHED_EXACTLY;-if(match[matchlen-1]=='/'||name[matchlen]=='/')-returnMATCHED_RECURSIVELY;-return0;-}--/*-*Givenanameandalistofpathspecs,seeifthenamematches-*anyofthepathspecs.Thecallerisalsointerestedinseeing-*allpathspecmatchessomenamesitcallsthisfunctionwith-*(otherwisetheusercouldhavemistypedtheunmatchedpathspec),-*andamarkisleftinseen[]arrayforpathspecelementthat-*actuallymatchedanything.-*/-intmatch_pathspec(constchar**pathspec,constchar*name,intnamelen,-intprefix,char*seen)-{-inti,retval=0;--if(!pathspec)-return1;--name+=prefix;-namelen-=prefix;--for(i=0;pathspec[i]!=NULL;i++){-inthow;-constchar*match=pathspec[i]+prefix;-if(seen&&seen[i]==MATCHED_EXACTLY)-continue;-how=match_one(match,name,namelen);-if(how){-if(retval<how)-retval=how;-if(seen&&seen[i]<how)-seen[i]=how;-}-}-returnretval;-}--/*-*Does'match'matchthegivenname?-*Amatchisfoundif-*-*(1)the'match'stringisleadingdirectoryof'name',or-*(2)the'match'stringisawildcardandmatches'name',or-*(3)the'match'stringisexactlythesameas'name'.-*-*andthereturnvaluetellswhichcaseitwas.-*-*Itreturns0whenthereisnomatch.-*/staticintmatch_pathspec_item(conststructpathspec_item*item,intprefix,constchar*name,intnamelen){
This patch is essentially no-op. It helps catching new use of this
field though. This field is introduced as an intermediate step for the
pathspec conversion and will be removed eventually. At this stage no
more access sites should be introduced.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
archive.c | 2 +-
builtin/add.c | 2 +-
builtin/checkout.c | 2 +-
builtin/ls-tree.c | 2 +-
cache.h | 2 +-
dir.c | 4 ++--
setup.c | 6 +++---
7 files changed, 10 insertions(+), 10 deletions(-)
@@ -108,7 +108,7 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)len=common_prefix_len(pathspec);/* Read the directory and prune it */-read_directory(dir,pathspec->nr?pathspec->raw[0]:"",len,pathspec);+read_directory(dir,pathspec->nr?pathspec->_raw[0]:"",len,pathspec);returnlen;}
@@ -1235,7 +1235,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const struif(has_symlink_leading_path(path,len))returndir->nr;-simplify=create_simplify(pathspec?pathspec->raw:NULL);+simplify=create_simplify(pathspec?pathspec->_raw:NULL);if(!len||treat_leading_path(dir,path,len,simplify))read_directory_recursive(dir,path,len,0,simplify);free_simplify(simplify);
From: Martin von Zweigbergk <hidden> Date: 2016-06-15 22:55:50
Hi,
I was tempted to ask this before, and the recent thread regarding "add
-u/A" [1] convinced me to.
On Sun, Jan 13, 2013 at 4:35 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
We have two ways of dealing with empty pathspec:
1. limit it to current prefix
2. match the entire working directory
Some commands go with #1, some with #2. get_pathspec() and
parse_pathspec() only supports #1. Make it support #2 too via
PATHSPEC_EMPTY_MATCH_ALL flag.
If #2 is indeed the direction we want to go, then maybe we should make
that the default behavior from parse_pathspec()? I.e. rename the flag
"PATHSPEC_EMPTY_MATCH_PREFIX" (or something). Makes sense?
Btw, Matthieu was asking where we use #1. If you do invert the name
and meaning of the flag, then the answer to that question should be
(mostly?) obvious from a re-roll of your series (i.e. all the places
where PATHSPEC_EMPTY_MATCH_PREFIX is used).
Martin
[1] http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214113
On Tue, Jan 22, 2013 at 6:12 AM, Martin von Zweigbergk
[off-list ref] wrote:
Hi,
I was tempted to ask this before, and the recent thread regarding "add
-u/A" [1] convinced me to.
On Sun, Jan 13, 2013 at 4:35 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted
We have two ways of dealing with empty pathspec:
1. limit it to current prefix
2. match the entire working directory
Some commands go with #1, some with #2. get_pathspec() and
parse_pathspec() only supports #1. Make it support #2 too via
PATHSPEC_EMPTY_MATCH_ALL flag.
If #2 is indeed the direction we want to go, then maybe we should make
that the default behavior from parse_pathspec()? I.e. rename the flag
"PATHSPEC_EMPTY_MATCH_PREFIX" (or something). Makes sense?
No problem with me. Will do unless someone objects.
Btw, Matthieu was asking where we use #1. If you do invert the name
and meaning of the flag, then the answer to that question should be
(mostly?) obvious from a re-roll of your series (i.e. all the places
where PATHSPEC_EMPTY_MATCH_PREFIX is used).
Martin
[1] http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214113