Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On Fri, Jun 07, 2013 at 11:50:31PM +0200, benoit.per...@ensimag.fr wrote: The #7 issue on git-mediawiki's issue tracker [1] states that the ability to preview content without pushing would be a nice thing to have. Sounds like a useful goal. The default behaviour for the `preview` subcommand is: 1- Find the remote name of the current branch's upstream and check if it's a wiki one with its url (ie: mediawiki://) 2- Parse the content of the local file (given as argument) using the distant wiki's API. Makes sense. 3- Retrieve the current page on the distant mediawiki. 4- Merge those those contents. I'm not sure what these steps are for. You are trying to preview not just your local version, but pulling in any changes that have happened upstream since the work you built on top of? It seems like that is a separate, orthogonal issue, and git would be the right place to do that merge. IOW, as a user, your workflow would be something like: 1. git fetch, pulling down the latest copy from the server 2. make your changes on top 3. use this command to preview your changes 4. use git fetch to check whether anything else happened on the server. a. If not, go ahead and push. b. If upstream changed, use git diff to inspect the changes to the wiki source. Merge or rebase your changes with respect to what's on the server. Goto step 3 to make sure they look good. I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. It works but a couple of points trouble me: 1- I had to copy two functions from `git-remote-mediawiki.perl`, I don't really know how we could factorize those things ? I don't think it makes much sense to create a package just for them ? You could make a Git::MediaWiki.pm module, but installing that would significantly complicate the build procedure, and potentially be annoying for users. One trick I have done in the past is to concatenate bits of perl script together in the Makefile, like this: foo: common.pl foo.pl { \ echo '$(PERL_PATH_SQ)' \ for i in $^; do \ echo #line 1 $src \ cat $src \ done \ } $@+ mv $@+ $@ That would conflict a bit with the way we chain to git's Makefile, though. I suspect you could do something complicated like build foo.pl from common.pl and foo-main.pl, then chain to git's Makefile to build foo from foo.pl. 2- The current behavior is to crash if the current branch do not have an upstream branch on a valid mediawiki remote. To find that specific remote, it runs `git rev-parse --symbolic-full-name @{upstream}` which will return something like `/refs/remotes/$remote_name/master`. 2a- maybe there is a better way to find that remote name ? If you just care about the remote name and not the name of the local branch, you can just ask for my $curr_branch = `git symbolic-ref HEAD`; my $remote = `git config branch.$curr_branch.remote`; my $url = `git config remote.$remote.url`; Of course you would want some error checks and probably some chomp()s in there, too. 2b- would it be useful to add a fallback if that search fails ? searching for a valid mediawiki remote url in all the remotes returned by `git remote` for instance ? That is probably OK as long as there is only one such remote, and it would help the case where you have branched off of a local branch (so your upstream remote is .). If there are two mediawiki remotes, though, it would make sense to simply fail, as you don't know which to use. But I'd expect the common case by far to be that you simply have one such remote. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On Sat, Jun 08, 2013 at 09:00:30PM +0200, Matthieu Moy wrote: + # Auto-loading in browser + if ($autoload) { + open(my $browser, -|:encoding(UTF-8), xdg-open .$preview_file_name); That could be read from Git's configuration, and default to xdg-open. But you don't want to hardcode it in the middle of the code. In fact, I think we provide the git-web--browse tool for just this purpose. It takes care of the trickiness of looking at the web.browser config, resolving custom browser tools defined by browser.tool.*, and handling backgrounding of the browser (which you want to do for graphical browsers but not for terminal ones). -Peff PS I agreed with all of the other comments in your review. :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/45] struct pathspec conversion and :(glob)
This series finishes off struct pathspec conversion that was started 2 years ago when this struct was added. In the end we can pass more information down the callchain than simply a series of strings. This makes it possible to add more pathspec magic, the :(glob) in the end of this series is an example. This also changes --literal-pathspecs slightly, to not just make the pathspec literal, but to disable all magic. Next step: introduction of :(icase) magic for case-insensitive matching (definitely won't take another 2 years as the groundwork is pretty much done). Nguyễn Thái Ngọc Duy (45): clean: remove unused variable seen Move struct pathspec and related functions to pathspec.[ch] pathspec: i18n-ize error strings in pathspec parsing code pathspec: add copy_pathspec Add parse_pathspec() that converts cmdline args to struct pathspec parse_pathspec: save original pathspec for reporting parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL} Convert some get_pathspec() calls to parse_pathspec() parse_pathspec: a special flag for max_depth feature parse_pathspec: support stripping submodule trailing slashes parse_pathspec: support stripping/checking submodule paths parse_pathspec: support prefixing original patterns Guard against new pathspec magic in pathspec matching code clean: convert to use parse_pathspec 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 ls-files: convert to use parse_pathspec archive: convert to use parse_pathspec check-ignore: convert to use parse_pathspec add: convert to use parse_pathspec reset: convert to use parse_pathspec line-log: convert to use parse_pathspec Convert read_cache_preload() to take struct pathspec Convert run_add_interactive to use 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 parse_pathspec: make sure the prefix part is wildcard-free parse_pathspec: preserve prefix length via PATHSPEC_PREFIX_ORIGIN Kill limit_pathspec_to_literal() as it's only used by parse_pathspec() pathspec: support :(literal) syntax for noglob pathspec pathspec: make --literal-pathspecs disable pathspec magic pathspec: support :(glob) syntax Rename field raw to _raw in struct pathspec Documentation/git.txt | 23 +- Documentation/glossary-content.txt| 49 +++- Documentation/technical/api-setup.txt | 38 ++- archive.c | 18 +- archive.h | 4 +- builtin/add.c | 164 ++--- builtin/blame.c | 14 +- builtin/check-ignore.c| 35 ++- builtin/checkout.c| 40 +-- builtin/clean.c | 24 +- builtin/commit.c | 37 ++- builtin/diff-files.c | 2 +- builtin/diff-index.c | 2 +- builtin/diff.c| 6 +- builtin/grep.c| 10 +- builtin/log.c | 2 +- builtin/ls-files.c| 75 +++--- builtin/ls-tree.c | 13 +- builtin/mv.c | 13 +- builtin/rerere.c | 8 +- builtin/reset.c | 33 ++- builtin/rm.c | 24 +- builtin/update-index.c| 6 +- cache.h | 34 +-- commit.h | 2 +- diff-lib.c| 3 +- diff.h| 3 +- dir.c | 261 +--- dir.h | 18 +- git.c | 8 + line-log.c| 2 +- merge-recursive.c | 2 +- notes-merge.c | 4 +- path.c| 15 +- pathspec.c| 442 +++--- pathspec.h| 68 +- preload-index.c | 21 +- read-cache.c | 5 +- rerere.c | 7 +- rerere.h | 4 +- resolve-undo.c| 4 +- resolve-undo.h| 2 +-
[PATCH 03/45] pathspec: i18n-ize error strings in pathspec parsing code
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pathspec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pathspec.c b/pathspec.c index 133f8be..403095b 100644 --- a/pathspec.c +++ b/pathspec.c @@ -167,11 +167,11 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char break; } if (ARRAY_SIZE(pathspec_magic) = i) - die(Invalid pathspec magic '%.*s' in '%s', + die(_(Invalid pathspec magic '%.*s' in '%s'), (int) len, copyfrom, elt); } if (*copyfrom != ')') - die(Missing ')' at the end of pathspec magic in '%s', elt); + die(_(Missing ')' at the end of pathspec magic in '%s'), elt); copyfrom++; } else { /* shorthand */ @@ -188,7 +188,7 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char break; } if (ARRAY_SIZE(pathspec_magic) = i) - die(Unimplemented pathspec magic '%c' in '%s', + die(_(Unimplemented pathspec magic '%c' in '%s'), ch, elt); } if (*copyfrom == ':') -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/45] Move struct pathspec and related functions to pathspec.[ch]
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- archive.c | 1 + builtin/grep.c | 1 + builtin/ls-files.c | 1 + builtin/ls-tree.c | 1 + builtin/update-index.c | 1 + cache.h| 22 +--- diff.h | 1 + dir.c | 1 + pathspec.c | 150 + pathspec.h | 21 +++ preload-index.c| 1 + setup.c| 149 tree-walk.c| 1 + 13 files changed, 182 insertions(+), 169 deletions(-) diff --git a/archive.c b/archive.c index d254fa5..c699a2d 100644 --- a/archive.c +++ b/archive.c @@ -5,6 +5,7 @@ #include archive.h #include parse-options.h #include unpack-trees.h +#include pathspec.h static char const * const archive_usage[] = { N_(git archive [options] tree-ish [path...]), diff --git a/builtin/grep.c b/builtin/grep.c index 159e65d..4de49df 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -17,6 +17,7 @@ #include grep.h #include quote.h #include dir.h +#include pathspec.h static char const * const grep_usage[] = { N_(git grep [options] [-e] pattern [rev...] [[--] path...]), diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 2202072..a0b7e30 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -13,6 +13,7 @@ #include parse-options.h #include resolve-undo.h #include string-list.h +#include pathspec.h static int abbrev; static int show_deleted; diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index fb76e38..93fc3a0 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -10,6 +10,7 @@ #include quote.h #include builtin.h #include parse-options.h +#include pathspec.h static int line_termination = '\n'; #define LS_RECURSIVE 1 diff --git a/builtin/update-index.c b/builtin/update-index.c index 5c7762e..b9c2bd0 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -11,6 +11,7 @@ #include refs.h #include resolve-undo.h #include parse-options.h +#include pathspec.h /* * Default to not allowing changes to the list of files. The diff --git a/cache.h b/cache.h index df532f8..5917375 100644 --- a/cache.h +++ b/cache.h @@ -185,6 +185,8 @@ struct cache_entry { #error CE_EXTENDED_FLAGS out of range #endif +struct pathspec; + /* * Copy the sha1 and stat state of a cache entry from one to * another. But we never change the name, or the hash state! @@ -483,28 +485,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int); -#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR */ - -struct pathspec { - const char **raw; /* get_pathspec() result, not freed by free_pathspec() */ - int nr; - unsigned int has_wildcard:1; - unsigned int recursive:1; - int max_depth; - struct pathspec_item { - const char *match; - int len; - int nowildcard_len; - int flags; - } *items; -}; - -extern int init_pathspec(struct pathspec *, const char **); -extern void free_pathspec(struct pathspec *); extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec); -extern int limit_pathspec_to_literal(void); - #define HASH_WRITE_OBJECT 1 #define HASH_FORMAT_CHECK 2 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags); diff --git a/diff.h b/diff.h index 78b4091..d1bc914 100644 --- a/diff.h +++ b/diff.h @@ -5,6 +5,7 @@ #define DIFF_H #include tree-walk.h +#include pathspec.h struct rev_info; struct diff_options; diff --git a/dir.c b/dir.c index 897c874..9291c9a 100644 --- a/dir.c +++ b/dir.c @@ -11,6 +11,7 @@ #include dir.h #include refs.h #include wildmatch.h +#include pathspec.h struct path_simplify { int len; diff --git a/pathspec.c b/pathspec.c index 284f397..133f8be 100644 --- a/pathspec.c +++ b/pathspec.c @@ -99,3 +99,153 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix) die(_('%s' is beyond a symbolic link), path + len); } } + +/* + * Magic pathspec + * + * NEEDSWORK: These need to be moved to dir.h or even to a new + * pathspec.h when we restructure get_pathspec() users to use the + * struct pathspec interface. + * + * Possible future magic semantics include stuff like: + * + * { PATHSPEC_NOGLOB, '!', noglob }, + * { PATHSPEC_ICASE, '\0', icase }, + * { PATHSPEC_RECURSIVE, '*', recursive }, + * { PATHSPEC_REGEXP, '\0', regexp }, + * + */ +#define PATHSPEC_FROMTOP(10) + +static struct pathspec_magic { + unsigned bit; + char mnemonic; /* this cannot
[PATCH 01/45] clean: remove unused variable seen
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/clean.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 04e396b..f955a40 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -155,7 +155,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix) struct string_list exclude_list = STRING_LIST_INIT_NODUP; struct exclude_list *el; const char *qname; - char *seen = NULL; struct option options[] = { OPT__QUIET(quiet, N_(do not print names of files removed)), OPT__DRY_RUN(dry_run, N_(dry run)), @@ -214,9 +213,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix) fill_directory(dir, pathspec); - if (pathspec) - seen = xmalloc(argc 0 ? argc : 1); - for (i = 0; i dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; int len, pos; @@ -250,11 +246,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (lstat(ent-name, st)) continue; - if (pathspec) { - memset(seen, 0, argc 0 ? argc : 1); + if (pathspec) matches = match_pathspec(pathspec, ent-name, len, -0, seen); - } +0, NULL); if (S_ISDIR(st.st_mode)) { strbuf_addstr(directory, ent-name); @@ -281,7 +275,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix) } } } - free(seen); strbuf_release(directory); string_list_clear(exclude_list, 0); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/45] pathspec: add copy_pathspec
The function is made to use with free_pathspec() because a simple struct assignment is not enough (free_pathspec wants to free items pointer). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/mv.c | 13 +++-- pathspec.c | 8 pathspec.h | 1 + 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 034fec9..16ce99b 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -15,8 +15,9 @@ static const char * const builtin_mv_usage[] = { NULL }; -static const char **copy_pathspec(const char *prefix, const char **pathspec, - int count, int base_name) +static const char **internal_copy_pathspec(const char *prefix, + const char **pathspec, + int count, int base_name) { int i; const char **result = xmalloc((count + 1) * sizeof(const char *)); @@ -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(enum update_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); else if (!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]); diff --git a/pathspec.c b/pathspec.c index 403095b..8fe56cd 100644 --- a/pathspec.c +++ b/pathspec.c @@ -249,3 +249,11 @@ const char **get_pathspec(const char *prefix, const char **pathspec) return NULL; return pathspec; } + +void copy_pathspec(struct pathspec *dst, const struct pathspec *src) +{ + *dst = *src; + dst-items = xmalloc(sizeof(struct pathspec_item) * dst-nr); + memcpy(dst-items, src-items, + sizeof(struct pathspec_item) * dst-nr); +} diff --git a/pathspec.h b/pathspec.h index 7884068..a621676 100644 --- a/pathspec.h +++ b/pathspec.h @@ -18,6 +18,7 @@ struct pathspec { }; extern int init_pathspec(struct pathspec *, const char **); +extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src); extern void free_pathspec(struct pathspec *); extern int limit_pathspec_to_literal(void); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/45] parse_pathspec: save original pathspec for reporting
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 pclo...@gmail.com --- dir.c | 1 + pathspec.c | 2 ++ pathspec.h | 1 + 3 files changed, 4 insertions(+) diff --git a/dir.c b/dir.c index b8e68c2..c0b284f 100644 --- a/dir.c +++ b/dir.c @@ -1601,6 +1601,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths) const char *path = paths[i]; item-match = path; + item-original = path; item-len = strlen(path); item-flags = 0; if (limit_pathspec_to_literal()) { diff --git a/pathspec.c b/pathspec.c index b49bd51..89bdc7f 100644 --- a/pathspec.c +++ b/pathspec.c @@ -203,6 +203,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, else match = prefix_path(prefix, prefixlen, copyfrom); *raw = item-match = match; + item-original = elt; item-len = strlen(item-match); if (limit_pathspec_to_literal()) item-nowildcard_len = item-len; @@ -277,6 +278,7 @@ void parse_pathspec(struct pathspec *pathspec, pathspec-items = item = xmalloc(sizeof(*item)); memset(item, 0, sizeof(*item)); item-match = prefix; + item-original = prefix; item-nowildcard_len = item-len = strlen(prefix); raw[0] = prefix; raw[1] = NULL; diff --git a/pathspec.h b/pathspec.h index 937ec91..cc5841b 100644 --- a/pathspec.h +++ b/pathspec.h @@ -16,6 +16,7 @@ struct pathspec { int max_depth; struct pathspec_item { const char *match; + const char *original; unsigned magic; int len; int nowildcard_len; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/45] parse_pathspec: support stripping submodule trailing slashes
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 pclo...@gmail.com --- pathspec.c | 9 + pathspec.h | 2 ++ 2 files changed, 11 insertions(+) diff --git a/pathspec.c b/pathspec.c index a352ef1..e3c8339 100644 --- a/pathspec.c +++ b/pathspec.c @@ -205,6 +205,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item, *raw = item-match = match; item-original = elt; item-len = strlen(item-match); + + if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) + (item-len = 1 item-match[item-len - 1] == '/') + (i = cache_name_pos(item-match, item-len - 1)) = 0 + S_ISGITLINK(active_cache[i]-ce_mode)) { + item-len--; + match[item-len] = '\0'; + } + if (limit_pathspec_to_literal()) item-nowildcard_len = item-len; else diff --git a/pathspec.h b/pathspec.h index aa98597..7935b26 100644 --- a/pathspec.h +++ b/pathspec.h @@ -31,6 +31,8 @@ struct pathspec { #define PATHSPEC_PREFER_CWD (10) /* No args means match cwd */ #define PATHSPEC_PREFER_FULL (11) /* No args means match everything */ #define PATHSPEC_MAXDEPTH_VALID (12) /* max_depth field is valid */ +/* stripping the trailing slash if the given path is a gitlink */ +#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (13) extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/45] Add parse_pathspec() that converts cmdline args to struct pathspec
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 pclo...@gmail.com --- Documentation/technical/api-setup.txt | 19 +++- dir.c | 4 +- dir.h | 2 + pathspec.c| 168 ++ pathspec.h| 11 +++ 5 files changed, 163 insertions(+), 41 deletions(-) diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt index 4f63a04..90d1aff 100644 --- a/Documentation/technical/api-setup.txt +++ b/Documentation/technical/api-setup.txt @@ -8,6 +8,23 @@ Talk about * is_inside_git_dir() * is_inside_work_tree() * setup_work_tree() -* get_pathspec() (Dscho) + +Pathspec + + +See glossary-context.txt for the syntax of pathspec. In memory, a +pathspec set is represented by struct pathspec and is prepared by +parse_pathspec(). This function takes several arguments: + +- magic_mask specifies what features that are NOT supported by the + following code. If a user attempts to use such a feature, + parse_pathspec() can reject it early. + +- flags specifies other things that the caller wants parse_pathspec to + perform. + +- prefix and args come from cmd_* functions + +get_pathspec() is obsolete and should never be used in new code. diff --git a/dir.c b/dir.c index 9291c9a..b8e68c2 100644 --- a/dir.c +++ b/dir.c @@ -381,7 +381,7 @@ int match_pathspec_depth(const struct pathspec *ps, /* * Return the length of the simple part of a path match limiter. */ -static int simple_length(const char *match) +int simple_length(const char *match) { int len = -1; @@ -393,7 +393,7 @@ static int simple_length(const char *match) } } -static int no_wildcard(const char *string) +int no_wildcard(const char *string) { return string[simple_length(string)] == '\0'; } diff --git a/dir.h b/dir.h index 3d6b80c..229ccc8 100644 --- a/dir.h +++ b/dir.h @@ -128,6 +128,8 @@ struct dir_struct { #define MATCHED_RECURSIVELY 1 #define MATCHED_FNMATCH 2 #define MATCHED_EXACTLY 3 +extern int simple_length(const char *match); +extern int no_wildcard(const char *string); extern char *common_prefix(const char **pathspec); extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen); extern int match_pathspec_depth(const struct pathspec *pathspec, diff --git a/pathspec.c b/pathspec.c index 8fe56cd..b49bd51 100644 --- a/pathspec.c +++ b/pathspec.c @@ -103,10 +103,6 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix) /* * Magic pathspec * - * NEEDSWORK: These need to be moved to dir.h or even to a new - * pathspec.h when we restructure get_pathspec() users to use the - * struct pathspec interface. - * * Possible future magic semantics include stuff like: * * { PATHSPEC_NOGLOB, '!', noglob }, @@ -115,7 +111,6 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix) * { PATHSPEC_REGEXP, '\0', regexp }, * */ -#define PATHSPEC_FROMTOP(10) static struct pathspec_magic { unsigned bit; @@ -127,7 +122,7 @@ static struct pathspec_magic { /* * Take an element of a pathspec and check for magic signatures. - * Append the result to the prefix. + * Append the result to the prefix. Return the magic bitmap. * * For now, we only parse the syntax and throw out anything other than * top magic. @@ -138,10 +133,15 @@ static struct pathspec_magic { * the prefix part must always match literally, and a single stupid * string cannot express such a case. */ -static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) +static unsigned prefix_pathspec(struct pathspec_item *item, +
[PATCH 07/45] parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
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 #2. get_pathspec() and parse_pathspec() only support #1. Make parse_pathspec() reject empty pathspec by default. #1 and #2 can be specified via new flags. This makes it more expressive about default behavior at command level. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pathspec.c | 13 - pathspec.h | 4 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pathspec.c b/pathspec.c index 89bdc7f..9e68321 100644 --- a/pathspec.c +++ b/pathspec.c @@ -271,10 +271,20 @@ void parse_pathspec(struct pathspec *pathspec, if (!entry !prefix) return; + if ((flags PATHSPEC_PREFER_CWD) + (flags PATHSPEC_PREFER_FULL)) + die(BUG: PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are incompatible); + /* No arguments with prefix - prefix pathspec */ if (!entry) { static const char *raw[2]; + if (flags PATHSPEC_PREFER_FULL) + return; + + if (!(flags PATHSPEC_PREFER_CWD)) + die(BUG: parse_pathspec cannot take no arguments in this case); + pathspec-items = item = xmalloc(sizeof(*item)); memset(item, 0, sizeof(*item)); item-match = prefix; @@ -340,7 +350,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec) struct pathspec ps; parse_pathspec(ps, PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP, - 0, prefix, pathspec); + PATHSPEC_PREFER_CWD, + prefix, pathspec); return ps.raw; } diff --git a/pathspec.h b/pathspec.h index cc5841b..d630e8b 100644 --- a/pathspec.h +++ b/pathspec.h @@ -24,6 +24,10 @@ struct pathspec { } *items; }; +/* parse_pathspec flags */ +#define PATHSPEC_PREFER_CWD (10) /* No args means match cwd */ +#define PATHSPEC_PREFER_FULL (11) /* No args means match everything */ + extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, unsigned magic_mask, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/45] parse_pathspec: support stripping/checking submodule paths
PATHSPEC_SYMLINK_LEADING_PATH and _STRIP_SUBMODULE_SLASH_EXPENSIVE are respectively the alternate implementation of pathspec.c:die_if_path_beyond_symlink() and pathspec.c:check_path_for_gitlink(). They are intended to replace those functions when builtin/add.c and builtin/check-ignore.c are converted to use parse_pathspec. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pathspec.c | 26 ++ pathspec.h | 10 ++ 2 files changed, 36 insertions(+) diff --git a/pathspec.c b/pathspec.c index e3c8339..9aaec36 100644 --- a/pathspec.c +++ b/pathspec.c @@ -214,6 +214,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++) { + struct cache_entry *ce = active_cache[i]; + int ce_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); + } + if (limit_pathspec_to_literal()) item-nowildcard_len = item-len; else @@ -329,6 +349,12 @@ void parse_pathspec(struct pathspec *pathspec, unsupported_magic(entry, item[i].magic magic_mask, short_magic); + + if ((flags PATHSPEC_SYMLINK_LEADING_PATH) + has_symlink_leading_path(item[i].match, item[i].len)) { + die(_(pathspec '%s' is beyond a symbolic link), entry); + } + if (item[i].nowildcard_len item[i].len) pathspec-has_wildcard = 1; pathspec-magic |= item[i].magic; diff --git a/pathspec.h b/pathspec.h index 7935b26..b631514 100644 --- a/pathspec.h +++ b/pathspec.h @@ -33,6 +33,16 @@ struct pathspec { #define PATHSPEC_MAXDEPTH_VALID (12) /* max_depth field is valid */ /* stripping the trailing slash if the given path is a gitlink */ #define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (13) +/* die if a symlink is part of the given path's directory */ +#define PATHSPEC_SYMLINK_LEADING_PATH (14) +/* + * This is like a combination of ..LEADING_PATH and .._SLASH_CHEAP + * (but not the same): it strips the trailing slash if the given path + * is a gitlink but also checks and dies if gitlink is part of the + * leading path (i.e. the given path goes beyond a submodule). It's + * safer than _SLASH_CHEAP and also more expensive. + */ +#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (15) extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/45] Convert some get_pathspec() calls to parse_pathspec()
These call sites follow the pattern: paths = get_pathspec(prefix, argv); init_pathspec(pathspec, paths); which can be converted into a single parse_pathspec() call. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/grep.c | 6 +++--- builtin/ls-tree.c | 10 +- builtin/update-index.c | 5 +++-- revision.c | 4 ++-- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 4de49df..1a6c028 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -631,7 +631,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix) const char *show_in_pager = NULL, *default_pager = dummy; struct grep_opt opt; struct object_array list = OBJECT_ARRAY_INIT; - const char **paths = NULL; struct pathspec pathspec; struct string_list path_list = STRING_LIST_INIT_NODUP; int i; @@ -858,8 +857,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) verify_filename(prefix, argv[j], j == i); } - paths = get_pathspec(prefix, argv + i); - init_pathspec(pathspec, paths); + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, argv + i); pathspec.max_depth = opt.max_depth; pathspec.recursive = 1; diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 93fc3a0..bdb03f3 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -167,7 +167,15 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) if (get_sha1(argv[0], sha1)) die(Not a valid object name %s, argv[0]); - init_pathspec(pathspec, get_pathspec(prefix, argv + 1)); + /* +* show_recursive() rolls its own matching code and is +* generally ignorant of 'struct pathspec'. The magic mask +* cannot be lifted until it is converted to use +* match_pathspec_depth() or tree_entry_interesting() +*/ + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, argv + 1); for (i = 0; i pathspec.nr; i++) pathspec.items[i].nowildcard_len = pathspec.items[i].len; pathspec.has_wildcard = 0; diff --git a/builtin/update-index.c b/builtin/update-index.c index b9c2bd0..e795818 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -547,10 +547,11 @@ static int do_reupdate(int ac, const char **av, */ int pos; int has_head = 1; - const char **paths = get_pathspec(prefix, av + 1); struct pathspec pathspec; - init_pathspec(pathspec, paths); + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, av + 1); if (read_ref(HEAD, head_sha1)) /* If there is no HEAD, that means it is an initial diff --git a/revision.c b/revision.c index 518cd08..f9185d4 100644 --- a/revision.c +++ b/revision.c @@ -1871,8 +1871,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s */ ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc); prune_data.path[prune_data.nr++] = NULL; - init_pathspec(revs-prune_data, - get_pathspec(revs-prefix, prune_data.path)); + parse_pathspec(revs-prune_data, 0, 0, + revs-prefix, prune_data.path); } if (revs-def == NULL) -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/45] parse_pathspec: a special flag for max_depth feature
match_pathspec_depth() and tree_entry_interesting() check max_depth field in order to support git grep --max-depth. The feature activation is tied to recursive field, which led to some unwanted activation, e.g. 5c8eeb8 (diff-index: enable recursive pathspec matching in unpack_trees - 2012-01-15). This patch decouples the activation from recursive field, puts it in magic field instead. This makes sure that only git grep can activate this feature. And because parse_pathspec knows when the feature is not used, it does not need to sort pathspec (required for max_depth to work correctly). A small win for non-grep cases. Even though a new magic flag is introduced, no magic syntax is. The magic can be only enabled by parse_pathspec() caller. We might someday want to support :(maxdepth:10)src. It all depends on actual use cases. max_depth feature cannot be enabled via init_pathspec() anymore. But that's ok because init_pathspec() is on its way to /dev/null. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/grep.c | 3 ++- diff-lib.c | 1 - dir.c | 8 ++-- pathspec.c | 8 ++-- pathspec.h | 6 +- tree-diff.c| 1 - tree-walk.c| 8 ++-- 7 files changed, 25 insertions(+), 10 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 1a6c028..4bc0754 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -858,7 +858,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } parse_pathspec(pathspec, 0, - PATHSPEC_PREFER_CWD, + PATHSPEC_PREFER_CWD | + (opt.max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0), prefix, argv + i); pathspec.max_depth = opt.max_depth; pathspec.recursive = 1; diff --git a/diff-lib.c b/diff-lib.c index f35de0f..4729157 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -474,7 +474,6 @@ static int diff_cache(struct rev_info *revs, opts.dst_index = NULL; opts.pathspec = revs-diffopt.pathspec; opts.pathspec-recursive = 1; - opts.pathspec-max_depth = -1; init_tree_desc(t, tree-buffer, tree-size); return unpack_trees(1, t, opts); diff --git a/dir.c b/dir.c index c0b284f..cfcdda5 100644 --- a/dir.c +++ b/dir.c @@ -341,7 +341,9 @@ int match_pathspec_depth(const struct pathspec *ps, int i, retval = 0; if (!ps-nr) { - if (!ps-recursive || ps-max_depth == -1) + if (!ps-recursive || + !(ps-magic PATHSPEC_MAXDEPTH) || + ps-max_depth == -1) return MATCHED_RECURSIVELY; if (within_depth(name, namelen, 0, ps-max_depth)) @@ -358,7 +360,9 @@ int match_pathspec_depth(const struct pathspec *ps, if (seen seen[i] == MATCHED_EXACTLY) continue; how = match_pathspec_item(ps-items+i, prefix, name, namelen); - if (ps-recursive ps-max_depth != -1 + if (ps-recursive + (ps-magic PATHSPEC_MAXDEPTH) + ps-max_depth != -1 how how != MATCHED_FNMATCH) { int len = ps-items[i].len; if (name[len] == '/') diff --git a/pathspec.c b/pathspec.c index 9e68321..a352ef1 100644 --- a/pathspec.c +++ b/pathspec.c @@ -267,6 +267,9 @@ void parse_pathspec(struct pathspec *pathspec, memset(pathspec, 0, sizeof(*pathspec)); + if (flags PATHSPEC_MAXDEPTH_VALID) + pathspec-magic |= PATHSPEC_MAXDEPTH; + /* No arguments, no prefix - no pathspec */ if (!entry !prefix) return; @@ -322,8 +325,9 @@ void parse_pathspec(struct pathspec *pathspec, pathspec-magic |= item[i].magic; } - qsort(pathspec-items, pathspec-nr, - sizeof(struct pathspec_item), pathspec_item_cmp); + if (pathspec-magic PATHSPEC_MAXDEPTH) + qsort(pathspec-items, pathspec-nr, + sizeof(struct pathspec_item), pathspec_item_cmp); } /* diff --git a/pathspec.h b/pathspec.h index d630e8b..aa98597 100644 --- a/pathspec.h +++ b/pathspec.h @@ -3,7 +3,10 @@ /* Pathspec magic */ #define PATHSPEC_FROMTOP (10) -#define PATHSPEC_ALL_MAGIC PATHSPEC_FROMTOP +#define PATHSPEC_MAXDEPTH (11) +#define PATHSPEC_ALL_MAGIC \ + (PATHSPEC_FROMTOP | \ +PATHSPEC_MAXDEPTH) #define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR */ @@ -27,6 +30,7 @@ struct pathspec { /* parse_pathspec flags */ #define PATHSPEC_PREFER_CWD (10) /* No args means match cwd */ #define PATHSPEC_PREFER_FULL (11) /* No args means match everything */ +#define PATHSPEC_MAXDEPTH_VALID (12) /* max_depth field is valid */ extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, diff --git a/tree-diff.c b/tree-diff.c
[PATCH 15/45] commit: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/commit.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index d2f30d9..0efe269 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -279,17 +279,17 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, { int fd; struct string_list partial; - const char **pathspec = NULL; + struct pathspec pathspec; char *old_index_env = NULL; int refresh_flags = REFRESH_QUIET; if (is_status) refresh_flags |= REFRESH_UNMERGED; + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_FULL, + prefix, argv); - if (*argv) - pathspec = get_pathspec(prefix, argv); - - if (read_cache_preload(pathspec) 0) + if (read_cache_preload(pathspec.raw) 0) die(_(index file corrupt)); if (interactive) { @@ -331,9 +331,9 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, * (A) if all goes well, commit the real index; * (B) on failure, rollback the real index. */ - if (all || (also pathspec *pathspec)) { + if (all || (also pathspec.nr)) { fd = hold_locked_index(index_lock, 1); - add_files_to_cache(also ? prefix : NULL, pathspec, 0); + add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0); refresh_cache_or_die(refresh_flags); update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || @@ -352,7 +352,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, * and create commit from the_index. * We still need to refresh the index here. */ - if (!only (!pathspec || !*pathspec)) { + if (!only !pathspec.nr) { fd = hold_locked_index(index_lock, 1); refresh_cache_or_die(refresh_flags); if (active_cache_changed) { @@ -397,7 +397,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, memset(partial, 0, sizeof(partial)); partial.strdup_strings = 1; - if (list_paths(partial, !current_head ? NULL : HEAD, prefix, pathspec)) + if (list_paths(partial, !current_head ? NULL : HEAD, prefix, pathspec.raw)) exit(1); discard_cache(); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/45] status: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/commit.c | 9 + wt-status.c | 16 +++- wt-status.h | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 0efe269..833c7be 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1197,11 +1197,12 @@ int cmd_status(int argc, const char **argv, const char *prefix) handle_untracked_files_arg(s); if (show_ignored_in_status) s.show_ignored_files = 1; - if (*argv) - s.pathspec = get_pathspec(prefix, argv); + parse_pathspec(s.pathspec, 0, + PATHSPEC_PREFER_FULL, + prefix, argv); - read_cache_preload(s.pathspec); - refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL); + read_cache_preload(s.pathspec.raw); + refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL); fd = hold_locked_index(index_lock, 0); if (0 = fd) diff --git a/wt-status.c b/wt-status.c index bf84a86..99302e6 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1,4 +1,5 @@ #include cache.h +#include pathspec.h #include wt-status.h #include object.h #include dir.h @@ -437,7 +438,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s) } rev.diffopt.format_callback = wt_status_collect_changed_cb; rev.diffopt.format_callback_data = s; - init_pathspec(rev.prune_data, s-pathspec); + copy_pathspec(rev.prune_data, s-pathspec); run_diff_files(rev, 0); } @@ -462,22 +463,20 @@ static void wt_status_collect_changes_index(struct wt_status *s) rev.diffopt.detect_rename = 1; rev.diffopt.rename_limit = 200; rev.diffopt.break_opt = 0; - init_pathspec(rev.prune_data, s-pathspec); + copy_pathspec(rev.prune_data, s-pathspec); run_diff_index(rev, 1); } static void wt_status_collect_changes_initial(struct wt_status *s) { - struct pathspec pathspec; int i; - init_pathspec(pathspec, s-pathspec); for (i = 0; i active_nr; i++) { struct string_list_item *it; struct wt_status_change_data *d; struct cache_entry *ce = active_cache[i]; - if (!ce_path_match(ce, pathspec)) + if (!ce_path_match(ce, s-pathspec)) continue; it = string_list_insert(s-change, ce-name); d = it-util; @@ -492,7 +491,6 @@ static void wt_status_collect_changes_initial(struct wt_status *s) else d-index_status = DIFF_STATUS_ADDED; } - free_pathspec(pathspec); } static void wt_status_collect_untracked(struct wt_status *s) @@ -515,12 +513,12 @@ static void wt_status_collect_untracked(struct wt_status *s) dir.flags |= DIR_SHOW_IGNORED_TOO; setup_standard_excludes(dir); - fill_directory(dir, s-pathspec); + fill_directory(dir, s-pathspec.raw); for (i = 0; i dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; if (cache_name_is_other(ent-name, ent-len) - match_pathspec(s-pathspec, ent-name, ent-len, 0, NULL)) + match_pathspec_depth(s-pathspec, ent-name, ent-len, 0, NULL)) string_list_insert(s-untracked, ent-name); free(ent); } @@ -528,7 +526,7 @@ static void wt_status_collect_untracked(struct wt_status *s) for (i = 0; i dir.ignored_nr; i++) { struct dir_entry *ent = dir.ignored[i]; if (cache_name_is_other(ent-name, ent-len) - match_pathspec(s-pathspec, ent-name, ent-len, 0, NULL)) + match_pathspec_depth(s-pathspec, ent-name, ent-len, 0, NULL)) string_list_insert(s-ignored, ent-name); free(ent); } diff --git a/wt-status.h b/wt-status.h index 4121bc2..8463672 100644 --- a/wt-status.h +++ b/wt-status.h @@ -44,7 +44,7 @@ struct wt_status { int is_initial; char *branch; const char *reference; - const char **pathspec; + struct pathspec pathspec; int verbose; int amend; enum commit_whence whence; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/45] rm: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/rm.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin/rm.c b/builtin/rm.c index 7b91d52..0b38de3 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -10,6 +10,7 @@ #include tree-walk.h #include parse-options.h #include submodule.h +#include pathspec.h static const char * const builtin_rm_usage[] = { N_(git rm [options] [--] file...), @@ -216,7 +217,7 @@ static struct option builtin_rm_options[] = { int cmd_rm(int argc, const char **argv, const char *prefix) { int i, newfd; - const char **pathspec; + struct pathspec pathspec; char *seen; git_config(git_default_config, NULL); @@ -249,31 +250,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, 0, PATHSPEC_PREFER_CWD, 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++) { struct cache_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) { - const char *match; + if (pathspec.nr) { + const char *original; int seen_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 +281,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) -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/45] rerere: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/rerere.c | 8 +--- rerere.c | 9 + rerere.h | 4 +++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/builtin/rerere.c b/builtin/rerere.c index dc1708e..4e51add 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -6,6 +6,7 @@ #include rerere.h #include xdiff/xdiff.h #include xdiff-interface.h +#include pathspec.h static const char * const rerere_usage[] = { N_(git rerere [clear | forget path... | status | remaining | diff | gc]), @@ -68,11 +69,12 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) return rerere(flags); if (!strcmp(argv[0], forget)) { - const char **pathspec; + struct pathspec pathspec; if (argc 2) warning('git rerere forget' without paths is deprecated); - pathspec = get_pathspec(prefix, argv + 1); - return rerere_forget(pathspec); + parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, + prefix, argv + 1); + return rerere_forget(pathspec); } fd = setup_rerere(merge_rr, flags); diff --git a/rerere.c b/rerere.c index 98e3e29..27afbfe 100644 --- a/rerere.c +++ b/rerere.c @@ -6,6 +6,7 @@ #include resolve-undo.h #include ll-merge.h #include attr.h +#include pathspec.h #define RESOLVED 0 #define PUNTED 1 @@ -656,7 +657,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr) return 0; } -int rerere_forget(const char **pathspec) +int rerere_forget(struct pathspec *pathspec) { int i, fd; struct string_list conflict = STRING_LIST_INIT_DUP; @@ -667,12 +668,12 @@ int rerere_forget(const char **pathspec) fd = setup_rerere(merge_rr, RERERE_NOAUTOUPDATE); - unmerge_cache(pathspec); + unmerge_cache(pathspec-raw); find_conflict(conflict); for (i = 0; i conflict.nr; i++) { struct string_list_item *it = conflict.items[i]; - if (!match_pathspec(pathspec, it-string, strlen(it-string), - 0, NULL)) + if (!match_pathspec_depth(pathspec, it-string, strlen(it-string), + 0, NULL)) continue; rerere_forget_one_path(it-string, merge_rr); } diff --git a/rerere.h b/rerere.h index 156d2aa..4aa06c9 100644 --- a/rerere.h +++ b/rerere.h @@ -3,6 +3,8 @@ #include string-list.h +struct pathspec; + #define RERERE_AUTOUPDATE 01 #define RERERE_NOAUTOUPDATE 02 @@ -16,7 +18,7 @@ extern void *RERERE_RESOLVED; extern int setup_rerere(struct string_list *, int); extern int rerere(int); extern const char *rerere_path(const char *hex, const char *file); -extern int rerere_forget(const char **); +extern int rerere_forget(struct pathspec *); extern int rerere_remaining(struct string_list *); extern void rerere_clear(struct string_list *); extern void rerere_gc(struct string_list *); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/45] parse_pathspec: support prefixing original patterns
This makes 'original' suitable for passing to an external command because all pathspec magic is left in place, provided that the external command understands pathspec. The prefixing is needed because we usually launch a subcommand at worktree's top directory and the subcommand can no longer calculate the prefix itself. This slightly affects the original purpose of 'original' (i.e. reporting). We should report without prefixing. So only turn this flag on when you know you are about to pass the result straight away to an external command. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pathspec.c | 12 +++- pathspec.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pathspec.c b/pathspec.c index 9aaec36..ba0a41d 100644 --- a/pathspec.c +++ b/pathspec.c @@ -203,7 +203,17 @@ static unsigned prefix_pathspec(struct pathspec_item *item, else match = prefix_path(prefix, prefixlen, copyfrom); *raw = item-match = match; - item-original = elt; + /* +* Prefix the pathspec (keep all magic) and put to +* original. Useful for passing to another command. +*/ + if (flags PATHSPEC_PREFIX_ORIGIN) { + struct strbuf sb = STRBUF_INIT; + strbuf_add(sb, elt, copyfrom - elt); + strbuf_addstr(sb, match); + item-original = strbuf_detach(sb, NULL); + } else + item-original = elt; item-len = strlen(item-match); if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) diff --git a/pathspec.h b/pathspec.h index b631514..3ca6636 100644 --- a/pathspec.h +++ b/pathspec.h @@ -43,6 +43,7 @@ struct pathspec { * safer than _SLASH_CHEAP and also more expensive. */ #define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (15) +#define PATHSPEC_PREFIX_ORIGIN (16) extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/45] checkout: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 34 +- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index f5b50e5..197198b 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -46,7 +46,7 @@ struct checkout_opts { int branch_exists; const char *prefix; - const char **pathspec; + struct pathspec pathspec; struct tree *source_tree; }; @@ -257,20 +257,18 @@ static int checkout_paths(const struct checkout_opts *opts, if (opts-patch_mode) return run_add_interactive(revision, --patch=checkout, - opts-pathspec); + opts-pathspec.raw); lock_file = xcalloc(1, sizeof(struct lock_file)); newfd = hold_locked_index(lock_file, 1); - if (read_cache_preload(opts-pathspec) 0) + if (read_cache_preload(opts-pathspec.raw) 0) return error(_(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); /* * Make sure all pathspecs participated in locating the paths @@ -304,12 +302,12 @@ static int checkout_paths(const struct checkout_opts *opts, * match_pathspec() for _all_ entries when * opts-source_tree != NULL. */ - if (match_pathspec(opts-pathspec, ce-name, ce_namelen(ce), + if (match_pathspec_depth(opts-pathspec, ce-name, ce_namelen(ce), 0, ps_matched)) ce-ce_flags |= CE_MATCHED; } - if (report_path_error(ps_matched, opts-pathspec, opts-prefix)) { + if (report_path_error(ps_matched, opts-pathspec.raw, opts-prefix)) { free(ps_matched); return 1; } @@ -994,7 +992,7 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts) static int checkout_branch(struct checkout_opts *opts, struct branch_info *new) { - if (opts-pathspec) + if (opts-pathspec.nr) die(_(paths cannot be used with switching branches)); if (opts-patch_mode) @@ -1146,9 +1144,19 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) } if (argc) { - opts.pathspec = get_pathspec(prefix, argv); + /* +* In patch mode (opts.patch_mode != 0), we pass the +* pathspec to an external program, git-add--interactive. +* Do not accept any kind of magic that that program +* cannot handle. Magic mask is pretty safe to be +* lifted for new magic when opts.patch_mode == 0. +*/ + parse_pathspec(opts.pathspec, + opts.patch_mode == 0 ? 0 : + (PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP), + 0, prefix, argv); - if (!opts.pathspec) + if (!opts.pathspec.nr) die(_(invalid path specification)); /* @@ -1180,7 +1188,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) strbuf_release(buf); } - if (opts.patch_mode || opts.pathspec) + if (opts.patch_mode || opts.pathspec.nr) return checkout_paths(opts, new.name); else return checkout_branch(opts, new); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/45] clean: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/clean.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index f955a40..fdd4980 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -13,6 +13,7 @@ #include refs.h #include string-list.h #include quote.h +#include pathspec.h static int force = -1; /* unset */ @@ -150,7 +151,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT; struct strbuf directory = STRBUF_INIT; struct dir_struct dir; - static const char **pathspec; + struct pathspec pathspec; struct strbuf buf = STRBUF_INIT; struct string_list exclude_list = STRING_LIST_INIT_NODUP; struct exclude_list *el; @@ -209,9 +210,11 @@ int cmd_clean(int argc, const char **argv, const char *prefix) for (i = 0; i exclude_list.nr; i++) add_exclude(exclude_list.items[i].string, , 0, el, -(i+1)); - pathspec = get_pathspec(prefix, argv); + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_CWD, + prefix, argv); - fill_directory(dir, pathspec); + fill_directory(dir, pathspec.raw); for (i = 0; i dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; @@ -246,9 +249,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (lstat(ent-name, st)) continue; - if (pathspec) - matches = match_pathspec(pathspec, ent-name, len, -0, NULL); + if (pathspec.nr) + matches = match_pathspec_depth(pathspec, ent-name, + len, 0, NULL); if (S_ISDIR(st.st_mode)) { strbuf_addstr(directory, ent-name); @@ -262,7 +265,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) } strbuf_reset(directory); } else { - if (pathspec !matches) + if (pathspec.nr !matches) continue; res = dry_run ? 0 : unlink(ent-name); if (res) { -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/45] Guard against new pathspec magic in pathspec matching code
GUARD_PATHSPEC() marks pathspec-sensitive code, basically all those that touch anything in 'struct pathspec' except fields nr and original. GUARD_PATHSPEC() is not supposed to fail. It's mainly to help the designers to catch unsupported codepaths. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/api-setup.txt | 19 +++ builtin/diff.c| 2 ++ dir.c | 2 ++ pathspec.h| 7 +++ tree-diff.c | 19 +++ tree-walk.c | 2 ++ 6 files changed, 51 insertions(+) diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt index 90d1aff..540e455 100644 --- a/Documentation/technical/api-setup.txt +++ b/Documentation/technical/api-setup.txt @@ -28,3 +28,22 @@ parse_pathspec(). This function takes several arguments: - prefix and args come from cmd_* functions get_pathspec() is obsolete and should never be used in new code. + +parse_pathspec() helps catch unsupported features and reject them +politely. At a lower level, different pathspec-related functions may +not support the same set of features. Such pathspec-sensitive +functions are guarded with GUARD_PATHSPEC(), which will die in an +unfriendly way when an unsupported feature is requested. + +The command designers are supposed to make sure that GUARD_PATHSPEC() +never dies. They have to make sure all unsupported features are caught +by parse_pathspec(), not by GUARD_PATHSPEC. grepping GUARD_PATHSPEC() +should give the designers all pathspec-sensitive codepaths and what +features they support. + +A similar process is applied when a new pathspec magic is added. The +designer lifts the GUARD_PATHSPEC restriction in the functions that +support the new magic. At the same time (s)he has to make sure this +new feature will be caught at parse_pathspec() in commands that cannot +handle the new magic in some cases. grepping parse_pathspec() should +help. diff --git a/builtin/diff.c b/builtin/diff.c index 8c2af6c..d237e0a 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -371,6 +371,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) die(_(unhandled object '%s' given.), name); } if (rev.prune_data.nr) { + /* builtin_diff_b_f() */ + GUARD_PATHSPEC(rev.prune_data, PATHSPEC_FROMTOP); if (!path) path = rev.prune_data.items[0].match; paths += rev.prune_data.nr; diff --git a/dir.c b/dir.c index cfcdda5..fcc0b97 100644 --- a/dir.c +++ b/dir.c @@ -340,6 +340,8 @@ int match_pathspec_depth(const struct pathspec *ps, { int i, retval = 0; + GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + if (!ps-nr) { if (!ps-recursive || !(ps-magic PATHSPEC_MAXDEPTH) || diff --git a/pathspec.h b/pathspec.h index 3ca6636..7068f7d 100644 --- a/pathspec.h +++ b/pathspec.h @@ -27,6 +27,13 @@ struct pathspec { } *items; }; +#define GUARD_PATHSPEC(ps, mask) \ + do { \ + if ((ps)-magic ~(mask)) \ + die(BUG:%s:%d: unsupported magic %x, \ + __FILE__, __LINE__, (ps)-magic ~(mask)); \ + } while (0) + /* parse_pathspec flags */ #define PATHSPEC_PREFER_CWD (10) /* No args means match cwd */ #define PATHSPEC_PREFER_FULL (11) /* No args means match everything */ diff --git a/tree-diff.c b/tree-diff.c index 826512e..5a87614 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -198,6 +198,25 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co const char *paths[1]; int i; + /* +* follow-rename code is very specific, we need exactly one +* path. Magic that matches more than one path is not +* supported. +*/ + GUARD_PATHSPEC(opt-pathspec, PATHSPEC_FROMTOP); +#if 0 + /* +* We should reject wildcards as well. Unfortunately we +* haven't got a reliable way to detect that 'foo\*bar' in +* fact has no wildcards. nowildcard_len is merely a hint for +* optimization. Let it slip for now until wildmatch is taught +* about dry-run mode and returns wildcard info. +*/ + 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; diff --git a/tree-walk.c b/tree-walk.c index d399ca9..37b157e 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -636,6 +636,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry, enum interesting never_interesting = ps-has_wildcard ? entry_not_interesting :
[PATCH 25/45] line-log: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- line-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/line-log.c b/line-log.c index 4bbb09b..843a334 100644 --- a/line-log.c +++ b/line-log.c @@ -747,7 +747,7 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list r = r-next; } paths[count] = NULL; - init_pathspec(rev-diffopt.pathspec, paths); + parse_pathspec(rev-diffopt.pathspec, 0, 0, , paths); free(paths); } } -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/45] ls-files: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/ls-files.c | 46 +- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index a0b7e30..1534a39 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -31,7 +31,7 @@ static int debug_mode; static const char *prefix; static int max_prefix_len; static int prefix_len; -static const char **pathspec; +static struct pathspec pathspec; static int error_unmatch; static char *ps_matched; static const char *with_tree; @@ -60,7 +60,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent) if (len = ent-len) die(git ls-files: internal error - directory entry not superset of prefix); - if (!match_pathspec(pathspec, ent-name, ent-len, len, ps_matched)) + if (!match_pathspec_depth(pathspec, ent-name, ent-len, len, ps_matched)) return; fputs(tag, stdout); @@ -135,7 +135,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce) if (len = ce_namelen(ce)) die(git ls-files: internal error - cache entry not superset of prefix); - if (!match_pathspec(pathspec, ce-name, ce_namelen(ce), len, ps_matched)) + if (!match_pathspec_depth(pathspec, ce-name, ce_namelen(ce), len, ps_matched)) return; if (tag *tag show_valid_bit @@ -189,7 +189,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]) @@ -214,7 +214,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) @@ -282,21 +282,6 @@ static void prune_cache(const char *prefix) active_nr = last; } -static void strip_trailing_slash_from_submodules(void) -{ - const char **p; - - for (p = pathspec; *p != NULL; p++) { - int len = strlen(*p), pos; - - if (len 1 || (*p)[len - 1] != '/') - continue; - pos = cache_name_pos(*p, len - 1); - if (pos = 0 S_ISGITLINK(active_cache[pos]-ce_mode)) - *p = xstrndup(*p, len - 1); - } -} - /* * Read the tree specified with --with-tree option * (typically, HEAD) into stage #1 and then @@ -550,23 +535,18 @@ 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, 0, + PATHSPEC_PREFER_CWD | + 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) { - int num; - 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); @@ -593,7 +573,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (ps_matched) { int bad; - 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); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 24/45] reset: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/reset.c | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 6032131..da7282e 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -171,7 +171,10 @@ static void die_if_unmerged_cache(int reset_type) } -static const char **parse_args(const char **argv, const char *prefix, const char **rev_ret) +static void parse_args(struct pathspec *pathspec, + const char **argv, const char *prefix, + int patch_mode, + const char **rev_ret) { const char *rev = HEAD; unsigned char unused[20]; @@ -213,7 +216,10 @@ static const char **parse_args(const char **argv, const char *prefix, const char } } *rev_ret = rev; - return argv[0] ? get_pathspec(prefix, argv) : NULL; + parse_pathspec(pathspec, + patch_mode ? PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP : 0, + PATHSPEC_PREFER_FULL, + prefix, argv); } static int update_refs(const char *rev, const unsigned char *sha1) @@ -243,7 +249,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) int patch_mode = 0, unborn; const char *rev; unsigned char sha1[20]; - const char **pathspec = NULL; + struct pathspec pathspec; const struct option options[] = { OPT__QUIET(quiet, N_(be quiet, only report errors)), OPT_SET_INT(0, mixed, reset_type, @@ -263,13 +269,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_reset_usage, PARSE_OPT_KEEP_DASHDASH); - pathspec = parse_args(argv, prefix, rev); + parse_args(pathspec, argv, prefix, patch_mode, rev); unborn = !strcmp(rev, HEAD) get_sha1(HEAD, sha1); if (unborn) { /* reset on unborn branch: treat as reset to empty tree */ hashcpy(sha1, EMPTY_TREE_SHA1_BIN); - } else if (!pathspec) { + } else if (!pathspec.nr) { struct commit *commit; if (get_sha1_committish(rev, sha1)) die(_(Failed to resolve '%s' as a valid revision.), rev); @@ -290,13 +296,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (patch_mode) { if (reset_type != NONE) die(_(--patch is incompatible with --{hard,mixed,soft})); - return run_add_interactive(sha1_to_hex(sha1), --patch=reset, pathspec); + return run_add_interactive(sha1_to_hex(sha1), --patch=reset, pathspec.raw); } /* git reset tree [--] paths... can be used to * load chosen paths from the tree into the index without * affecting the working tree nor HEAD. */ - if (pathspec) { + if (pathspec.nr) { if (reset_type == MIXED) warning(_(--mixed with paths is deprecated; use 'git reset -- paths' instead.)); else if (reset_type != NONE) @@ -323,7 +329,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int newfd = hold_locked_index(lock, 1); if (reset_type == MIXED) { - if (read_from_tree(pathspec, sha1)) + if (read_from_tree(pathspec.raw, sha1)) return 1; } else { int err = reset_index(sha1, reset_type, quiet); @@ -344,7 +350,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_(Could not write new index file.)); } - if (!pathspec !unborn) { + if (!pathspec.nr !unborn) { /* Any resets without paths update HEAD to the head being * switched to, saving the previous head in ORIG_HEAD before. */ update_ref_status = update_refs(rev, sha1); @@ -352,7 +358,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type == HARD !update_ref_status !quiet) print_new_head_line(lookup_commit_reference(sha1)); } - if (!pathspec) + if (!pathspec.nr) remove_branch_state(); return update_ref_status; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/45] Convert read_cache_preload() to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 2 +- builtin/commit.c | 4 ++-- builtin/diff-files.c | 2 +- builtin/diff-index.c | 2 +- builtin/diff.c | 4 ++-- cache.h | 2 +- preload-index.c | 20 +++- 7 files changed, 19 insertions(+), 17 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 197198b..d7a65e4 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -262,7 +262,7 @@ static int checkout_paths(const struct checkout_opts *opts, lock_file = xcalloc(1, sizeof(struct lock_file)); newfd = hold_locked_index(lock_file, 1); - if (read_cache_preload(opts-pathspec.raw) 0) + if (read_cache_preload(opts-pathspec) 0) return error(_(corrupt index file)); if (opts-source_tree) diff --git a/builtin/commit.c b/builtin/commit.c index 833c7be..171de9f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -289,7 +289,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, PATHSPEC_PREFER_FULL, prefix, argv); - if (read_cache_preload(pathspec.raw) 0) + if (read_cache_preload(pathspec) 0) die(_(index file corrupt)); if (interactive) { @@ -1201,7 +1201,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) PATHSPEC_PREFER_FULL, prefix, argv); - read_cache_preload(s.pathspec.raw); + read_cache_preload(s.pathspec); refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL); fd = hold_locked_index(index_lock, 0); diff --git a/builtin/diff-files.c b/builtin/diff-files.c index 46085f8..9200069 100644 --- a/builtin/diff-files.c +++ b/builtin/diff-files.c @@ -61,7 +61,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix) (rev.diffopt.output_format DIFF_FORMAT_PATCH)) rev.combine_merges = rev.dense_combined_merges = 1; - if (read_cache_preload(rev.diffopt.pathspec.raw) 0) { + if (read_cache_preload(rev.diffopt.pathspec) 0) { perror(read_cache_preload); return -1; } diff --git a/builtin/diff-index.c b/builtin/diff-index.c index 1c737f7..ce15b23 100644 --- a/builtin/diff-index.c +++ b/builtin/diff-index.c @@ -43,7 +43,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix) usage(diff_cache_usage); if (!cached) { setup_work_tree(); - if (read_cache_preload(rev.diffopt.pathspec.raw) 0) { + if (read_cache_preload(rev.diffopt.pathspec) 0) { perror(read_cache_preload); return -1; } diff --git a/builtin/diff.c b/builtin/diff.c index d237e0a..6b4e3f9 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -140,7 +140,7 @@ static int builtin_diff_index(struct rev_info *revs, usage(builtin_diff_usage); if (!cached) { setup_work_tree(); - if (read_cache_preload(revs-diffopt.pathspec.raw) 0) { + if (read_cache_preload(revs-diffopt.pathspec) 0) { perror(read_cache_preload); return -1; } @@ -240,7 +240,7 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv revs-combine_merges = revs-dense_combined_merges = 1; setup_work_tree(); - if (read_cache_preload(revs-diffopt.pathspec.raw) 0) { + if (read_cache_preload(revs-diffopt.pathspec) 0) { perror(read_cache_preload); return -1; } diff --git a/cache.h b/cache.h index 5917375..19e0d87 100644 --- a/cache.h +++ b/cache.h @@ -443,7 +443,7 @@ extern int init_db(const char *template_dir, unsigned int flags); /* Initialize and use the cache information */ extern int read_index(struct index_state *); -extern int read_index_preload(struct index_state *, const char **pathspec); +extern int read_index_preload(struct index_state *, const struct pathspec *pathspec); extern int read_index_from(struct index_state *, const char *path); extern int is_index_unborn(struct index_state *); extern int read_index_unmerged(struct index_state *); diff --git a/preload-index.c b/preload-index.c index cddfffa..8c44ceb 100644 --- a/preload-index.c +++ b/preload-index.c @@ -5,7 +5,8 @@ #include pathspec.h #ifdef NO_PTHREADS -static void preload_index(struct index_state *index, const char **pathspec) +static void preload_index(struct index_state *index, + const struct pathspec *pathspec) { ; /* nothing */ } @@ -25,7 +26,7 @@ static void preload_index(struct index_state *index, const char **pathspec) struct thread_data { pthread_t pthread; struct index_state *index; -
[PATCH 22/45] check-ignore: convert to use parse_pathspec
check-ignore (at least the test suite) seems to rely on the pattern order. PATHSPEC_KEEP_ORDER is introduced to explictly express this. The lack of PATHSPEC_MAXDEPTH_VALID is sufficient because it's the only flag that reorders pathspecs, but it's less obvious that way. Cc: Adam Spiers g...@adamspiers.org Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/check-ignore.c | 35 ++- pathspec.c | 6 +- pathspec.h | 1 + t/t0008-ignores.sh | 8 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c index 4a8fc70..d49c083 100644 --- a/builtin/check-ignore.c +++ b/builtin/check-ignore.c @@ -64,37 +64,45 @@ static void output_exclude(const char *path, struct exclude *exclude) } static int check_ignore(struct dir_struct *dir, - const char *prefix, const char **pathspec) + const char *prefix, int argc, const char **argv) { - const char *path, *full_path; + const char *full_path; char *seen; int num_ignored = 0, dtype = DT_UNKNOWN, i; struct exclude *exclude; + struct pathspec pathspec; - if (!pathspec || !*pathspec) { + if (!argc) { if (!quiet) fprintf(stderr, no pathspec given.\n); return 0; } /* +* check-ignore just needs paths. Magic beyond :/ is really +* irrelevant. +*/ + parse_pathspec(pathspec, + PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP, + PATHSPEC_SYMLINK_LEADING_PATH | + PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE | + PATHSPEC_KEEP_ORDER, + prefix, argv); + + /* * look for pathspecs matching entries in the index, since these * should not be ignored, in order to be consistent with * 'git status', 'git add' etc. */ - seen = find_pathspecs_matching_against_index(pathspec); - for (i = 0; pathspec[i]; i++) { - path = pathspec[i]; - full_path = prefix_path(prefix, prefix - ? strlen(prefix) : 0, path); - full_path = check_path_for_gitlink(full_path); - die_if_path_beyond_symlink(full_path, prefix); + seen = find_pathspecs_matching_against_index(pathspec.raw); + for (i = 0; i pathspec.nr; i++) { + full_path = pathspec.raw[i]; exclude = NULL; if (!seen[i]) { exclude = last_exclude_matching(dir, full_path, dtype); } if (!quiet (exclude || show_non_matching)) - output_exclude(path, exclude); + output_exclude(pathspec.items[i].original, exclude); if (exclude) num_ignored++; } @@ -120,7 +128,8 @@ static int check_ignore_stdin_paths(struct dir_struct *dir, const char *prefix) strbuf_swap(buf, nbuf); } pathspec[0] = buf.buf; - num_ignored += check_ignore(dir, prefix, (const char **)pathspec); + num_ignored += check_ignore(dir, prefix, + 1, (const char **)pathspec); maybe_flush_or_die(stdout, check-ignore to stdout); } strbuf_release(buf); @@ -166,7 +175,7 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix) if (stdin_paths) { num_ignored = check_ignore_stdin_paths(dir, prefix); } else { - num_ignored = check_ignore(dir, prefix, argv); + num_ignored = check_ignore(dir, prefix, argc, argv); maybe_flush_or_die(stdout, ignore to stdout); } diff --git a/pathspec.c b/pathspec.c index ba0a41d..152f9b5 100644 --- a/pathspec.c +++ b/pathspec.c @@ -370,9 +370,13 @@ void parse_pathspec(struct pathspec *pathspec, pathspec-magic |= item[i].magic; } - if (pathspec-magic PATHSPEC_MAXDEPTH) + + if (pathspec-magic PATHSPEC_MAXDEPTH) { + if (flags PATHSPEC_KEEP_ORDER) + die(BUG: PATHSPEC_MAXDEPTH_VALID and PATHSPEC_KEEP_ORDER are incompatible); qsort(pathspec-items, pathspec-nr, sizeof(struct pathspec_item), pathspec_item_cmp); + } } /* diff --git a/pathspec.h b/pathspec.h index 7068f7d..4f144fd 100644 --- a/pathspec.h +++ b/pathspec.h @@ -51,6 +51,7 @@ struct pathspec { */ #define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (15) #define PATHSPEC_PREFIX_ORIGIN (16) +#define PATHSPEC_KEEP_ORDER (17) extern int init_pathspec(struct pathspec *, const char **); extern void parse_pathspec(struct pathspec *pathspec, diff --git a/t/t0008-ignores.sh
[PATCH 29/45] checkout: convert read_tree_some to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 9 +++-- tree.c | 4 ++-- tree.h | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 0e93af0..be08061 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -83,12 +83,9 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen, return 0; } -static int read_tree_some(struct tree *tree, const char **pathspec) +static int read_tree_some(struct tree *tree, const struct pathspec *pathspec) { - struct pathspec ps; - 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 * for all args, expanding wildcards, and exit @@ -266,7 +263,7 @@ static int checkout_paths(const struct checkout_opts *opts, return error(_(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); diff --git a/tree.c b/tree.c index 62fed63..ff72f67 100644 --- a/tree.c +++ b/tree.c @@ -47,7 +47,7 @@ static int read_one_entry_quick(const unsigned char *sha1, const char *base, int } static int read_tree_1(struct tree *tree, struct strbuf *base, - int stage, struct pathspec *pathspec, + int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context) { struct tree_desc desc; @@ -116,7 +116,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base, int read_tree_recursive(struct tree *tree, const char *base, int baselen, - int stage, struct pathspec *pathspec, + int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context) { struct strbuf sb = STRBUF_INIT; diff --git a/tree.h b/tree.h index 69bcb5e..9dc90ba 100644 --- a/tree.h +++ b/tree.h @@ -25,7 +25,7 @@ typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const ch extern int read_tree_recursive(struct tree *tree, const char *base, int baselen, - int stage, struct pathspec *pathspec, + int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context); extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 23/45] add: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c | 103 +- pathspec.c| 43 2 files changed, 45 insertions(+), 101 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index f45d9d4..9a7235e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -226,21 +226,6 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, return seen; } -/* - * Checks the index to see whether any path in pathspec refers to - * something inside a submodule. If so, dies with an error message. - */ -static void treat_gitlinks(const char **pathspec) -{ - int i; - - if (!pathspec || !*pathspec) - return; - - for (i = 0; pathspec[i]; i++) - pathspec[i] = check_path_for_gitlink(pathspec[i]); -} - static void refresh(int verbose, const char **pathspec) { char *seen; @@ -258,25 +243,6 @@ static void refresh(int verbose, const char **pathspec) free(seen); } -/* - * Normalizes argv relative to prefix, via get_pathspec(), and then - * runs die_if_path_beyond_symlink() on each path in the normalized - * list. - */ -static const char **validate_pathspec(const char **argv, const char *prefix) -{ - const char **pathspec = get_pathspec(prefix, argv); - - if (pathspec) { - const char **p; - for (p = pathspec; *p; p++) { - die_if_path_beyond_symlink(*p, prefix); - } - } - - return pathspec; -} - int run_add_interactive(const char *revision, const char *patch_mode, const char **pathspec) { @@ -308,17 +274,23 @@ int run_add_interactive(const char *revision, const char *patch_mode, int interactive_add(int argc, const char **argv, const char *prefix, int patch) { - const char **pathspec = NULL; + struct pathspec pathspec; - if (argc) { - pathspec = validate_pathspec(argv, prefix); - if (!pathspec) - return -1; - } + /* +* git-add--interactive itself does not parse pathspec. It +* simply passes the pathspec to other builtin commands. Let's +* hope all of them support all magic, or we'll need to limit +* the magic here. There is still a problem with prefix. But +* that'll be worked on later on. +*/ + parse_pathspec(pathspec, PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP, + PATHSPEC_PREFER_FULL | + PATHSPEC_SYMLINK_LEADING_PATH, + prefix, argv); return run_add_interactive(NULL, patch ? --patch : NULL, - pathspec); + pathspec.raw); } static int edit_patch(int argc, const char **argv, const char *prefix) @@ -445,7 +417,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) { int exit_status = 0; int newfd; - const char **pathspec; + struct pathspec pathspec; struct dir_struct dir; int flags; int add_new_files; @@ -526,11 +498,19 @@ int cmd_add(int argc, const char **argv, const char *prefix) fprintf(stderr, _(Maybe you wanted to say 'git add .'?\n)); return 0; } - pathspec = validate_pathspec(argv, prefix); if (read_cache() 0) die(_(index file corrupt)); - treat_gitlinks(pathspec); + + /* +* Check the pathspec '%s' did not match any files block +* below before enabling new magic. +*/ + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_FULL | + PATHSPEC_SYMLINK_LEADING_PATH | + PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE, + prefix, argv); if (add_new_files) { int baselen; @@ -543,34 +523,40 @@ int cmd_add(int argc, const char **argv, const char *prefix) } /* This picks up the paths that are not tracked */ - baselen = fill_directory(dir, implicit_dot ? NULL : pathspec); - if (pathspec) - seen = prune_directory(dir, pathspec, baselen, + baselen = fill_directory(dir, implicit_dot ? NULL : pathspec.raw); + if (pathspec.nr) + seen = prune_directory(dir, pathspec.raw, baselen, implicit_dot ? WARN_IMPLICIT_DOT : 0); } if (refresh_only) { - refresh(verbose, pathspec); + refresh(verbose, pathspec.raw); goto finish; } if (implicit_dot prefix) refresh_cache(REFRESH_QUIET); - if (pathspec) { + if (pathspec.nr) { int i; if (!seen) -
[PATCH 30/45] Convert report_path_error to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 2 +- builtin/commit.c | 14 ++ builtin/ls-files.c | 19 +++ cache.h| 2 +- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index be08061..f543533 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -304,7 +304,7 @@ static int checkout_paths(const struct checkout_opts *opts, ce-ce_flags |= CE_MATCHED; } - if (report_path_error(ps_matched, opts-pathspec.raw, opts-prefix)) { + if (report_path_error(ps_matched, opts-pathspec, opts-prefix)) { free(ps_matched); return 1; } diff --git a/builtin/commit.c b/builtin/commit.c index 171de9f..e5a3bb5 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -183,20 +183,18 @@ static int commit_index_files(void) * and return the paths that match the given pattern in list. */ static int list_paths(struct string_list *list, const char *with_tree, - const char *prefix, const char **pattern) + const char *prefix, const struct pathspec *pattern) { int i; char *m; - if (!pattern) + if (!pattern-nr) return 0; - for (i = 0; pattern[i]; i++) - ; - m = xcalloc(1, i); + m = xcalloc(1, pattern-nr); if (with_tree) { - char *max_prefix = common_prefix(pattern); + char *max_prefix = common_prefix(pattern-raw); overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : prefix); free(max_prefix); } @@ -207,7 +205,7 @@ static int list_paths(struct string_list *list, const char *with_tree, if (ce-ce_flags CE_UPDATE) continue; - if (!match_pathspec(pattern, ce-name, ce_namelen(ce), 0, m)) + if (!match_pathspec_depth(pattern, ce-name, ce_namelen(ce), 0, m)) continue; item = string_list_insert(list, ce-name); if (ce_skip_worktree(ce)) @@ -397,7 +395,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, memset(partial, 0, sizeof(partial)); partial.strdup_strings = 1; - if (list_paths(partial, !current_head ? NULL : HEAD, prefix, pathspec.raw)) + if (list_paths(partial, !current_head ? NULL : HEAD, prefix, pathspec)) exit(1); discard_cache(); diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 1534a39..f6db498 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -344,15 +344,16 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix) } } -int report_path_error(const char *ps_matched, const char **pathspec, const char *prefix) +int report_path_error(const char *ps_matched, + const struct pathspec *pathspec, + const char *prefix) { /* * Make sure all pathspec matched; otherwise it is an error. */ struct strbuf sb = STRBUF_INIT; - const char *name; int num, errors = 0; - for (num = 0; pathspec[num]; num++) { + for (num = 0; num pathspec-nr; num++) { int other, found_dup; if (ps_matched[num]) @@ -360,13 +361,16 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char /* * The caller might have fed identical pathspec * twice. Do not barf on such a mistake. +* FIXME: parse_pathspec should have eliminated +* duplicate pathspec. */ for (found_dup = other = 0; -!found_dup pathspec[other]; +!found_dup other pathspec-nr; other++) { if (other == num || !ps_matched[other]) continue; - if (!strcmp(pathspec[other], pathspec[num])) + if (!strcmp(pathspec-items[other].original, + pathspec-items[num].original)) /* * Ok, we have a match already. */ @@ -375,9 +379,8 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char if (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); @@ -573,7 +576,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (ps_matched) {
[PATCH 21/45] archive: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- archive.c | 19 +++ archive.h | 4 +++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/archive.c b/archive.c index c699a2d..99fadc8 100644 --- a/archive.c +++ b/archive.c @@ -5,7 +5,6 @@ #include archive.h #include parse-options.h #include unpack-trees.h -#include pathspec.h static char const * const archive_usage[] = { N_(git archive [options] tree-ish [path...]), @@ -152,7 +151,6 @@ int write_archive_entries(struct archiver_args *args, struct archiver_context context; struct unpack_trees_options opts; struct tree_desc t; - struct pathspec pathspec; int err; if (args-baselen 0 args-base[args-baselen - 1] == '/') { @@ -187,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; return err; @@ -223,7 +219,7 @@ static int path_exists(struct tree *tree, const char *path) struct pathspec pathspec; int ret; - init_pathspec(pathspec, paths); + parse_pathspec(pathspec, 0, 0, , paths); ret = read_tree_recursive(tree, , 0, 0, pathspec, reject_entry, NULL); free_pathspec(pathspec); return ret != 0; @@ -232,11 +228,18 @@ static int path_exists(struct tree *tree, const char *path) static void parse_pathspec_arg(const char **pathspec, struct archiver_args *ar_args) { - ar_args-pathspec = pathspec = get_pathspec(, pathspec); + /* +* must be consistent with parse_pathspec in path_exists() +* Also if pathspec patterns are dependent, we're in big +* trouble as we test each one separately +*/ + parse_pathspec(ar_args-pathspec, 0, + PATHSPEC_PREFER_FULL, + , pathspec); if (pathspec) { while (*pathspec) { if (**pathspec !path_exists(ar_args-tree, *pathspec)) - die(path not found: %s, *pathspec); + die(_(pathspec '%s' did not match any files), *pathspec); pathspec++; } } diff --git a/archive.h b/archive.h index 895afcd..4a791e1 100644 --- a/archive.h +++ b/archive.h @@ -1,6 +1,8 @@ #ifndef ARCHIVE_H #define ARCHIVE_H +#include pathspec.h + struct archiver_args { const char *base; size_t baselen; @@ -8,7 +10,7 @@ struct archiver_args { const unsigned char *commit_sha1; const struct commit *commit; time_t time; - const char **pathspec; + struct pathspec pathspec; unsigned int verbose : 1; unsigned int worktree_attributes : 1; unsigned int convert : 1; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 28/45] Convert unmerge_cache to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- rerere.c | 2 +- resolve-undo.c | 4 ++-- resolve-undo.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rerere.c b/rerere.c index 27afbfe..4105bca 100644 --- a/rerere.c +++ b/rerere.c @@ -668,7 +668,7 @@ int rerere_forget(struct pathspec *pathspec) fd = setup_rerere(merge_rr, RERERE_NOAUTOUPDATE); - unmerge_cache(pathspec-raw); + unmerge_cache(pathspec); find_conflict(conflict); for (i = 0; i conflict.nr; i++) { struct string_list_item *it = conflict.items[i]; diff --git a/resolve-undo.c b/resolve-undo.c index 639eb9c..4b78e6f 100644 --- a/resolve-undo.c +++ b/resolve-undo.c @@ -173,7 +173,7 @@ void unmerge_marked_index(struct index_state *istate) } } -void unmerge_index(struct index_state *istate, const char **pathspec) +void unmerge_index(struct index_state *istate, const struct pathspec *pathspec) { int i; @@ -182,7 +182,7 @@ void unmerge_index(struct index_state *istate, const char **pathspec) for (i = 0; i istate-cache_nr; i++) { struct cache_entry *ce = istate-cache[i]; - if (!match_pathspec(pathspec, ce-name, ce_namelen(ce), 0, NULL)) + if (!match_pathspec_depth(pathspec, ce-name, ce_namelen(ce), 0, NULL)) continue; i = unmerge_index_entry_at(istate, i); } diff --git a/resolve-undo.h b/resolve-undo.h index 7a30206..4630645 100644 --- a/resolve-undo.h +++ b/resolve-undo.h @@ -11,7 +11,7 @@ extern void resolve_undo_write(struct strbuf *, struct string_list *); extern struct string_list *resolve_undo_read(const char *, unsigned long); extern void resolve_undo_clear_index(struct index_state *); extern int unmerge_index_entry_at(struct index_state *, int); -extern void unmerge_index(struct index_state *, const char **); +extern void unmerge_index(struct index_state *, const struct pathspec *); extern void unmerge_marked_index(struct index_state *); #endif -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 35/45] Remove diff_tree_{setup,release}_paths
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/blame.c | 12 ++-- builtin/reset.c | 9 + diff.h | 2 -- notes-merge.c | 4 ++-- revision.c | 5 +++-- tree-diff.c | 18 -- 6 files changed, 20 insertions(+), 30 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 079dcd3..5bd721d 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -408,7 +408,7 @@ static struct origin *find_origin(struct scoreboard *sb, paths[0] = origin-path; paths[1] = NULL; - diff_tree_setup_paths(paths, diff_opts); + init_pathspec(diff_opts.pathspec, paths); diff_setup_done(diff_opts); if (is_null_sha1(origin-commit-object.sha1)) @@ -458,7 +458,7 @@ static struct origin *find_origin(struct scoreboard *sb, } } diff_flush(diff_opts); - diff_tree_release_paths(diff_opts); + free_pathspec(diff_opts.pathspec); if (porigin) { /* * Create a freestanding copy that is not part of @@ -494,7 +494,7 @@ static struct origin *find_rename(struct scoreboard *sb, diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_opts.single_follow = origin-path; paths[0] = NULL; - diff_tree_setup_paths(paths, diff_opts); + init_pathspec(diff_opts.pathspec, paths); diff_setup_done(diff_opts); if (is_null_sha1(origin-commit-object.sha1)) @@ -516,7 +516,7 @@ static struct origin *find_rename(struct scoreboard *sb, } } diff_flush(diff_opts); - diff_tree_release_paths(diff_opts); + free_pathspec(diff_opts.pathspec); return porigin; } @@ -1079,7 +1079,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; @@ -1162,7 +1162,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); return retval; } diff --git a/builtin/reset.c b/builtin/reset.c index 1c8e4a5..b665218 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -130,12 +130,13 @@ static void update_index_from_diff(struct diff_queue_struct *q, } } -static int read_from_tree(const char **pathspec, unsigned char *tree_sha1) +static int read_from_tree(const struct pathspec *pathspec, + unsigned char *tree_sha1) { struct diff_options opt; memset(opt, 0, sizeof(opt)); - diff_tree_setup_paths(pathspec, opt); + copy_pathspec(opt.pathspec, pathspec); opt.output_format = DIFF_FORMAT_CALLBACK; opt.format_callback = update_index_from_diff; @@ -144,7 +145,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1) return 1; diffcore_std(opt); diff_flush(opt); - diff_tree_release_paths(opt); + free_pathspec(opt.pathspec); return 0; } @@ -329,7 +330,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int newfd = hold_locked_index(lock, 1); if (reset_type == MIXED) { - if (read_from_tree(pathspec.raw, sha1)) + if (read_from_tree(pathspec, sha1)) return 1; } else { int err = reset_index(sha1, reset_type, quiet); diff --git a/diff.h b/diff.h index d1bc914..b8df245 100644 --- a/diff.h +++ b/diff.h @@ -180,8 +180,6 @@ const char *diff_line_prefix(struct diff_options *); extern const char mime_boundary_leader[]; -extern void diff_tree_setup_paths(const char **paths, struct diff_options *); -extern void diff_tree_release_paths(struct diff_options *); extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt); extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new, diff --git a/notes-merge.c b/notes-merge.c index 0f67bd3..f1a3b39 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -169,7 +169,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o, sha1_to_hex(mp-remote)); } diff_flush(opt); - diff_tree_release_paths(opt); + free_pathspec(opt.pathspec); *num_changes = len; return changes; @@ -255,7 +255,7 @@ static void diff_tree_local(struct notes_merge_options *o, sha1_to_hex(mp-local)); } diff_flush(opt); -
[PATCH 33/45] Convert add_files_to_cache to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c| 11 +++ builtin/commit.c | 2 +- cache.h | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 34c9358..a47aeb4 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -166,14 +166,16 @@ static void update_callback(struct diff_queue_struct *q, } } -static void update_files_in_cache(const char *prefix, const char **pathspec, +static void update_files_in_cache(const char *prefix, + const struct pathspec *pathspec, struct update_callback_data *data) { struct rev_info rev; init_revisions(rev, prefix); setup_revisions(0, NULL, rev, NULL); - init_pathspec(rev.prune_data, pathspec); + if (pathspec) + copy_pathspec(rev.prune_data, pathspec); rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; rev.diffopt.format_callback = update_callback; rev.diffopt.format_callback_data = data; @@ -181,7 +183,8 @@ static void update_files_in_cache(const char *prefix, const char **pathspec, run_diff_files(rev, DIFF_RACY_IS_MODIFIED); } -int add_files_to_cache(const char *prefix, const char **pathspec, int flags) +int add_files_to_cache(const char *prefix, + const struct pathspec *pathspec, int flags) { struct update_callback_data data; @@ -571,7 +574,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) memset(pathspec, 0, sizeof(pathspec)); } update_data.flags = flags ~ADD_CACHE_IMPLICIT_DOT; - update_files_in_cache(prefix, pathspec.raw, update_data); + update_files_in_cache(prefix, pathspec, update_data); exit_status |= !!update_data.add_errors; if (add_new_files) diff --git a/builtin/commit.c b/builtin/commit.c index 56bc7be..2ddff4e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -331,7 +331,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, */ if (all || (also pathspec.nr)) { fd = hold_locked_index(index_lock, 1); - add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0); + add_files_to_cache(also ? prefix : NULL, pathspec, 0); refresh_cache_or_die(refresh_flags); update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || diff --git a/cache.h b/cache.h index 25410b9..b3d8b29 100644 --- a/cache.h +++ b/cache.h @@ -1260,7 +1260,7 @@ void packet_trace_identity(const char *prog); * return 0 if success, 1 - if addition of a file failed and * ADD_FILES_IGNORE_ERRORS was specified in flags */ -int add_files_to_cache(const char *prefix, const char **pathspec, int flags); +int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags); /* diff.c */ extern int diff_auto_refresh_index; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 27/45] Convert run_add_interactive to use struct pathspec
This passes the pathspec, more or less unmodified, to git-add--interactive. The command itself does not process pathspec. It simply passes the pathspec to other builtin commands. So if all those commands support pathspec, we're good. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c | 26 ++ builtin/checkout.c | 9 - builtin/reset.c| 8 commit.h | 2 +- 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 9a7235e..d039fc9 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -244,16 +244,12 @@ static void refresh(int verbose, const char **pathspec) } int run_add_interactive(const char *revision, const char *patch_mode, - const char **pathspec) + const struct pathspec *pathspec) { - int status, ac, pc = 0; + int status, ac, i; const char **args; - if (pathspec) - while (pathspec[pc]) - pc++; - - args = xcalloc(sizeof(const char *), (pc + 5)); + args = xcalloc(sizeof(const char *), (pathspec-nr + 6)); ac = 0; args[ac++] = add--interactive; if (patch_mode) @@ -261,11 +257,9 @@ int run_add_interactive(const char *revision, const char *patch_mode, if (revision) args[ac++] = revision; args[ac++] = --; - if (pc) { - memcpy((args[ac]), pathspec, sizeof(const char *) * pc); - ac += pc; - } - args[ac] = NULL; + for (i = 0; i pathspec-nr; i++) + /* pass original pathspec, to be re-parsed */ + args[ac++] = pathspec-items[i].original; status = run_command_v_opt(args, RUN_GIT_CMD); free(args); @@ -280,17 +274,17 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch) * git-add--interactive itself does not parse pathspec. It * simply passes the pathspec to other builtin commands. Let's * hope all of them support all magic, or we'll need to limit -* the magic here. There is still a problem with prefix. But -* that'll be worked on later on. +* the magic here. */ parse_pathspec(pathspec, PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP, PATHSPEC_PREFER_FULL | - PATHSPEC_SYMLINK_LEADING_PATH, + PATHSPEC_SYMLINK_LEADING_PATH | + PATHSPEC_PREFIX_ORIGIN, prefix, argv); return run_add_interactive(NULL, patch ? --patch : NULL, - pathspec.raw); + pathspec); } static int edit_patch(int argc, const char **argv, const char *prefix) diff --git a/builtin/checkout.c b/builtin/checkout.c index d7a65e4..0e93af0 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -257,7 +257,7 @@ static int checkout_paths(const struct checkout_opts *opts, if (opts-patch_mode) return run_add_interactive(revision, --patch=checkout, - opts-pathspec.raw); + opts-pathspec); lock_file = xcalloc(1, sizeof(struct lock_file)); @@ -1151,10 +1151,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) * cannot handle. Magic mask is pretty safe to be * lifted for new magic when opts.patch_mode == 0. */ - parse_pathspec(opts.pathspec, - opts.patch_mode == 0 ? 0 : - (PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP), - 0, prefix, argv); + parse_pathspec(opts.pathspec, 0, + opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0, + prefix, argv); if (!opts.pathspec.nr) die(_(invalid path specification)); diff --git a/builtin/reset.c b/builtin/reset.c index da7282e..1c8e4a5 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -216,9 +216,9 @@ static void parse_args(struct pathspec *pathspec, } } *rev_ret = rev; - parse_pathspec(pathspec, - patch_mode ? PATHSPEC_ALL_MAGIC ~PATHSPEC_FROMTOP : 0, - PATHSPEC_PREFER_FULL, + parse_pathspec(pathspec, 0, + PATHSPEC_PREFER_FULL | + (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0), prefix, argv); } @@ -296,7 +296,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (patch_mode) { if (reset_type != NONE) die(_(--patch is incompatible with --{hard,mixed,soft})); - return run_add_interactive(sha1_to_hex(sha1),
[PATCH 34/45] Convert common_prefix() to use struct pathspec
The code now takes advantage of nowildcard_len field. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/commit.c | 2 +- builtin/ls-files.c | 2 +- dir.c | 31 +++ dir.h | 2 +- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 2ddff4e..5b7adb0 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -194,7 +194,7 @@ static int list_paths(struct string_list *list, const char *with_tree, m = xcalloc(1, pattern-nr); if (with_tree) { - char *max_prefix = common_prefix(pattern-raw); + char *max_prefix = common_prefix(pattern); overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : prefix); free(max_prefix); } diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 7feedba..67feda6 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -544,7 +544,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 */ diff --git a/dir.c b/dir.c index 863ee39..9d20691 100644 --- a/dir.c +++ b/dir.c @@ -103,26 +103,25 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen, return match_status; } -static size_t common_prefix_len(const char **pathspec) +static size_t common_prefix_len(const struct pathspec *pathspec) { - const char *n, *first; + int n; size_t max = 0; - int literal = limit_pathspec_to_literal(); - if (!pathspec) - return max; - - first = *pathspec; - while ((n = *pathspec++)) { - size_t i, len = 0; - for (i = 0; first == n || i max; i++) { - char c = n[i]; - if (!c || c != first[i] || (!literal is_glob_special(c))) + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + + for (n = 0; n pathspec-nr; n++) { + size_t i = 0, len = 0; + while (i pathspec-items[n].nowildcard_len + (n == 0 || i max)) { + char c = pathspec-items[n].match[i]; + if (c != pathspec-items[0].match[i]) break; if (c == '/') len = i + 1; + i++; } - if (first == n || len max) { + if (n == 0 || len max) { max = len; if (!max) break; @@ -135,11 +134,11 @@ static size_t common_prefix_len(const char **pathspec) * Returns a copy of the longest leading path common among all * pathspecs. */ -char *common_prefix(const char **pathspec) +char *common_prefix(const struct pathspec *pathspec) { unsigned long len = common_prefix_len(pathspec); - return len ? xmemdupz(*pathspec, len) : NULL; + return len ? xmemdupz(pathspec-items[0].match, len) : NULL; } int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec) @@ -150,7 +149,7 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec) * Calculate common prefix for the pathspec, and * use that to optimize the directory walk */ - 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); diff --git a/dir.h b/dir.h index 076dd96..ba40e69 100644 --- a/dir.h +++ b/dir.h @@ -130,7 +130,7 @@ struct dir_struct { #define MATCHED_EXACTLY 3 extern int simple_length(const char *match); extern int no_wildcard(const char *string); -extern char *common_prefix(const char **pathspec); +extern char *common_prefix(const struct pathspec *pathspec); extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen); extern int match_pathspec_depth(const struct pathspec *pathspec, const char *name, int namelen, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 37/45] Remove match_pathspec() in favor of match_pathspec_depth()
match_pathspec_depth was created to replace match_pathspec (see 61cf282 (pathspec: add match_pathspec_depth() - 2010-12-15). It took more than two years, but the replacement finally happens :-) Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c | 30 +++--- builtin/check-ignore.c | 4 +- dir.c | 107 - dir.h | 1 - pathspec.c | 19 - pathspec.h | 4 +- 6 files changed, 25 insertions(+), 140 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index a47aeb4..0b80fa8 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -195,23 +195,21 @@ int add_files_to_cache(const char *prefix, } #define WARN_IMPLICIT_DOT (1u 0) -static char *prune_directory(struct dir_struct *dir, const char **pathspec, +static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, int prefix, unsigned flag) { char *seen; - int i, specs; + int i; struct dir_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) { struct dir_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; else if (flag WARN_IMPLICIT_DOT) /* @@ -225,7 +223,7 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, warn_pathless_add(); } dir-nr = dst - dir-entries; - add_pathspec_matches_against_index(pathspec, seen, specs); + add_pathspec_matches_against_index(pathspec, seen); return seen; } @@ -523,7 +521,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, implicit_dot ? empty_pathspec : pathspec); if (pathspec.nr) - seen = prune_directory(dir, pathspec.raw, baselen, + seen = prune_directory(dir, pathspec, baselen, implicit_dot ? WARN_IMPLICIT_DOT : 0); } @@ -538,23 +536,23 @@ int cmd_add(int argc, const char **argv, const char *prefix) int i; if (!seen) - seen = find_pathspecs_matching_against_index(pathspec.raw); + seen = find_pathspecs_matching_against_index(pathspec); /* * file_exists() assumes exact match */ 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++) { + const char *path = pathspec.items[i].match; + if (!seen[i] !file_exists(path)) { if (ignore_missing) { int dtype = DT_UNKNOWN; - if (is_excluded(dir, pathspec.raw[i], dtype)) - dir_add_ignored(dir, pathspec.raw[i], strlen(pathspec.raw[i])); + if (is_excluded(dir, path, dtype)) + dir_add_ignored(dir, path, pathspec.items[i].len); } else die(_(pathspec '%s' did not match any files), - pathspec.raw[i]); + pathspec.items[i].original); } } free(seen); diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c index d49c083..70537c8 100644 --- a/builtin/check-ignore.c +++ b/builtin/check-ignore.c @@ -94,9 +94,9 @@ static int check_ignore(struct dir_struct *dir, * should not be ignored, in order to be consistent with * 'git status', 'git add' etc. */ - seen = find_pathspecs_matching_against_index(pathspec.raw); + seen = find_pathspecs_matching_against_index(pathspec); for (i = 0; i pathspec.nr; i++) { - full_path = pathspec.raw[i]; + full_path = pathspec.items[i].match; exclude = NULL; if (!seen[i]) { exclude = last_exclude_matching(dir, full_path, dtype); diff --git
[PATCH 36/45] Remove init_pathspec() in favor of parse_pathspec()
While at there, move free_pathspec() to pathspec.c Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/blame.c| 8 +--- builtin/log.c | 2 +- builtin/ls-files.c | 11 +-- diff-lib.c | 2 +- dir.c | 58 -- merge-recursive.c | 2 +- pathspec.c | 6 ++ pathspec.h | 1 - revision.c | 2 +- tree-diff.c| 10 +- 10 files changed, 21 insertions(+), 81 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 5bd721d..56e3d6b 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -408,7 +408,7 @@ static struct origin *find_origin(struct scoreboard *sb, paths[0] = origin-path; paths[1] = NULL; - init_pathspec(diff_opts.pathspec, paths); + parse_pathspec(diff_opts.pathspec, PATHSPEC_ALL_MAGIC, 0, , paths); diff_setup_done(diff_opts); if (is_null_sha1(origin-commit-object.sha1)) @@ -486,15 +486,12 @@ static struct origin *find_rename(struct scoreboard *sb, struct origin *porigin = NULL; struct diff_options diff_opts; int i; - const char *paths[2]; diff_setup(diff_opts); DIFF_OPT_SET(diff_opts, RECURSIVE); diff_opts.detect_rename = DIFF_DETECT_RENAME; diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_opts.single_follow = origin-path; - paths[0] = NULL; - init_pathspec(diff_opts.pathspec, paths); diff_setup_done(diff_opts); if (is_null_sha1(origin-commit-object.sha1)) @@ -1064,7 +1061,6 @@ static int find_copy_in_parent(struct scoreboard *sb, int opt) { struct diff_options diff_opts; - const char *paths[1]; int i, j; int retval; struct blame_list *blame_list; @@ -1078,8 +1074,6 @@ static int find_copy_in_parent(struct scoreboard *sb, DIFF_OPT_SET(diff_opts, RECURSIVE); diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; - paths[0] = NULL; - init_pathspec(diff_opts.pathspec, paths); diff_setup_done(diff_opts); /* Try find copies harder on new path if requested; diff --git a/builtin/log.c b/builtin/log.c index 9e21232..3798955 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -503,7 +503,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) init_grep_defaults(); git_config(git_log_config, NULL); - init_pathspec(match_all, NULL); + memset(match_all, 0, sizeof(match_all)); init_revisions(rev, prefix); rev.diff = 1; rev.always_show_header = 1; diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 67feda6..7dbd23d 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -313,13 +313,12 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix) } if (prefix) { - static const char *(matchbuf[2]); - matchbuf[0] = prefix; - matchbuf[1] = NULL; - init_pathspec(pathspec, matchbuf); - pathspec.items[0].nowildcard_len = pathspec.items[0].len; + static const char *(matchbuf[1]); + matchbuf[0] = NULL; + parse_pathspec(pathspec, PATHSPEC_ALL_MAGIC, + PATHSPEC_PREFER_CWD, prefix, matchbuf); } else - init_pathspec(pathspec, NULL); + memset(pathspec, 0, sizeof(pathspec)); if (read_tree(tree, 1, pathspec)) die(unable to read tree entries %s, tree_name); diff --git a/diff-lib.c b/diff-lib.c index 4729157..68fc991 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -499,7 +499,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt) struct rev_info revs; init_revisions(revs, NULL); - init_pathspec(revs.prune_data, opt-pathspec.raw); + copy_pathspec(revs.prune_data, opt-pathspec); revs.diffopt = *opt; if (diff_cache(revs, tree_sha1, NULL, 1)) diff --git a/dir.c b/dir.c index 9d20691..269ff85 100644 --- a/dir.c +++ b/dir.c @@ -1582,64 +1582,6 @@ int remove_path(const char *name) return 0; } -static int pathspec_item_cmp(const void *a_, const void *b_) -{ - struct pathspec_item *a, *b; - - a = (struct pathspec_item *)a_; - b = (struct pathspec_item *)b_; - return strcmp(a-match, b-match); -} - -int init_pathspec(struct pathspec *pathspec, const char **paths) -{ - const char **p = paths; - int i; - - memset(pathspec, 0, sizeof(*pathspec)); - if (!p) - return 0; - while (*p) - p++; - pathspec-raw = paths; - pathspec-nr = p - paths; - if (!pathspec-nr) - return 0; - - pathspec-items = xmalloc(sizeof(struct pathspec_item)*pathspec-nr); - for (i = 0; i pathspec-nr; i++) { - struct pathspec_item *item
[PATCH 31/45] Convert refresh_index to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c| 15 +++ builtin/commit.c | 2 +- builtin/rm.c | 2 +- cache.h | 2 +- read-cache.c | 5 +++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index d039fc9..f5d6a33 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -226,19 +226,18 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, return seen; } -static void refresh(int verbose, const char **pathspec) +static void refresh(int verbose, const struct pathspec *pathspec) { char *seen; - int i, specs; + int i; - 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); } @@ -524,7 +523,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) } if (refresh_only) { - refresh(verbose, pathspec.raw); + refresh(verbose, pathspec); goto finish; } if (implicit_dot prefix) diff --git a/builtin/commit.c b/builtin/commit.c index e5a3bb5..56bc7be 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1200,7 +1200,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) prefix, argv); read_cache_preload(s.pathspec); - refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL); + refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL); fd = hold_locked_index(index_lock, 0); if (0 = fd) diff --git a/builtin/rm.c b/builtin/rm.c index 0b38de3..9afc1c5 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -251,7 +251,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix) } parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv); - refresh_index(the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL); + refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL); seen = NULL; seen = xcalloc(pathspec.nr, 1); diff --git a/cache.h b/cache.h index f978e88..25410b9 100644 --- a/cache.h +++ b/cache.h @@ -499,7 +499,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st); #define REFRESH_IGNORE_MISSING 0x0008 /* ignore non-existent */ #define REFRESH_IGNORE_SUBMODULES 0x0010 /* ignore submodules */ #define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not needs update */ -extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen, const char *header_msg); +extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg); struct lock_file { struct lock_file *next; diff --git a/read-cache.c b/read-cache.c index 04ed561..27bbc35 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1097,7 +1097,8 @@ static void show_file(const char * fmt, const char * name, int in_porcelain, printf(fmt, name); } -int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec, +int refresh_index(struct index_state *istate, unsigned int flags, + const struct pathspec *pathspec, char *seen, const char *header_msg) { int i; @@ -1132,7 +1133,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p continue; if (pathspec - !match_pathspec(pathspec, ce-name, ce_namelen(ce), 0, seen)) + !match_pathspec_depth(pathspec, ce-name, ce_namelen(ce), 0, seen)) filtered = 1; if (ce_stage(ce)) { -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 32/45] Convert {read,fill}_directory to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/add.c | 4 +++- builtin/clean.c| 2 +- builtin/grep.c | 2 +- builtin/ls-files.c | 2 +- dir.c | 16 +++- dir.h | 4 ++-- wt-status.c| 2 +- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index f5d6a33..34c9358 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -507,6 +507,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (add_new_files) { int baselen; + struct pathspec empty_pathspec; /* Set up the default git porcelain excludes */ memset(dir, 0, sizeof(dir)); @@ -515,8 +516,9 @@ int cmd_add(int argc, const char **argv, const char *prefix) setup_standard_excludes(dir); } + memset(empty_pathspec, 0, sizeof(empty_pathspec)); /* This picks up the paths that are not tracked */ - baselen = fill_directory(dir, implicit_dot ? NULL : pathspec.raw); + baselen = fill_directory(dir, implicit_dot ? empty_pathspec : pathspec); if (pathspec.nr) seen = prune_directory(dir, pathspec.raw, baselen, implicit_dot ? WARN_IMPLICIT_DOT : 0); diff --git a/builtin/clean.c b/builtin/clean.c index fdd4980..d540ca4 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -214,7 +214,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) PATHSPEC_PREFER_CWD, prefix, argv); - fill_directory(dir, pathspec.raw); + fill_directory(dir, pathspec); for (i = 0; i dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; diff --git a/builtin/grep.c b/builtin/grep.c index 4bc0754..76a6a60 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -523,7 +523,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec, if (exc_std) setup_standard_excludes(dir); - fill_directory(dir, pathspec-raw); + fill_directory(dir, pathspec); for (i = 0; i dir.nr; i++) { const char *name = dir.entries[i]-name; int namelen = strlen(name); diff --git a/builtin/ls-files.c b/builtin/ls-files.c index f6db498..7feedba 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -214,7 +214,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) diff --git a/dir.c b/dir.c index fcc0b97..863ee39 100644 --- a/dir.c +++ b/dir.c @@ -142,7 +142,7 @@ char *common_prefix(const char **pathspec) return len ? xmemdupz(*pathspec, len) : NULL; } -int fill_directory(struct dir_struct *dir, const char **pathspec) +int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec) { size_t len; @@ -150,10 +150,10 @@ int fill_directory(struct dir_struct *dir, const char **pathspec) * Calculate common prefix for the pathspec, and * use that to optimize the directory walk */ - 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); return len; } @@ -1390,14 +1390,20 @@ static int treat_leading_path(struct dir_struct *dir, return rc; } -int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec) +int read_directory(struct dir_struct *dir, const char *path, int len, const struct pathspec *pathspec) { struct path_simplify *simplify; + /* +* Check out create_simplify() +*/ + if (pathspec) + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + if (has_symlink_leading_path(path, len)) return dir-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); diff --git a/dir.h b/dir.h index 229ccc8..076dd96 100644 --- a/dir.h +++ b/dir.h @@ -137,8 +137,8 @@ extern int match_pathspec_depth(const struct pathspec *pathspec, int prefix, char *seen); extern int within_depth(const char *name, int namelen, int depth, int max_depth); -extern int
[PATCH 39/45] parse_pathspec: make sure the prefix part is wildcard-free
Prepending prefix to pathspec is a trick to workaround the fact that commands can be executed in a subdirectory, but all git commands run at worktree's root. The prefix part should always be treated as literal string. Make it so. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- cache.h| 2 ++ path.c | 15 ++- pathspec.c | 21 + pathspec.h | 2 +- setup.c| 24 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/cache.h b/cache.h index b3d8b29..276330e 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ extern void setup_work_tree(void); extern const char *setup_git_directory_gently(int *); extern const char *setup_git_directory(void); extern char *prefix_path(const char *prefix, int len, const char *path); +extern char *prefix_path_gently(const char *prefix, int len, int *remaining, const char *path); extern const char *prefix_filename(const char *prefix, int len, const char *path); extern int check_filename(const char *prefix, const char *name); extern void verify_filename(const char *prefix, @@ -720,6 +721,7 @@ const char *real_path(const char *path); const char *real_path_if_valid(const char *path); const char *absolute_path(const char *path); const char *relative_path(const char *abs, const char *base); +int normalize_path_copy_len(char *dst, const char *src, int *prefix_len); int normalize_path_copy(char *dst, const char *src); int longest_ancestor_length(const char *path, struct string_list *prefixes); char *strip_path_suffix(const char *path, const char *suffix); diff --git a/path.c b/path.c index 04ff148..f4b49d6 100644 --- a/path.c +++ b/path.c @@ -492,8 +492,14 @@ const char *relative_path(const char *abs, const char *base) * * Note that this function is purely textual. It does not follow symlinks, * verify the existence of the path, or make any system calls. + * + * prefix_len != NULL is for a specific case of prefix_pathspec(): + * assume that src == dst and src[0..prefix_len-1] is already + * normalized, any time ../ eats up to the prefix_len part, + * prefix_len is reduced. In the end prefix_len is the remaining + * prefix that has not been overridden by user pathspec. */ -int normalize_path_copy(char *dst, const char *src) +int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) { char *dst0; @@ -568,11 +574,18 @@ int normalize_path_copy(char *dst, const char *src) /* Windows: dst[-1] cannot be backslash anymore */ while (dst0 dst dst[-1] != '/') dst--; + if (prefix_len *prefix_len dst - dst0) + *prefix_len = dst - dst0; } *dst = '\0'; return 0; } +int normalize_path_copy(char *dst, const char *src) +{ + return normalize_path_copy_len(dst, src, NULL); +} + /* * path = Canonical absolute path * prefixes = string_list containing normalized, absolute paths without diff --git a/pathspec.c b/pathspec.c index 17f25d6..e2ee268 100644 --- a/pathspec.c +++ b/pathspec.c @@ -150,10 +150,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item, magic |= short_magic; *p_short_magic = short_magic; - if (magic PATHSPEC_FROMTOP) + if (magic PATHSPEC_FROMTOP) { match = xstrdup(copyfrom); - else - match = prefix_path(prefix, prefixlen, copyfrom); + prefixlen = 0; + } else { + match = prefix_path_gently(prefix, prefixlen, prefixlen, copyfrom); + if (!match) + die(_(%s: '%s' is outside repository), elt, copyfrom); + } *raw = item-match = match; /* * Prefix the pathspec (keep all magic) and put to @@ -167,6 +171,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, } else item-original = elt; item-len = strlen(item-match); + item-prefix = prefixlen; if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) (item-len = 1 item-match[item-len - 1] == '/') @@ -198,13 +203,20 @@ static unsigned prefix_pathspec(struct pathspec_item *item, if (limit_pathspec_to_literal()) item-nowildcard_len = item-len; - else + else { item-nowildcard_len = simple_length(item-match); + if (item-nowildcard_len prefixlen) + item-nowildcard_len = prefixlen; + } item-flags = 0; if (item-nowildcard_len item-len item-match[item-nowildcard_len] == '*' no_wildcard(item-match + item-nowildcard_len + 1)) item-flags |= PATHSPEC_ONESTAR; + + /* sanity checks, pathspec matchers assume these are sane */ + assert(item-nowildcard_len = item-len + item-prefix = item-len); return magic; } @@ -284,6 +296,7 @@ void parse_pathspec(struct
[PATCH 44/45] pathspec: support :(glob) syntax
:(glob)path differs from plain pathspec that it uses wildmatch with WM_PATHNAME while the other uses fnmatch without FNM_PATHNAME. The difference lies in how '*' (and '**') is processed. With the introduction of :(glob) and :(literal) and their global options --[no]glob-pathspecs, the user can: - make everything literal by default via --noglob-pathspecs --literal-pathspecs cannot be used for this purpose as it disables _all_ pathspec magic. - individually turn on globbing with :(glob) - make everything globbing by default via --glob-pathspecs - individually turn off globbing with :(literal) The implication behind this is, there is no way to gain the default matching behavior (i.e. fnmatch without FNM_PATHNAME). You either get new globbing or literal. The old fnmatch behavior is considered deprecated and discouraged to use. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git.txt | 19 Documentation/glossary-content.txt | 29 ++ builtin/add.c | 9 -- builtin/ls-tree.c | 2 +- cache.h| 2 ++ dir.c | 28 + dir.h | 9 +++--- git.c | 8 + pathspec.c | 47 +--- pathspec.h | 4 ++- t/t6130-pathspec-noglob.sh | 63 ++ tree-walk.c| 9 +++--- 12 files changed, 198 insertions(+), 31 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index a3fbc59..52b85b6 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -452,6 +452,17 @@ help ...`. This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment variable to `1`. +--glob-pathspecs: + Add glob magic to all pathspec. This is equivalent to setting + the `GIT_GLOB_PATHSPECS` environment variable to `1`. Disabling + globbing on individual pathspecs can be done using pathspec + magic :(literal) + +--noglob-pathspecs: + Add literal magic to all pathspec. This is equivalent to setting + the `GIT_NOGLOB_PATHSPECS` environment variable to `1`. Enabling + globbing on individual pathspecs can be done using pathspec + magic :(glob) GIT COMMANDS @@ -847,6 +858,14 @@ GIT_LITERAL_PATHSPECS:: literal paths to Git (e.g., paths previously given to you by `git ls-tree`, `--raw` diff output, etc). +GIT_GLOB_PATHSPECS:: + Setting this variable to `1` will cause Git to treat all + pathspecs as glob patterns (aka glob magic). + +GIT_NOGLOB_PATHSPECS:: + Setting this variable to `1` will cause Git to treat all + pathspecs as literal (aka literal magic). + Discussion[[Discussion]] diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index 186b566..5c8da50 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -333,6 +333,35 @@ top `/`;; literal;; Wildcards in the pattern such as `*` or `?` are treated as literal characters. + +glob;; + Git treats the pattern as a shell glob suitable for + consumption by fnmatch(3) with the FNM_PATHNAME flag: + wildcards in the pattern will not match a / in the pathname. + For example, Documentation/{asterisk}.html matches + Documentation/git.html but not Documentation/ppc/ppc.html + or tools/perf/Documentation/perf.html. ++ +Two consecutive asterisks (`**`) in patterns matched against +full pathname may have special meaning: + + - A leading `**` followed by a slash means match in all + directories. For example, `**/foo` matches file or directory + `foo` anywhere, the same as pattern `foo`. **/foo/bar + matches file or directory `bar` anywhere that is directly + under directory `foo`. + + - A trailing /** matches everything inside. For example, + abc/** matches all files inside directory abc, relative + to the location of the `.gitignore` file, with infinite depth. + + - A slash followed by two consecutive asterisks then a slash + matches zero or more directories. For example, `a/**/b` + matches `a/b`, `a/x/b`, `a/x/y/b` and so on. + + - Other consecutive asterisks are considered invalid. ++ +Glob magic is incompatible with literal magic. -- + Currently only the slash `/` is recognized as the magic signature, diff --git a/builtin/add.c b/builtin/add.c index 663ddd1..1dab246 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -541,11 +541,16 @@ int cmd_add(int argc, const char **argv, const char *prefix) /* * file_exists() assumes exact match */ - GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_LITERAL); + GUARD_PATHSPEC(pathspec, +
[PATCH 38/45] tree-diff: remove the use of pathspec's raw[] in follow-rename codepath
Put a checkpoint to guard unsupported pathspec features in future. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- tree-diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree-diff.c b/tree-diff.c index e1145c6..21a50d8 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -224,7 +224,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co DIFF_OPT_SET(diff_opts, RECURSIVE); DIFF_OPT_SET(diff_opts, FIND_COPIES_HARDER); diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; - diff_opts.single_follow = opt-pathspec.raw[0]; + diff_opts.single_follow = opt-pathspec.items[0].match; diff_opts.break_opt = opt-break_opt; diff_opts.rename_score = opt-rename_score; diff_setup_done(diff_opts); @@ -243,7 +243,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co * the future! */ if ((p-status == 'R' || p-status == 'C') - !strcmp(p-two-path, opt-pathspec.raw[0])) { + !strcmp(p-two-path, opt-pathspec.items[0].match)) { const char *path[2]; /* Switch the file-pairs around */ -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 43/45] pathspec: make --literal-pathspecs disable pathspec magic
--literal-pathspecs and its equivalent environment variable are probably used for scripting. In that setting, pathspec magic may be unwanted. Disabling globbing in individual pathspec can be done via :(literal) magic. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git.txt | 4 ++-- pathspec.c | 2 +- t/t6130-pathspec-noglob.sh | 6 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 68f1ee6..a3fbc59 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -448,8 +448,8 @@ help ...`. linkgit:git-replace[1] for more information. --literal-pathspecs:: - Treat pathspecs literally, rather than as glob patterns. This is - equivalent to setting the `GIT_LITERAL_PATHSPECS` environment + Treat pathspecs literally (i.e. no globbing, no pathspec magic). + This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment variable to `1`. diff --git a/pathspec.c b/pathspec.c index cc6545f..9802829 100644 --- a/pathspec.c +++ b/pathspec.c @@ -103,7 +103,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, if (literal_global) global_magic |= PATHSPEC_LITERAL; - if (elt[0] != ':') { + if (elt[0] != ':' || literal_global) { ; /* nothing to do */ } else if (elt[1] == '(') { /* longhand */ diff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh index 49c148e..8551b02 100755 --- a/t/t6130-pathspec-noglob.sh +++ b/t/t6130-pathspec-noglob.sh @@ -77,6 +77,12 @@ test_expect_success 'no-glob option matches literally (bracket)' ' test_cmp expect actual ' +test_expect_success 'no-glob option disables :(literal)' ' + : expect + git --literal-pathspecs log --format=%s -- :(literal)foo actual + test_cmp expect actual +' + test_expect_success 'no-glob environment variable works' ' echo star expect GIT_LITERAL_PATHSPECS=1 git log --format=%s -- f* actual -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 41/45] Kill limit_pathspec_to_literal() as it's only used by parse_pathspec()
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- dir.c | 8 pathspec.c | 8 ++-- pathspec.h | 2 -- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/dir.c b/dir.c index 1098133..76a2e1a 100644 --- a/dir.c +++ b/dir.c @@ -1475,14 +1475,6 @@ int remove_path(const char *name) return 0; } -int limit_pathspec_to_literal(void) -{ - static int flag = -1; - if (flag 0) - flag = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0); - return flag; -} - /* * Frees memory within dir which was allocated for exclude lists and * the exclude_stack. Does not free dir itself. diff --git a/pathspec.c b/pathspec.c index 2bd400a..69adaba 100644 --- a/pathspec.c +++ b/pathspec.c @@ -91,11 +91,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item, const char *prefix, int prefixlen, const char *elt) { + static int literal_global = -1; unsigned magic = 0, short_magic = 0; const char *copyfrom = elt, *long_magic_end = NULL; char *match; int i, pathspec_prefix = -1; + if (literal_global 0) + literal_global = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0); + if (elt[0] != ':') { ; /* nothing to do */ } else if (elt[1] == '(') { @@ -184,7 +188,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, if (flags PATHSPEC_PREFIX_ORIGIN) { struct strbuf sb = STRBUF_INIT; const char *start = elt; - if (prefixlen !limit_pathspec_to_literal()) { + if (prefixlen !literal_global) { /* Preserve the actual prefix length of each pattern */ if (long_magic_end) { strbuf_add(sb, start, long_magic_end - start); @@ -232,7 +236,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, elt, ce_len, ce-name); } - if (limit_pathspec_to_literal()) + if (literal_global) item-nowildcard_len = item-len; else { item-nowildcard_len = simple_length(item-match); diff --git a/pathspec.h b/pathspec.h index bbcfa74..4ebaadc 100644 --- a/pathspec.h +++ b/pathspec.h @@ -61,8 +61,6 @@ extern void parse_pathspec(struct pathspec *pathspec, extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src); extern void free_pathspec(struct pathspec *); -extern int limit_pathspec_to_literal(void); - extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec); extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen); extern const char *check_path_for_gitlink(const char *path); -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 40/45] parse_pathspec: preserve prefix length via PATHSPEC_PREFIX_ORIGIN
The prefix length is passed from one command to another via the new magic 'prefix'. The magic is for parse_pathspec's internal use only, not visible to parse_pathspec's callers. Prefix length is not preserved across commands when --literal-pathspecs is specified (no magic is allowed, including 'prefix'). That's OK because we know all paths are literal. No magic, no special treatment regarding prefix. (This may be no longer true if we make :(glob) default) Other options to preserve the prefix include saving it to env variable or quoting. Env var way (at least _one_ env var) is not suitable because the prefix is not the same for all pathspecs. Pathspecs starting with ../ will eat into the prefix part. We could also preserve 'prefix' across commands by quoting the prefix part, then dequoting on receiving. But it may not be 100% accurate, we may dequote longer than the original prefix part, for example. That may be good or not, but it's not the purpose. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pathspec.c | 41 - 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/pathspec.c b/pathspec.c index e2ee268..2bd400a 100644 --- a/pathspec.c +++ b/pathspec.c @@ -92,9 +92,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item, const char *elt) { unsigned magic = 0, short_magic = 0; - const char *copyfrom = elt; + const char *copyfrom = elt, *long_magic_end = NULL; char *match; - int i; + int i, pathspec_prefix = -1; if (elt[0] != ':') { ; /* nothing to do */ @@ -112,18 +112,29 @@ static unsigned prefix_pathspec(struct pathspec_item *item, nextat = copyfrom + len; if (!len) continue; - for (i = 0; i ARRAY_SIZE(pathspec_magic); i++) + for (i = 0; i ARRAY_SIZE(pathspec_magic); i++) { if (strlen(pathspec_magic[i].name) == len !strncmp(pathspec_magic[i].name, copyfrom, len)) { magic |= pathspec_magic[i].bit; break; } + if (!prefixcmp(copyfrom, prefix:)) { + char *endptr; + pathspec_prefix = strtol(copyfrom + 7, +endptr, 10); + if (endptr - copyfrom != len) + die(_(invalid parameter for pathspec magic 'prefix')); + /* i would be wrong, but it does not matter */ + break; + } + } if (ARRAY_SIZE(pathspec_magic) = i) die(_(Invalid pathspec magic '%.*s' in '%s'), (int) len, copyfrom, elt); } if (*copyfrom != ')') die(_(Missing ')' at the end of pathspec magic in '%s'), elt); + long_magic_end = copyfrom; copyfrom++; } else { /* shorthand */ @@ -150,7 +161,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item, magic |= short_magic; *p_short_magic = short_magic; - if (magic PATHSPEC_FROMTOP) { + if (pathspec_prefix = 0 + (prefixlen || (prefix *prefix))) + die(BUG: 'prefix' magic is supposed to be used at worktree's root); + + if (pathspec_prefix = 0) { + match = xstrdup(copyfrom); + prefixlen = pathspec_prefix; + } else if (magic PATHSPEC_FROMTOP) { match = xstrdup(copyfrom); prefixlen = 0; } else { @@ -165,7 +183,20 @@ static unsigned prefix_pathspec(struct pathspec_item *item, */ if (flags PATHSPEC_PREFIX_ORIGIN) { struct strbuf sb = STRBUF_INIT; - strbuf_add(sb, elt, copyfrom - elt); + const char *start = elt; + if (prefixlen !limit_pathspec_to_literal()) { + /* Preserve the actual prefix length of each pattern */ + if (long_magic_end) { + strbuf_add(sb, start, long_magic_end - start); + strbuf_addf(sb, ,prefix:%d, prefixlen); + start = long_magic_end; + } else { + if (*start == ':') + start++; + strbuf_addf(sb, :(prefix:%d), prefixlen); + } + } +
[PATCH 42/45] pathspec: support :(literal) syntax for noglob pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/glossary-content.txt | 20 builtin/add.c | 2 +- builtin/diff.c | 2 +- dir.c | 15 --- pathspec.c | 11 --- pathspec.h | 4 +++- t/t6130-pathspec-noglob.sh | 18 ++ tree-diff.c| 2 +- tree-walk.c| 5 - 9 files changed, 64 insertions(+), 15 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index db2a74d..186b566 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -322,10 +322,22 @@ and a close parentheses `)`, and the remainder is the pattern to match against the path. + The magic signature consists of an ASCII symbol that is not -alphanumeric. Currently only the slash `/` is recognized as a -magic signature: it makes the pattern match from the root of -the working tree, even when you are running the command from -inside a subdirectory. +alphanumeric. ++ +-- +top `/`;; + The magic word `top` (mnemonic: `/`) makes the pattern match + from the root of the working tree, even when you are running + the command from inside a subdirectory. + +literal;; + Wildcards in the pattern such as `*` or `?` are treated + as literal characters. +-- ++ +Currently only the slash `/` is recognized as the magic signature, +but it is envisioned that we will support more types of magic in later +versions of Git. + A pathspec with only a colon means there is no pathspec. This form should not be combined with other pathspec. diff --git a/builtin/add.c b/builtin/add.c index 0b80fa8..663ddd1 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -541,7 +541,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) /* * file_exists() assumes exact match */ - GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP); + GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_LITERAL); for (i = 0; i pathspec.nr; i++) { const char *path = pathspec.items[i].match; diff --git a/builtin/diff.c b/builtin/diff.c index 6b4e3f9..b78435f 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -372,7 +372,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) } if (rev.prune_data.nr) { /* builtin_diff_b_f() */ - GUARD_PATHSPEC(rev.prune_data, PATHSPEC_FROMTOP); + GUARD_PATHSPEC(rev.prune_data, PATHSPEC_FROMTOP | PATHSPEC_LITERAL); if (!path) path = rev.prune_data.items[0].match; paths += rev.prune_data.nr; diff --git a/dir.c b/dir.c index 76a2e1a..65cac36 100644 --- a/dir.c +++ b/dir.c @@ -108,7 +108,10 @@ static size_t common_prefix_len(const struct pathspec *pathspec) int n; size_t max = 0; - GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + GUARD_PATHSPEC(pathspec, + PATHSPEC_FROMTOP | + PATHSPEC_MAXDEPTH | + PATHSPEC_LITERAL); for (n = 0; n pathspec-nr; n++) { size_t i = 0, len = 0; @@ -232,7 +235,10 @@ int match_pathspec_depth(const struct pathspec *ps, { int i, retval = 0; - GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + GUARD_PATHSPEC(ps, + PATHSPEC_FROMTOP | + PATHSPEC_MAXDEPTH | + PATHSPEC_LITERAL); if (!ps-nr) { if (!ps-recursive || @@ -1290,7 +1296,10 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru * Check out create_simplify() */ if (pathspec) - GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH); + GUARD_PATHSPEC(pathspec, + PATHSPEC_FROMTOP | + PATHSPEC_MAXDEPTH | + PATHSPEC_LITERAL); if (has_symlink_leading_path(path, len)) return dir-nr; diff --git a/pathspec.c b/pathspec.c index 69adaba..cc6545f 100644 --- a/pathspec.c +++ b/pathspec.c @@ -70,6 +70,7 @@ static struct pathspec_magic { const char *name; } pathspec_magic[] = { { PATHSPEC_FROMTOP, '/', top }, + { PATHSPEC_LITERAL, 0, literal }, }; /* @@ -92,13 +93,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item, const char *elt) { static int literal_global = -1; - unsigned magic = 0, short_magic = 0; + unsigned magic = 0, short_magic = 0, global_magic = 0; const char *copyfrom = elt, *long_magic_end = NULL; char *match; int i,
[PATCH 45/45] Rename field raw to _raw in struct pathspec
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 pclo...@gmail.com --- builtin/ls-tree.c | 2 +- dir.c | 4 ++-- pathspec.c| 6 +++--- pathspec.h| 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 1056634..7882352 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -36,7 +36,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname) if (ls_options LS_RECURSIVE) return 1; - s = pathspec.raw; + s = pathspec._raw; if (!s) return 0; diff --git a/dir.c b/dir.c index 7e931af..0a79929 100644 --- a/dir.c +++ b/dir.c @@ -158,7 +158,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); return len; } @@ -1308,7 +1308,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru if (has_symlink_leading_path(path, len)) return dir-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); diff --git a/pathspec.c b/pathspec.c index e3581da..8ae1f8c 100644 --- a/pathspec.c +++ b/pathspec.c @@ -376,7 +376,7 @@ void parse_pathspec(struct pathspec *pathspec, raw[0] = prefix; raw[1] = NULL; pathspec-nr = 1; - pathspec-raw = raw; + pathspec-_raw = raw; return; } @@ -386,7 +386,7 @@ void parse_pathspec(struct pathspec *pathspec, pathspec-nr = n; pathspec-items = item = xmalloc(sizeof(*item) * n); - pathspec-raw = argv; + pathspec-_raw = argv; prefixlen = prefix ? strlen(prefix) : 0; for (i = 0; i n; i++) { @@ -447,7 +447,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec) ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL), PATHSPEC_PREFER_CWD, prefix, pathspec); - return ps.raw; + return ps._raw; } void copy_pathspec(struct pathspec *dst, const struct pathspec *src) diff --git a/pathspec.h b/pathspec.h index 7608172..5ba42e5 100644 --- a/pathspec.h +++ b/pathspec.h @@ -15,7 +15,7 @@ #define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR */ struct pathspec { - const char **raw; /* get_pathspec() result, not freed by free_pathspec() */ + const char **_raw; /* get_pathspec() result, not freed by free_pathspec() */ int nr; unsigned int has_wildcard:1; unsigned int recursive:1; -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] core: use env variable instead of config var to turn on logging pack access
On Sun, Jun 9, 2013 at 1:22 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: 5f44324 (core: log offset pack data accesses happened - 2011-07-06) provides a way to observe pack access patterns via a config switch. Setting an environment variable looks more obvious than a config var, especially when you just need to _observe_, and more inline with other tracing knobs we have. Document it as it may be useful for remote troubleshooting. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git.txt | 7 +++ cache.h | 3 --- config.c | 3 --- environment.c | 1 - sha1_file.c | 14 -- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 68f1ee6..c760918 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -838,6 +838,13 @@ for further details. as a file path and will try to write the trace messages into it. +'GIT_TRACE_PACK_ACCESS':: + If this variable is set to a path, a file will be created at + the given path logging all accesses to any packs. For each + access, the pack file name and an offset in the pack is + recorded. This may be helpful for troubleshooting some + pack-related performance problems. + GIT_LITERAL_PATHSPECS:: Setting this variable to `1` will cause Git to treat all pathspecs literally, rather than as glob patterns. For example, diff --git a/cache.h b/cache.h index df532f8..4f41606 100644 --- a/cache.h +++ b/cache.h @@ -772,9 +772,6 @@ extern int parse_sha1_header(const char *hdr, unsigned long *sizep); /* global flag to enable extra checks when accessing packed objects */ extern int do_check_packed_object_crc; -/* for development: log offset of pack access */ -extern const char *log_pack_access; - extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type); extern int move_temp_to_file(const char *tmpfile, const char *filename); diff --git a/config.c b/config.c index 7a85ebd..d04e815 100644 --- a/config.c +++ b/config.c @@ -688,9 +688,6 @@ static int git_default_core_config(const char *var, const char *value) return 0; } - if (!strcmp(var, core.logpackaccess)) - return git_config_string(log_pack_access, var, value); - if (!strcmp(var, core.autocrlf)) { if (value !strcasecmp(value, input)) { if (core_eol == EOL_CRLF) diff --git a/environment.c b/environment.c index e2e75c1..0cb67b2 100644 --- a/environment.c +++ b/environment.c @@ -37,7 +37,6 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; size_t delta_base_cache_limit = 16 * 1024 * 1024; unsigned long big_file_threshold = 512 * 1024 * 1024; -const char *log_pack_access; const char *pager_program; int pager_use_color = 1; const char *editor_program; diff --git a/sha1_file.c b/sha1_file.c index b114cc9..4b23bb8 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -36,6 +36,9 @@ static inline uintmax_t sz_fmt(size_t s) { return s; } const unsigned char null_sha1[20]; +static const char *no_log_pack_access = no_log_pack_access; +static const char *log_pack_access; + /* * This is meant to hold a *small* number of objects that you would * want read_sha1_file() to be able to return, but yet you do not want @@ -1956,12 +1959,19 @@ static void write_pack_access_log(struct packed_git *p, off_t obj_offset) { static FILE *log_file; + if (!log_pack_access) + log_pack_access = getenv(GIT_TRACE_PACK_ACCESS); + if (!log_pack_access) + log_pack_access = no_log_pack_access; + if (log_pack_access == no_log_pack_access) + return; + if (!log_file) { log_file = fopen(log_pack_access, w); if (!log_file) { error(cannot open pack access log '%s' for writing: %s, log_pack_access, strerror(errno)); - log_pack_access = NULL; + log_pack_access = no_log_pack_access; return; } } @@ -1992,7 +2002,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset, int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC; int base_from_cache = 0; - if (log_pack_access) + if (log_pack_access != no_log_pack_access) write_pack_access_log(p, obj_offset); /* PHASE 1: drill down to the innermost base object */ -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH 1/2] core: use env variable instead of config var to turn on logging pack access
On Sun, Jun 9, 2013 at 1:22 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: 5f44324 (core: log offset pack data accesses happened - 2011-07-06) provides a way to observe pack access patterns via a config switch. Setting an environment variable looks more obvious than a config var, especially when you just need to _observe_, and more inline with other tracing knobs we have. s/inline/in line/ (IMHO) Or: ..., and akin to other tracing knobs we have. Document it as it may be useful for remote troubleshooting. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git.txt: document GIT_TRACE_PACKET
On Sun, Jun 9, 2013 at 1:53 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: This can help with debugging object negotiation or other protocol issues. s///g Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: add --ignore-blank-lines option
On Sat, Jun 8, 2013 at 4:44 PM, Antoine Pelisse apeli...@gmail.com wrote: The goal of the patch is to introduce the GNU diff -B/--ignore-blank-lines as closely as possible. The short option is not available because it's already used for break-rewrites. When this option is used, git-diff will not create hunks that simply adds or removes empty lines, but will still show empty lines s/hunks/a hunk/ ...or... s/adds or removes/add or remove/ addition/suppression if they are close enough to valuable changes. diff --git a/xdiff/xemit.c b/xdiff/xemit.c index d11dbf9..04b4bb1 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -56,16 +56,49 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t * /* * Starting at the passed change atom, find the latest change atom to be included * inside the differential hunk according to the specified configuration. + * Also advance xscr if the first changes must be discareded. */ s/discareded/discarded/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/45] pathspec: add copy_pathspec
On Sun, Jun 9, 2013 at 2:25 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: The function is made to use with free_pathspec() because a simple struct assignment is not enough (free_pathspec wants to free items pointer). I had to read this about five or six times before I could parse and understand it. Perhaps rewrite something like this: free_pathspec() frees pathspec-items, hence simple struct assignment does not properly duplicate a pathspec. Introduce copy_pathspec() to clone a pathspec correctly by deeply copying the items member. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFH/PATCH 0/4] git http tests with apache 2.4
Apache 2.4 recently shipped in Debian unstable, and I noticed that all of the git httpd tests stopped working. It turns out that some configuration directives have changed between 2.2 and 2.4, and the httpd server would not start at all. With this series, the tests run again (for me, at least). The IfVersion checks hopefully mean that there are no regressions for people running 2.2 and lower. However, the final patch is very unsatisfactory. We have to pick an MPM module to mention in the config, but we don't have any idea what's available. I suspect what I have provided will work on most Unix-ish systems. Under Windows, there is a totally different MPM. But I am not sure that our http tests run at all on Windows, as we seem to check in lib-httpd.sh for a Unix-ish apache module path. As far as I know, Apache does not have a try to load this module and fallback directive. We could perhaps look in the module directory and try to do something clever in the shell before starting Apache. Advice from Apache gurus is welcome. [1/4]: t/lib-httpd/apache.conf: do not use LockFile in apache = 2.4 [2/4]: t/lib-httpd/apache.conf: load extra auth modules in apache 2.4 [3/4]: t/lib-httpd/apache.conf: load compat access module in apache 2.4 [4/4]: t/lib-httpd/apache.conf: configure an MPM module for apache 2.4 -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] t/lib-httpd/apache.conf: do not use LockFile in apache = 2.4
The LockFile directive from earlier versions of apache has been replaced by the Mutex directive. The latter seems to give sane defaults and does not need any specific customization, so we can get away with just adding a version check to the use of LockFile. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd/apache.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index b5bce45..38699bb 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -1,5 +1,7 @@ LockFile accept.lock ServerName dummy +IfVersion 2.4 LockFile accept.lock +/IfVersion PidFile httpd.pid DocumentRoot www LogFormat %h %l %u %t \%r\ %s %b common -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] t/lib-httpd/apache.conf: load compat access module in apache 2.4
In apache 2.4, the Order directive has gone away in favor of a new system in mod_authz_host. However, since we want our config file to remain compatible across multiple Apache versions, we can use mod_access_compat to keep using the older style. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd/apache.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 1d3615b..4883b8c 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -54,6 +54,9 @@ ErrorLog error.log IfModule !mod_authz_core.c LoadModule authz_core_module modules/mod_authz_core.so /IfModule +IfModule !mod_access_compat.c + LoadModule access_compat_module modules/mod_access_compat.so +/IfModule /IfVersion PassEnv GIT_VALGRIND -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] t/lib-httpd/apache.conf: load extra auth modules in apache 2.4
In apache 2.4, the Auth* and Require directives have moved into the authn_core and authz_core modules, respectively. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd/apache.conf | 9 + 1 file changed, 9 insertions(+) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 38699bb..1d3615b 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -47,6 +47,15 @@ ErrorLog error.log /IfModule /IfVersion +IfVersion = 2.4 +IfModule !mod_authn_core.c + LoadModule authn_core_module modules/mod_authn_core.so +/IfModule +IfModule !mod_authz_core.c + LoadModule authz_core_module modules/mod_authz_core.so +/IfModule +/IfVersion + PassEnv GIT_VALGRIND PassEnv GIT_VALGRIND_OPTIONS -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] t/lib-httpd/apache.conf: configure an MPM module for apache 2.4
Versions of Apache before 2.4 always had a MultiProcessing Module (MPM) statically built in, which manages the worker threads/processes. We do not care which one, as it is largely a performance issue, and we put only a light load on the server during our testing. As of Apache 2.4, the MPM module is loadable just like any other module, but exactly one such module must be loaded. On a system where the MPMs are compiled dynamically (e.g., Debian unstable), this means that our test Apache server will not start unless we provide the appropriate configuration. Unfortunately, we do not actually know which MPM modules are available or appropriate for the system on which the tests are running. This patch picks the prefork module, as it is likely to be available on all Unix-like systems. Signed-off-by: Jeff King p...@peff.net --- t/lib-httpd/apache.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 4883b8c..56ae548 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -57,6 +57,9 @@ ErrorLog error.log IfModule !mod_access_compat.c LoadModule access_compat_module modules/mod_access_compat.so /IfModule +IfModule !mod_mpm_prefork.c + LoadModule mpm_prefork_module modules/mod_mpm_prefork.so +/IfModule /IfVersion PassEnv GIT_VALGRIND -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
Johannes Schindelin wrote: Correct. The opinions of inactive community members and non-contributors are less useful. I humbly suggest to treat other people's contribution with the same respect you want yours' to be treated. What?! When did I disrespect other people's contributions? git.git is what it is today because of everyone's contributions: if I disrespected them, why would I work on improving git? My opinion has nothing to do with me, or my contributions. I have already stated multiple times on the list that I take no pride in my contributions whatsoever*: I have no ego to speak of. I said the opinions of inactive community members and non-contributors are less useful [than those of active contributors], and I'm still scratching my head over what you inferred. Do you think that the opinions of inactive community members and non-contributors are _more_ valuable than those of active contributors, or am I missing something? * You'd know that if you read emails on the list. But you don't, for some mysterious unstated reason. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Jeff King wrote: Sorry that I cannot show you the source code, but you may interested to know that libgit2 powers: Yes, I'm well aware of these: libgit2 is LGPL, because of which these three proprietary applications have been made possible. Isn't it completely orthogonal to the discussion about how best to lib'ify git.git though? From what I understand, fc is not interested in building another application leveraging libgit.a or libgit2; he's interested in improving libgit.a and getting more users. It is definitely not feature-complete when compared with git.git. But I do think it is in a state that is usable for quite a few tasks. What is this task you are discussing? fc is talking about improving libgit.a and getting an official git library with many users. Answer the question: what should we do now? 1. Start moving irrelevant code out of libgit.a, and use inspiration from libgit2 to improve it (this might or might not involve taking code from libgit2). Get _users_ of libgit.a via ruby bindings (or something) asap, so it puts pressure on fixing it. 2. Wait indefinitely until libgit2.git magically becomes ready to be usable by git.git as-is. Then throw libgit.a out the window, and rewrite git.git to call into libgit2.a instead [*1*]. What you seem to be saying is 3. Work on libgit2 (and abandon git.git?) [*2*], or worse: 2. fc is in favor of 1. Unless you are in favor of _not_ improving libgit.a, don't stand in his way: you might personally think that it is a difficult (or impossible) task, but that's no reason to stop fc from trying. I personally think his goal is admirable, and I'm nobody to say that it cannot be done: therefore, I will review his patches and help him in whatever little way I can. [Footnote] *1* You have dismissed 1 as being unworkable, but do you realize how unrealistic this sounds? *2* git.git has _far_ more users and a _lot_ more contributors. Don't be unwelcoming to contributors by asking them to go away and work on something else. The three proprietary applications you have given as counter-examples (?) is not helping anyone. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Document .git/modules
A note in the begging of this document describe the behavior already. This patch just add where to find the repositories. Signed-off-by: Fredrik Gustafsson iv...@iveqy.com --- Documentation/gitrepository-layout.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt index d6f3393..aa03882 100644 --- a/Documentation/gitrepository-layout.txt +++ b/Documentation/gitrepository-layout.txt @@ -211,6 +211,9 @@ shallow:: and maintained by shallow clone mechanism. See `--depth` option to linkgit:git-clone[1] and linkgit:git-fetch[1]. +modules:: + Contains the git-repositories of the submodules. + SEE ALSO linkgit:git-init[1], -- 1.8.1.5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document .git/modules
On Sun, Jun 9, 2013 at 6:37 AM, Fredrik Gustafsson iv...@iveqy.com wrote: A note in the begging of this document describe the behavior already. s/begging/beginning/ s/describe/describes/ This patch just add where to find the repositories. s/add/adds/ Signed-off-by: Fredrik Gustafsson iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
Jeff King p...@peff.net writes: 1- Find the remote name of the current branch's upstream and check if it's a wiki one with its url (ie: mediawiki://) 2- Parse the content of the local file (given as argument) using the distant wiki's API. Makes sense. 3- Retrieve the current page on the distant mediawiki. 4- Merge those those contents. I'm not sure what these steps are for. You are trying to preview not just your local version, but pulling in any changes that have happened upstream since the work you built on top of? Same question here. I'd expect git mw preview in a mediawiki workflow to do what pdflatex foo evince foo.pdf do in a latex workflow: see in rendered form what I've been doing. In a latex flow, if I want to see how my local changes merge with the remote ones, I do git merge pdflatex, and I'd do the same with git mw. I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Next step could even be git mw diff $from $to, using the wiki to render the diff. Not a priority, but could be funny. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Document .git/modules
A note in the beginning of this document describes the behavior already. This patch just adds where to find the repositories. Signed-off-by: Fredrik Gustafsson iv...@iveqy.com --- Documentation/gitrepository-layout.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt index d6f3393..aa03882 100644 --- a/Documentation/gitrepository-layout.txt +++ b/Documentation/gitrepository-layout.txt @@ -211,6 +211,9 @@ shallow:: and maintained by shallow clone mechanism. See `--depth` option to linkgit:git-clone[1] and linkgit:git-fetch[1]. +modules:: + Contains the git-repositories of the submodules. + SEE ALSO linkgit:git-init[1], -- 1.8.1.5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document .git/modules
On Sun, Jun 09, 2013 at 06:59:17AM -0400, Eric Sunshine wrote: On Sun, Jun 9, 2013 at 6:37 AM, Fredrik Gustafsson iv...@iveqy.com wrote: A note in the begging of this document describe the behavior already. s/begging/beginning/ s/describe/describes/ This patch just add where to find the repositories. s/add/adds/ Signed-off-by: Fredrik Gustafsson iv...@iveqy.com Thanks! Should be fixed in the next patch (which I failed to mark as v2, but that's a reply to your email) -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 11:34 PM, Jeff King p...@peff.net wrote: On Sat, Jun 08, 2013 at 09:20:54AM -0500, Felipe Contreras wrote: Let the code speak. Show me a script in any language that does something useful using libgit2, doing the equivalent to at least a couple of 'git foo' commands. Sorry that I cannot show you the source code, but you may interested to know that libgit2 powers: 1. Microsoft's Visual Studio Tools for Git plugin 2. GitHub's native Mac and Windows clients (using Objective C and C# bindings); some operations still shell out to git where the functionality is not yet implemented in libgit2. 3. Parts of the web view of GitHub.com via Ruby bindings It is definitely not feature-complete when compared with git.git. But I do think it is in a state that is usable for quite a few tasks. That's not what a I asked. We have perl, and shell, and python, and ruby scripts in git.git, they all use 'git foo' commands to get things done. But to do that, forks are needed, many of them, constantly. The proposal was to use libgit2 to avoid such forks. Well, show me a *script* that does that and is worthy of inclusion to git.git, even if it's to contrib. I didn't ask for irrelevant 3rd parties, I asked for something worthy of inclusion into git.git, a script that does something useful using libgit2. Duy Nguyen seems to think it's easy to do that. I'm waiting. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sun, Jun 9, 2013 at 12:26 AM, Jeff King p...@peff.net wrote: On Sat, Jun 08, 2013 at 09:17:56PM -0500, Felipe Contreras wrote: Definitely, yes. Even if you look at the impact on code alone and don't care about the people, destroying a collegial work environment is harmful enough to the code to outweigh the (admittedly often useful) patches. A collegial work environment is overrated, and proof of that the Linux kernel, where honest and straight talk is the bread and butter of the mailing list. And the Linux kernel is the most successful software project in history by far. It's code that speaks. Sorry, but I don't agree, and I want to publicly state my opinion so that Jonathan (and other bystanders on the list) knows that he is not alone in his opinions. You don't agree that 1) a collegial work environment is overrated, 2) that the Linux kernel doesn't put an emphasis on being collegial, or 3) that it's the most successful software project in history? I have consistently found your demeanor on the list to be very unfriendly and difficult to work with. It is one thing to have honest and straight talk, and another thing to be obstinate, unmindful of feedback (both with respect to technical details, as well as to communication styles), and disrespectful of other people. Go back to my 261 commits, show me one that is unmindful of technical details. It is certainly your choice about how you will communicate. But likewise it is the choice of readers and reviewers to choose how much of their time to give to your writings. Exactly. Nobody is forcing you to read my emails. But somehow you already know that ignoring them is not in the best interest of the project. And by that I mean it's in the best interest of our users, without which our project is nothing. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
Matthieu Moy matthieu@grenoble-inp.fr writes: Same question here. I'd expect git mw preview in a mediawiki workflow to do what pdflatex foo evince foo.pdf do in a latex workflow: see in rendered form what I've been doing. In a latex flow, if I want to see how my local changes merge with the remote ones, I do git merge pdflatex, and I'd do the same with git mw. In fact, I should not have used merge to describe how the two contents (page template + new parsed content) are combined together. For now, the code simply replaces the template page's text content (the one retrieved from the remote) with the new one. It does not really care if the remote has changes or not. (And, to be honest, I did not thought about that issue ;) ). But, like both of you said : in a typical workflow, the merging would be left to the user so the current behavior is fine I think ? I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Next step could even be git mw diff $from $to, using the wiki to render the diff. Not a priority, but could be funny. I searched in the Mediawiki API if there was a way to diff from a stored revision and raw text content but I've found nothing :/ . We could make a little hack to do that by saving as a new revision the local content, and use the DeleteRevision-thingy from Mediawiki [1] to hide this useless revision but it would floods the remote DB and usually users to not have the permission to use that tool. So, for now I would say it's a no-go :/ . [1] http://www.mediawiki.org/wiki/Manual:RevisionDelete Benoit Person -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On 9 June 2013 08:08, Jeff King p...@peff.net wrote: I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Hum, so `git mw preview origin:page.mw` would just do the get request to the remote mediawiki, save it locally and - maybe - load it in the browser ? Is it really better than just opening the browser and typing the right URL ? Currently, this URL is one click away when you have preview file loaded in a web browser. It works but a couple of points trouble me: 1- I had to copy two functions from `git-remote-mediawiki.perl`, I don't really know how we could factorize those things ? I don't think it makes much sense to create a package just for them ? You could make a Git::MediaWiki.pm module, but installing that would significantly complicate the build procedure, and potentially be annoying for users. One trick I have done in the past is to concatenate bits of perl script together in the Makefile, like this: foo: common.pl foo.pl { \ echo '$(PERL_PATH_SQ)' \ for i in $^; do \ echo #line 1 $src \ cat $src \ done \ } $@+ mv $@+ $@ That would conflict a bit with the way we chain to git's Makefile, though. I suspect you could do something complicated like build foo.pl from common.pl and foo-main.pl, then chain to git's Makefile to build foo from foo.pl. ok, thanks a lot. 2- The current behavior is to crash if the current branch do not have an upstream branch on a valid mediawiki remote. To find that specific remote, it runs `git rev-parse --symbolic-full-name @{upstream}` which will return something like `/refs/remotes/$remote_name/master`. 2a- maybe there is a better way to find that remote name ? If you just care about the remote name and not the name of the local branch, you can just ask for my $curr_branch = `git symbolic-ref HEAD`; my $remote = `git config branch.$curr_branch.remote`; my $url = `git config remote.$remote.url`; Of course you would want some error checks and probably some chomp()s in there, too. The fact is, `git symbolic-ref HEAD` result would have to be parsed in order to extract the current branch name like I currently extract the remote name. So, is it really better than `git rev-parse --symbolic-full-name @{upstream}` ? 2b- would it be useful to add a fallback if that search fails ? searching for a valid mediawiki remote url in all the remotes returned by `git remote` for instance ? That is probably OK as long as there is only one such remote, and it would help the case where you have branched off of a local branch (so your upstream remote is .). If there are two mediawiki remotes, though, it would make sense to simply fail, as you don't know which to use. But I'd expect the common case by far to be that you simply have one such remote. Well, I thought that `git mw preview` could provide an interactive mode where, if the first search fails, it would find all the mediawiki remotes, and offers to the user a way to choose the remote ? Benoit Person -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-daemon: needs /root/.config/git/config?
* Ian Kumlien po...@vapor.com [130605 13:31]: Yes, i agree, it's suboptimal but I for one would use getpwuid to get the home directory of the executing user to avoid this - though i don't know how portable it is (or if there is any other issues) It's not only suboptimal but simply wrong. getpwuid gives at best the initial home directory, and even there it is only a guess. (If you are looking for some home directory of a different user it might be a good guess). But using getpwuid(getuid())-pw_dir if HOME is set is a serious mistake, as you throw out the good value for some almost but not quite totally unrelated value. Bernhard R. Link -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Jeff King wrote: Definitely, yes. Even if you look at the impact on code alone and don't care about the people, destroying a collegial work environment is harmful enough to the code to outweigh the (admittedly often useful) patches. A collegial work environment is overrated, and proof of that the Linux kernel, where honest and straight talk is the bread and butter of the mailing list. And the Linux kernel is the most successful software project in history by far. It's code that speaks. I have consistently found your demeanor on the list to be very unfriendly and difficult to work with. It is one thing to have honest and straight talk, and another thing to be obstinate, unmindful of feedback (both with respect to technical details, as well as to communication styles), and disrespectful of other people. While I agree that being rude and obstinate is definitely undesirable, and that a healthy on-list environment is important, I have something to add: Being super-tactful comes at a cost. Regulars on the mailing list have to spend 3~4x the amount of time to compose an email (reading and re-reading their drafts to see how to express them in a more friendly way); this leads to a lot of inefficiency and creates a suffocating environment in which people don't have freedom of expression. I would much rather prefer straight talk where nobody reads into what is written and takes offense. In this case, jrn took offense and talked about how he would ban fc from the list if he were managing it: while I'm not defending fc's tone, I'm not defending jrn's comment either. jrn has been around since mid-2008, and fc has been around since early-2009. It's mid-2013, and they still haven't learnt to work with each other. Disagreement is healthy, and is the foundation of progress. When it comes to sensitive issues, stern disagreement is often mis-interpreted as disrespect (or worse). If we keep beating up disagreements on the basis of tone and demeanor, git.git would go nowhere. Sure, it would be more ideal if fc's tone were friendlier [*1*], but it isn't: let's deal with the issue instead of constantly whining about it. [Footnotes] *1* Oh, and mine too. I've been told several times off-list that my tone is unfriendly. I'm working on fixing the issue, but I don't enjoy the constant suffocation: I should be able to say what I want without too much effort. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sun, Jun 9, 2013 at 7:48 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Jeff King wrote: Definitely, yes. Even if you look at the impact on code alone and don't care about the people, destroying a collegial work environment is harmful enough to the code to outweigh the (admittedly often useful) patches. A collegial work environment is overrated, and proof of that the Linux kernel, where honest and straight talk is the bread and butter of the mailing list. And the Linux kernel is the most successful software project in history by far. It's code that speaks. I have consistently found your demeanor on the list to be very unfriendly and difficult to work with. It is one thing to have honest and straight talk, and another thing to be obstinate, unmindful of feedback (both with respect to technical details, as well as to communication styles), and disrespectful of other people. While I agree that being rude and obstinate is definitely undesirable, and that a healthy on-list environment is important, I have something to add: Being super-tactful comes at a cost. Regulars on the mailing list have to spend 3~4x the amount of time to compose an email (reading and re-reading their drafts to see how to express them in a more friendly way); this leads to a lot of inefficiency and creates a suffocating environment in which people don't have freedom of expression. That's exactly the reason why they don't put emphasis on that in the Linux kernel mailing list. Besides, when something is really fucked up, the most effective way to convey the fact that you think it's totally fucked up, is to say precisely that. If somebody's feelings get hurt along the way, and they decide to leave the project, they were not cut for Linux development anyway. I would much rather prefer straight talk where nobody reads into what is written and takes offense. In this case, jrn took offense and talked about how he would ban fc from the list if he were managing it: while I'm not defending fc's tone, I'm not defending jrn's comment either. jrn has been around since mid-2008, and fc has been around since early-2009. It's mid-2013, and they still haven't learnt to work with each other. We don't _need_ to work with each other. If he helps the project, and I help the project, what's wrong with that? Disagreement is healthy, and is the foundation of progress. When it comes to sensitive issues, stern disagreement is often mis-interpreted as disrespect (or worse). If we keep beating up disagreements on the basis of tone and demeanor, git.git would go nowhere. Sure, it would be more ideal if fc's tone were friendlier [*1*], but it isn't: let's deal with the issue instead of constantly whining about it. Completely agree. Disagreement is not disrespect. Besides, ideas don't have feelings, ideas don't need respect; ideas should be criticized. Any rational person, specially scientists, understand that it's not healthy to have an emotional attachment to ideas; they might be wrong, and they need scrutiny. If one doesn't tolerate criticism of one's ideas (however straightforward or delicate it might be), one is never going to find the truth. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
Benoît Person benoit.per...@ensimag.fr writes: On 9 June 2013 08:08, Jeff King p...@peff.net wrote: I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Hum, so `git mw preview origin:page.mw` would just do the get request to the remote mediawiki, save it locally and - maybe - load it in the browser ? Is it really better than just opening the browser and typing the right URL ? Currently, this URL is one click away when you have preview file loaded in a web browser. origin:page.mw is not necessarily the best example, but HEAD^:page.mw can exist in your local history and not on the wiki. Even HEAD:page.mw can be interesting if you have uncommited changes. Not that it's terribly useful to be able to preview it, but once you can deal with local files, it should be easy enough to generalize it to any blob. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote: Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. Why should sequencer.c move into builtin/ to solve this? Why not pull init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into notes.c? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/9] sequencer: run post-rewrite hook
On Thu, Jun 6, 2013 at 1:40 PM, Junio C Hamano gits...@pobox.com wrote: It probably is worth inserting a commit before 4/9 that adds rewrite.[ch], and - introduces struct rewritten_list[_item]; - moves run_rewrite_hook() in builtin/commit.c to rewrite.c; - changes its function signature so that it takes char *action_name and struct rewritten * as parameters; and - adjust its sole call site in cmd_commit() to feed a single-item rewritten_list to it. Then 4/9 can teach cherry-pick to prepare the rewritten-list and probably this commit can be part of that to call run_rewrite_hook(). Done. I'll send the patches in a moment. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 9, 2013 at 10:12 AM, John Keeping j...@keeping.me.uk wrote: On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote: Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. Why should sequencer.c move into builtin/ to solve this? Why not pull init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into notes.c? Because finish_copy_notes_for_rewrite is only useful for builtin commands, so it belongs in builtin/. If there's any meaning to the ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just squash all objects into libgit.a and be done with it. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 9, 2013 at 9:56 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. I already explained this multiple times; code from ./*.o can't access code from ./builtin/*.o. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 10:12 AM, John Keeping j...@keeping.me.uk wrote: On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote: Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. Why should sequencer.c move into builtin/ to solve this? Why not pull init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into notes.c? Because finish_copy_notes_for_rewrite is only useful for builtin commands, so it belongs in builtin/. If there's any meaning to the ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just squash all objects into libgit.a and be done with it. How is it only useful for builtin commands? By that logic everything belongs in builtin/ because it's all only used by builtin commands (I realise that is what you're arguing towards). But we make a distinction between things that are specific to one command (especially argument parsing and user interaction) and more generally useful features. Copying notes around in the notes tree is generally useful so why shouldn't it be in notes.c with the other note manipulation functions? The current API may not be completely suitable but that doesn't mean that it cannot be extracted into notes.c. In fact, other than the commit message saying Notes added by 'git notes copy' I don't see what's wrong with the current functions being extracted as-is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 9, 2013 at 11:02 AM, John Keeping j...@keeping.me.uk wrote: On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 10:12 AM, John Keeping j...@keeping.me.uk wrote: On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote: Felipe Contreras wrote: The plan is simple; make libgit.a a proper library, starting by clarifying what goes into libgit.a, and what doesn't. If there's any hopes of ever having a public library, it's clear what code doesn't belong in libgit.a; code that is meant for builtins, that code belongs in builtins/lib.a, or similar. Give this a try: --- a/sequencer.c +++ b/sequencer.c libgit.a(sequencer.o): In function `copy_notes': /home/felipec/dev/git/sequencer.c:110: undefined reference to `init_copy_notes_for_rewrite' /home/felipec/dev/git/sequencer.c:114: undefined reference to `finish_copy_notes_for_rewrite' This is a good example: yes, I'm convinced that the code does need to be reorganized. Please resend your {sequencer.c - builtin/sequencer.c} patch with this example as the rationale, and let's work towards improving libgit.a. Why should sequencer.c move into builtin/ to solve this? Why not pull init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into notes.c? Because finish_copy_notes_for_rewrite is only useful for builtin commands, so it belongs in builtin/. If there's any meaning to the ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just squash all objects into libgit.a and be done with it. How is it only useful for builtin commands? By that logic everything belongs in builtin/ because it's all only used by builtin commands (I realise that is what you're arguing towards). Which is precisely the point of this patch. If everything is for builtin commands, then we don't have a git library, and git.a should contain everything under builtin/*.o. But we make a distinction between things that are specific to one command (especially argument parsing and user interaction) and more generally useful features. No, we don't. Everything under ./*.o goes to libgit.a, and everything under ./builtin/*.o goes to 'git'. So builtin/commit.o can access code from builtin/notes.o, but sequencer.o can't. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] submodule: remove redundant check for the_index.initialized
read_cache already performs the same check and returns immediately if the cache has already been loaded. Signed-off-by: René Scharfe rene.scha...@lsrfire.ath.cx --- submodule.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/submodule.c b/submodule.c index e728025..1821a5b 100644 --- a/submodule.c +++ b/submodule.c @@ -603,9 +603,8 @@ int fetch_populated_submodules(const struct argv_array *options, if (!work_tree) goto out; - if (!the_index.initialized) - if (read_cache() 0) - die(index file corrupt); + if (read_cache() 0) + die(index file corrupt); argv_array_push(argv, fetch); for (i = 0; i options-argc; i++) -- 1.8.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
John Keeping wrote: How is it only useful for builtin commands? By that logic everything belongs in builtin/ because it's all only used by builtin commands (I realise that is what you're arguing towards). sequencer.c is merely a common API for builtins to invoke continuations: i.e. stop the program persisting enough state to let to user continue, allow the user to do whatever conflict resolutions using whatever tools, allow the user to continue the original operation. sequencer.c provides a uniform UI (--continue|--skip|--abort), and a uniform way to persist state (.git/sequencer/todo). It mainly abstracts out the boring details of reading/writing the todo lines. Currently, sequencer.c has no callers other than those in builtin/revert.c. In its current shape, it's incapable of being used by anything else: while an external ruby script (possibly a rebase implementation) could call into the sequencer to persist state, I don't think it is going to happen anytime soon. We might get a proper public api sequencer sometime in the distant future, but don't confuse that with the current shape of sequencer.c. But we make a distinction between things that are specific to one command (especially argument parsing and user interaction) and more generally useful features. Copying notes around in the notes tree is generally useful so why shouldn't it be in notes.c with the other note manipulation functions? The current API may not be completely suitable but that doesn't mean that it cannot be extracted into notes.c. In fact, other than the commit message saying Notes added by 'git notes copy' I don't see what's wrong with the current functions being extracted as-is. Sure, notes could have a better public api and so could a lot of other things: worktree operations like reset and checkout come to mind. The problem is that we seem to be at some frozen in some sort of stalemate, and some reorganization is definitely required. What would you suggest? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/45] Massive improvents to rebase and cherry-pick
Hi, These are improvements to 'git rebase' by using a much improved 'git cherry-pick'. I already sent some of these, but they have been revamped. These changes require reorganization of the code in order to have a builtin/lib.a library. A new builtin/rewrite.c helper is added, and builtin/commit updated to use that. A new git-rebase--cherypick mode is added, and it replaces git-rebase--am and git-rebase--merge. I also added the new rebase tests by Martin von Zweigbergk to make sure everything works, and in fact, it works better than before, since now all the rebase modes are consistent with each other. I don't have any hopes of these getting merged, because people have no interest in fixing the ./*.o ./builin/*.o divide problem, only interested in arguing that libgit.a is not a real library, and it should never be. So my obviously cleanup is rejected. Felipe Contreras (38): build: generate and clean test scripts build: do not install git-remote-testgit build: trivial cleanup build: add builtin lib log-tree: remove dependency from sequencer Move sequencer to builtin unpack-trees: plug a memory leak read-cache: plug a few leaks sequencer: remove useless indentation sequencer: trivial fix cherry-pick: don't barf when there's nothing to do cherry-pick: add --skip-empty option revert/cherry-pick: add --quiet option revert/cherry-pick: add --skip option builtin: add rewrite helper cherry-pick: store rewritten commits cherry-pick: don't store skipped commit builtin: move run_rewrite_hook() to rewrite.c builtin: add copy_rewrite_notes() cherry-pick: copy notes and run hooks cherry-pick: add --action-name option cherry-pick: remember rerere-autoupdate rebase: split the cherry-pick stuff rebase: cherry-pick: fix mode storage rebase: cherry-pick: fix sequence continuation rebase: cherry-pick: fix abort of cherry mode rebase: cherry-pick: fix command invocations rebase: cherry-pick: fix status messages rebase: cherry-pick: automatically commit stage rebase: cherry-pick: set correct action-name rebase: trivial cleanup rebase: use 'cherrypick' mode instead of 'am' rebase: cherry-pick: fix for shell prompt rebase: cherry-pick: add merge options rebase: remove merge mode rebase: cherry-pick: add copyright tests: fix autostash tests: update topology tests Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 .gitignore | 1 + Documentation/git-cherry-pick.txt | 10 +- Documentation/git-revert.txt | 7 +- Documentation/sequencer.txt| 3 + Makefile | 31 +-- builtin/commit.c | 46 + builtin/revert.c | 17 ++ builtin/rewrite.c | 124 builtin/rewrite.h | 20 ++ sequencer.c = builtin/sequencer.c | 263 ++--- sequencer.h = builtin/sequencer.h | 12 +- contrib/completion/git-prompt.sh | 4 +- git-rebase--am.sh | 12 +- git-rebase--cherrypick.sh | 72 +++ git-rebase--interactive.sh | 4 +- git-rebase--merge.sh | 151 -- git-rebase.sh | 16 +- log-tree.c | 161 ++- log-tree.h | 3 + read-cache.c | 4 + t/lib-rebase.sh| 33 t/t3400-rebase.sh | 53 + t/t3401-rebase-partial.sh | 69 --- t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 56 +++--- t/t3407-rebase-abort.sh| 2 +- t/t3409-rebase-preserve-merges.sh | 53 - t/t3420-rebase-autostash.sh| 5 +- t/t3421-rebase-topology-linear.sh | 350 + t/t3425-rebase-topology-merges.sh | 255 t/t3508-cherry-pick-many-commits.sh| 13 ++ t/t3510-cherry-pick-sequence.sh| 14 +- t/t5520-pull.sh| 2 +- t/t9106-git-svn-commit-diff-clobber.sh | 2 +- t/t9903-bash-prompt.sh | 2 +- unpack-trees.c | 4 +- 36 files changed, 1268 insertions(+), 616 deletions(-) create mode 100644 builtin/rewrite.c create mode 100644 builtin/rewrite.h rename sequencer.c = builtin/sequencer.c (86%) rename sequencer.h = builtin/sequencer.h (86%) create mode 100644 git-rebase--cherrypick.sh delete mode 100644 git-rebase--merge.sh delete mode 100755 t/t3401-rebase-partial.sh create mode 100755
[PATCH v4 10/45] sequencer: trivial fix
We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/sequencer.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/sequencer.c b/builtin/sequencer.c index b2c8c94..23b01b7 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -539,8 +539,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } allow = allow_empty(opts, commit); - if (allow 0) - return allow; + if (allow 0) { + res = allow; + goto leave; + } if (!opts-no_commit) res = run_git_commit(defmsg, opts, allow); -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 04/45] build: add builtin lib
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Makefile | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 3b6cd5a..d2af207 100644 --- a/Makefile +++ b/Makefile @@ -631,6 +631,7 @@ export PERL_PATH export PYTHON_PATH LIB_FILE = libgit.a +BUILTIN_LIB = builtin/lib.a XDIFF_LIB = xdiff/lib.a VCSSVN_LIB = vcs-svn/lib.a @@ -1713,9 +1714,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \ '-DGIT_INFO_PATH=$(infodir_relative_SQ)' -git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) +git$X: git.o GIT-LDFLAGS $(BUILTIN_LIB) $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \ - $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) + $(ALL_LDFLAGS) $(BUILTIN_LIB) $(LIBS) help.sp help.s help.o: common-cmds.h @@ -2070,6 +2071,9 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ +$(BUILTIN_LIB): $(BUILTIN_OBJS) + $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ + $(XDIFF_LIB): $(XDIFF_OBJS) $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ @@ -2482,7 +2486,7 @@ profile-clean: clean: profile-clean coverage-clean $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \ - builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) + builtin/*.o $(LIB_FILE) $(BUILTIN_LIB) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) $(RM) -r bin-wrappers $(dep_dirs) -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/45] unpack-trees: plug a memory leak
Before overwriting the destination index, first let's discard its contents. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- unpack-trees.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unpack-trees.c b/unpack-trees.c index 57b4074..abe2576 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1166,8 +1166,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o-src_index = NULL; ret = check_updates(o) ? (-2) : 0; - if (o-dst_index) + if (o-dst_index) { + discard_index(o-dst_index); *o-dst_index = o-result; + } done: clear_exclude_list(el); -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 11/45] cherry-pick: don't barf when there's nothing to do
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/sequencer.c | 4 ++-- t/t3510-cherry-pick-sequence.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/sequencer.c b/builtin/sequencer.c index 23b01b7..4d7dc8b 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -565,8 +565,8 @@ static void prepare_revs(struct replay_opts *opts) if (prepare_revision_walk(opts-revs)) die(_(revision walk setup failed)); - if (!opts-revs-commits) - die(_(empty commit set passed)); + if (!opts-revs-commits !opts-quiet) + error(_(empty commit set passed)); } static void read_and_refresh_cache(struct replay_opts *opts) diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 7b7a89d..33c5512 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -472,7 +472,7 @@ test_expect_success 'malformed instruction sheet 2' ' test_expect_success 'empty commit set' ' pristine_detach initial - test_expect_code 128 git cherry-pick base..base + git cherry-pick base..base ' test_expect_success 'malformed instruction sheet 3' ' -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 12/45] cherry-pick: add --skip-empty option
Pretty much what it says on the tin. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/git-cherry-pick.txt | 3 +++ builtin/revert.c| 8 builtin/sequencer.c | 6 ++ builtin/sequencer.h | 1 + t/t3508-cherry-pick-many-commits.sh | 13 + 5 files changed, 31 insertions(+) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index c205d23..fccd936 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -129,6 +129,9 @@ effect to your index in a row. redundant commits are ignored. This option overrides that behavior and creates an empty commit object. Implies `--allow-empty`. +--skip-empty:: + Instead of failing, skip commits that are or become empty. + --strategy=strategy:: Use the given merge strategy. Should only be used once. See the MERGE STRATEGIES section in linkgit:git-merge[1] diff --git a/builtin/revert.c b/builtin/revert.c index 0401fdb..5a8453d 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -118,6 +118,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_END(), OPT_END(), OPT_END(), + OPT_END(), }; if (opts-action == REPLAY_PICK) { @@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_BOOLEAN(0, allow-empty, opts-allow_empty, N_(preserve initially empty commits)), OPT_BOOLEAN(0, allow-empty-message, opts-allow_empty_message, N_(allow commits with empty messages)), OPT_BOOLEAN(0, keep-redundant-commits, opts-keep_redundant_commits, N_(keep redundant, empty commits)), + OPT_BOOLEAN(0, skip-empty, opts-skip_empty, N_(skip empty commits)), OPT_END(), }; if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra)) @@ -144,6 +146,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) --abort, rollback, NULL); + verify_opt_mutually_compatible(me, + --allow-empty, opts-allow_empty, + --skip-empty, opts-skip_empty, + --keep-redundant-commits, opts-keep_redundant_commits, + NULL); + /* implies allow_empty */ if (opts-keep_redundant_commits) opts-allow_empty = 1; diff --git a/builtin/sequencer.c b/builtin/sequencer.c index 4d7dc8b..56551bb 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -538,6 +538,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) goto leave; } + if (opts-skip_empty is_index_unchanged() == 1) { + warning(_(skipping %s... %s), + find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV), + msg.subject); + goto leave; + } allow = allow_empty(opts, commit); if (allow 0) { res = allow; diff --git a/builtin/sequencer.h b/builtin/sequencer.h index c341918..86e2eee 100644 --- a/builtin/sequencer.h +++ b/builtin/sequencer.h @@ -32,6 +32,7 @@ struct replay_opts { int allow_empty; int allow_empty_message; int keep_redundant_commits; + int skip_empty; int mainline; diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 19c99d7..3dc19c6 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' ' check_head_differs_from fourth ' +test_expect_success 'cherry-pick skip empty' ' + git clean -fxd + git checkout -b empty fourth + git commit --allow-empty -m empty + test_commit ontop + git checkout -f master + git reset --hard fourth + git cherry-pick --skip-empty fourth..empty + echo ontop expected + git log --format=%s fourth..HEAD actual + test_cmp expected actual +' + test_done -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/45] sequencer: remove useless indentation
By using good ol' goto. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/sequencer.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/builtin/sequencer.c b/builtin/sequencer.c index e92e039..b2c8c94 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -390,7 +390,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; char *defmsg = NULL; struct strbuf msgbuf = STRBUF_INIT; - int res, unborn = 0; + int res, unborn = 0, allow; if (opts-no_commit) { /* @@ -535,14 +535,16 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) msg.subject); print_advice(res == 1, opts); rerere(opts-allow_rerere_auto); - } else { - int allow = allow_empty(opts, commit); + goto leave; + } + + allow = allow_empty(opts, commit); if (allow 0) return allow; if (!opts-no_commit) res = run_git_commit(defmsg, opts, allow); - } +leave: free_message(msg); free(defmsg); -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/45] read-cache: plug a few leaks
We are not freeing 'istate-cache' properly. We can't rely on 'initialized' to keep track of the 'istate-cache', because it doesn't really mean it's initialized. So assume it always has data, and free it before overwriting it. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- read-cache.c | 4 1 file changed, 4 insertions(+) diff --git a/read-cache.c b/read-cache.c index 5e30746..a1dd04d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1451,6 +1451,7 @@ int read_index_from(struct index_state *istate, const char *path) istate-version = ntohl(hdr-hdr_version); istate-cache_nr = ntohl(hdr-hdr_entries); istate-cache_alloc = alloc_nr(istate-cache_nr); + free(istate-cache); istate-cache = xcalloc(istate-cache_alloc, sizeof(*istate-cache)); istate-initialized = 1; @@ -1512,6 +1513,9 @@ int discard_index(struct index_state *istate) for (i = 0; i istate-cache_nr; i++) free(istate-cache[i]); + free(istate-cache); + istate-cache = NULL; + istate-cache_alloc = 0; resolve_undo_clear_index(istate); istate-cache_nr = 0; istate-cache_changed = 0; -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/45] build: trivial cleanup
There's no need to list again the prerequisites. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 34f1240..3b6cd5a 100644 --- a/Makefile +++ b/Makefile @@ -2068,13 +2068,13 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIB_FILE): $(LIB_OBJS) - $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $(LIB_OBJS) + $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ $(XDIFF_LIB): $(XDIFF_OBJS) - $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $(XDIFF_OBJS) + $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ $(VCSSVN_LIB): $(VCSSVN_OBJS) - $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $(VCSSVN_OBJS) + $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $^ export DEFAULT_EDITOR DEFAULT_PAGER -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/45] build: generate and clean test scripts
Commit 416fda6 (build: do not install git-remote-testpy) made it so git-remote-testpy is not only not installed, but also not generated by default, let's make sure tests scripts (NO_INSTALL) are generated as ell. Comments-by: Junio C Hamano gits...@pobox.com Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 03524d0..51c812d 100644 --- a/Makefile +++ b/Makefile @@ -2232,6 +2232,7 @@ endif test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X)) +all:: $(NO_INSTALL) all:: $(TEST_PROGRAMS) $(test_bindir_programs) bin-wrappers/%: wrap-for-bin.sh @@ -2482,7 +2483,7 @@ clean: profile-clean coverage-clean $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \ builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X - $(RM) $(TEST_PROGRAMS) + $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope* -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/45] revert/cherry-pick: add --skip option
Akin to 'am --skip' and 'rebase --skip'. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/git-cherry-pick.txt | 1 + Documentation/git-revert.txt | 1 + Documentation/sequencer.txt | 3 +++ builtin/revert.c | 6 ++ builtin/sequencer.c | 24 builtin/sequencer.h | 3 ++- t/t3510-cherry-pick-sequence.sh | 12 7 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index da0bd81..d95c63c 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] commit... 'git cherry-pick' --continue +'git cherry-pick' --skip 'git cherry-pick' --quit 'git cherry-pick' --abort diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt index 98a8e7a..52e146e 100644 --- a/Documentation/git-revert.txt +++ b/Documentation/git-revert.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] commit... 'git revert' --continue +'git revert' --skip 'git revert' --quit 'git revert' --abort diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt index 5747f44..df2d355 100644 --- a/Documentation/sequencer.txt +++ b/Documentation/sequencer.txt @@ -3,6 +3,9 @@ '.git/sequencer'. Can be used to continue after resolving conflicts in a failed cherry-pick or revert. +--skip:: + Skip the current commit, and then continue. + --quit:: Forget about the current operation in progress. Can be used to clear the sequencer state after a failed cherry-pick or diff --git a/builtin/revert.c b/builtin/revert.c index ec83748..d3d5600 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) int remove_state = 0; int contin = 0; int rollback = 0; + int skip = 0; struct option options[] = { OPT__QUIET(opts-quiet, N_(suppress progress reporting)), OPT_BOOLEAN(0, quit, remove_state, N_(end revert or cherry-pick sequence)), OPT_BOOLEAN(0, continue, contin, N_(resume revert or cherry-pick sequence)), OPT_BOOLEAN(0, abort, rollback, N_(cancel revert or cherry-pick sequence)), + OPT_BOOLEAN(0, skip, skip, N_(skip current commit in the sequence)), OPT_BOOLEAN('n', no-commit, opts-no_commit, N_(don't automatically commit)), OPT_BOOLEAN('e', edit, opts-edit, N_(edit the commit message)), OPT_NOOP_NOARG('r', NULL), @@ -164,6 +166,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) opts-subcommand = REPLAY_CONTINUE; else if (rollback) opts-subcommand = REPLAY_ROLLBACK; + else if (skip) + opts-subcommand = REPLAY_SKIP; else opts-subcommand = REPLAY_NONE; @@ -174,6 +178,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) this_operation = --quit; else if (opts-subcommand == REPLAY_CONTINUE) this_operation = --continue; + else if (opts-subcommand == REPLAY_SKIP) + this_operation = --skip; else { assert(opts-subcommand == REPLAY_ROLLBACK); this_operation = --abort; diff --git a/builtin/sequencer.c b/builtin/sequencer.c index 0f50942..2b1b30a 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -961,6 +961,28 @@ static int sequencer_continue(struct replay_opts *opts) return pick_commits(todo_list, opts); } +static int sequencer_skip(struct replay_opts *opts) +{ + const char *argv[4]; /* reset --hard HEAD + NULL */ + struct string_list merge_rr = STRING_LIST_INIT_DUP; + int ret; + + if (setup_rerere(merge_rr, 0) = 0) { + rerere_clear(merge_rr); + string_list_clear(merge_rr, 1); + } + + argv[0] = reset; + argv[1] = --hard; + argv[2] = HEAD; + argv[3] = NULL; + ret = run_command_v_opt(argv, RUN_GIT_CMD); + if (ret) + return ret; + + return sequencer_continue(opts); +} + static int single_pick(struct commit *cmit, struct replay_opts *opts) { setenv(GIT_REFLOG_ACTION, action_name(opts), 0); @@ -991,6 +1013,8 @@ int sequencer_pick_revisions(struct replay_opts *opts) return sequencer_rollback(opts); if (opts-subcommand == REPLAY_CONTINUE) return sequencer_continue(opts); + if (opts-subcommand == REPLAY_SKIP) + return sequencer_skip(opts);
[PATCH v4 15/45] builtin: add rewrite helper
So that we can load and store rewrites, as well as other operations on a list of rewritten commits. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Makefile | 1 + builtin/rewrite.c | 74 +++ builtin/rewrite.h | 18 ++ 3 files changed, 93 insertions(+) create mode 100644 builtin/rewrite.c create mode 100644 builtin/rewrite.h diff --git a/Makefile b/Makefile index 4c7bb88..a167e68 100644 --- a/Makefile +++ b/Makefile @@ -991,6 +991,7 @@ BUILTIN_OBJS += builtin/verify-tag.o BUILTIN_OBJS += builtin/write-tree.o BUILTIN_LIB_OBJS += builtin/sequencer.o +BUILTIN_LIB_OBJS += builtin/rewrite.o BUILTIN_LIB_OBJS += $(BUILTIN_OBJS) GITLIBS = $(LIB_FILE) $(XDIFF_LIB) diff --git a/builtin/rewrite.c b/builtin/rewrite.c new file mode 100644 index 000..2519352 --- /dev/null +++ b/builtin/rewrite.c @@ -0,0 +1,74 @@ +#include cache.h +#include rewrite.h + +void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to) +{ + struct rewritten_item *item; + if (list-nr + 1 = list-alloc) { + list-alloc += 32; + list-items = xrealloc(list-items, list-alloc * sizeof(*list-items)); + } + item = list-items[list-nr]; + hashcpy(item-from, from); + hashcpy(item-to, to); + list-nr++; +} + +int store_rewritten(struct rewritten *list, const char *file) +{ + static struct lock_file lock; + struct strbuf buf = STRBUF_INIT; + int fd, i, ret = 0; + + fd = hold_lock_file_for_update(lock, file, LOCK_DIE_ON_ERROR); + for (i = 0; i list-nr; i++) { + struct rewritten_item *item = list-items[i]; + strbuf_addf(buf, %s %s\n, sha1_to_hex(item-from), sha1_to_hex(item-to)); + } + if (write_in_full(fd, buf.buf, buf.len) 0) { + error(_(Could not write to %s), file); + ret = 1; + goto leave; + } + if (commit_lock_file(lock) 0) { + error(_(Error wrapping up %s.), file); + ret = 1; + goto leave; + } +leave: + strbuf_release(buf); + return ret; +} + +void load_rewritten(struct rewritten *list, const char *file) +{ + struct strbuf buf = STRBUF_INIT; + char *p; + int fd; + + fd = open(file, O_RDONLY); + if (fd 0) + return; + if (strbuf_read(buf, fd, 0) 0) { + close(fd); + strbuf_release(buf); + return; + } + close(fd); + + for (p = buf.buf; *p;) { + unsigned char from[20]; + unsigned char to[20]; + char *eol = strchrnul(p, '\n'); + if (eol - p != 81) + /* wrong size */ + break; + if (get_sha1_hex(p, from)) + break; + if (get_sha1_hex(p + 41, to)) + break; + add_rewritten(list, from, to); + p = *eol ? eol + 1 : eol; + } + strbuf_release(buf); +} diff --git a/builtin/rewrite.h b/builtin/rewrite.h new file mode 100644 index 000..09e7222 --- /dev/null +++ b/builtin/rewrite.h @@ -0,0 +1,18 @@ +#ifndef REWRITE_H +#define REWRITE_H + +struct rewritten_item { + unsigned char from[20]; + unsigned char to[20]; +}; + +struct rewritten { + struct rewritten_item *items; + unsigned int nr, alloc; +}; + +void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to); +int store_rewritten(struct rewritten *list, const char *file); +void load_rewritten(struct rewritten *list, const char *file); + +#endif -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 35/45] rebase: remove merge mode
The cherrypick mode does the job. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Makefile | 1 - contrib/completion/git-prompt.sh | 4 +- git-rebase--merge.sh | 151 --- git-rebase.sh| 13 +--- t/t3406-rebase-message.sh| 15 t/t9903-bash-prompt.sh | 2 +- 6 files changed, 3 insertions(+), 183 deletions(-) delete mode 100644 git-rebase--merge.sh diff --git a/Makefile b/Makefile index 4719979..609fa9e 100644 --- a/Makefile +++ b/Makefile @@ -475,7 +475,6 @@ SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--am SCRIPT_LIB += git-rebase--cherrypick SCRIPT_LIB += git-rebase--interactive -SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 3d10f21..5036795 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -352,12 +352,10 @@ __git_ps1 () total=$(cat $g/rebase-merge/end) if [ -f $g/rebase-merge/interactive ]; then r=|REBASE-i - elif [ -f $g/rebase-merge/cherrypick ]; then + else r=|REBASE step=$(cat $g/sequencer/rewritten | wc -l) let step+=1 - else - r=|REBASE-m fi else if [ -d $g/rebase-apply ]; then diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh deleted file mode 100644 index 16d1817..000 --- a/git-rebase--merge.sh +++ /dev/null @@ -1,151 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2010 Junio C Hamano. -# - -prec=4 - -read_state () { - onto_name=$(cat $state_dir/onto_name) - end=$(cat $state_dir/end) - msgnum=$(cat $state_dir/msgnum) -} - -continue_merge () { - test -d $state_dir || die $state_dir directory does not exist - - unmerged=$(git ls-files -u) - if test -n $unmerged - then - echo You still have unmerged paths in your index - echo did you forget to use git add? - die $resolvemsg - fi - - cmt=`cat $state_dir/current` - if ! git diff-index --quiet --ignore-submodules HEAD -- - then - if ! git commit --no-verify -C $cmt - then - echo Commit failed, please do not call \git commit\ - echo directly, but instead do one of the following: - die $resolvemsg - fi - if test -z $GIT_QUIET - then - printf Committed: %0${prec}d $msgnum - fi - echo $cmt $(git rev-parse HEAD^0) $state_dir/rewritten - else - if test -z $GIT_QUIET - then - printf Already applied: %0${prec}d $msgnum - fi - fi - test -z $GIT_QUIET - GIT_PAGER='' git log --format=%s -1 $cmt - - # onto the next patch: - msgnum=$(($msgnum + 1)) - echo $msgnum $state_dir/msgnum -} - -call_merge () { - cmt=$(cat $state_dir/cmt.$1) - echo $cmt $state_dir/current - hd=$(git rev-parse --verify HEAD) - cmt_name=$(git symbolic-ref HEAD 2 /dev/null || echo HEAD) - msgnum=$(cat $state_dir/msgnum) - eval GITHEAD_$cmt='${cmt_name##refs/heads/}~$(($end - $msgnum))' - eval GITHEAD_$hd='$onto_name' - export GITHEAD_$cmt GITHEAD_$hd - if test -n $GIT_QUIET - then - GIT_MERGE_VERBOSITY=1 export GIT_MERGE_VERBOSITY - fi - test -z $strategy strategy=recursive - eval 'git-merge-$strategy' $strategy_opts '$cmt^ -- $hd $cmt' - rv=$? - case $rv in - 0) - unset GITHEAD_$cmt GITHEAD_$hd - return - ;; - 1) - git rerere $allow_rerere_autoupdate - die $resolvemsg - ;; - 2) - echo Strategy: $strategy failed, try another 12 - die $resolvemsg - ;; - *) - die Unknown exit code ($rv) from command: \ - git-merge-$strategy $cmt^ -- HEAD $cmt - ;; - esac -} - -finish_rb_merge () { - move_to_original_branch - if test -s $state_dir/rewritten - then - git notes copy --for-rewrite=rebase $state_dir/rewritten - if test -x $GIT_DIR/hooks/post-rewrite - then - $GIT_DIR/hooks/post-rewrite rebase $state_dir/rewritten - fi - fi - say All done. -} - -case $action in -continue) - read_state - continue_merge - while test $msgnum
[PATCH v4 22/45] cherry-pick: remember rerere-autoupdate
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/sequencer.c | 8 1 file changed, 8 insertions(+) diff --git a/builtin/sequencer.c b/builtin/sequencer.c index a419387..ddd369f 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -753,6 +753,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data) opts-xopts[opts-xopts_nr++] = xstrdup(value); } else if (!strcmp(key, options.action-name)) git_config_string(opts-action_name, key, value); + else if (!strcmp(key, options.allow-rerere-auto)) + opts-allow_rerere_auto = git_config_int(key, value); else return error(_(Invalid key: %s), key); @@ -932,6 +934,12 @@ static void save_opts(struct replay_opts *opts) } if (opts-action_name) git_config_set_in_file(opts_file, options.action-name, opts-action_name); + if (opts-allow_rerere_auto) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(buf, %d, opts-allow_rerere_auto); + git_config_set_in_file(opts_file, options.allow-rerere-auto, buf.buf); + strbuf_release(buf); + } } static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 17/45] cherry-pick: don't store skipped commit
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/sequencer.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/sequencer.c b/builtin/sequencer.c index 24034f2..d40fda9 100644 --- a/builtin/sequencer.c +++ b/builtin/sequencer.c @@ -953,7 +953,7 @@ static int continue_single_pick(void) return run_command_v_opt(argv, RUN_GIT_CMD); } -static int sequencer_continue(struct replay_opts *opts) +static int sequencer_continue(struct replay_opts *opts, int skip) { struct commit_list *todo_list = NULL; @@ -973,7 +973,7 @@ static int sequencer_continue(struct replay_opts *opts) } if (index_differs_from(HEAD, 0)) return error_dirty_index(opts); - if (opts-action == REPLAY_PICK) { + if (opts-action == REPLAY_PICK !skip) { unsigned char to[20]; if (!read_ref(HEAD, to)) add_rewritten(rewritten, todo_list-item-object.sha1, to); @@ -1001,7 +1001,7 @@ static int sequencer_skip(struct replay_opts *opts) if (ret) return ret; - return sequencer_continue(opts); + return sequencer_continue(opts, 1); } static int single_pick(struct commit *cmit, struct replay_opts *opts) @@ -1033,7 +1033,7 @@ int sequencer_pick_revisions(struct replay_opts *opts) if (opts-subcommand == REPLAY_ROLLBACK) return sequencer_rollback(opts); if (opts-subcommand == REPLAY_CONTINUE) - return sequencer_continue(opts); + return sequencer_continue(opts, 0); if (opts-subcommand == REPLAY_SKIP) return sequencer_skip(opts); -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 30/45] rebase: cherry-pick: set correct action-name
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase--cherrypick.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index 241cda7..d36b0dc 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -45,7 +45,7 @@ else fi test -n $GIT_QUIET extra=$extra -q test -z $force_rebase extra=$extra --ff -git cherry-pick --no-merges --right-only --topo-order --do-walk $extra $revisions +git cherry-pick --no-merges --right-only --topo-order --do-walk --action-name rebase $extra $revisions ret=$? if test 0 != $ret -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 23/45] rebase: split the cherry-pick stuff
They do something completely different from 'git am', it belongs in a different file. No functional changes. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- .gitignore| 1 + Makefile | 1 + git-rebase--am.sh | 11 +-- git-rebase--cherrypick.sh | 30 ++ git-rebase.sh | 4 5 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 git-rebase--cherrypick.sh diff --git a/.gitignore b/.gitignore index 1640c3a..2a3dbae 100644 --- a/.gitignore +++ b/.gitignore @@ -113,6 +113,7 @@ /git-read-tree /git-rebase /git-rebase--am +/git-rebase--cherrypick /git-rebase--interactive /git-rebase--merge /git-receive-pack diff --git a/Makefile b/Makefile index a167e68..4719979 100644 --- a/Makefile +++ b/Makefile @@ -473,6 +473,7 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--am +SCRIPT_LIB += git-rebase--cherrypick SCRIPT_LIB += git-rebase--interactive SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 34e3102..6460028 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -19,15 +19,7 @@ esac test -n $rebase_root root_flag=--root ret=0 -if test -n $keep_empty -then - # we have to do this the hard way. git format-patch completely squashes - # empty commits and even if it didn't the format doesn't really lend - # itself well to recording empty patches. fortunately, cherry-pick - # makes this easy - git cherry-pick --allow-empty $revisions - ret=$? -else + rm -f $GIT_DIR/rebased-patches git format-patch -k --stdout --full-index --ignore-if-in-upstream \ @@ -63,7 +55,6 @@ else ret=$? rm -f $GIT_DIR/rebased-patches -fi if test 0 != $ret then diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh new file mode 100644 index 000..2c16995 --- /dev/null +++ b/git-rebase--cherrypick.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# +# Copyright (c) 2010 Junio C Hamano. +# + +case $action in +continue) + git am --resolved --resolvemsg=$resolvemsg + move_to_original_branch + return + ;; +skip) + git am --skip --resolvemsg=$resolvemsg + move_to_original_branch + return + ;; +esac + +test -n $rebase_root root_flag=--root + +git cherry-pick --allow-empty $revisions +ret=$? + +if test 0 != $ret +then + test -d $state_dir write_basic_state + return $ret +fi + +move_to_original_branch diff --git a/git-rebase.sh b/git-rebase.sh index d0c11a9..70762f1 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -413,6 +413,10 @@ elif test -n $do_merge then type=merge state_dir=$merge_dir +elif test -n $keep_empty +then + type=cherrypick + state_dir=$apply_dir else type=am state_dir=$apply_dir -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 26/45] rebase: cherry-pick: fix abort of cherry mode
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-rebase.sh b/git-rebase.sh index 4465daf..0937e2c 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -369,6 +369,7 @@ skip) run_specific_rebase ;; abort) + test $type == cherrypick git cherry-pick --abort git rerere clear read_basic_state case $head_name in -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html