* [PATCH 0/3] recursive support for ls-files @ 2016-09-24 0:13 Brandon Williams 2016-09-24 0:13 ` [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar Brandon Williams ` (4 more replies) 0 siblings, 5 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-24 0:13 UTC (permalink / raw) To: git; +Cc: Brandon Williams After looking at the feedback I rerolled a few things, in particular the --submodule_prefix option that existed to give a submodule context about where it had been invoked from. People didn't seem to like the idea of exposing this to the users (yet anyways) so I removed it as an option and instead have it being passed to a child process via an environment variable GIT_INTERNAL_SUBMODULE_PREFIX. This way we don't have to support anything to external users at the moment. Also fixed a bug (and added a test) for the -z options as pointed out by Jeff King. Brandon Williams (3): submodules: make submodule-prefix option an envvar ls-files: optionally recurse into submodules ls-files: add pathspec matching for submodules Documentation/git-ls-files.txt | 7 +- builtin/ls-files.c | 173 ++++++++++++++++++++------- cache.h | 1 + dir.c | 46 +++++++- dir.h | 4 + environment.c | 1 + t/t3007-ls-files-recurse-submodules.sh | 209 +++++++++++++++++++++++++++++++++ 7 files changed, 398 insertions(+), 43 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams @ 2016-09-24 0:13 ` Brandon Williams 2016-09-25 23:34 ` Junio C Hamano 2016-09-24 0:13 ` [PATCH 2/3 v3] ls-files: optionally recurse into submodules Brandon Williams ` (3 subsequent siblings) 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-24 0:13 UTC (permalink / raw) To: git; +Cc: Brandon Williams Add a submodule-prefix enviorment variable 'GIT_INTERNAL_SUBMODULE_PREFIX' which can be used by commands which have --recurse-submodule options. Signed-off-by: Brandon Williams <bmwill@google.com> --- cache.h | 1 + environment.c | 1 + 2 files changed, 2 insertions(+) diff --git a/cache.h b/cache.h index 3556326..ae88a35 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" +#define GIT_SUBMODULE_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUBMODULE_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" diff --git a/environment.c b/environment.c index ca72464..7380815 100644 --- a/environment.c +++ b/environment.c @@ -120,6 +120,7 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, + GIT_SUBMODULE_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar 2016-09-24 0:13 ` [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar Brandon Williams @ 2016-09-25 23:34 ` Junio C Hamano 0 siblings, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-25 23:34 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > Add a submodule-prefix enviorment variable environment? > 'GIT_INTERNAL_SUBMODULE_PREFIX' which can be used by commands which have > --recurse-submodule options. > > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > cache.h | 1 + > environment.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/cache.h b/cache.h > index 3556326..ae88a35 100644 > --- a/cache.h > +++ b/cache.h > @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) > #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" > #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" > #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" > +#define GIT_SUBMODULE_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUBMODULE_PREFIX" > #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" > #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" > #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" > diff --git a/environment.c b/environment.c > index ca72464..7380815 100644 > --- a/environment.c > +++ b/environment.c > @@ -120,6 +120,7 @@ const char * const local_repo_env[] = { > NO_REPLACE_OBJECTS_ENVIRONMENT, > GIT_REPLACE_REF_BASE_ENVIRONMENT, > GIT_PREFIX_ENVIRONMENT, > + GIT_SUBMODULE_PREFIX_ENVIRONMENT, > GIT_SHALLOW_FILE_ENVIRONMENT, > GIT_COMMON_DIR_ENVIRONMENT, > NULL ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 2/3 v3] ls-files: optionally recurse into submodules 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams 2016-09-24 0:13 ` [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar Brandon Williams @ 2016-09-24 0:13 ` Brandon Williams 2016-09-24 0:13 ` [PATCH 3/3 v3] ls-files: add pathspec matching for submodules Brandon Williams ` (2 subsequent siblings) 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-24 0:13 UTC (permalink / raw) To: git; +Cc: Brandon Williams Allow ls-files to recognize submodules in order to retrieve a list of files from a repository's submodules. This is done by forking off a process to recursively call ls-files on all submodules. Use environment variable `GIT_INTERNAL_SUBMODULE_PREFIX` to pass a path to the submodule which it can use to prepend to output or pathspec matching logic. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 7 ++- builtin/ls-files.c | 63 ++++++++++++++++++++++ t/t3007-ls-files-recurse-submodules.sh | 99 ++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 0d933ac..446209e 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -18,7 +18,8 @@ SYNOPSIS [--exclude-per-directory=<file>] [--exclude-standard] [--error-unmatch] [--with-tree=<tree-ish>] - [--full-name] [--abbrev] [--] [<file>...] + [--full-name] [--recurse-submodules] + [--abbrev] [--] [<file>...] DESCRIPTION ----------- @@ -137,6 +138,10 @@ a space) at the start of each line: option forces paths to be output relative to the project top directory. +--recurse-submodules:: + Recursively calls ls-files on each submodule in the repository. + Currently there is only support for the --cached mode. + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object lines, show only a partial prefix. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00ea91a..54ab765 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -14,6 +14,7 @@ #include "resolve-undo.h" #include "string-list.h" #include "pathspec.h" +#include "run-command.h" static int abbrev; static int show_deleted; @@ -28,6 +29,8 @@ static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; static int show_eol; +static int recurse_submodules; +static const char *submodule_prefix; static const char *prefix; static int max_prefix_len; @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) static void write_name(const char *name) { /* + * NEEDSWORK: To make this thread-safe, full_name would have to be owned + * by the caller. + * + * full_name get reused across output lines to minimize the allocation + * churn. + */ + static struct strbuf full_name = STRBUF_INIT; + if (submodule_prefix && *submodule_prefix) { + strbuf_reset(&full_name); + strbuf_addstr(&full_name, submodule_prefix); + strbuf_addstr(&full_name, name); + name = full_name.buf; + } + + /* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ @@ -152,6 +170,27 @@ static void show_killed_files(struct dir_struct *dir) } } +/** + * Recursively call ls-files on a submodule + */ +static void show_gitlink(const struct cache_entry *ce) +{ + struct child_process cp = CHILD_PROCESS_INIT; + int status; + + argv_array_push(&cp.args, "ls-files"); + argv_array_push(&cp.args, "--recurse-submodules"); + argv_array_pushf(&cp.env_array, "%s=%s%s/", + GIT_SUBMODULE_PREFIX_ENVIRONMENT, + submodule_prefix ? submodule_prefix : "", + ce->name); + cp.git_cmd = 1; + cp.dir = ce->name; + status = run_command(&cp); + if (status) + exit(status); +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { int len = max_prefix_len; @@ -163,6 +202,10 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) len, ps_matched, S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) return; + if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + show_gitlink(ce); + return; + } if (tag && *tag && show_valid_bit && (ce->ce_flags & CE_VALID)) { @@ -468,6 +511,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) { OPTION_SET_INT, 0, "full-name", &prefix_len, NULL, N_("make the output relative to the project top directory"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOL(0, "recurse-submodules", &recurse_submodules, + N_("recurse through submodules")), OPT_BOOL(0, "error-unmatch", &error_unmatch, N_("if any <file> is not in the index, treat this as an error")), OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"), @@ -519,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + submodule_prefix = getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT); + + if (recurse_submodules && + (show_stage || show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo || + show_valid_bit || show_tag || show_eol)) + die("ls-files --recurse-submodules can only be used in " + "--cached mode"); + + if (recurse_submodules && error_unmatch) + die("ls-files --recurse-submodules does not support " + "--error-unmatch"); + + if (recurse_submodules && argc) + die("ls-files --recurse-submodules does not support path " + "arguments"); + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD | PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh new file mode 100755 index 0000000..caf3815 --- /dev/null +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -0,0 +1,99 @@ +#!/bin/sh + +test_description='Test ls-files recurse-submodules feature + +This test verifies the recurse-submodules feature correctly lists files from +submodules. +' + +. ./test-lib.sh + +test_expect_success 'setup directory structure and submodules' ' + echo a >a && + mkdir b && + echo b >b/b && + git add a b && + git commit -m "add a and b" && + git init submodule && + echo c >submodule/c && + git -C submodule add c && + git -C submodule commit -m "add c" && + git submodule add ./submodule && + git commit -m "added submodule" +' + +test_expect_success 'ls-files correctly outputs files in submodule' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files does not output files not added to a repo' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + echo a >not_added && + echo b >b/not_added && + echo c >submodule/not_added && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files recurses more than 1 level' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/.gitmodules + submodule/c + submodule/subsub/d + EOF + + git init submodule/subsub && + echo d >submodule/subsub/d && + git -C submodule/subsub add d && + git -C submodule/subsub commit -m "add d" && + git -C submodule submodule add ./subsub && + git -C submodule commit -m "added subsub" && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules does not support using path arguments' ' + test_must_fail git ls-files --recurse-submodules b 2>actual && + test_i18ngrep "does not support path arguments" actual +' + +test_expect_success '--recurse-submodules does not support --error-unmatch' ' + test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual && + test_i18ngrep "does not support --error-unmatch" actual +' + +test_incompatible_with_recurse_submodules () { + test_expect_success "--recurse-submodules and $1 are incompatible" " + test_must_fail git ls-files --recurse-submodules $1 2>actual && + test_i18ngrep 'can only be used in --cached mode' actual + " +} + +test_incompatible_with_recurse_submodules -v +test_incompatible_with_recurse_submodules -t +test_incompatible_with_recurse_submodules --deleted +test_incompatible_with_recurse_submodules --modified +test_incompatible_with_recurse_submodules --others +test_incompatible_with_recurse_submodules --stage +test_incompatible_with_recurse_submodules --killed +test_incompatible_with_recurse_submodules --unmerged +test_incompatible_with_recurse_submodules --eol + +test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH 3/3 v3] ls-files: add pathspec matching for submodules 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams 2016-09-24 0:13 ` [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar Brandon Williams 2016-09-24 0:13 ` [PATCH 2/3 v3] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-24 0:13 ` Brandon Williams 2016-09-25 7:17 ` [PATCH 0/3] recursive support for ls-files Jeff King 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-24 0:13 UTC (permalink / raw) To: git; +Cc: Brandon Williams Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 134 ++++++++++++++++++++------------- dir.c | 46 ++++++++++- dir.h | 4 + t/t3007-ls-files-recurse-submodules.sh | 126 +++++++++++++++++++++++++++++-- 4 files changed, 248 insertions(+), 62 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 54ab765..8ecffd1 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -177,6 +177,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + int i; argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); @@ -184,6 +185,29 @@ static void show_gitlink(const struct cache_entry *ce) GIT_SUBMODULE_PREFIX_ENVIRONMENT, submodule_prefix ? submodule_prefix : "", ce->name); + /* add options */ + if (show_eol) + argv_array_push(&cp.args, "--eol"); + if (show_valid_bit) + argv_array_push(&cp.args, "-v"); + if (show_stage) + argv_array_push(&cp.args, "--stage"); + if (show_cached) + argv_array_push(&cp.args, "--cached"); + if (debug_mode) + argv_array_push(&cp.args, "--debug"); + if (line_terminator == '\0') + argv_array_push(&cp.args, "-z"); + + /* + * Pass in the original pathspec args. The submodule will be + * responsible for prepending the 'submodule_prefix' prior to comparing + * against the pathspec for matches. + */ + argv_array_push(&cp.args, "--"); + for (i = 0; i < pathspec.nr; i++) + argv_array_push(&cp.args, pathspec.items[i].original); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -193,57 +217,62 @@ static void show_gitlink(const struct cache_entry *ce) static void show_ce_entry(const char *tag, const struct cache_entry *ce) { + struct strbuf name = STRBUF_INIT; int len = max_prefix_len; + if (submodule_prefix) + strbuf_addstr(&name, submodule_prefix); + strbuf_addstr(&name, ce->name); 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, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) - return; - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && + submodule_path_match(&pathspec, name.buf, ps_matched)) { show_gitlink(ce); - return; - } + } else if (match_pathspec(&pathspec, name.buf, name.len, + len, ps_matched, + S_ISDIR(ce->ce_mode) || + S_ISGITLINK(ce->ce_mode))) { + if (tag && *tag && show_valid_bit && + (ce->ce_flags & CE_VALID)) { + static char alttag[4]; + memcpy(alttag, tag, 3); + if (isalpha(tag[0])) + alttag[0] = tolower(tag[0]); + else if (tag[0] == '?') + alttag[0] = '!'; + else { + alttag[0] = 'v'; + alttag[1] = tag[0]; + alttag[2] = ' '; + alttag[3] = 0; + } + tag = alttag; + } - if (tag && *tag && show_valid_bit && - (ce->ce_flags & CE_VALID)) { - static char alttag[4]; - memcpy(alttag, tag, 3); - if (isalpha(tag[0])) - alttag[0] = tolower(tag[0]); - else if (tag[0] == '?') - alttag[0] = '!'; - else { - alttag[0] = 'v'; - alttag[1] = tag[0]; - alttag[2] = ' '; - alttag[3] = 0; + if (!show_stage) { + fputs(tag, stdout); + } else { + printf("%s%06o %s %d\t", + tag, + ce->ce_mode, + find_unique_abbrev(ce->sha1,abbrev), + ce_stage(ce)); + } + write_eolinfo(ce, ce->name); + write_name(ce->name); + if (debug_mode) { + const struct stat_data *sd = &ce->ce_stat_data; + + printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); + printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); + printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); + printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); + printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); } - tag = alttag; } - if (!show_stage) { - fputs(tag, stdout); - } else { - printf("%s%06o %s %d\t", - tag, - ce->ce_mode, - find_unique_abbrev(ce->sha1,abbrev), - ce_stage(ce)); - } - write_eolinfo(ce, ce->name); - write_name(ce->name); - if (debug_mode) { - const struct stat_data *sd = &ce->ce_stat_data; - - printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); - printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); - printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); - printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); - printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); - } + strbuf_release(&name); } static void show_ru_info(void) @@ -568,27 +597,28 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) submodule_prefix = getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT); if (recurse_submodules && - (show_stage || show_deleted || show_others || show_unmerged || - show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol)) - die("ls-files --recurse-submodules can only be used in " - "--cached mode"); + (show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo)) + die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) die("ls-files --recurse-submodules does not support " "--error-unmatch"); - if (recurse_submodules && argc) - die("ls-files --recurse-submodules does not support path " - "arguments"); - 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); + /* + * Find common prefix for all pathspec's + * This is used as a performance optimization which unfortunately cannot + * be done when recursing into submodules + */ + if (recurse_submodules) + max_prefix = NULL; + else + 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 0ea235f..28e9736 100644 --- a/dir.c +++ b/dir.c @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen, return 1; } -#define DO_MATCH_EXCLUDE 1 -#define DO_MATCH_DIRECTORY 2 +#define DO_MATCH_EXCLUDE (1<<0) +#define DO_MATCH_DIRECTORY (1<<1) +#define DO_MATCH_SUBMODULE (1<<2) /* * Does 'match' match the given name? @@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, item->nowildcard_len - prefix)) return MATCHED_FNMATCH; + /* Perform checks to see if "name" is a super set of the pathspec */ + if (flags & DO_MATCH_SUBMODULE) { + /* name is a literal prefix of the pathspec */ + if ((namelen < matchlen) && + (match[namelen] == '/') && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_RECURSIVELY; + + /* name" doesn't match up to the first wild character */ + if (item->nowildcard_len < item->len && + ps_strncmp(item, match, name, + item->nowildcard_len - prefix)) + return 0; + + /* + * Here is where we would perform a wildmatch to check if + * "name" can be matched as a directory (or a prefix) against + * the pathspec. Since wildmatch doesn't have this capability + * at the present we have to punt and say that it is a match, + * potentially returning a false positive + * The submodules themselves will be able to perform more + * accurate matching to determine if the pathspec matches. + */ + return MATCHED_RECURSIVELY; + } + return 0; } @@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps, return negative ? 0 : positive; } +/** + * Check if a submodule is a superset of the pathspec + */ +int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen) +{ + int matched = do_match_pathspec(ps, submodule_name, + strlen(submodule_name), + 0, seen, + DO_MATCH_DIRECTORY | + DO_MATCH_SUBMODULE); + return matched; +} + int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix) diff --git a/dir.h b/dir.h index da1a858..97c83bb 100644 --- a/dir.h +++ b/dir.h @@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +extern int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen); + static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index caf3815..4a51d38 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + cat | tr "\n" "\0" >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -69,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' -test_expect_success '--recurse-submodules does not support using path arguments' ' - test_must_fail git ls-files --recurse-submodules b 2>actual && - test_i18ngrep "does not support path arguments" actual +test_expect_success '--recurse-submodules and pathspecs setup' ' + echo e >submodule/subsub/e.txt && + git -C submodule/subsub add e.txt && + git -C submodule/subsub commit -m "adding e.txt" && + echo f >submodule/f.TXT && + echo g >submodule/g.txt && + git -C submodule add f.TXT g.txt && + git -C submodule commit -m "add f and g" && + echo h >h.txt && + mkdir sib && + echo sib >sib/file && + git add h.txt sib/file && + git commit -m "add h and sib/file" && + git init sub && + echo sub >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git submodule add ./sub && + git commit -m "added sub" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub/d + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + cat actual && + git ls-files --recurse-submodules "*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules "*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sub/file + EOF + + git ls-files --recurse-submodules "sub" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su*/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su?/file" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sib/file + sub/file + EOF + + git ls-files --recurse-submodules "s??/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s???file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s*file" >actual && + test_cmp expect actual ' test_expect_success '--recurse-submodules does not support --error-unmatch' ' @@ -82,18 +196,14 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' ' test_incompatible_with_recurse_submodules () { test_expect_success "--recurse-submodules and $1 are incompatible" " test_must_fail git ls-files --recurse-submodules $1 2>actual && - test_i18ngrep 'can only be used in --cached mode' actual + test_i18ngrep 'unsupported mode' actual " } -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others -test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams ` (2 preceding siblings ...) 2016-09-24 0:13 ` [PATCH 3/3 v3] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-09-25 7:17 ` Jeff King 2016-09-25 16:32 ` Brandon Williams 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams 4 siblings, 1 reply; 67+ messages in thread From: Jeff King @ 2016-09-25 7:17 UTC (permalink / raw) To: Brandon Williams; +Cc: git On Fri, Sep 23, 2016 at 05:13:31PM -0700, Brandon Williams wrote: > After looking at the feedback I rerolled a few things, in particular the > --submodule_prefix option that existed to give a submodule context about where > it had been invoked from. People didn't seem to like the idea of exposing this > to the users (yet anyways) so I removed it as an option and instead have it > being passed to a child process via an environment variable > GIT_INTERNAL_SUBMODULE_PREFIX. This way we don't have to support anything to > external users at the moment. I think we can still have it as a command-line argument and declare it internal. It's not like environment variables cannot also be set by our callers. :) I don't mind it as an environment variable, though. In some ways it makes things easier. I just think "internal versus external" and the exact implementation are orthogonal. > Also fixed a bug (and added a test) for the -z options as pointed out by Jeff > King. Hmm. It is broken after patch 2, and then fixed in patch 3. Usually we'd try not to have a broken state in the history. It's less important in this case, because the breakage is not a regression (--recurse-submodules is a new feature, so you could consider it "not working" until the 3rd patch). But I think it's still a good rule to follow, because it makes the commits easier to review, look at later, etc. For that matter, I do not understand why options like "-s" get enabled in patch 3. I do not mind them starting as disabled in patch 2, but it seems like "pass along some known-safe options" should be its own patch somewhere between patches 2 and 3. There are some other options that are ignored (neither disabled nor passed along to children). Most of them are related to exclusions, which I _think_ are safe to ignore (they do not do anything interesting unless you specify "-o", which is explicitly disabled). I'm not sure about --with-tree, though (or what it would even mean in the context of recursing). -Peff ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-25 7:17 ` [PATCH 0/3] recursive support for ls-files Jeff King @ 2016-09-25 16:32 ` Brandon Williams 2016-09-25 18:38 ` Junio C Hamano 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-25 16:32 UTC (permalink / raw) To: Jeff King; +Cc: git On 09/25, Jeff King wrote: > On Fri, Sep 23, 2016 at 05:13:31PM -0700, Brandon Williams wrote: > > > After looking at the feedback I rerolled a few things, in particular the > > --submodule_prefix option that existed to give a submodule context about where > > it had been invoked from. People didn't seem to like the idea of exposing this > > to the users (yet anyways) so I removed it as an option and instead have it > > being passed to a child process via an environment variable > > GIT_INTERNAL_SUBMODULE_PREFIX. This way we don't have to support anything to > > external users at the moment. > > I think we can still have it as a command-line argument and declare it > internal. It's not like environment variables cannot also be set by our > callers. :) > > I don't mind it as an environment variable, though. In some ways it > makes things easier. I just think "internal versus external" and the > exact implementation are orthogonal. > We may still want it to be an option at some point in the future. This way we can revisit making it an option once we know more about the other uses it could have (aside from just being for submodules as someone suggested). > > Also fixed a bug (and added a test) for the -z options as pointed out by Jeff > > King. > > Hmm. It is broken after patch 2, and then fixed in patch 3. Usually we'd > try not to have a broken state in the history. It's less important in > this case, because the breakage is not a regression > (--recurse-submodules is a new feature, so you could consider it "not > working" until the 3rd patch). But I think it's still a good rule to > follow, because it makes the commits easier to review, look at later, > etc. > > For that matter, I do not understand why options like "-s" get enabled > in patch 3. I do not mind them starting as disabled in patch 2, but it > seems like "pass along some known-safe options" should be its own patch > somewhere between patches 2 and 3. I'll keep that in mind for future patches. I figured that since it was fixed in the end that would be fine but if things shouldn't be broken at any state in the patch series I'll make sure to not do that in the future. > There are some other options that are ignored (neither disabled nor > passed along to children). Most of them are related to exclusions, which > I _think_ are safe to ignore (they do not do anything interesting unless > you specify "-o", which is explicitly disabled). I'm not sure about > --with-tree, though (or what it would even mean in the context of > recursing). These other features that are disabled now could be enabled in a future patch. You're right though I'd have to think about the --with-tree option a bit more and what it would mean with submodules. -Brandon ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-25 16:32 ` Brandon Williams @ 2016-09-25 18:38 ` Junio C Hamano 2016-09-26 17:04 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-25 18:38 UTC (permalink / raw) To: Brandon Williams; +Cc: Jeff King, git Brandon Williams <bmwill@google.com> writes: > On 09/25, Jeff King wrote: >> On Fri, Sep 23, 2016 at 05:13:31PM -0700, Brandon Williams wrote: >> >> > After looking at the feedback I rerolled a few things, in particular the >> > --submodule_prefix option that existed to give a submodule context about where >> > it had been invoked from. People didn't seem to like the idea of exposing this >> > to the users (yet anyways) so I removed it as an option and instead have it >> > being passed to a child process via an environment variable >> > GIT_INTERNAL_SUBMODULE_PREFIX. This way we don't have to support anything to >> > external users at the moment. >> >> I think we can still have it as a command-line argument and declare it >> internal. It's not like environment variables cannot also be set by our >> callers. :) >> >> I don't mind it as an environment variable, though. In some ways it >> makes things easier. I just think "internal versus external" and the >> exact implementation are orthogonal. > > We may still want it to be an option at some point in the future. This > way we can revisit making it an option once we know more about the other > uses it could have (aside from just being for submodules as someone > suggested). I do not think it makes too much of a difference between environment and command line option. We need an update to the "git" potty to say "you told me to use the submodule-prefix feature, but this subcommand is not prepared to accept it (yet)" and cause it to error out either way, which would mean that a series that introduces the feature needs to touch "git.c" anyway, so I would have expected us to add command line option first, simply because "git.c" is where it happens, optionally with the support for the environment variable, not the other way around. >> > Also fixed a bug (and added a test) for the -z options as pointed out by Jeff >> > King. >> >> Hmm. It is broken after patch 2, and then fixed in patch 3. Usually we'd >> try not to have a broken state in the history. It's less important in >> this case, because the breakage is not a regression >> (--recurse-submodules is a new feature, so you could consider it "not >> working" until the 3rd patch). But I think it's still a good rule to >> follow, because it makes the commits easier to review, look at later, >> etc. >> >> For that matter, I do not understand why options like "-s" get enabled >> in patch 3. I do not mind them starting as disabled in patch 2, but it >> seems like "pass along some known-safe options" should be its own patch >> somewhere between patches 2 and 3. Yes, exactly. An obvious lazy way out to avoid breakage-in-the-middle and make incremental progress would be to squash everything into one patch, but we should and we should be able to do better. I'd imagine this three-patch series would be more pleasant for future readers if it were structured like: [1/3] introduces the submodule-prefix as a global feature; at the least it needs a way to invoke (either an environment, or an option to "git" potty, or both) and prevent mistakes by erroring out when it is attempted to call a subcommand that does not support the feature (yet). [2/3] adds the --recurse-submodule feature in a limited form to "ls-files". I'd suggest for this step to pass through all options and arguments that are safe and reasonably useful to pass through without needing anything more than "ah, this option was given, so let's stuff it to the argv-array". An attempt to give things that are not yet passed through until 3/3 to lead to an error that says it is not allowed (yet). [3-N] each of the remaining steps after 3/N adds support for one more thing to be passed that 2/3 refrained from doing, by doing more than just "pass it in argv-array", and then remove the "not yet supported" error that added by 2/3 for that one thing. The first of these "more things" would be to support pathspecs as the receiving side would need code changes for the matching logic. There may be more, or there may be nothing else that requires 4/N, 5/N, etc. >> There are some other options that are ignored (neither disabled nor >> passed along to children). Most of them are related to exclusions, which >> I _think_ are safe to ignore (they do not do anything interesting unless >> you specify "-o", which is explicitly disabled). Sure. I agree that some options fall outside of "safe and reasonably useful to pass through" criteria and it is OK not to support passing them through. I however think we should detect mistakes in the caller, though. I wonder if this will be common enough in the future that we are better off adding a bit or two to the parse-options infrastructure. Thinking out loud, would an enhancement like this be sufficient for this particular series, and still useful in more general cases? - allow the caller of parse_options() to mark each entry with a handful of bits with no meaning to parse-options API. "ls-files" may use this mechanism to mark options that to be passed through and options that must be rejected when --recurse-submodules is asked. - parse_options() will mark each entry as "this option was given" while doing its work. Note that "--no-foo" counts as "option 'foo' was given". With these, after your call to parse_options() returns, you could notice that recurse-submodules was asked for by inspecting your own "static int recurse_submodules" global variable, and then iterate over builtin_ls_files_options[] array yourself to see if an option that you marked with "must be rejected while recursing" bit using mechanism #1 was seen by parse_options() by relying on mechanism #2. You could also add a new helper macro to parse-options API that iterates over the options[] array, i.e. for_each_parse_options_array(opt) { if (!opt_was_used(opt)) continue; if (opt_custom_bit(opt) & NOT_IN_RECURSIVE) die("'%s' cannot be used with --recurse-submodules", parse_options_name(opt)) } or something like that, but we'd need to gain experience with the mechanism #1 and #2 first before deciding if such a helper is useful (iow, I do not think we need it from day one, if we go this route). ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-25 18:38 ` Junio C Hamano @ 2016-09-26 17:04 ` Brandon Williams 2016-09-26 18:17 ` Junio C Hamano 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-26 17:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git On 09/25, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > On 09/25, Jeff King wrote: > >> On Fri, Sep 23, 2016 at 05:13:31PM -0700, Brandon Williams wrote: > >> > >> > After looking at the feedback I rerolled a few things, in particular the > >> > --submodule_prefix option that existed to give a submodule context about where > >> > it had been invoked from. People didn't seem to like the idea of exposing this > >> > to the users (yet anyways) so I removed it as an option and instead have it > >> > being passed to a child process via an environment variable > >> > GIT_INTERNAL_SUBMODULE_PREFIX. This way we don't have to support anything to > >> > external users at the moment. > >> > >> I think we can still have it as a command-line argument and declare it > >> internal. It's not like environment variables cannot also be set by our > >> callers. :) > >> > >> I don't mind it as an environment variable, though. In some ways it > >> makes things easier. I just think "internal versus external" and the > >> exact implementation are orthogonal. > > > > We may still want it to be an option at some point in the future. This > > way we can revisit making it an option once we know more about the other > > uses it could have (aside from just being for submodules as someone > > suggested). > > I do not think it makes too much of a difference between environment > and command line option. We need an update to the "git" potty to > say "you told me to use the submodule-prefix feature, but this > subcommand is not prepared to accept it (yet)" and cause it to error > out either way, which would mean that a series that introduces the > feature needs to touch "git.c" anyway, so I would have expected us > to add command line option first, simply because "git.c" is where it > happens, optionally with the support for the environment variable, > not the other way around. In a previous email you mentioned that this feature should be completely hidden from users, which is why I removed the command line option for this latest series. If that isn't what you intended that I can definitely add the option to git.c. And you would rather we perform the checking in git.c to see if a subcommand supports the prefix versus silently ignoring it if it hasn't? I'm assuming this checking would also be done in git.c? > > >> > Also fixed a bug (and added a test) for the -z options as pointed out by Jeff > >> > King. > >> > >> Hmm. It is broken after patch 2, and then fixed in patch 3. Usually we'd > >> try not to have a broken state in the history. It's less important in > >> this case, because the breakage is not a regression > >> (--recurse-submodules is a new feature, so you could consider it "not > >> working" until the 3rd patch). But I think it's still a good rule to > >> follow, because it makes the commits easier to review, look at later, > >> etc. > >> > >> For that matter, I do not understand why options like "-s" get enabled > >> in patch 3. I do not mind them starting as disabled in patch 2, but it > >> seems like "pass along some known-safe options" should be its own patch > >> somewhere between patches 2 and 3. > > Yes, exactly. > > An obvious lazy way out to avoid breakage-in-the-middle and make > incremental progress would be to squash everything into one patch, > but we should and we should be able to do better. > > I'd imagine this three-patch series would be more pleasant for > future readers if it were structured like: > > [1/3] introduces the submodule-prefix as a global feature; at the > least it needs a way to invoke (either an environment, or an > option to "git" potty, or both) and prevent mistakes by > erroring out when it is attempted to call a subcommand that > does not support the feature (yet). > > [2/3] adds the --recurse-submodule feature in a limited form to > "ls-files". I'd suggest for this step to pass through all > options and arguments that are safe and reasonably useful > to pass through without needing anything more than "ah, this > option was given, so let's stuff it to the argv-array". An > attempt to give things that are not yet passed through until > 3/3 to lead to an error that says it is not allowed (yet). > > [3-N] each of the remaining steps after 3/N adds support for one > more thing to be passed that 2/3 refrained from doing, by > doing more than just "pass it in argv-array", and then remove > the "not yet supported" error that added by 2/3 for that one > thing. The first of these "more things" would be to support > pathspecs as the receiving side would need code changes for > the matching logic. There may be more, or there may be > nothing else that requires 4/N, 5/N, etc. I can do another rework and structure the patch series more inline with what you are suggesting here. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-26 17:04 ` Brandon Williams @ 2016-09-26 18:17 ` Junio C Hamano 2016-09-26 18:38 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-26 18:17 UTC (permalink / raw) To: Brandon Williams; +Cc: Jeff King, git Brandon Williams <bmwill@google.com> writes: > In a previous email you mentioned that this feature should be completely > hidden from users, which is why I removed the command line option for > this latest series. I may have said something like that; I do not recall, though, so a more accurate description might be "I may have said something that can be (mis)interpreted like so". Sorry for the confusion. In any case, an environment is not "completely hidden from users", so it is not fundamentallly different from a command line option anyway ;-) > If that isn't what you intended that I can > definitely add the option to git.c. And you would rather we perform the > checking in git.c to see if a subcommand supports the prefix versus > silently ignoring it if it hasn't? I'm assuming this checking would > also be done in git.c? I actually do not care strongly _where_ the check happens. It was just that in the subcommand dispatcher would be the single place that is easiest-to-implement to perform that check, that made me suggest that. We already have various bits like NEEDS_WORK_TREE, RUN_SETUP, etc. so REJECT_EXTERNAL_PREFIX (or whatever its name be; I do not offhand recall current proposal) bit would fit there naturally, I would think. Of course, non-built-in commands need to protect themselves separately, if they want to. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-26 18:17 ` Junio C Hamano @ 2016-09-26 18:38 ` Brandon Williams 2016-09-26 18:48 ` Junio C Hamano 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-26 18:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git On 09/26, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > In a previous email you mentioned that this feature should be completely > > hidden from users, which is why I removed the command line option for > > this latest series. > > I may have said something like that; I do not recall, though, so a > more accurate description might be "I may have said something that > can be (mis)interpreted like so". Sorry for the confusion. > > In any case, an environment is not "completely hidden from users", > so it is not fundamentallly different from a command line option > anyway ;-) No worries, I'm still trying to get a hang of all this :) True, that wouldn't be completely hidden either. > > If that isn't what you intended that I can > > definitely add the option to git.c. And you would rather we perform the > > checking in git.c to see if a subcommand supports the prefix versus > > silently ignoring it if it hasn't? I'm assuming this checking would > > also be done in git.c? > > I actually do not care strongly _where_ the check happens. It was > just that in the subcommand dispatcher would be the single place > that is easiest-to-implement to perform that check, that made me > suggest that. We already have various bits like NEEDS_WORK_TREE, > RUN_SETUP, etc. so REJECT_EXTERNAL_PREFIX (or whatever its name be; > I do not offhand recall current proposal) bit would fit there > naturally, I would think. Of course, non-built-in commands need to > protect themselves separately, if they want to. That makes sense. I have an idea of where the check could be made. And with those flags it may make sense to have the flag be an indicator that the builtin is ready for submodule type commands ie SUPPORTS_SUBMODULES or something along those lines. That way we only need to add the flag to each command as we go (instead of all commands which don't support it) and just make sure that the flag is set if the submodule prefix option is being used. How does that sound? -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 0/3] recursive support for ls-files 2016-09-26 18:38 ` Brandon Williams @ 2016-09-26 18:48 ` Junio C Hamano 0 siblings, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-26 18:48 UTC (permalink / raw) To: Brandon Williams; +Cc: Jeff King, git Brandon Williams <bmwill@google.com> writes: > or something along those lines. That way we only need to add the flag > to each command as we go ... Sounds good. Thanks. ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 0/4 v4] recursive support for ls-files 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams ` (3 preceding siblings ...) 2016-09-25 7:17 ` [PATCH 0/3] recursive support for ls-files Jeff King @ 2016-09-26 22:46 ` Brandon Williams 2016-09-26 22:46 ` [PATCH 1/4 v4] submodules: make submodule-prefix option Brandon Williams ` (4 more replies) 4 siblings, 5 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-26 22:46 UTC (permalink / raw) To: git; +Cc: Brandon Williams A couple things have changed in v4: - Restructured the patch series to prevent a breakage mid-way. - Added an additional patch in the middle to pass through safe options. This way the series is structured in a more coherent manor. - Added --submodule-prefix to top-level git.c Hopefully this series addresses some of issues brought up in v3 Brandon Williams (4): submodules: make submodule-prefix option ls-files: optionally recurse into submodules ls-files: pass through safe options for --recurse-submodules ls-files: add pathspec matching for submodules Documentation/git-ls-files.txt | 7 +- Documentation/git.txt | 5 + builtin/ls-files.c | 187 ++++++++++++++++++++++------- cache.h | 1 + dir.c | 46 +++++++- dir.h | 4 + environment.c | 1 + git.c | 21 +++- t/t3007-ls-files-recurse-submodules.sh | 209 +++++++++++++++++++++++++++++++++ 9 files changed, 437 insertions(+), 44 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 1/4 v4] submodules: make submodule-prefix option 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams @ 2016-09-26 22:46 ` Brandon Williams 2016-09-27 18:17 ` Junio C Hamano 2016-09-26 22:46 ` [PATCH 2/4 v4] ls-files: optionally recurse into submodules Brandon Williams ` (3 subsequent siblings) 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-26 22:46 UTC (permalink / raw) To: git; +Cc: Brandon Williams Add a submodule-prefix environment variable 'GIT_INTERNAL_SUBMODULE_PREFIX' which can be used by commands which have --recurse-submodule options to give context to submodules about how they were invoked. This option is only allowed for builtins which have submodule support. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git.txt | 5 +++++ cache.h | 1 + environment.c | 1 + git.c | 19 +++++++++++++++++++ 4 files changed, 26 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7913fc2..d29967a 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -13,6 +13,7 @@ SYNOPSIS [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>] + [--submodule-prefix=<path>] <command> [<args>] DESCRIPTION @@ -601,6 +602,10 @@ foo.bar= ...`) sets `foo.bar` to the empty string. details. Equivalent to setting the `GIT_NAMESPACE` environment variable. +--submodule-prefix=<path>:: + Set a prefix which gives submodules context about the superproject that + invoked it. Only allowed for commands which support submodules. + --bare:: Treat the repository as a bare repository. If GIT_DIR environment is not set, it is set to the current working diff --git a/cache.h b/cache.h index 3556326..ae88a35 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" +#define GIT_SUBMODULE_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUBMODULE_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" diff --git a/environment.c b/environment.c index ca72464..7380815 100644 --- a/environment.c +++ b/environment.c @@ -120,6 +120,7 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, + GIT_SUBMODULE_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL diff --git a/git.c b/git.c index 1c61151..b2b096a 100644 --- a/git.c +++ b/git.c @@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); if (envchanged) *envchanged = 1; + } else if (!strcmp(cmd, "--submodule-prefix")) { + if (*argc < 2) { + fprintf(stderr, "No prefix given for --submodule-prefix.\n" ); + usage(git_usage_string); + } + setenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT, (*argv)[1], 1); + if (envchanged) + *envchanged = 1; + (*argv)++; + (*argc)--; + } else if (skip_prefix(cmd, "--submodule-prefix=", &cmd)) { + setenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT, cmd, 1); + if (envchanged) + *envchanged = 1; } else if (!strcmp(cmd, "--bare")) { char *cwd = xgetcwd(); is_bare_repository_cfg = 1; @@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv) * RUN_SETUP for reading from the configuration file. */ #define NEED_WORK_TREE (1<<3) +#define SUPPORT_SUBMODULES (1<<4) struct cmd_struct { const char *cmd; @@ -344,6 +359,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) } commit_pager_choice(); + if (!help && (getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT) && + !(p->option & SUPPORT_SUBMODULES))) + die("%s doesn't support submodules", p->cmd); + if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 1/4 v4] submodules: make submodule-prefix option 2016-09-26 22:46 ` [PATCH 1/4 v4] submodules: make submodule-prefix option Brandon Williams @ 2016-09-27 18:17 ` Junio C Hamano 2016-09-27 20:29 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 18:17 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > +--submodule-prefix=<path>:: > + Set a prefix which gives submodules context about the superproject that > + invoked it. Only allowed for commands which support submodules. This, and also the message in die(), uses a phrase "support submodules", but it is unclear what it exactly means to the end users and readers. A "ls-files" that is recursively run as an implementation detail of the "grep --recurse-submodules" would be taught to support this option with this series. Who is supporting submodules in that context? I'd imagine (close to) 100% of the people would say it is "grep" that is supporting submodules, not "ls-files", but what this paragraph and die() message want to express by the phrase "support submodules" is the fact that "ls-files" knows how to react to "--submodule-prefix" option. I'd suggest not to worry too much about this phrasing at this point, until we figure out exactly how we want to present these to end users. For now, perhaps drop the second sentence and replace it with "The end-users are not expected to use this option" or something like that? > diff --git a/git.c b/git.c > index 1c61151..b2b096a 100644 > --- a/git.c > +++ b/git.c > @@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) > setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); > if (envchanged) > *envchanged = 1; > + } else if (!strcmp(cmd, "--submodule-prefix")) { > + if (*argc < 2) { > + fprintf(stderr, "No prefix given for --submodule-prefix.\n" ); > + usage(git_usage_string); > + } > + setenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT, (*argv)[1], 1); > + if (envchanged) > + *envchanged = 1; > + (*argv)++; > + (*argc)--; > + } else if (skip_prefix(cmd, "--submodule-prefix=", &cmd)) { > + setenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT, cmd, 1); > + if (envchanged) > + *envchanged = 1; > } else if (!strcmp(cmd, "--bare")) { > char *cwd = xgetcwd(); > is_bare_repository_cfg = 1; > @@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv) > * RUN_SETUP for reading from the configuration file. > */ > #define NEED_WORK_TREE (1<<3) > +#define SUPPORT_SUBMODULES (1<<4) > > struct cmd_struct { > const char *cmd; > @@ -344,6 +359,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) > } > commit_pager_choice(); > > + if (!help && (getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT) && > + !(p->option & SUPPORT_SUBMODULES))) > + die("%s doesn't support submodules", p->cmd); s/submodules/submodule-prefix/ at least. > if (!help && p->option & NEED_WORK_TREE) > setup_work_tree(); ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 1/4 v4] submodules: make submodule-prefix option 2016-09-27 18:17 ` Junio C Hamano @ 2016-09-27 20:29 ` Brandon Williams 2016-09-27 20:35 ` Junio C Hamano 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > +--submodule-prefix=<path>:: > > + Set a prefix which gives submodules context about the superproject that > > + invoked it. Only allowed for commands which support submodules. > > This, and also the message in die(), uses a phrase "support > submodules", but it is unclear what it exactly means to the end > users and readers. > > A "ls-files" that is recursively run as an implementation detail of > the "grep --recurse-submodules" would be taught to support this > option with this series. Who is supporting submodules in that > context? > > I'd imagine (close to) 100% of the people would say it is "grep" > that is supporting submodules, not "ls-files", but what this > paragraph and die() message want to express by the phrase "support > submodules" is the fact that "ls-files" knows how to react to > "--submodule-prefix" option. > > I'd suggest not to worry too much about this phrasing at this point, > until we figure out exactly how we want to present these to end > users. For now, perhaps drop the second sentence and replace it > with "The end-users are not expected to use this option" or > something like that? K can do. The intention is that each command has to do whatever internal rework needed so that it understands how to interact with subomodules (be that calling another command or internally supporting it). > > + die("%s doesn't support submodules", p->cmd); > > s/submodules/submodule-prefix/ at least. So should the #define be SUPPORT_SUBMODULE_PREFIX instead? That may be too narrow minded and not looking toward future submodule options support but I'm not sure. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 1/4 v4] submodules: make submodule-prefix option 2016-09-27 20:29 ` Brandon Williams @ 2016-09-27 20:35 ` Junio C Hamano 2016-09-27 20:43 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 20:35 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: >> s/submodules/submodule-prefix/ at least. > > So should the #define be SUPPORT_SUBMODULE_PREFIX instead? That may be > too narrow minded and not looking toward future submodule options > support but I'm not sure. I am not convinced that this prefix needs to be tied/limited to submodule, at least not yet, though. I view it as a prefix that points from above the repository's top, of which submodule support may be a good sample user, but the caller may not necessarily be doing or interested in "submodule support". That's also part of figuring out how we want define the semantics of this thing and how we want to present it to the end-users, I guess, so we may have to rename it when we know more, but that's OK. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 1/4 v4] submodules: make submodule-prefix option 2016-09-27 20:35 ` Junio C Hamano @ 2016-09-27 20:43 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > >> s/submodules/submodule-prefix/ at least. > > > > So should the #define be SUPPORT_SUBMODULE_PREFIX instead? That may be > > too narrow minded and not looking toward future submodule options > > support but I'm not sure. > > I am not convinced that this prefix needs to be tied/limited to > submodule, at least not yet, though. I view it as a prefix that > points from above the repository's top, of which submodule support > may be a good sample user, but the caller may not necessarily be > doing or interested in "submodule support". > > That's also part of figuring out how we want define the semantics of > this thing and how we want to present it to the end-users, I guess, > so we may have to rename it when we know more, but that's OK. K we can leave the name as is for now then. Thankfully it should be a simple name change since it only lives in 1 file. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 2/4 v4] ls-files: optionally recurse into submodules 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams 2016-09-26 22:46 ` [PATCH 1/4 v4] submodules: make submodule-prefix option Brandon Williams @ 2016-09-26 22:46 ` Brandon Williams 2016-09-27 18:29 ` Junio C Hamano 2016-09-26 22:46 ` [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules Brandon Williams ` (2 subsequent siblings) 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-26 22:46 UTC (permalink / raw) To: git; +Cc: Brandon Williams Allow ls-files to recognize submodules in order to retrieve a list of files from a repository's submodules. This is done by forking off a process to recursively call ls-files on all submodules. Use top-level --submodule_prefix option to pass a path to the submodule which it can use to prepend to output or pathspec matching logic. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 7 +- builtin/ls-files.c | 143 ++++++++++++++++++++++++--------- git.c | 2 +- t/t3007-ls-files-recurse-submodules.sh | 100 +++++++++++++++++++++++ 4 files changed, 212 insertions(+), 40 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 0d933ac..446209e 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -18,7 +18,8 @@ SYNOPSIS [--exclude-per-directory=<file>] [--exclude-standard] [--error-unmatch] [--with-tree=<tree-ish>] - [--full-name] [--abbrev] [--] [<file>...] + [--full-name] [--recurse-submodules] + [--abbrev] [--] [<file>...] DESCRIPTION ----------- @@ -137,6 +138,10 @@ a space) at the start of each line: option forces paths to be output relative to the project top directory. +--recurse-submodules:: + Recursively calls ls-files on each submodule in the repository. + Currently there is only support for the --cached mode. + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object lines, show only a partial prefix. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00ea91a..d4bfc60 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -14,6 +14,7 @@ #include "resolve-undo.h" #include "string-list.h" #include "pathspec.h" +#include "run-command.h" static int abbrev; static int show_deleted; @@ -28,6 +29,8 @@ static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; static int show_eol; +static int recurse_submodules; +static const char *submodule_prefix; static const char *prefix; static int max_prefix_len; @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) static void write_name(const char *name) { /* + * NEEDSWORK: To make this thread-safe, full_name would have to be owned + * by the caller. + * + * full_name get reused across output lines to minimize the allocation + * churn. + */ + static struct strbuf full_name = STRBUF_INIT; + if (submodule_prefix && *submodule_prefix) { + strbuf_reset(&full_name); + strbuf_addstr(&full_name, submodule_prefix); + strbuf_addstr(&full_name, name); + name = full_name.buf; + } + + /* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ @@ -152,55 +170,84 @@ static void show_killed_files(struct dir_struct *dir) } } +/** + * Recursively call ls-files on a submodule + */ +static void show_gitlink(const struct cache_entry *ce) +{ + struct child_process cp = CHILD_PROCESS_INIT; + int status; + + argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/", + submodule_prefix ? submodule_prefix : "", + ce->name); + argv_array_push(&cp.args, "ls-files"); + argv_array_push(&cp.args, "--recurse-submodules"); + + cp.git_cmd = 1; + cp.dir = ce->name; + status = run_command(&cp); + if (status) + exit(status); +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { + struct strbuf name = STRBUF_INIT; int len = max_prefix_len; + if (submodule_prefix) + strbuf_addstr(&name, submodule_prefix); + strbuf_addstr(&name, ce->name); 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, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) - return; + if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + show_gitlink(ce); + } else if (match_pathspec(&pathspec, name.buf, name.len, + len, ps_matched, + S_ISDIR(ce->ce_mode) || + S_ISGITLINK(ce->ce_mode))) { + if (tag && *tag && show_valid_bit && + (ce->ce_flags & CE_VALID)) { + static char alttag[4]; + memcpy(alttag, tag, 3); + if (isalpha(tag[0])) + alttag[0] = tolower(tag[0]); + else if (tag[0] == '?') + alttag[0] = '!'; + else { + alttag[0] = 'v'; + alttag[1] = tag[0]; + alttag[2] = ' '; + alttag[3] = 0; + } + tag = alttag; + } - if (tag && *tag && show_valid_bit && - (ce->ce_flags & CE_VALID)) { - static char alttag[4]; - memcpy(alttag, tag, 3); - if (isalpha(tag[0])) - alttag[0] = tolower(tag[0]); - else if (tag[0] == '?') - alttag[0] = '!'; - else { - alttag[0] = 'v'; - alttag[1] = tag[0]; - alttag[2] = ' '; - alttag[3] = 0; + if (!show_stage) { + fputs(tag, stdout); + } else { + printf("%s%06o %s %d\t", + tag, + ce->ce_mode, + find_unique_abbrev(ce->sha1,abbrev), + ce_stage(ce)); + } + write_eolinfo(ce, ce->name); + write_name(ce->name); + if (debug_mode) { + const struct stat_data *sd = &ce->ce_stat_data; + + printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); + printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); + printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); + printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); + printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); } - tag = alttag; } - if (!show_stage) { - fputs(tag, stdout); - } else { - printf("%s%06o %s %d\t", - tag, - ce->ce_mode, - find_unique_abbrev(ce->sha1,abbrev), - ce_stage(ce)); - } - write_eolinfo(ce, ce->name); - write_name(ce->name); - if (debug_mode) { - const struct stat_data *sd = &ce->ce_stat_data; - - printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); - printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); - printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); - printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); - printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); - } + strbuf_release(&name); } static void show_ru_info(void) @@ -468,6 +515,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) { OPTION_SET_INT, 0, "full-name", &prefix_len, NULL, N_("make the output relative to the project top directory"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOL(0, "recurse-submodules", &recurse_submodules, + N_("recurse through submodules")), OPT_BOOL(0, "error-unmatch", &error_unmatch, N_("if any <file> is not in the index, treat this as an error")), OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"), @@ -519,6 +568,24 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + submodule_prefix = getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT); + + if (recurse_submodules && + (show_stage || show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo || + show_valid_bit || show_tag || show_eol || with_tree || + (line_terminator == '\0'))) + die("ls-files --recurse-submodules unsupported mode"); + + if (recurse_submodules && error_unmatch) + die("ls-files --recurse-submodules does not support " + "--error-unmatch"); + + if (recurse_submodules && argc) + die("ls-files --recurse-submodules does not support path " + "arguments"); + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD | PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, diff --git a/git.c b/git.c index b2b096a..510b6a2 100644 --- a/git.c +++ b/git.c @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = { { "init-db", cmd_init_db }, { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY }, { "log", cmd_log, RUN_SETUP }, - { "ls-files", cmd_ls_files, RUN_SETUP }, + { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUBMODULES }, { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY }, { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo }, diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh new file mode 100755 index 0000000..7d225ac --- /dev/null +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -0,0 +1,100 @@ +#!/bin/sh + +test_description='Test ls-files recurse-submodules feature + +This test verifies the recurse-submodules feature correctly lists files from +submodules. +' + +. ./test-lib.sh + +test_expect_success 'setup directory structure and submodules' ' + echo a >a && + mkdir b && + echo b >b/b && + git add a b && + git commit -m "add a and b" && + git init submodule && + echo c >submodule/c && + git -C submodule add c && + git -C submodule commit -m "add c" && + git submodule add ./submodule && + git commit -m "added submodule" +' + +test_expect_success 'ls-files correctly outputs files in submodule' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files does not output files not added to a repo' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + echo a >not_added && + echo b >b/not_added && + echo c >submodule/not_added && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files recurses more than 1 level' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/.gitmodules + submodule/c + submodule/subsub/d + EOF + + git init submodule/subsub && + echo d >submodule/subsub/d && + git -C submodule/subsub add d && + git -C submodule/subsub commit -m "add d" && + git -C submodule submodule add ./subsub && + git -C submodule commit -m "added subsub" && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules does not support using path arguments' ' + test_must_fail git ls-files --recurse-submodules b 2>actual && + test_i18ngrep "does not support path arguments" actual +' + +test_expect_success '--recurse-submodules does not support --error-unmatch' ' + test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual && + test_i18ngrep "does not support --error-unmatch" actual +' + +test_incompatible_with_recurse_submodules () { + test_expect_success "--recurse-submodules and $1 are incompatible" " + test_must_fail git ls-files --recurse-submodules $1 2>actual && + test_i18ngrep 'unsupported mode' actual + " +} + +test_incompatible_with_recurse_submodules -z +test_incompatible_with_recurse_submodules -v +test_incompatible_with_recurse_submodules -t +test_incompatible_with_recurse_submodules --deleted +test_incompatible_with_recurse_submodules --modified +test_incompatible_with_recurse_submodules --others +test_incompatible_with_recurse_submodules --stage +test_incompatible_with_recurse_submodules --killed +test_incompatible_with_recurse_submodules --unmerged +test_incompatible_with_recurse_submodules --eol + +test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 2/4 v4] ls-files: optionally recurse into submodules 2016-09-26 22:46 ` [PATCH 2/4 v4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-27 18:29 ` Junio C Hamano 2016-09-27 20:33 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 18:29 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > Allow ls-files to recognize submodules in order to retrieve a list of > files from a repository's submodules. This is done by forking off a > process to recursively call ls-files on all submodules. Use top-level > --submodule_prefix option to pass a path to the submodule which it can > use to prepend to output or pathspec matching logic. > > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > Documentation/git-ls-files.txt | 7 +- > builtin/ls-files.c | 143 ++++++++++++++++++++++++--------- > git.c | 2 +- > t/t3007-ls-files-recurse-submodules.sh | 100 +++++++++++++++++++++++ > 4 files changed, 212 insertions(+), 40 deletions(-) > create mode 100755 t/t3007-ls-files-recurse-submodules.sh > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > index 0d933ac..446209e 100644 > --- a/Documentation/git-ls-files.txt > +++ b/Documentation/git-ls-files.txt > @@ -18,7 +18,8 @@ SYNOPSIS > [--exclude-per-directory=<file>] > [--exclude-standard] > [--error-unmatch] [--with-tree=<tree-ish>] > - [--full-name] [--abbrev] [--] [<file>...] > + [--full-name] [--recurse-submodules] > + [--abbrev] [--] [<file>...] > > DESCRIPTION > ----------- > @@ -137,6 +138,10 @@ a space) at the start of each line: > option forces paths to be output relative to the project > top directory. > > +--recurse-submodules:: > + Recursively calls ls-files on each submodule in the repository. > + Currently there is only support for the --cached mode. > + > --abbrev[=<n>]:: > Instead of showing the full 40-byte hexadecimal object > lines, show only a partial prefix. > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index 00ea91a..d4bfc60 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -14,6 +14,7 @@ > #include "resolve-undo.h" > #include "string-list.h" > #include "pathspec.h" > +#include "run-command.h" > > static int abbrev; > static int show_deleted; > @@ -28,6 +29,8 @@ static int show_valid_bit; > static int line_terminator = '\n'; > static int debug_mode; > static int show_eol; > +static int recurse_submodules; > +static const char *submodule_prefix; I would have expected this to added to environment.c in the previous step, but it is OK--I'd imagine you'd grab this from the environment and carrying a piece of information from git.c to here by setenv() followed by getenv() feels somewhat roundabout, though. > static const char *prefix; > static int max_prefix_len; > @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) > static void write_name(const char *name) > { > /* > + * NEEDSWORK: To make this thread-safe, full_name would have to be owned > + * by the caller. As Peff mentioned in his review in another thread, a large number of functions in git are not reentrant, and I do not think we would want to give the impression that those missing a warning are safe to use. Other than that, this step looks OK. 3/4 and later would be a lot more fun to review ;-) ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 2/4 v4] ls-files: optionally recurse into submodules 2016-09-27 18:29 ` Junio C Hamano @ 2016-09-27 20:33 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > +static const char *submodule_prefix; > > I would have expected this to added to environment.c in the previous > step, but it is OK--I'd imagine you'd grab this from the environment > and carrying a piece of information from git.c to here by setenv() > followed by getenv() feels somewhat roundabout, though. If it would make sense to do the caching of prefix string in environment.c I can move it there and add a get_submodule_prefix() function which either reads the envvar or the cached value if its already been read. Would that be a better route? > > > static const char *prefix; > > static int max_prefix_len; > > @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) > > static void write_name(const char *name) > > { > > /* > > + * NEEDSWORK: To make this thread-safe, full_name would have to be owned > > + * by the caller. > > As Peff mentioned in his review in another thread, a large number of > functions in git are not reentrant, and I do not think we would want > to give the impression that those missing a warning are safe to use. > > Other than that, this step looks OK. 3/4 and later would be a lot > more fun to review ;-) Oh yes, I can remove the comment. Seemed to miss that bit while rerolling the series. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams 2016-09-26 22:46 ` [PATCH 1/4 v4] submodules: make submodule-prefix option Brandon Williams 2016-09-26 22:46 ` [PATCH 2/4 v4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-26 22:46 ` Brandon Williams 2016-09-27 18:40 ` Junio C Hamano 2016-09-27 18:43 ` Junio C Hamano 2016-09-26 22:46 ` [PATCH 4/4 v4] ls-files: add pathspec matching for submodules Brandon Williams 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams 4 siblings, 2 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-26 22:46 UTC (permalink / raw) To: git; +Cc: Brandon Williams Pass through some known-safe options when recursing into submodules. (--cached, --stage, -v, -t, -z, --debug, --eol) Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 34 ++++++++++++++++++++++++++++++---- t/t3007-ls-files-recurse-submodules.sh | 17 ++++++++++++----- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index d4bfc60..a39367f 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -31,6 +31,7 @@ static int debug_mode; static int show_eol; static int recurse_submodules; static const char *submodule_prefix; +static struct argv_array recurse_submodules_opts = ARGV_ARRAY_INIT; static const char *prefix; static int max_prefix_len; @@ -170,6 +171,27 @@ static void show_killed_files(struct dir_struct *dir) } } +/* + * Compile an argv_array with all of the options supported by --recurse_submodules + */ +static void compile_submodule_options(int show_tag) +{ + if (show_cached) + argv_array_push(&recurse_submodules_opts, "--cached"); + if (show_stage) + argv_array_push(&recurse_submodules_opts, "--stage"); + if (show_valid_bit) + argv_array_push(&recurse_submodules_opts, "-v"); + if (show_tag) + argv_array_push(&recurse_submodules_opts, "-t"); + if (line_terminator == '\0') + argv_array_push(&recurse_submodules_opts, "-z"); + if (debug_mode) + argv_array_push(&recurse_submodules_opts, "--debug"); + if (show_eol) + argv_array_push(&recurse_submodules_opts, "--eol"); +} + /** * Recursively call ls-files on a submodule */ @@ -184,6 +206,9 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); + /* add supported options */ + argv_array_pushv(&cp.args, recurse_submodules_opts.argv); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -568,14 +593,15 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); - if (recurse_submodules) + if (recurse_submodules) { submodule_prefix = getenv(GIT_SUBMODULE_PREFIX_ENVIRONMENT); + compile_submodule_options(show_tag); + } if (recurse_submodules && - (show_stage || show_deleted || show_others || show_unmerged || + (show_deleted || show_others || show_unmerged || show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol || with_tree || - (line_terminator == '\0'))) + with_tree)) die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index 7d225ac..40767da 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + cat | tr "\n" "\0" >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -86,15 +98,10 @@ test_incompatible_with_recurse_submodules () { " } -test_incompatible_with_recurse_submodules -z -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others -test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-26 22:46 ` [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-09-27 18:40 ` Junio C Hamano 2016-09-27 20:11 ` Junio C Hamano 2016-09-27 20:49 ` Brandon Williams 2016-09-27 18:43 ` Junio C Hamano 1 sibling, 2 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 18:40 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > Pass through some known-safe options when recursing into submodules. > (--cached, --stage, -v, -t, -z, --debug, --eol) > > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > builtin/ls-files.c | 34 ++++++++++++++++++++++++++++++---- > t/t3007-ls-files-recurse-submodules.sh | 17 ++++++++++++----- > 2 files changed, 42 insertions(+), 9 deletions(-) > > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index d4bfc60..a39367f 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -31,6 +31,7 @@ static int debug_mode; > static int show_eol; > static int recurse_submodules; > static const char *submodule_prefix; > +static struct argv_array recurse_submodules_opts = ARGV_ARRAY_INIT; I'd imagine that this is also thread-unsafe, but we do not have to comment it ;-) > @@ -170,6 +171,27 @@ static void show_killed_files(struct dir_struct *dir) > } > } > > +/* > + * Compile an argv_array with all of the options supported by --recurse_submodules > + */ > +static void compile_submodule_options(int show_tag) > +{ > + if (show_cached) > + argv_array_push(&recurse_submodules_opts, "--cached"); > + if (show_stage) > + argv_array_push(&recurse_submodules_opts, "--stage"); > + if (show_valid_bit) > + argv_array_push(&recurse_submodules_opts, "-v"); > + if (show_tag) > + argv_array_push(&recurse_submodules_opts, "-t"); > + if (line_terminator == '\0') > + argv_array_push(&recurse_submodules_opts, "-z"); > + if (debug_mode) > + argv_array_push(&recurse_submodules_opts, "--debug"); > + if (show_eol) > + argv_array_push(&recurse_submodules_opts, "--eol"); > +} OK. These are only the safe ones to pass through? "compile" or "assemble" is much less important thing to say than how these are chosen. "pass_supported_options()" or something? I dunno. > if (recurse_submodules && > - (show_stage || show_deleted || show_others || show_unmerged || > + (show_deleted || show_others || show_unmerged || > show_killed || show_modified || show_resolve_undo || > - show_valid_bit || show_tag || show_eol || with_tree || > - (line_terminator == '\0'))) > + with_tree)) > die("ls-files --recurse-submodules unsupported mode"); Makes sense. > +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' > + cat | tr "\n" "\0" >expect <<-\EOF && > + .gitmodules > + a > + b/b > + submodule/c > + EOF Hmm, what do you need "cat" for here? In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh) we seem to avoid using "tr", even though q_to_cr and others do use it. I wonder if we had some portability issues with passing NUL through tr or something? ... digs and finds e85fe4d8 ("more tr portability test script fixes", 2008-03-12) So use something like perl -pe 'y/\012/\000/' <<\-EOF ... EOF instead, perhaps? > + git ls-files --recurse-submodules -z >actual && > + test_cmp expect actual > +' ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 18:40 ` Junio C Hamano @ 2016-09-27 20:11 ` Junio C Hamano 2016-09-27 20:52 ` Brandon Williams 2016-09-28 17:24 ` Brandon Williams 2016-09-27 20:49 ` Brandon Williams 1 sibling, 2 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 20:11 UTC (permalink / raw) To: Brandon Williams; +Cc: git Junio C Hamano <gitster@pobox.com> writes: > In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh) > we seem to avoid using "tr", even though q_to_cr and others do use > it. I wonder if we had some portability issues with passing NUL > through tr or something? > > ... digs and finds e85fe4d8 ("more tr portability test script > fixes", 2008-03-12) > > So use something like > > perl -pe 'y/\012/\000/' <<\-EOF > ... > EOF > > instead, perhaps? I actually think it would make more sense to add lf_to_nul () { perl -pe 'y/\012/\000/' } to t/test-lib-functions.sh somewhere near q_to_nul if we were to go this route. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 20:11 ` Junio C Hamano @ 2016-09-27 20:52 ` Brandon Williams 2016-09-27 20:58 ` Junio C Hamano 2016-09-27 20:59 ` Stefan Beller 2016-09-28 17:24 ` Brandon Williams 1 sibling, 2 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh) > > we seem to avoid using "tr", even though q_to_cr and others do use > > it. I wonder if we had some portability issues with passing NUL > > through tr or something? > > > > ... digs and finds e85fe4d8 ("more tr portability test script > > fixes", 2008-03-12) > > > > So use something like > > > > perl -pe 'y/\012/\000/' <<\-EOF > > ... > > EOF > > > > instead, perhaps? > > I actually think it would make more sense to add > > lf_to_nul () { > perl -pe 'y/\012/\000/' > } > > to t/test-lib-functions.sh somewhere near q_to_nul if we were to go > this route. my mind is drawing a blank, what does the 'lf' in 'lf_to_nul' stand for? line feed? -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 20:52 ` Brandon Williams @ 2016-09-27 20:58 ` Junio C Hamano 2016-09-27 20:59 ` Stefan Beller 1 sibling, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 20:58 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > my mind is drawing a blank, what does the 'lf' in 'lf_to_nul' stand for? > line feed? Yup. "man 7 ascii" ;-) ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 20:52 ` Brandon Williams 2016-09-27 20:58 ` Junio C Hamano @ 2016-09-27 20:59 ` Stefan Beller 1 sibling, 0 replies; 67+ messages in thread From: Stefan Beller @ 2016-09-27 20:59 UTC (permalink / raw) To: Brandon Williams; +Cc: Junio C Hamano, git On Tue, Sep 27, 2016 at 1:52 PM, Brandon Williams <bmwill@google.com> wrote: > On 09/27, Junio C Hamano wrote: >> Junio C Hamano <gitster@pobox.com> writes: >> >> > In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh) >> > we seem to avoid using "tr", even though q_to_cr and others do use >> > it. I wonder if we had some portability issues with passing NUL >> > through tr or something? >> > >> > ... digs and finds e85fe4d8 ("more tr portability test script >> > fixes", 2008-03-12) >> > >> > So use something like >> > >> > perl -pe 'y/\012/\000/' <<\-EOF >> > ... >> > EOF >> > >> > instead, perhaps? >> >> I actually think it would make more sense to add >> >> lf_to_nul () { >> perl -pe 'y/\012/\000/' >> } >> >> to t/test-lib-functions.sh somewhere near q_to_nul if we were to go >> this route. > > my mind is drawing a blank, what does the 'lf' in 'lf_to_nul' stand for? > line feed? > Yes, line feed. (Note that Git has to deal with this cross platform new lines e.g. CRLF is common on Windows, CR was common on MAC, and LF is Windows, so naming the new line as they are, makes sense here.) ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 20:11 ` Junio C Hamano 2016-09-27 20:52 ` Brandon Williams @ 2016-09-28 17:24 ` Brandon Williams 2016-09-28 18:59 ` Junio C Hamano 1 sibling, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-28 17:24 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh) > > we seem to avoid using "tr", even though q_to_cr and others do use > > it. I wonder if we had some portability issues with passing NUL > > through tr or something? > > > > ... digs and finds e85fe4d8 ("more tr portability test script > > fixes", 2008-03-12) > > > > So use something like > > > > perl -pe 'y/\012/\000/' <<\-EOF > > ... > > EOF > > > > instead, perhaps? > > I actually think it would make more sense to add > > lf_to_nul () { > perl -pe 'y/\012/\000/' > } > > to t/test-lib-functions.sh somewhere near q_to_nul if we were to go > this route. Turns out this function already exists in test-lib-functions.sh -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-28 17:24 ` Brandon Williams @ 2016-09-28 18:59 ` Junio C Hamano 0 siblings, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-28 18:59 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: >> I actually think it would make more sense to add >> >> lf_to_nul () { >> perl -pe 'y/\012/\000/' >> } >> >> to t/test-lib-functions.sh somewhere near q_to_nul if we were to go >> this route. > > Turns out this function already exists in test-lib-functions.sh ;-) ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 18:40 ` Junio C Hamano 2016-09-27 20:11 ` Junio C Hamano @ 2016-09-27 20:49 ` Brandon Williams 1 sibling, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > @@ -170,6 +171,27 @@ static void show_killed_files(struct dir_struct *dir) > > } > > } > > > > +/* > > + * Compile an argv_array with all of the options supported by --recurse_submodules > > + */ > > +static void compile_submodule_options(int show_tag) > > +{ > > + if (show_cached) > > + argv_array_push(&recurse_submodules_opts, "--cached"); > > + if (show_stage) > > + argv_array_push(&recurse_submodules_opts, "--stage"); > > + if (show_valid_bit) > > + argv_array_push(&recurse_submodules_opts, "-v"); > > + if (show_tag) > > + argv_array_push(&recurse_submodules_opts, "-t"); > > + if (line_terminator == '\0') > > + argv_array_push(&recurse_submodules_opts, "-z"); > > + if (debug_mode) > > + argv_array_push(&recurse_submodules_opts, "--debug"); > > + if (show_eol) > > + argv_array_push(&recurse_submodules_opts, "--eol"); > > +} > > OK. These are only the safe ones to pass through? "compile" or > "assemble" is much less important thing to say than how these are > chosen. "pass_supported_options()" or something? I dunno. Alternatively we can change this to compile all potential options (not just the ones that are supported now) and then just error the caller out, as you've suggested, if an unsupported option used. 'pass' may not be the most descriptive word for this function as it isn't actually doing the passing but rather generating an argv of options that will be passed in the event a submodule is found and we need to kick off a child process for it. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-26 22:46 ` [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules Brandon Williams 2016-09-27 18:40 ` Junio C Hamano @ 2016-09-27 18:43 ` Junio C Hamano 2016-09-27 20:44 ` Brandon Williams 1 sibling, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 18:43 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > if (recurse_submodules && > - (show_stage || show_deleted || show_others || show_unmerged || > + (show_deleted || show_others || show_unmerged || > show_killed || show_modified || show_resolve_undo || > - show_valid_bit || show_tag || show_eol || with_tree || > - (line_terminator == '\0'))) > + with_tree)) > die("ls-files --recurse-submodules unsupported mode"); Ahh, one more thing, and this comment probably applies to 2/4 not this one, but if the intention is to shrink this "not supported yet" check as the series progresses, in the earlier step the check would need to make sure no pathspec is given, which is first supported in 4/4, I think. It is not a big deal to require rerolling by itself, bit if you are rerolling 2/4 for other reasons, then... ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 18:43 ` Junio C Hamano @ 2016-09-27 20:44 ` Brandon Williams 2016-09-27 20:59 ` Junio C Hamano 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > if (recurse_submodules && > > - (show_stage || show_deleted || show_others || show_unmerged || > > + (show_deleted || show_others || show_unmerged || > > show_killed || show_modified || show_resolve_undo || > > - show_valid_bit || show_tag || show_eol || with_tree || > > - (line_terminator == '\0'))) > > + with_tree)) > > die("ls-files --recurse-submodules unsupported mode"); > > Ahh, one more thing, and this comment probably applies to 2/4 not > this one, but if the intention is to shrink this "not supported yet" > check as the series progresses, in the earlier step the check would > need to make sure no pathspec is given, which is first supported in > 4/4, I think. It is not a big deal to require rerolling by itself, > bit if you are rerolling 2/4 for other reasons, then... Oh there is a separate if gaurd for pathspecs which is introduced in 2/4 and then removed once pathspec support has been added in 4/4. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules 2016-09-27 20:44 ` Brandon Williams @ 2016-09-27 20:59 ` Junio C Hamano 0 siblings, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 20:59 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > Oh there is a separate if gaurd for pathspecs which is introduced in 2/4 > and then removed once pathspec support has been added in 4/4. Thanks; I missed to spot that when I wrote the message you are responding to, but it indeed is there ;-) ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH 4/4 v4] ls-files: add pathspec matching for submodules 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams ` (2 preceding siblings ...) 2016-09-26 22:46 ` [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-09-26 22:46 ` Brandon Williams 2016-09-27 20:01 ` Junio C Hamano 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-26 22:46 UTC (permalink / raw) To: git; +Cc: Brandon Williams Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 28 ++++++--- dir.c | 46 +++++++++++++- dir.h | 4 ++ t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++- 4 files changed, 174 insertions(+), 12 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index a39367f..2488a02 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -199,6 +199,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + int i; argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/", submodule_prefix ? submodule_prefix : "", @@ -209,6 +210,15 @@ static void show_gitlink(const struct cache_entry *ce) /* add supported options */ argv_array_pushv(&cp.args, recurse_submodules_opts.argv); + /* + * Pass in the original pathspec args. The submodule will be + * responsible for prepending the 'submodule_prefix' prior to comparing + * against the pathspec for matches. + */ + argv_array_push(&cp.args, "--"); + for (i = 0; i < pathspec.nr; i++) + argv_array_push(&cp.args, pathspec.items[i].original); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -227,7 +237,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && + submodule_path_match(&pathspec, name.buf, ps_matched)) { show_gitlink(ce); } else if (match_pathspec(&pathspec, name.buf, name.len, len, ps_matched, @@ -608,17 +619,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) die("ls-files --recurse-submodules does not support " "--error-unmatch"); - if (recurse_submodules && argc) - die("ls-files --recurse-submodules does not support path " - "arguments"); - 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); + /* + * Find common prefix for all pathspec's + * This is used as a performance optimization which unfortunately cannot + * be done when recursing into submodules + */ + if (recurse_submodules) + max_prefix = NULL; + else + 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 0ea235f..28e9736 100644 --- a/dir.c +++ b/dir.c @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen, return 1; } -#define DO_MATCH_EXCLUDE 1 -#define DO_MATCH_DIRECTORY 2 +#define DO_MATCH_EXCLUDE (1<<0) +#define DO_MATCH_DIRECTORY (1<<1) +#define DO_MATCH_SUBMODULE (1<<2) /* * Does 'match' match the given name? @@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, item->nowildcard_len - prefix)) return MATCHED_FNMATCH; + /* Perform checks to see if "name" is a super set of the pathspec */ + if (flags & DO_MATCH_SUBMODULE) { + /* name is a literal prefix of the pathspec */ + if ((namelen < matchlen) && + (match[namelen] == '/') && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_RECURSIVELY; + + /* name" doesn't match up to the first wild character */ + if (item->nowildcard_len < item->len && + ps_strncmp(item, match, name, + item->nowildcard_len - prefix)) + return 0; + + /* + * Here is where we would perform a wildmatch to check if + * "name" can be matched as a directory (or a prefix) against + * the pathspec. Since wildmatch doesn't have this capability + * at the present we have to punt and say that it is a match, + * potentially returning a false positive + * The submodules themselves will be able to perform more + * accurate matching to determine if the pathspec matches. + */ + return MATCHED_RECURSIVELY; + } + return 0; } @@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps, return negative ? 0 : positive; } +/** + * Check if a submodule is a superset of the pathspec + */ +int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen) +{ + int matched = do_match_pathspec(ps, submodule_name, + strlen(submodule_name), + 0, seen, + DO_MATCH_DIRECTORY | + DO_MATCH_SUBMODULE); + return matched; +} + int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix) diff --git a/dir.h b/dir.h index da1a858..97c83bb 100644 --- a/dir.h +++ b/dir.h @@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +extern int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen); + static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index 40767da..4a51d38 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' -test_expect_success '--recurse-submodules does not support using path arguments' ' - test_must_fail git ls-files --recurse-submodules b 2>actual && - test_i18ngrep "does not support path arguments" actual +test_expect_success '--recurse-submodules and pathspecs setup' ' + echo e >submodule/subsub/e.txt && + git -C submodule/subsub add e.txt && + git -C submodule/subsub commit -m "adding e.txt" && + echo f >submodule/f.TXT && + echo g >submodule/g.txt && + git -C submodule add f.TXT g.txt && + git -C submodule commit -m "add f and g" && + echo h >h.txt && + mkdir sib && + echo sib >sib/file && + git add h.txt sib/file && + git commit -m "add h and sib/file" && + git init sub && + echo sub >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git submodule add ./sub && + git commit -m "added sub" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub/d + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + cat actual && + git ls-files --recurse-submodules "*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules "*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sub/file + EOF + + git ls-files --recurse-submodules "sub" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su*/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su?/file" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sib/file + sub/file + EOF + + git ls-files --recurse-submodules "s??/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s???file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s*file" >actual && + test_cmp expect actual ' test_expect_success '--recurse-submodules does not support --error-unmatch' ' -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH 4/4 v4] ls-files: add pathspec matching for submodules 2016-09-26 22:46 ` [PATCH 4/4 v4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-09-27 20:01 ` Junio C Hamano 2016-09-27 20:40 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-27 20:01 UTC (permalink / raw) To: Brandon Williams; +Cc: git Brandon Williams <bmwill@google.com> writes: > - /* Find common prefix for all pathspec's */ > - max_prefix = common_prefix(&pathspec); > + /* > + * Find common prefix for all pathspec's > + * This is used as a performance optimization which unfortunately cannot > + * be done when recursing into submodules > + */ > + if (recurse_submodules) > + max_prefix = NULL; > + else > + max_prefix = common_prefix(&pathspec); > max_prefix_len = max_prefix ? strlen(max_prefix) : 0; I still wonder if we can do better than this, as this would be a big cycle-saver especially in recurse-submodules case. When you get max_prefix that is "a/b/c", there are three cases: * a/b/c is a path prefix for an entry in the index, e.g. a/b/c/d; you then can safely use it and you do not have to do any recursive invocation of ls-files outside "a/b/c". You may match a/b/c/d in the toplevel, or you may recurse a/b/c/e that is a submodule, but you won't have to pay attention to submodules outside. * a leading path of a/b/c, e.g. a/b, is a gitlink or a blob in the index; you can use a/b and you only have to recurse into a/b if that is a submodule; if a/b is a blob, you'd show nothing. * a/b/c itself and no leading path of it appears in the index; you know that nothing will match once you know that you are in this situation. Because a gitlink "a/b" sorts at the same location in the index as a regular blob "a/b" would, by feeding the max_prefix common_prefix() gives you (i.e. "a/b/c") to index_name_pos() to see which one of the three situations you are in can be done fairly cheaply, I would think. The index_name_pos() call may find "a/b/c" exactly (case 1), or return a location where "a/b/c" would be inserted in the list of existing entries. If there were "a/b" (or "a") in the index, there wouldn't be any "a/b/x" (or "a/x") at the same time, so a query for "a/b/c" would land you next to (just after) an existing entry that is a leading path of it, if such an entry exists, no? That would allow you to tell case 2 above fairly cheaply, I would expect. It is a separate issue if adding that support to 4/4 is a good idea; I personally think doing it as a separate follow-up patch would make more sense, so all of the above is tangent. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH 4/4 v4] ls-files: add pathspec matching for submodules 2016-09-27 20:01 ` Junio C Hamano @ 2016-09-27 20:40 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-27 20:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 09/27, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > - /* Find common prefix for all pathspec's */ > > - max_prefix = common_prefix(&pathspec); > > + /* > > + * Find common prefix for all pathspec's > > + * This is used as a performance optimization which unfortunately cannot > > + * be done when recursing into submodules > > + */ > > + if (recurse_submodules) > > + max_prefix = NULL; > > + else > > + max_prefix = common_prefix(&pathspec); > > max_prefix_len = max_prefix ? strlen(max_prefix) : 0; > > I still wonder if we can do better than this, as this would be a big > cycle-saver especially in recurse-submodules case. > > When you get max_prefix that is "a/b/c", there are three cases: > > * a/b/c is a path prefix for an entry in the index, e.g. a/b/c/d; > you then can safely use it and you do not have to do any > recursive invocation of ls-files outside "a/b/c". You may match > a/b/c/d in the toplevel, or you may recurse a/b/c/e that is a > submodule, but you won't have to pay attention to submodules > outside. > > * a leading path of a/b/c, e.g. a/b, is a gitlink or a blob in the > index; you can use a/b and you only have to recurse into a/b if > that is a submodule; if a/b is a blob, you'd show nothing. > > * a/b/c itself and no leading path of it appears in the index; you > know that nothing will match once you know that you are in this > situation. > > Because a gitlink "a/b" sorts at the same location in the index as a > regular blob "a/b" would, by feeding the max_prefix common_prefix() > gives you (i.e. "a/b/c") to index_name_pos() to see which one of the > three situations you are in can be done fairly cheaply, I would > think. The index_name_pos() call may find "a/b/c" exactly (case 1), > or return a location where "a/b/c" would be inserted in the list of > existing entries. If there were "a/b" (or "a") in the index, there > wouldn't be any "a/b/x" (or "a/x") at the same time, so a query for > "a/b/c" would land you next to (just after) an existing entry that > is a leading path of it, if such an entry exists, no? That would > allow you to tell case 2 above fairly cheaply, I would expect. > > It is a separate issue if adding that support to 4/4 is a good idea; > I personally think doing it as a separate follow-up patch would make > more sense, so all of the above is tangent. I agree that this could be a big cycle saver. At the present I was working towards getting a working implementation but this should definitely be addressed in a follow-up patch to introduce the optimization to the recurse-submodule mode. It hopefully wouldn't be too hard to implement seeing as its using string literals. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v5 0/4] recursive support for ls-files 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams ` (3 preceding siblings ...) 2016-09-26 22:46 ` [PATCH 4/4 v4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-09-28 21:50 ` Brandon Williams 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams ` (4 more replies) 4 siblings, 5 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-28 21:50 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster The big change in this version is the introduction of a --super-prefix option to the top level git. After much discussion this seemed to be a better naming scheme than 'submodule-prefix' as it could be an option other cmds could use independent of submodules. In 3/4 I changed the compile_submodule_options function to compile all options that can be realistically passed through and when an option that isn't supported (or rather safe) yet is provided the caller will be errored out. Brandon Williams (4): git: make super-prefix option ls-files: optionally recurse into submodules ls-files: pass through safe options for --recurse-submodules ls-files: add pathspec matching for submodules Documentation/git-ls-files.txt | 7 +- Documentation/git.txt | 6 + builtin/ls-files.c | 202 ++++++++++++++++++++++++------- cache.h | 2 + dir.c | 46 +++++++- dir.h | 4 + environment.c | 10 ++ git.c | 24 +++- t/t3007-ls-files-recurse-submodules.sh | 209 +++++++++++++++++++++++++++++++++ 9 files changed, 466 insertions(+), 44 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v5 1/4] git: make super-prefix option 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams @ 2016-09-28 21:50 ` Brandon Williams 2016-09-28 22:01 ` Stefan Beller ` (2 more replies) 2016-09-28 21:50 ` [PATCH v5 2/4] ls-files: optionally recurse into submodules Brandon Williams ` (3 subsequent siblings) 4 siblings, 3 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-28 21:50 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' which can be used to specify a path from above a repository down to its root. The immediate use of this option is by commands which have a --recurse-submodule option in order to give context to submodules about how they were invoked. This option is currently only allowed for builtins which support a super-prefix. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git.txt | 6 ++++++ cache.h | 2 ++ environment.c | 10 ++++++++++ git.c | 22 ++++++++++++++++++++++ 4 files changed, 40 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7913fc2..e3309b9 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -13,6 +13,7 @@ SYNOPSIS [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>] + [--super-prefix=<path>] <command> [<args>] DESCRIPTION @@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string. details. Equivalent to setting the `GIT_NAMESPACE` environment variable. +--super-prefix=<path>:: + Set a prefix which gives a path from above a repository down to its + root. One use is to give submodules context about the superproject that + invoked it. Currently for internal use only. + --bare:: Treat the repository as a bare repository. If GIT_DIR environment is not set, it is set to the current working diff --git a/cache.h b/cache.h index 3556326..01730e1 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" +#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" @@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); extern int get_common_dir(struct strbuf *sb, const char *gitdir); extern const char *get_git_namespace(void); extern const char *strip_namespace(const char *namespaced_ref); +extern const char *get_super_prefix(); extern const char *get_git_work_tree(void); /* diff --git a/environment.c b/environment.c index ca72464..1a2a779 100644 --- a/environment.c +++ b/environment.c @@ -100,6 +100,8 @@ static char *work_tree; static const char *namespace; static size_t namespace_len; +static const char *super_prefix; + static const char *git_dir, *git_common_dir; static char *git_object_dir, *git_index_file, *git_graft_file; int git_db_env, git_index_env, git_graft_env, git_common_dir_env; @@ -120,6 +122,7 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, + GIT_SUPER_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL @@ -222,6 +225,13 @@ const char *strip_namespace(const char *namespaced_ref) return namespaced_ref + namespace_len; } +const char *get_super_prefix() +{ + if (!super_prefix) + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); + return super_prefix; +} + static int git_work_tree_initialized; /* diff --git a/git.c b/git.c index 1c61151..3e5cd63 100644 --- a/git.c +++ b/git.c @@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); if (envchanged) *envchanged = 1; + } else if (!strcmp(cmd, "--super-prefix")) { + if (*argc < 2) { + fprintf(stderr, "No prefix given for --super-prefix.\n" ); + usage(git_usage_string); + } + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); + if (envchanged) + *envchanged = 1; + (*argv)++; + (*argc)--; + } else if (skip_prefix(cmd, "--super-prefix=", &cmd)) { + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1); + if (envchanged) + *envchanged = 1; } else if (!strcmp(cmd, "--bare")) { char *cwd = xgetcwd(); is_bare_repository_cfg = 1; @@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv) * RUN_SETUP for reading from the configuration file. */ #define NEED_WORK_TREE (1<<3) +#define SUPPORT_SUPER_PREFIX (1<<4) struct cmd_struct { const char *cmd; @@ -344,6 +359,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) } commit_pager_choice(); + if (!help && get_super_prefix()) { + if (!(p->option & SUPPORT_SUPER_PREFIX)) + die("%s doesn't support --super-prefix", p->cmd); + if (prefix) + die("can't have both a prefix and super-prefix"); + } + if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v5 1/4] git: make super-prefix option 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams @ 2016-09-28 22:01 ` Stefan Beller 2016-09-28 22:19 ` Junio C Hamano 2016-09-29 18:39 ` Jeff King 2 siblings, 0 replies; 67+ messages in thread From: Stefan Beller @ 2016-09-28 22:01 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Wed, Sep 28, 2016 at 2:50 PM, Brandon Williams <bmwill@google.com> wrote: > > DESCRIPTION > @@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string. > details. Equivalent to setting the `GIT_NAMESPACE` environment > variable. > > +--super-prefix=<path>:: > + Set a prefix which gives a path from above a repository down to its > + root. One use is to give submodules context about the superproject that > + invoked it. Currently for internal use only. I would put the "Currently for internal use only." at the beginning, so end users don't have to bother reading though the description, when they want to only use kosher flags. (Well, checking `man git fetch` and searching for 'internal', there doesn't seem to be a consistent way how to document internal flags :(. It doesn't however advertise the flag in the SYNOPSIS. Ok it doesn't advertise a lot of flags in its SYNOPSIS) > +const char *get_super_prefix() > +{ > + if (!super_prefix) > + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); The getenv() function returns a pointer to the value in the environment, or NULL if there is no match. So in case this is not set (when e.g. the user did not specify the super prefix), we would probe it a couple of times. The caching effect only occurs when the string is set. So this looks like we save repetitive calls, but we do not always do that. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v5 1/4] git: make super-prefix option 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams 2016-09-28 22:01 ` Stefan Beller @ 2016-09-28 22:19 ` Junio C Hamano 2016-09-29 18:39 ` Jeff King 2 siblings, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-28 22:19 UTC (permalink / raw) To: Brandon Williams; +Cc: git, sbeller, peff Brandon Williams <bmwill@google.com> writes: > Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' > which can be used to specify a path from above a repository down to its > root. The immediate use of this option is by commands which have a > --recurse-submodule option in order to give context to submodules about > how they were invoked. This option is currently only allowed for > builtins which support a super-prefix. Yes, it can be used to specify that, but it is unclear (1) how such a path thusly specified is used, (2) how it affects the outcome of the operations, and (3) what it means (the same applies to the documentation update). It would help future readers of "git log", "tig blame", etc. to have a few sentences after the first sentence above. Perhaps ... specify a path from above ... down to its root. When such a super-prefix is specified, the paths reported by Git are prefixed with it to make them relative to that directory "above". The paths given by the users on the command line (e.g. "git subcmd --output-file=path/to/a/file" and pathspecs) are taken relative to that directory "above" to match. or something like that? > @@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); > extern int get_common_dir(struct strbuf *sb, const char *gitdir); > extern const char *get_git_namespace(void); > extern const char *strip_namespace(const char *namespaced_ref); > +extern const char *get_super_prefix(); Nice, but extern const char *get_super_prefix(void); > +const char *get_super_prefix() Likewise. > +{ > + if (!super_prefix) > + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); > + return super_prefix; > +} Good. > commit_pager_choice(); > > + if (!help && get_super_prefix()) { > + if (!(p->option & SUPPORT_SUPER_PREFIX)) > + die("%s doesn't support --super-prefix", p->cmd); > + if (prefix) > + die("can't have both a prefix and super-prefix"); > + } Nice. I'd phrase the latter as "can't use--super-prefix from a subdirectory", though. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v5 1/4] git: make super-prefix option 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams 2016-09-28 22:01 ` Stefan Beller 2016-09-28 22:19 ` Junio C Hamano @ 2016-09-29 18:39 ` Jeff King 2016-09-29 18:44 ` Brandon Williams 2 siblings, 1 reply; 67+ messages in thread From: Jeff King @ 2016-09-29 18:39 UTC (permalink / raw) To: Brandon Williams; +Cc: git, sbeller, gitster On Wed, Sep 28, 2016 at 02:50:40PM -0700, Brandon Williams wrote: > Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' > which can be used to specify a path from above a repository down to its > root. The immediate use of this option is by commands which have a > --recurse-submodule option in order to give context to submodules about > how they were invoked. This option is currently only allowed for > builtins which support a super-prefix. What about non-builtins? E.g., what should git --super-prefix=foo bar do? Should the externals and scripts check the presence of GIT_INTERNAL_SUPER_PREFIX and barf if it is set? Most scripts would probably notice eventually when calling some other builtin that doesn't support SUPER_PREFIX, but it seems hacky to count on that. There's also the question of 3rd-party programs. If we want to be conservative, I think you'd want to just always bail in execv_dashed_external() if --super-prefix is in use. That doesn't give an option for scripts to say "hey, I support this", but we can perhaps worry about loosening later. -Peff ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v5 1/4] git: make super-prefix option 2016-09-29 18:39 ` Jeff King @ 2016-09-29 18:44 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-29 18:44 UTC (permalink / raw) To: Jeff King; +Cc: git, sbeller, gitster On 09/29, Jeff King wrote: > On Wed, Sep 28, 2016 at 02:50:40PM -0700, Brandon Williams wrote: > > > Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' > > which can be used to specify a path from above a repository down to its > > root. The immediate use of this option is by commands which have a > > --recurse-submodule option in order to give context to submodules about > > how they were invoked. This option is currently only allowed for > > builtins which support a super-prefix. > > What about non-builtins? > > E.g., what should > > git --super-prefix=foo bar > > do? Should the externals and scripts check the presence of > GIT_INTERNAL_SUPER_PREFIX and barf if it is set? Most scripts would > probably notice eventually when calling some other builtin that doesn't > support SUPER_PREFIX, but it seems hacky to count on that. > > There's also the question of 3rd-party programs. If we want to be > conservative, I think you'd want to just always bail in > execv_dashed_external() if --super-prefix is in use. That doesn't give > an option for scripts to say "hey, I support this", but we can perhaps > worry about loosening later. > > -Peff That makes sense. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v5 2/4] ls-files: optionally recurse into submodules 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams @ 2016-09-28 21:50 ` Brandon Williams 2016-09-28 22:11 ` Stefan Beller 2016-09-28 22:22 ` Junio C Hamano 2016-09-28 21:50 ` [PATCH v5 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams ` (2 subsequent siblings) 4 siblings, 2 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-28 21:50 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Allow ls-files to recognize submodules in order to retrieve a list of files from a repository's submodules. This is done by forking off a process to recursively call ls-files on all submodules. Use top-level --super-prefix option to pass a path to the submodule which it can use to prepend to output or pathspec matching logic. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 7 +- builtin/ls-files.c | 139 ++++++++++++++++++++++++--------- git.c | 2 +- t/t3007-ls-files-recurse-submodules.sh | 100 ++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 40 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 0d933ac..446209e 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -18,7 +18,8 @@ SYNOPSIS [--exclude-per-directory=<file>] [--exclude-standard] [--error-unmatch] [--with-tree=<tree-ish>] - [--full-name] [--abbrev] [--] [<file>...] + [--full-name] [--recurse-submodules] + [--abbrev] [--] [<file>...] DESCRIPTION ----------- @@ -137,6 +138,10 @@ a space) at the start of each line: option forces paths to be output relative to the project top directory. +--recurse-submodules:: + Recursively calls ls-files on each submodule in the repository. + Currently there is only support for the --cached mode. + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object lines, show only a partial prefix. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00ea91a..e0e5cf5 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -14,6 +14,7 @@ #include "resolve-undo.h" #include "string-list.h" #include "pathspec.h" +#include "run-command.h" static int abbrev; static int show_deleted; @@ -28,8 +29,10 @@ static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; static int show_eol; +static int recurse_submodules; static const char *prefix; +static const char *super_prefix; static int max_prefix_len; static int prefix_len; static struct pathspec pathspec; @@ -68,6 +71,19 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) static void write_name(const char *name) { /* + * Prepend the super_prefix to name to construct the full_name to be + * written. 'full_name' gets reused across output lines to minimize the + * allocation churn. + */ + static struct strbuf full_name = STRBUF_INIT; + if (super_prefix && *super_prefix) { + strbuf_reset(&full_name); + strbuf_addstr(&full_name, super_prefix); + strbuf_addstr(&full_name, name); + name = full_name.buf; + } + + /* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ @@ -152,55 +168,84 @@ static void show_killed_files(struct dir_struct *dir) } } +/** + * Recursively call ls-files on a submodule + */ +static void show_gitlink(const struct cache_entry *ce) +{ + struct child_process cp = CHILD_PROCESS_INIT; + int status; + + argv_array_pushf(&cp.args, "--super-prefix=%s%s/", + super_prefix ? super_prefix : "", + ce->name); + argv_array_push(&cp.args, "ls-files"); + argv_array_push(&cp.args, "--recurse-submodules"); + + cp.git_cmd = 1; + cp.dir = ce->name; + status = run_command(&cp); + if (status) + exit(status); +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { + struct strbuf name = STRBUF_INIT; int len = max_prefix_len; + if (super_prefix) + strbuf_addstr(&name, super_prefix); + strbuf_addstr(&name, ce->name); 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, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) - return; + if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + show_gitlink(ce); + } else if (match_pathspec(&pathspec, name.buf, name.len, + len, ps_matched, + S_ISDIR(ce->ce_mode) || + S_ISGITLINK(ce->ce_mode))) { + if (tag && *tag && show_valid_bit && + (ce->ce_flags & CE_VALID)) { + static char alttag[4]; + memcpy(alttag, tag, 3); + if (isalpha(tag[0])) + alttag[0] = tolower(tag[0]); + else if (tag[0] == '?') + alttag[0] = '!'; + else { + alttag[0] = 'v'; + alttag[1] = tag[0]; + alttag[2] = ' '; + alttag[3] = 0; + } + tag = alttag; + } - if (tag && *tag && show_valid_bit && - (ce->ce_flags & CE_VALID)) { - static char alttag[4]; - memcpy(alttag, tag, 3); - if (isalpha(tag[0])) - alttag[0] = tolower(tag[0]); - else if (tag[0] == '?') - alttag[0] = '!'; - else { - alttag[0] = 'v'; - alttag[1] = tag[0]; - alttag[2] = ' '; - alttag[3] = 0; + if (!show_stage) { + fputs(tag, stdout); + } else { + printf("%s%06o %s %d\t", + tag, + ce->ce_mode, + find_unique_abbrev(ce->sha1,abbrev), + ce_stage(ce)); + } + write_eolinfo(ce, ce->name); + write_name(ce->name); + if (debug_mode) { + const struct stat_data *sd = &ce->ce_stat_data; + + printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); + printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); + printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); + printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); + printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); } - tag = alttag; } - if (!show_stage) { - fputs(tag, stdout); - } else { - printf("%s%06o %s %d\t", - tag, - ce->ce_mode, - find_unique_abbrev(ce->sha1,abbrev), - ce_stage(ce)); - } - write_eolinfo(ce, ce->name); - write_name(ce->name); - if (debug_mode) { - const struct stat_data *sd = &ce->ce_stat_data; - - printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); - printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); - printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); - printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); - printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); - } + strbuf_release(&name); } static void show_ru_info(void) @@ -468,6 +513,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) { OPTION_SET_INT, 0, "full-name", &prefix_len, NULL, N_("make the output relative to the project top directory"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOL(0, "recurse-submodules", &recurse_submodules, + N_("recurse through submodules")), OPT_BOOL(0, "error-unmatch", &error_unmatch, N_("if any <file> is not in the index, treat this as an error")), OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"), @@ -484,6 +531,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) prefix = cmd_prefix; if (prefix) prefix_len = strlen(prefix); + super_prefix = get_super_prefix(); git_config(git_default_config, NULL); if (read_cache() < 0) @@ -519,6 +567,21 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules && + (show_stage || show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo || + show_valid_bit || show_tag || show_eol || with_tree || + (line_terminator == '\0'))) + die("ls-files --recurse-submodules unsupported mode"); + + if (recurse_submodules && error_unmatch) + die("ls-files --recurse-submodules does not support " + "--error-unmatch"); + + if (recurse_submodules && argc) + die("ls-files --recurse-submodules does not support path " + "arguments"); + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD | PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, diff --git a/git.c b/git.c index 3e5cd63..86d9be4 100644 --- a/git.c +++ b/git.c @@ -443,7 +443,7 @@ static struct cmd_struct commands[] = { { "init-db", cmd_init_db }, { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY }, { "log", cmd_log, RUN_SETUP }, - { "ls-files", cmd_ls_files, RUN_SETUP }, + { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX }, { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY }, { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo }, diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh new file mode 100755 index 0000000..7d225ac --- /dev/null +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -0,0 +1,100 @@ +#!/bin/sh + +test_description='Test ls-files recurse-submodules feature + +This test verifies the recurse-submodules feature correctly lists files from +submodules. +' + +. ./test-lib.sh + +test_expect_success 'setup directory structure and submodules' ' + echo a >a && + mkdir b && + echo b >b/b && + git add a b && + git commit -m "add a and b" && + git init submodule && + echo c >submodule/c && + git -C submodule add c && + git -C submodule commit -m "add c" && + git submodule add ./submodule && + git commit -m "added submodule" +' + +test_expect_success 'ls-files correctly outputs files in submodule' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files does not output files not added to a repo' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + echo a >not_added && + echo b >b/not_added && + echo c >submodule/not_added && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files recurses more than 1 level' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/.gitmodules + submodule/c + submodule/subsub/d + EOF + + git init submodule/subsub && + echo d >submodule/subsub/d && + git -C submodule/subsub add d && + git -C submodule/subsub commit -m "add d" && + git -C submodule submodule add ./subsub && + git -C submodule commit -m "added subsub" && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules does not support using path arguments' ' + test_must_fail git ls-files --recurse-submodules b 2>actual && + test_i18ngrep "does not support path arguments" actual +' + +test_expect_success '--recurse-submodules does not support --error-unmatch' ' + test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual && + test_i18ngrep "does not support --error-unmatch" actual +' + +test_incompatible_with_recurse_submodules () { + test_expect_success "--recurse-submodules and $1 are incompatible" " + test_must_fail git ls-files --recurse-submodules $1 2>actual && + test_i18ngrep 'unsupported mode' actual + " +} + +test_incompatible_with_recurse_submodules -z +test_incompatible_with_recurse_submodules -v +test_incompatible_with_recurse_submodules -t +test_incompatible_with_recurse_submodules --deleted +test_incompatible_with_recurse_submodules --modified +test_incompatible_with_recurse_submodules --others +test_incompatible_with_recurse_submodules --stage +test_incompatible_with_recurse_submodules --killed +test_incompatible_with_recurse_submodules --unmerged +test_incompatible_with_recurse_submodules --eol + +test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v5 2/4] ls-files: optionally recurse into submodules 2016-09-28 21:50 ` [PATCH v5 2/4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-28 22:11 ` Stefan Beller 2016-09-28 22:22 ` Junio C Hamano 1 sibling, 0 replies; 67+ messages in thread From: Stefan Beller @ 2016-09-28 22:11 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Wed, Sep 28, 2016 at 2:50 PM, Brandon Williams <bmwill@google.com> wrote: > Allow ls-files to recognize submodules in order to retrieve a list of > files from a repository's submodules. This is done by forking off a > process to recursively call ls-files on all submodules. Use top-level > --super-prefix option to pass a path to the submodule which it can > use to prepend to output or pathspec matching logic. > > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > Documentation/git-ls-files.txt | 7 +- > builtin/ls-files.c | 139 ++++++++++++++++++++++++--------- > git.c | 2 +- > t/t3007-ls-files-recurse-submodules.sh | 100 ++++++++++++++++++++++++ > 4 files changed, 208 insertions(+), 40 deletions(-) > create mode 100755 t/t3007-ls-files-recurse-submodules.sh > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > index 0d933ac..446209e 100644 > --- a/Documentation/git-ls-files.txt > +++ b/Documentation/git-ls-files.txt > @@ -18,7 +18,8 @@ SYNOPSIS > [--exclude-per-directory=<file>] > [--exclude-standard] > [--error-unmatch] [--with-tree=<tree-ish>] > - [--full-name] [--abbrev] [--] [<file>...] > + [--full-name] [--recurse-submodules] > + [--abbrev] [--] [<file>...] > > DESCRIPTION > ----------- > @@ -137,6 +138,10 @@ a space) at the start of each line: > option forces paths to be output relative to the project > top directory. > > +--recurse-submodules:: > + Recursively calls ls-files on each submodule in the repository. > + Currently there is only support for the --cached mode. > + > --abbrev[=<n>]:: > Instead of showing the full 40-byte hexadecimal object > lines, show only a partial prefix. > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index 00ea91a..e0e5cf5 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -14,6 +14,7 @@ > #include "resolve-undo.h" > #include "string-list.h" > #include "pathspec.h" > +#include "run-command.h" > > static int abbrev; > static int show_deleted; > @@ -28,8 +29,10 @@ static int show_valid_bit; > static int line_terminator = '\n'; > static int debug_mode; > static int show_eol; > +static int recurse_submodules; > > static const char *prefix; > +static const char *super_prefix; > static int max_prefix_len; > static int prefix_len; > static struct pathspec pathspec; > @@ -68,6 +71,19 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) > static void write_name(const char *name) > { > /* > + * Prepend the super_prefix to name to construct the full_name to be > + * written. 'full_name' gets reused across output lines to minimize the > + * allocation churn. > + */ When doing these tricks with the allocation churn (i.e. we make it hard to read and understand for a reader, then we should do it completely, i.e. keep full_name in the strbuf and only do a strbuf_setlen to reset the buffer just a bit. With this implementation we burden the reader/user to understand how the memory is kept over multiple calls to this function, but we still do more work than expected). So either I'd not worry about performance and provide an 'obvious correct' implementation, with e.g. no static here and we free the memory correctly. Or you'd go the performance route, but then we'd usually ask for numbers. (How much faster is it; Does the trickyness trade off well to the performance gain?) > + static struct strbuf full_name = STRBUF_INIT; > + if (super_prefix && *super_prefix) { Why do we have to check twice here? Wouldn't just if (super_prefix) { ... be enough? ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v5 2/4] ls-files: optionally recurse into submodules 2016-09-28 21:50 ` [PATCH v5 2/4] ls-files: optionally recurse into submodules Brandon Williams 2016-09-28 22:11 ` Stefan Beller @ 2016-09-28 22:22 ` Junio C Hamano 1 sibling, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-09-28 22:22 UTC (permalink / raw) To: Brandon Williams; +Cc: git, sbeller, peff Brandon Williams <bmwill@google.com> writes: > +--recurse-submodules:: > + Recursively calls ls-files on each submodule in the repository. > + Currently there is only support for the --cached mode. Good to describe it, but at this step, there is only support for the "--cached" mode and it does not take a pathspec. Also as the series progresses, I would expect this to be updated in patches $n/4 (2 < $n). > + if (recurse_submodules && argc) > + die("ls-files --recurse-submodules does not support path " > + "arguments"); Hmm, s/path arguments/pathspec/, perhaps, as the latter is specified in the glossary? ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v5 3/4] ls-files: pass through safe options for --recurse-submodules 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams 2016-09-28 21:50 ` [PATCH v5 2/4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-28 21:50 ` Brandon Williams 2016-09-28 21:50 ` [PATCH v5 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-28 21:50 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pass through some known-safe options when recursing into submodules. (--cached, --stage, -v, -t, -z, --debug, --eol) Other options are compiled into an argv_array but if an unsafe option is given the caller will be errored out. Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 51 ++++++++++++++++++++++++++++++++-- t/t3007-ls-files-recurse-submodules.sh | 17 ++++++++---- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e0e5cf5..f377e36 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -30,6 +30,7 @@ static int line_terminator = '\n'; static int debug_mode; static int show_eol; static int recurse_submodules; +static struct argv_array submodules_options = ARGV_ARRAY_INIT; static const char *prefix; static const char *super_prefix; @@ -168,6 +169,45 @@ static void show_killed_files(struct dir_struct *dir) } } +/* + * Compile an argv_array with all of the options supported by --recurse_submodules + */ +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) +{ + if (line_terminator == '\0') + argv_array_push(&submodules_options, "-z"); + if (show_tag) + argv_array_push(&submodules_options, "-t"); + if (show_valid_bit) + argv_array_push(&submodules_options, "-v"); + if (show_cached) + argv_array_push(&submodules_options, "--cached"); + if (show_deleted) + argv_array_push(&submodules_options, "--deleted"); + if (show_modified) + argv_array_push(&submodules_options, "--modified"); + if (show_others) + argv_array_push(&submodules_options, "--others"); + if (dir->flags & DIR_SHOW_IGNORED) + argv_array_push(&submodules_options, "--ignored"); + if (show_stage) + argv_array_push(&submodules_options, "--stage"); + if (show_killed) + argv_array_push(&submodules_options, "--killed"); + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) + argv_array_push(&submodules_options, "--directory"); + if (!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES)) + argv_array_push(&submodules_options, "--empty-directory"); + if (show_unmerged) + argv_array_push(&submodules_options, "--unmerged"); + if (show_resolve_undo) + argv_array_push(&submodules_options, "--resolve-undo"); + if (show_eol) + argv_array_push(&submodules_options, "--eol"); + if (debug_mode) + argv_array_push(&submodules_options, "--debug"); +} + /** * Recursively call ls-files on a submodule */ @@ -182,6 +222,9 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); + /* add supported options */ + argv_array_pushv(&cp.args, submodules_options.argv); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -567,11 +610,13 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + compile_submodule_options(&dir, show_tag); + if (recurse_submodules && - (show_stage || show_deleted || show_others || show_unmerged || + (show_deleted || show_others || show_unmerged || show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol || with_tree || - (line_terminator == '\0'))) + with_tree)) die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index 7d225ac..79107d8 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + lf_to_nul >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -86,15 +98,10 @@ test_incompatible_with_recurse_submodules () { " } -test_incompatible_with_recurse_submodules -z -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others -test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v5 4/4] ls-files: add pathspec matching for submodules 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams ` (2 preceding siblings ...) 2016-09-28 21:50 ` [PATCH v5 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-09-28 21:50 ` Brandon Williams 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-28 21:50 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 28 ++++++--- dir.c | 46 +++++++++++++- dir.h | 4 ++ t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++- 4 files changed, 174 insertions(+), 12 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index f377e36..c8c5c29 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -215,6 +215,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + int i; argv_array_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? super_prefix : "", @@ -225,6 +226,15 @@ static void show_gitlink(const struct cache_entry *ce) /* add supported options */ argv_array_pushv(&cp.args, submodules_options.argv); + /* + * Pass in the original pathspec args. The submodule will be + * responsible for prepending the 'submodule_prefix' prior to comparing + * against the pathspec for matches. + */ + argv_array_push(&cp.args, "--"); + for (i = 0; i < pathspec.nr; i++) + argv_array_push(&cp.args, pathspec.items[i].original); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -243,7 +253,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && + submodule_path_match(&pathspec, name.buf, ps_matched)) { show_gitlink(ce); } else if (match_pathspec(&pathspec, name.buf, name.len, len, ps_matched, @@ -623,17 +634,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) die("ls-files --recurse-submodules does not support " "--error-unmatch"); - if (recurse_submodules && argc) - die("ls-files --recurse-submodules does not support path " - "arguments"); - 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); + /* + * Find common prefix for all pathspec's + * This is used as a performance optimization which unfortunately cannot + * be done when recursing into submodules + */ + if (recurse_submodules) + max_prefix = NULL; + else + 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 0ea235f..28e9736 100644 --- a/dir.c +++ b/dir.c @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen, return 1; } -#define DO_MATCH_EXCLUDE 1 -#define DO_MATCH_DIRECTORY 2 +#define DO_MATCH_EXCLUDE (1<<0) +#define DO_MATCH_DIRECTORY (1<<1) +#define DO_MATCH_SUBMODULE (1<<2) /* * Does 'match' match the given name? @@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, item->nowildcard_len - prefix)) return MATCHED_FNMATCH; + /* Perform checks to see if "name" is a super set of the pathspec */ + if (flags & DO_MATCH_SUBMODULE) { + /* name is a literal prefix of the pathspec */ + if ((namelen < matchlen) && + (match[namelen] == '/') && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_RECURSIVELY; + + /* name" doesn't match up to the first wild character */ + if (item->nowildcard_len < item->len && + ps_strncmp(item, match, name, + item->nowildcard_len - prefix)) + return 0; + + /* + * Here is where we would perform a wildmatch to check if + * "name" can be matched as a directory (or a prefix) against + * the pathspec. Since wildmatch doesn't have this capability + * at the present we have to punt and say that it is a match, + * potentially returning a false positive + * The submodules themselves will be able to perform more + * accurate matching to determine if the pathspec matches. + */ + return MATCHED_RECURSIVELY; + } + return 0; } @@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps, return negative ? 0 : positive; } +/** + * Check if a submodule is a superset of the pathspec + */ +int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen) +{ + int matched = do_match_pathspec(ps, submodule_name, + strlen(submodule_name), + 0, seen, + DO_MATCH_DIRECTORY | + DO_MATCH_SUBMODULE); + return matched; +} + int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix) diff --git a/dir.h b/dir.h index da1a858..97c83bb 100644 --- a/dir.h +++ b/dir.h @@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +extern int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen); + static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index 79107d8..5475855 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' -test_expect_success '--recurse-submodules does not support using path arguments' ' - test_must_fail git ls-files --recurse-submodules b 2>actual && - test_i18ngrep "does not support path arguments" actual +test_expect_success '--recurse-submodules and pathspecs setup' ' + echo e >submodule/subsub/e.txt && + git -C submodule/subsub add e.txt && + git -C submodule/subsub commit -m "adding e.txt" && + echo f >submodule/f.TXT && + echo g >submodule/g.txt && + git -C submodule add f.TXT g.txt && + git -C submodule commit -m "add f and g" && + echo h >h.txt && + mkdir sib && + echo sib >sib/file && + git add h.txt sib/file && + git commit -m "add h and sib/file" && + git init sub && + echo sub >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git submodule add ./sub && + git commit -m "added sub" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub/d + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + cat actual && + git ls-files --recurse-submodules "*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules "*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sub/file + EOF + + git ls-files --recurse-submodules "sub" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su*/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su?/file" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sib/file + sub/file + EOF + + git ls-files --recurse-submodules "s??/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s???file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s*file" >actual && + test_cmp expect actual ' test_expect_success '--recurse-submodules does not support --error-unmatch' ' -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v6 0/4] recursive support for ls-files 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams ` (3 preceding siblings ...) 2016-09-28 21:50 ` [PATCH v5 4/4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-09-29 21:48 ` Brandon Williams 2016-09-29 21:48 ` [PATCH v6 1/4] git: make super-prefix option Brandon Williams ` (4 more replies) 4 siblings, 5 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-29 21:48 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Minor fixes per the comments on version 5. Brandon Williams (4): git: make super-prefix option ls-files: optionally recurse into submodules ls-files: pass through safe options for --recurse-submodules ls-files: add pathspec matching for submodules Documentation/git-ls-files.txt | 7 +- Documentation/git.txt | 6 + builtin/ls-files.c | 202 ++++++++++++++++++++++++------- cache.h | 2 + dir.c | 46 +++++++- dir.h | 4 + environment.c | 10 ++ git.c | 28 ++++- t/t3007-ls-files-recurse-submodules.sh | 209 +++++++++++++++++++++++++++++++++ 9 files changed, 470 insertions(+), 44 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh -- 2.10.0 ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v6 1/4] git: make super-prefix option 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams @ 2016-09-29 21:48 ` Brandon Williams 2016-10-04 17:31 ` Stefan Beller 2016-09-29 21:48 ` [PATCH v6 2/4] ls-files: optionally recurse into submodules Brandon Williams ` (3 subsequent siblings) 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-29 21:48 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' which can be used to specify a path from above a repository down to its root. When such a super-prefix is specified, the paths reported by Git are prefixed with it to make them relative to that directory "above". The paths given by the user on the command line (e.g. "git subcmd --output-file=path/to/a/file" and pathspecs) are taken relative to the directory "above" to match. The immediate use of this option is by commands which have a --recurse-submodule option in order to give context to submodules about how they were invoked. This option is currently only allowed for builtins which support a super-prefix. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git.txt | 6 ++++++ cache.h | 2 ++ environment.c | 10 ++++++++++ git.c | 26 ++++++++++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7913fc2..2188ae6 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -13,6 +13,7 @@ SYNOPSIS [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>] + [--super-prefix=<path>] <command> [<args>] DESCRIPTION @@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string. details. Equivalent to setting the `GIT_NAMESPACE` environment variable. +--super-prefix=<path>:: + Currently for internal use only. Set a prefix which gives a path from + above a repository down to its root. One use is to give submodules + context about the superproject that invoked it. + --bare:: Treat the repository as a bare repository. If GIT_DIR environment is not set, it is set to the current working diff --git a/cache.h b/cache.h index 3556326..8cf495d 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" +#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" @@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); extern int get_common_dir(struct strbuf *sb, const char *gitdir); extern const char *get_git_namespace(void); extern const char *strip_namespace(const char *namespaced_ref); +extern const char *get_super_prefix(void); extern const char *get_git_work_tree(void); /* diff --git a/environment.c b/environment.c index ca72464..13f3d70 100644 --- a/environment.c +++ b/environment.c @@ -100,6 +100,8 @@ static char *work_tree; static const char *namespace; static size_t namespace_len; +static const char *super_prefix; + static const char *git_dir, *git_common_dir; static char *git_object_dir, *git_index_file, *git_graft_file; int git_db_env, git_index_env, git_graft_env, git_common_dir_env; @@ -120,6 +122,7 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, + GIT_SUPER_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL @@ -222,6 +225,13 @@ const char *strip_namespace(const char *namespaced_ref) return namespaced_ref + namespace_len; } +const char *get_super_prefix(void) +{ + if (!super_prefix) + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); + return super_prefix; +} + static int git_work_tree_initialized; /* diff --git a/git.c b/git.c index 1c61151..f756b62 100644 --- a/git.c +++ b/git.c @@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); if (envchanged) *envchanged = 1; + } else if (!strcmp(cmd, "--super-prefix")) { + if (*argc < 2) { + fprintf(stderr, "No prefix given for --super-prefix.\n" ); + usage(git_usage_string); + } + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); + if (envchanged) + *envchanged = 1; + (*argv)++; + (*argc)--; + } else if (skip_prefix(cmd, "--super-prefix=", &cmd)) { + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1); + if (envchanged) + *envchanged = 1; } else if (!strcmp(cmd, "--bare")) { char *cwd = xgetcwd(); is_bare_repository_cfg = 1; @@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv) * RUN_SETUP for reading from the configuration file. */ #define NEED_WORK_TREE (1<<3) +#define SUPPORT_SUPER_PREFIX (1<<4) struct cmd_struct { const char *cmd; @@ -344,6 +359,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) } commit_pager_choice(); + if (!help && get_super_prefix()) { + if (!(p->option & SUPPORT_SUPER_PREFIX)) + die("%s doesn't support --super-prefix", p->cmd); + if (prefix) + die("can't use --super-prefix from a subdirectory"); + } + if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); @@ -558,6 +580,10 @@ static void execv_dashed_external(const char **argv) const char *tmp; int status; + if (get_super_prefix()) { + die("%s doesn't support --super-prefix", argv[0]); + } + if (use_pager == -1) use_pager = check_pager_config(argv[0]); commit_pager_choice(); -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v6 1/4] git: make super-prefix option 2016-09-29 21:48 ` [PATCH v6 1/4] git: make super-prefix option Brandon Williams @ 2016-10-04 17:31 ` Stefan Beller 2016-10-04 17:35 ` Junio C Hamano 2016-10-04 17:39 ` Jeff King 0 siblings, 2 replies; 67+ messages in thread From: Stefan Beller @ 2016-10-04 17:31 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Thu, Sep 29, 2016 at 2:48 PM, Brandon Williams <bmwill@google.com> wrote: > > +const char *get_super_prefix(void) > +{ > + if (!super_prefix) > + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); > + return super_prefix; > +} > + As said earlier, is the following a valid thought: > The getenv() function returns a pointer to the value in the > environment, or NULL if there is no match. > So in case this is not set (when e.g. the user did not specify the > super prefix), we would probe it a couple of times. > The caching effect only occurs when the string is set. So this looks > like we save repetitive calls, but we do not always do that. > > + if (get_super_prefix()) { > + die("%s doesn't support --super-prefix", argv[0]); > + } > + Nit: no braces, please. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v6 1/4] git: make super-prefix option 2016-10-04 17:31 ` Stefan Beller @ 2016-10-04 17:35 ` Junio C Hamano 2016-10-04 17:39 ` Jeff King 1 sibling, 0 replies; 67+ messages in thread From: Junio C Hamano @ 2016-10-04 17:35 UTC (permalink / raw) To: Stefan Beller; +Cc: Brandon Williams, git, Jeff King Stefan Beller <sbeller@google.com> writes: > On Thu, Sep 29, 2016 at 2:48 PM, Brandon Williams <bmwill@google.com> wrote: > >> >> +const char *get_super_prefix(void) >> +{ >> + if (!super_prefix) >> + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); >> + return super_prefix; >> +} >> + > > As said earlier, is the following a valid thought: > >> The getenv() function returns a pointer to the value in the >> environment, or NULL if there is no match. >> So in case this is not set (when e.g. the user did not specify the >> super prefix), we would probe it a couple of times. >> The caching effect only occurs when the string is set. So this looks >> like we save repetitive calls, but we do not always do that. That reading is correct. If the code wants to do the caching, it should do so correctly. Unless we expect that some callers might want to be able to invalidate the cache, get_super_prefix(void) { static int initialized; if (!initialized) { super_prefix = getenv(...); initialized = 1; } return super_prefix; } would suffice. Thanks for careful reading. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v6 1/4] git: make super-prefix option 2016-10-04 17:31 ` Stefan Beller 2016-10-04 17:35 ` Junio C Hamano @ 2016-10-04 17:39 ` Jeff King 1 sibling, 0 replies; 67+ messages in thread From: Jeff King @ 2016-10-04 17:39 UTC (permalink / raw) To: Stefan Beller; +Cc: Brandon Williams, git, Junio C Hamano On Tue, Oct 04, 2016 at 10:31:51AM -0700, Stefan Beller wrote: > On Thu, Sep 29, 2016 at 2:48 PM, Brandon Williams <bmwill@google.com> wrote: > > > > > +const char *get_super_prefix(void) > > +{ > > + if (!super_prefix) > > + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); > > + return super_prefix; > > +} > > + > > As said earlier, is the following a valid thought: > > > The getenv() function returns a pointer to the value in the > > environment, or NULL if there is no match. > > So in case this is not set (when e.g. the user did not specify the > > super prefix), we would probe it a couple of times. > > The caching effect only occurs when the string is set. So this looks > > like we save repetitive calls, but we do not always do that. I think your concern is valid. If it is not set, we will do an O(n) search through the whole environment on each call. I also think the result of getenv() needs to be copied. In some implementations it persists for the life of the program, but that's not guaranteed; it may be overwritten by unrelated calls to getenv() or setenv(). -Peff ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v6 2/4] ls-files: optionally recurse into submodules 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams 2016-09-29 21:48 ` [PATCH v6 1/4] git: make super-prefix option Brandon Williams @ 2016-09-29 21:48 ` Brandon Williams 2016-09-29 21:48 ` [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams ` (2 subsequent siblings) 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-29 21:48 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Allow ls-files to recognize submodules in order to retrieve a list of files from a repository's submodules. This is done by forking off a process to recursively call ls-files on all submodules. Use top-level --super-prefix option to pass a path to the submodule which it can use to prepend to output or pathspec matching logic. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 8 +- builtin/ls-files.c | 138 ++++++++++++++++++++++++--------- git.c | 2 +- t/t3007-ls-files-recurse-submodules.sh | 100 ++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 40 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 0d933ac..ea01d45 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -18,7 +18,8 @@ SYNOPSIS [--exclude-per-directory=<file>] [--exclude-standard] [--error-unmatch] [--with-tree=<tree-ish>] - [--full-name] [--abbrev] [--] [<file>...] + [--full-name] [--recurse-submodules] + [--abbrev] [--] [<file>...] DESCRIPTION ----------- @@ -137,6 +138,11 @@ a space) at the start of each line: option forces paths to be output relative to the project top directory. +--recurse-submodules:: + Recursively calls ls-files on each submodule in the repository. + Currently there is only support for the --cached mode without a + pathspec. + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object lines, show only a partial prefix. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00ea91a..63befed 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -14,6 +14,7 @@ #include "resolve-undo.h" #include "string-list.h" #include "pathspec.h" +#include "run-command.h" static int abbrev; static int show_deleted; @@ -28,8 +29,10 @@ static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; static int show_eol; +static int recurse_submodules; static const char *prefix; +static const char *super_prefix; static int max_prefix_len; static int prefix_len; static struct pathspec pathspec; @@ -68,11 +71,24 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) static void write_name(const char *name) { /* + * Prepend the super_prefix to name to construct the full_name to be + * written. + */ + struct strbuf full_name = STRBUF_INIT; + if (super_prefix) { + strbuf_addstr(&full_name, super_prefix); + strbuf_addstr(&full_name, name); + name = full_name.buf; + } + + /* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ write_name_quoted_relative(name, prefix_len ? prefix : NULL, stdout, line_terminator); + + strbuf_release(&full_name); } static void show_dir_entry(const char *tag, struct dir_entry *ent) @@ -152,55 +168,84 @@ static void show_killed_files(struct dir_struct *dir) } } +/** + * Recursively call ls-files on a submodule + */ +static void show_gitlink(const struct cache_entry *ce) +{ + struct child_process cp = CHILD_PROCESS_INIT; + int status; + + argv_array_pushf(&cp.args, "--super-prefix=%s%s/", + super_prefix ? super_prefix : "", + ce->name); + argv_array_push(&cp.args, "ls-files"); + argv_array_push(&cp.args, "--recurse-submodules"); + + cp.git_cmd = 1; + cp.dir = ce->name; + status = run_command(&cp); + if (status) + exit(status); +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { + struct strbuf name = STRBUF_INIT; int len = max_prefix_len; + if (super_prefix) + strbuf_addstr(&name, super_prefix); + strbuf_addstr(&name, ce->name); 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, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) - return; + if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + show_gitlink(ce); + } else if (match_pathspec(&pathspec, name.buf, name.len, + len, ps_matched, + S_ISDIR(ce->ce_mode) || + S_ISGITLINK(ce->ce_mode))) { + if (tag && *tag && show_valid_bit && + (ce->ce_flags & CE_VALID)) { + static char alttag[4]; + memcpy(alttag, tag, 3); + if (isalpha(tag[0])) + alttag[0] = tolower(tag[0]); + else if (tag[0] == '?') + alttag[0] = '!'; + else { + alttag[0] = 'v'; + alttag[1] = tag[0]; + alttag[2] = ' '; + alttag[3] = 0; + } + tag = alttag; + } - if (tag && *tag && show_valid_bit && - (ce->ce_flags & CE_VALID)) { - static char alttag[4]; - memcpy(alttag, tag, 3); - if (isalpha(tag[0])) - alttag[0] = tolower(tag[0]); - else if (tag[0] == '?') - alttag[0] = '!'; - else { - alttag[0] = 'v'; - alttag[1] = tag[0]; - alttag[2] = ' '; - alttag[3] = 0; + if (!show_stage) { + fputs(tag, stdout); + } else { + printf("%s%06o %s %d\t", + tag, + ce->ce_mode, + find_unique_abbrev(ce->sha1,abbrev), + ce_stage(ce)); + } + write_eolinfo(ce, ce->name); + write_name(ce->name); + if (debug_mode) { + const struct stat_data *sd = &ce->ce_stat_data; + + printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); + printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); + printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); + printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); + printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); } - tag = alttag; } - if (!show_stage) { - fputs(tag, stdout); - } else { - printf("%s%06o %s %d\t", - tag, - ce->ce_mode, - find_unique_abbrev(ce->sha1,abbrev), - ce_stage(ce)); - } - write_eolinfo(ce, ce->name); - write_name(ce->name); - if (debug_mode) { - const struct stat_data *sd = &ce->ce_stat_data; - - printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); - printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); - printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); - printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); - printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); - } + strbuf_release(&name); } static void show_ru_info(void) @@ -468,6 +513,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) { OPTION_SET_INT, 0, "full-name", &prefix_len, NULL, N_("make the output relative to the project top directory"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOL(0, "recurse-submodules", &recurse_submodules, + N_("recurse through submodules")), OPT_BOOL(0, "error-unmatch", &error_unmatch, N_("if any <file> is not in the index, treat this as an error")), OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"), @@ -484,6 +531,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) prefix = cmd_prefix; if (prefix) prefix_len = strlen(prefix); + super_prefix = get_super_prefix(); git_config(git_default_config, NULL); if (read_cache() < 0) @@ -519,6 +567,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules && + (show_stage || show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo || + show_valid_bit || show_tag || show_eol || with_tree || + (line_terminator == '\0'))) + die("ls-files --recurse-submodules unsupported mode"); + + if (recurse_submodules && error_unmatch) + die("ls-files --recurse-submodules does not support " + "--error-unmatch"); + + if (recurse_submodules && argc) + die("ls-files --recurse-submodules does not support pathspec"); + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD | PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, diff --git a/git.c b/git.c index f756b62..1bbd5ae 100644 --- a/git.c +++ b/git.c @@ -443,7 +443,7 @@ static struct cmd_struct commands[] = { { "init-db", cmd_init_db }, { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY }, { "log", cmd_log, RUN_SETUP }, - { "ls-files", cmd_ls_files, RUN_SETUP }, + { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX }, { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY }, { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo }, diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh new file mode 100755 index 0000000..b5a53c3 --- /dev/null +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -0,0 +1,100 @@ +#!/bin/sh + +test_description='Test ls-files recurse-submodules feature + +This test verifies the recurse-submodules feature correctly lists files from +submodules. +' + +. ./test-lib.sh + +test_expect_success 'setup directory structure and submodules' ' + echo a >a && + mkdir b && + echo b >b/b && + git add a b && + git commit -m "add a and b" && + git init submodule && + echo c >submodule/c && + git -C submodule add c && + git -C submodule commit -m "add c" && + git submodule add ./submodule && + git commit -m "added submodule" +' + +test_expect_success 'ls-files correctly outputs files in submodule' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files does not output files not added to a repo' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + echo a >not_added && + echo b >b/not_added && + echo c >submodule/not_added && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files recurses more than 1 level' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/.gitmodules + submodule/c + submodule/subsub/d + EOF + + git init submodule/subsub && + echo d >submodule/subsub/d && + git -C submodule/subsub add d && + git -C submodule/subsub commit -m "add d" && + git -C submodule submodule add ./subsub && + git -C submodule commit -m "added subsub" && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules does not support using path arguments' ' + test_must_fail git ls-files --recurse-submodules b 2>actual && + test_i18ngrep "does not support pathspec" actual +' + +test_expect_success '--recurse-submodules does not support --error-unmatch' ' + test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual && + test_i18ngrep "does not support --error-unmatch" actual +' + +test_incompatible_with_recurse_submodules () { + test_expect_success "--recurse-submodules and $1 are incompatible" " + test_must_fail git ls-files --recurse-submodules $1 2>actual && + test_i18ngrep 'unsupported mode' actual + " +} + +test_incompatible_with_recurse_submodules -z +test_incompatible_with_recurse_submodules -v +test_incompatible_with_recurse_submodules -t +test_incompatible_with_recurse_submodules --deleted +test_incompatible_with_recurse_submodules --modified +test_incompatible_with_recurse_submodules --others +test_incompatible_with_recurse_submodules --stage +test_incompatible_with_recurse_submodules --killed +test_incompatible_with_recurse_submodules --unmerged +test_incompatible_with_recurse_submodules --eol + +test_done -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams 2016-09-29 21:48 ` [PATCH v6 1/4] git: make super-prefix option Brandon Williams 2016-09-29 21:48 ` [PATCH v6 2/4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-09-29 21:48 ` Brandon Williams 2016-09-30 0:14 ` Junio C Hamano 2016-09-29 21:48 ` [PATCH v6 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-29 21:48 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pass through some known-safe options when recursing into submodules. (--cached, --stage, -v, -t, -z, --debug, --eol) Other options are compiled into an argv_array but if an unsafe option is given the caller will be errored out. Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 51 ++++++++++++++++++++++++++++++++-- t/t3007-ls-files-recurse-submodules.sh | 17 ++++++++---- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 63befed..6f744ef 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -30,6 +30,7 @@ static int line_terminator = '\n'; static int debug_mode; static int show_eol; static int recurse_submodules; +static struct argv_array submodules_options = ARGV_ARRAY_INIT; static const char *prefix; static const char *super_prefix; @@ -168,6 +169,45 @@ static void show_killed_files(struct dir_struct *dir) } } +/* + * Compile an argv_array with all of the options supported by --recurse_submodules + */ +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) +{ + if (line_terminator == '\0') + argv_array_push(&submodules_options, "-z"); + if (show_tag) + argv_array_push(&submodules_options, "-t"); + if (show_valid_bit) + argv_array_push(&submodules_options, "-v"); + if (show_cached) + argv_array_push(&submodules_options, "--cached"); + if (show_deleted) + argv_array_push(&submodules_options, "--deleted"); + if (show_modified) + argv_array_push(&submodules_options, "--modified"); + if (show_others) + argv_array_push(&submodules_options, "--others"); + if (dir->flags & DIR_SHOW_IGNORED) + argv_array_push(&submodules_options, "--ignored"); + if (show_stage) + argv_array_push(&submodules_options, "--stage"); + if (show_killed) + argv_array_push(&submodules_options, "--killed"); + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) + argv_array_push(&submodules_options, "--directory"); + if (!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES)) + argv_array_push(&submodules_options, "--empty-directory"); + if (show_unmerged) + argv_array_push(&submodules_options, "--unmerged"); + if (show_resolve_undo) + argv_array_push(&submodules_options, "--resolve-undo"); + if (show_eol) + argv_array_push(&submodules_options, "--eol"); + if (debug_mode) + argv_array_push(&submodules_options, "--debug"); +} + /** * Recursively call ls-files on a submodule */ @@ -182,6 +222,9 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); + /* add supported options */ + argv_array_pushv(&cp.args, submodules_options.argv); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -567,11 +610,13 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + compile_submodule_options(&dir, show_tag); + if (recurse_submodules && - (show_stage || show_deleted || show_others || show_unmerged || + (show_deleted || show_others || show_unmerged || show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol || with_tree || - (line_terminator == '\0'))) + with_tree)) die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index b5a53c3..e76fa30 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + lf_to_nul >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -86,15 +98,10 @@ test_incompatible_with_recurse_submodules () { " } -test_incompatible_with_recurse_submodules -z -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others -test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules 2016-09-29 21:48 ` [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-09-30 0:14 ` Junio C Hamano 2016-09-30 16:33 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Junio C Hamano @ 2016-09-30 0:14 UTC (permalink / raw) To: Brandon Williams; +Cc: git, sbeller, peff Brandon Williams <bmwill@google.com> writes: > +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) > +{ > + if (line_terminator == '\0') > + argv_array_push(&submodules_options, "-z"); > + if (show_tag) > + argv_array_push(&submodules_options, "-t"); > + if (show_valid_bit) > + argv_array_push(&submodules_options, "-v"); > + if (show_cached) > + argv_array_push(&submodules_options, "--cached"); > + if (show_deleted) > + argv_array_push(&submodules_options, "--deleted"); > + if (show_modified) > + argv_array_push(&submodules_options, "--modified"); > + if (show_others) > + argv_array_push(&submodules_options, "--others"); > + if (dir->flags & DIR_SHOW_IGNORED) > + argv_array_push(&submodules_options, "--ignored"); > + if (show_stage) > + argv_array_push(&submodules_options, "--stage"); > + if (show_killed) > + argv_array_push(&submodules_options, "--killed"); > + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) > + argv_array_push(&submodules_options, "--directory"); > + if (!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES)) > + argv_array_push(&submodules_options, "--empty-directory"); > + if (show_unmerged) > + argv_array_push(&submodules_options, "--unmerged"); > + if (show_resolve_undo) > + argv_array_push(&submodules_options, "--resolve-undo"); > + if (show_eol) > + argv_array_push(&submodules_options, "--eol"); > + if (debug_mode) > + argv_array_push(&submodules_options, "--debug"); > +} With this and 4/4 applied, the documentation still says "--cached" is the only supported option. Does it really make sense to pass all of these? I understand "-z" and I suspect things like "-t" and "-v" that affect "how" things are shown may also happen to work, but I am not sure how much it makes sense for options that affect "what" things are shown. What does it even mean to ask for say "--unmerged" to be shown, for example, from the superproject? Recurse into submodules whose cache entries in the index of the superproject are unmerged, or something else? I am inclined to say that it is probably better to keep the "--cached only" as documented, at least on the "what are shown" side. Thanks. ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules 2016-09-30 0:14 ` Junio C Hamano @ 2016-09-30 16:33 ` Brandon Williams 2016-09-30 17:01 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-30 16:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, sbeller, peff On 09/29, Junio C Hamano wrote: > Brandon Williams <bmwill@google.com> writes: > > > +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) > > +{ > > + if (line_terminator == '\0') > > + argv_array_push(&submodules_options, "-z"); > > + if (show_tag) > > + argv_array_push(&submodules_options, "-t"); > > + if (show_valid_bit) > > + argv_array_push(&submodules_options, "-v"); > > + if (show_cached) > > + argv_array_push(&submodules_options, "--cached"); > > + if (show_deleted) > > + argv_array_push(&submodules_options, "--deleted"); > > + if (show_modified) > > + argv_array_push(&submodules_options, "--modified"); > > + if (show_others) > > + argv_array_push(&submodules_options, "--others"); > > + if (dir->flags & DIR_SHOW_IGNORED) > > + argv_array_push(&submodules_options, "--ignored"); > > + if (show_stage) > > + argv_array_push(&submodules_options, "--stage"); > > + if (show_killed) > > + argv_array_push(&submodules_options, "--killed"); > > + if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) > > + argv_array_push(&submodules_options, "--directory"); > > + if (!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES)) > > + argv_array_push(&submodules_options, "--empty-directory"); > > + if (show_unmerged) > > + argv_array_push(&submodules_options, "--unmerged"); > > + if (show_resolve_undo) > > + argv_array_push(&submodules_options, "--resolve-undo"); > > + if (show_eol) > > + argv_array_push(&submodules_options, "--eol"); > > + if (debug_mode) > > + argv_array_push(&submodules_options, "--debug"); > > +} > > With this and 4/4 applied, the documentation still says "--cached" > is the only supported option. > > Does it really make sense to pass all of these? I understand "-z" > and I suspect things like "-t" and "-v" that affect "how" things are > shown may also happen to work, but I am not sure how much it makes > sense for options that affect "what" things are shown. > > What does it even mean to ask for say "--unmerged" to be shown, for > example, from the superproject? Recurse into submodules whose cache > entries in the index of the superproject are unmerged, or something > else? > > I am inclined to say that it is probably better to keep the > "--cached only" as documented, at least on the "what are shown" > side. > > Thanks. You're right that probably makes the most sense for now. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules 2016-09-30 16:33 ` Brandon Williams @ 2016-09-30 17:01 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-09-30 17:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, sbeller, peff Pass through some known-safe options when recursing into submodules. (--cached, -v, -t, -z, --debug, --eol) If other unsafe options are given the caller will be errored out. Signed-off-by: Brandon Williams <bmwill@google.com> --- Something more like this correct? I ditched the extra parameters and reworded the commit msg to reflect this. builtin/ls-files.c | 30 +++++++++++++++++++++++++++--- t/t3007-ls-files-recurse-submodules.sh | 16 ++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 63befed..b6144a5 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -30,6 +30,7 @@ static int line_terminator = '\n'; static int debug_mode; static int show_eol; static int recurse_submodules; +static struct argv_array submodules_options = ARGV_ARRAY_INIT; static const char *prefix; static const char *super_prefix; @@ -168,6 +169,25 @@ static void show_killed_files(struct dir_struct *dir) } } +/* + * Compile an argv_array with all of the options supported by --recurse_submodules + */ +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) +{ + if (line_terminator == '\0') + argv_array_push(&submodules_options, "-z"); + if (show_tag) + argv_array_push(&submodules_options, "-t"); + if (show_valid_bit) + argv_array_push(&submodules_options, "-v"); + if (show_cached) + argv_array_push(&submodules_options, "--cached"); + if (show_eol) + argv_array_push(&submodules_options, "--eol"); + if (debug_mode) + argv_array_push(&submodules_options, "--debug"); +} + /** * Recursively call ls-files on a submodule */ @@ -182,6 +202,9 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); + /* add supported options */ + argv_array_pushv(&cp.args, submodules_options.argv); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -567,11 +590,12 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + compile_submodule_options(&dir, show_tag); + if (recurse_submodules && (show_stage || show_deleted || show_others || show_unmerged || - show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol || with_tree || - (line_terminator == '\0'))) + show_killed || show_modified || show_resolve_undo || with_tree)) die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index b5a53c3..33a2ea7 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + lf_to_nul >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -86,15 +98,11 @@ test_incompatible_with_recurse_submodules () { " } -test_incompatible_with_recurse_submodules -z -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v6 4/4] ls-files: add pathspec matching for submodules 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams ` (2 preceding siblings ...) 2016-09-29 21:48 ` [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-09-29 21:48 ` Brandon Williams 2016-10-04 17:56 ` Stefan Beller 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams 4 siblings, 1 reply; 67+ messages in thread From: Brandon Williams @ 2016-09-29 21:48 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 3 +- builtin/ls-files.c | 27 +++++++-- dir.c | 46 +++++++++++++- dir.h | 4 ++ t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++- 5 files changed, 175 insertions(+), 13 deletions(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index ea01d45..51ec9a1 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -140,8 +140,7 @@ a space) at the start of each line: --recurse-submodules:: Recursively calls ls-files on each submodule in the repository. - Currently there is only support for the --cached mode without a - pathspec. + Currently there is only support for the --cached. --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 6f744ef..82ec811 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -215,6 +215,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + int i; argv_array_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? super_prefix : "", @@ -225,6 +226,15 @@ static void show_gitlink(const struct cache_entry *ce) /* add supported options */ argv_array_pushv(&cp.args, submodules_options.argv); + /* + * Pass in the original pathspec args. The submodule will be + * responsible for prepending the 'submodule_prefix' prior to comparing + * against the pathspec for matches. + */ + argv_array_push(&cp.args, "--"); + for (i = 0; i < pathspec.nr; i++) + argv_array_push(&cp.args, pathspec.items[i].original); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -243,7 +253,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && + submodule_path_match(&pathspec, name.buf, ps_matched)) { show_gitlink(ce); } else if (match_pathspec(&pathspec, name.buf, name.len, len, ps_matched, @@ -623,16 +634,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) die("ls-files --recurse-submodules does not support " "--error-unmatch"); - if (recurse_submodules && argc) - die("ls-files --recurse-submodules does not support pathspec"); - 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); + /* + * Find common prefix for all pathspec's + * This is used as a performance optimization which unfortunately cannot + * be done when recursing into submodules + */ + if (recurse_submodules) + max_prefix = NULL; + else + 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 0ea235f..28e9736 100644 --- a/dir.c +++ b/dir.c @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen, return 1; } -#define DO_MATCH_EXCLUDE 1 -#define DO_MATCH_DIRECTORY 2 +#define DO_MATCH_EXCLUDE (1<<0) +#define DO_MATCH_DIRECTORY (1<<1) +#define DO_MATCH_SUBMODULE (1<<2) /* * Does 'match' match the given name? @@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, item->nowildcard_len - prefix)) return MATCHED_FNMATCH; + /* Perform checks to see if "name" is a super set of the pathspec */ + if (flags & DO_MATCH_SUBMODULE) { + /* name is a literal prefix of the pathspec */ + if ((namelen < matchlen) && + (match[namelen] == '/') && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_RECURSIVELY; + + /* name" doesn't match up to the first wild character */ + if (item->nowildcard_len < item->len && + ps_strncmp(item, match, name, + item->nowildcard_len - prefix)) + return 0; + + /* + * Here is where we would perform a wildmatch to check if + * "name" can be matched as a directory (or a prefix) against + * the pathspec. Since wildmatch doesn't have this capability + * at the present we have to punt and say that it is a match, + * potentially returning a false positive + * The submodules themselves will be able to perform more + * accurate matching to determine if the pathspec matches. + */ + return MATCHED_RECURSIVELY; + } + return 0; } @@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps, return negative ? 0 : positive; } +/** + * Check if a submodule is a superset of the pathspec + */ +int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen) +{ + int matched = do_match_pathspec(ps, submodule_name, + strlen(submodule_name), + 0, seen, + DO_MATCH_DIRECTORY | + DO_MATCH_SUBMODULE); + return matched; +} + int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix) diff --git a/dir.h b/dir.h index da1a858..97c83bb 100644 --- a/dir.h +++ b/dir.h @@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +extern int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen); + static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index e76fa30..5475855 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' -test_expect_success '--recurse-submodules does not support using path arguments' ' - test_must_fail git ls-files --recurse-submodules b 2>actual && - test_i18ngrep "does not support pathspec" actual +test_expect_success '--recurse-submodules and pathspecs setup' ' + echo e >submodule/subsub/e.txt && + git -C submodule/subsub add e.txt && + git -C submodule/subsub commit -m "adding e.txt" && + echo f >submodule/f.TXT && + echo g >submodule/g.txt && + git -C submodule add f.TXT g.txt && + git -C submodule commit -m "add f and g" && + echo h >h.txt && + mkdir sib && + echo sib >sib/file && + git add h.txt sib/file && + git commit -m "add h and sib/file" && + git init sub && + echo sub >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git submodule add ./sub && + git commit -m "added sub" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub/d + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + cat actual && + git ls-files --recurse-submodules "*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules "*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sub/file + EOF + + git ls-files --recurse-submodules "sub" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su*/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su?/file" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sib/file + sub/file + EOF + + git ls-files --recurse-submodules "s??/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s???file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s*file" >actual && + test_cmp expect actual ' test_expect_success '--recurse-submodules does not support --error-unmatch' ' -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v6 4/4] ls-files: add pathspec matching for submodules 2016-09-29 21:48 ` [PATCH v6 4/4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-10-04 17:56 ` Stefan Beller 0 siblings, 0 replies; 67+ messages in thread From: Stefan Beller @ 2016-10-04 17:56 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Thu, Sep 29, 2016 at 2:48 PM, Brandon Williams <bmwill@google.com> wrote: > Pathspecs can be a bit tricky when trying to apply them to submodules. > The main challenge is that the pathspecs will be with respect to the > superproject and not with respect to paths in the submodule. The > approach this patch takes is to pass in the identical pathspec from the > superproject to the submodule in addition to the submodule-prefix, which > is the path from the root of the superproject to the submodule, and then > we can compare an entry in the submodule prepended with the > submodule-prefix to the pathspec in order to determine if there is a > match. > > This patch also permits the pathspec logic to perform a prefix match against > submodules since a pathspec could refer to a file inside of a submodule. > Due to limitations in the wildmatch logic, a prefix match is only done > literally. If any wildcard character is encountered we'll simply punt > and produce a false positive match. More accurate matching will be done > once inside the submodule. This is due to the superproject not knowing > what files could exist in the submodule. > > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > Documentation/git-ls-files.txt | 3 +- > builtin/ls-files.c | 27 +++++++-- > dir.c | 46 +++++++++++++- > dir.h | 4 ++ > t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++- > 5 files changed, 175 insertions(+), 13 deletions(-) > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt > index ea01d45..51ec9a1 100644 > --- a/Documentation/git-ls-files.txt > +++ b/Documentation/git-ls-files.txt > @@ -140,8 +140,7 @@ a space) at the start of each line: > > --recurse-submodules:: > Recursively calls ls-files on each submodule in the repository. > - Currently there is only support for the --cached mode without a > - pathspec. > + Currently there is only support for the --cached. s/--cached/--cached mode/ ? The "the" in front of --cached sounds a bit strange for a non native speaker here. > + /* > + * Find common prefix for all pathspec's > + * This is used as a performance optimization which unfortunately cannot > + * be done when recursing into submodules > + */ > + if (recurse_submodules) > + max_prefix = NULL; > + else > + max_prefix = common_prefix(&pathspec); Nit of the day: While this is readable, you may want to explore how this reads shorter as max_prefix = recurse_submodules ? NULL : common_prefix(&pathspec); ? > + git ls-files --recurse-submodules "sub" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "sub/" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "sub/file" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "su*/file" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "su?/file" >actual && > + test_cmp expect actual > +' > + > + git ls-files --recurse-submodules "s??/file" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "s???file" >actual && > + test_cmp expect actual && > + git ls-files --recurse-submodules "s*file" >actual && > + test_cmp expect actual > ' Thanks for the tests! ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v7 0/4] recursive support for ls-files 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams ` (3 preceding siblings ...) 2016-09-29 21:48 ` [PATCH v6 4/4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-10-07 18:18 ` Brandon Williams 2016-10-07 18:18 ` [PATCH v7 1/4] git: make super-prefix option Brandon Williams ` (4 more replies) 4 siblings, 5 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Few minor fixes pointed out Stefan Brandon Williams (4): git: make super-prefix option ls-files: optionally recurse into submodules ls-files: pass through safe options for --recurse-submodules ls-files: add pathspec matching for submodules Documentation/git-ls-files.txt | 7 +- Documentation/git.txt | 6 + builtin/ls-files.c | 181 +++++++++++++++++++++------- cache.h | 2 + dir.c | 46 +++++++- dir.h | 4 + environment.c | 13 ++ git.c | 27 ++++- t/t3007-ls-files-recurse-submodules.sh | 210 +++++++++++++++++++++++++++++++++ 9 files changed, 452 insertions(+), 44 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh -- 2.10.0 ^ permalink raw reply [flat|nested] 67+ messages in thread
* [PATCH v7 1/4] git: make super-prefix option 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams @ 2016-10-07 18:18 ` Brandon Williams 2016-10-07 18:18 ` [PATCH v7 2/4] ls-files: optionally recurse into submodules Brandon Williams ` (3 subsequent siblings) 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX' which can be used to specify a path from above a repository down to its root. When such a super-prefix is specified, the paths reported by Git are prefixed with it to make them relative to that directory "above". The paths given by the user on the command line (e.g. "git subcmd --output-file=path/to/a/file" and pathspecs) are taken relative to the directory "above" to match. The immediate use of this option is by commands which have a --recurse-submodule option in order to give context to submodules about how they were invoked. This option is currently only allowed for builtins which support a super-prefix. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git.txt | 6 ++++++ cache.h | 2 ++ environment.c | 13 +++++++++++++ git.c | 25 +++++++++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7913fc2..2188ae6 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -13,6 +13,7 @@ SYNOPSIS [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>] + [--super-prefix=<path>] <command> [<args>] DESCRIPTION @@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string. details. Equivalent to setting the `GIT_NAMESPACE` environment variable. +--super-prefix=<path>:: + Currently for internal use only. Set a prefix which gives a path from + above a repository down to its root. One use is to give submodules + context about the superproject that invoked it. + --bare:: Treat the repository as a bare repository. If GIT_DIR environment is not set, it is set to the current working diff --git a/cache.h b/cache.h index 3556326..8cf495d 100644 --- a/cache.h +++ b/cache.h @@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" +#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" @@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); extern int get_common_dir(struct strbuf *sb, const char *gitdir); extern const char *get_git_namespace(void); extern const char *strip_namespace(const char *namespaced_ref); +extern const char *get_super_prefix(void); extern const char *get_git_work_tree(void); /* diff --git a/environment.c b/environment.c index ca72464..d12d7db 100644 --- a/environment.c +++ b/environment.c @@ -100,6 +100,8 @@ static char *work_tree; static const char *namespace; static size_t namespace_len; +static const char *super_prefix; + static const char *git_dir, *git_common_dir; static char *git_object_dir, *git_index_file, *git_graft_file; int git_db_env, git_index_env, git_graft_env, git_common_dir_env; @@ -120,6 +122,7 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, + GIT_SUPER_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL @@ -222,6 +225,16 @@ const char *strip_namespace(const char *namespaced_ref) return namespaced_ref + namespace_len; } +const char *get_super_prefix(void) +{ + static int initialized; + if (!initialized) { + super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT); + initialized = 1; + } + return super_prefix; +} + static int git_work_tree_initialized; /* diff --git a/git.c b/git.c index 1c61151..469a83f 100644 --- a/git.c +++ b/git.c @@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); if (envchanged) *envchanged = 1; + } else if (!strcmp(cmd, "--super-prefix")) { + if (*argc < 2) { + fprintf(stderr, "No prefix given for --super-prefix.\n" ); + usage(git_usage_string); + } + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); + if (envchanged) + *envchanged = 1; + (*argv)++; + (*argc)--; + } else if (skip_prefix(cmd, "--super-prefix=", &cmd)) { + setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1); + if (envchanged) + *envchanged = 1; } else if (!strcmp(cmd, "--bare")) { char *cwd = xgetcwd(); is_bare_repository_cfg = 1; @@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv) * RUN_SETUP for reading from the configuration file. */ #define NEED_WORK_TREE (1<<3) +#define SUPPORT_SUPER_PREFIX (1<<4) struct cmd_struct { const char *cmd; @@ -344,6 +359,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) } commit_pager_choice(); + if (!help && get_super_prefix()) { + if (!(p->option & SUPPORT_SUPER_PREFIX)) + die("%s doesn't support --super-prefix", p->cmd); + if (prefix) + die("can't use --super-prefix from a subdirectory"); + } + if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); @@ -558,6 +580,9 @@ static void execv_dashed_external(const char **argv) const char *tmp; int status; + if (get_super_prefix()) + die("%s doesn't support --super-prefix", argv[0]); + if (use_pager == -1) use_pager = check_pager_config(argv[0]); commit_pager_choice(); -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v7 2/4] ls-files: optionally recurse into submodules 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams 2016-10-07 18:18 ` [PATCH v7 1/4] git: make super-prefix option Brandon Williams @ 2016-10-07 18:18 ` Brandon Williams 2016-10-07 18:18 ` [PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams ` (2 subsequent siblings) 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Allow ls-files to recognize submodules in order to retrieve a list of files from a repository's submodules. This is done by forking off a process to recursively call ls-files on all submodules. Use top-level --super-prefix option to pass a path to the submodule which it can use to prepend to output or pathspec matching logic. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 8 +- builtin/ls-files.c | 138 ++++++++++++++++++++++++--------- git.c | 2 +- t/t3007-ls-files-recurse-submodules.sh | 100 ++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 40 deletions(-) create mode 100755 t/t3007-ls-files-recurse-submodules.sh diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 0d933ac..ea01d45 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -18,7 +18,8 @@ SYNOPSIS [--exclude-per-directory=<file>] [--exclude-standard] [--error-unmatch] [--with-tree=<tree-ish>] - [--full-name] [--abbrev] [--] [<file>...] + [--full-name] [--recurse-submodules] + [--abbrev] [--] [<file>...] DESCRIPTION ----------- @@ -137,6 +138,11 @@ a space) at the start of each line: option forces paths to be output relative to the project top directory. +--recurse-submodules:: + Recursively calls ls-files on each submodule in the repository. + Currently there is only support for the --cached mode without a + pathspec. + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object lines, show only a partial prefix. diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 00ea91a..63befed 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -14,6 +14,7 @@ #include "resolve-undo.h" #include "string-list.h" #include "pathspec.h" +#include "run-command.h" static int abbrev; static int show_deleted; @@ -28,8 +29,10 @@ static int show_valid_bit; static int line_terminator = '\n'; static int debug_mode; static int show_eol; +static int recurse_submodules; static const char *prefix; +static const char *super_prefix; static int max_prefix_len; static int prefix_len; static struct pathspec pathspec; @@ -68,11 +71,24 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path) static void write_name(const char *name) { /* + * Prepend the super_prefix to name to construct the full_name to be + * written. + */ + struct strbuf full_name = STRBUF_INIT; + if (super_prefix) { + strbuf_addstr(&full_name, super_prefix); + strbuf_addstr(&full_name, name); + name = full_name.buf; + } + + /* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ write_name_quoted_relative(name, prefix_len ? prefix : NULL, stdout, line_terminator); + + strbuf_release(&full_name); } static void show_dir_entry(const char *tag, struct dir_entry *ent) @@ -152,55 +168,84 @@ static void show_killed_files(struct dir_struct *dir) } } +/** + * Recursively call ls-files on a submodule + */ +static void show_gitlink(const struct cache_entry *ce) +{ + struct child_process cp = CHILD_PROCESS_INIT; + int status; + + argv_array_pushf(&cp.args, "--super-prefix=%s%s/", + super_prefix ? super_prefix : "", + ce->name); + argv_array_push(&cp.args, "ls-files"); + argv_array_push(&cp.args, "--recurse-submodules"); + + cp.git_cmd = 1; + cp.dir = ce->name; + status = run_command(&cp); + if (status) + exit(status); +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { + struct strbuf name = STRBUF_INIT; int len = max_prefix_len; + if (super_prefix) + strbuf_addstr(&name, super_prefix); + strbuf_addstr(&name, ce->name); 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, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) - return; + if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + show_gitlink(ce); + } else if (match_pathspec(&pathspec, name.buf, name.len, + len, ps_matched, + S_ISDIR(ce->ce_mode) || + S_ISGITLINK(ce->ce_mode))) { + if (tag && *tag && show_valid_bit && + (ce->ce_flags & CE_VALID)) { + static char alttag[4]; + memcpy(alttag, tag, 3); + if (isalpha(tag[0])) + alttag[0] = tolower(tag[0]); + else if (tag[0] == '?') + alttag[0] = '!'; + else { + alttag[0] = 'v'; + alttag[1] = tag[0]; + alttag[2] = ' '; + alttag[3] = 0; + } + tag = alttag; + } - if (tag && *tag && show_valid_bit && - (ce->ce_flags & CE_VALID)) { - static char alttag[4]; - memcpy(alttag, tag, 3); - if (isalpha(tag[0])) - alttag[0] = tolower(tag[0]); - else if (tag[0] == '?') - alttag[0] = '!'; - else { - alttag[0] = 'v'; - alttag[1] = tag[0]; - alttag[2] = ' '; - alttag[3] = 0; + if (!show_stage) { + fputs(tag, stdout); + } else { + printf("%s%06o %s %d\t", + tag, + ce->ce_mode, + find_unique_abbrev(ce->sha1,abbrev), + ce_stage(ce)); + } + write_eolinfo(ce, ce->name); + write_name(ce->name); + if (debug_mode) { + const struct stat_data *sd = &ce->ce_stat_data; + + printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); + printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); + printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); + printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); + printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); } - tag = alttag; } - if (!show_stage) { - fputs(tag, stdout); - } else { - printf("%s%06o %s %d\t", - tag, - ce->ce_mode, - find_unique_abbrev(ce->sha1,abbrev), - ce_stage(ce)); - } - write_eolinfo(ce, ce->name); - write_name(ce->name); - if (debug_mode) { - const struct stat_data *sd = &ce->ce_stat_data; - - printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec); - printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec); - printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino); - printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid); - printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags); - } + strbuf_release(&name); } static void show_ru_info(void) @@ -468,6 +513,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) { OPTION_SET_INT, 0, "full-name", &prefix_len, NULL, N_("make the output relative to the project top directory"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL }, + OPT_BOOL(0, "recurse-submodules", &recurse_submodules, + N_("recurse through submodules")), OPT_BOOL(0, "error-unmatch", &error_unmatch, N_("if any <file> is not in the index, treat this as an error")), OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"), @@ -484,6 +531,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) prefix = cmd_prefix; if (prefix) prefix_len = strlen(prefix); + super_prefix = get_super_prefix(); git_config(git_default_config, NULL); if (read_cache() < 0) @@ -519,6 +567,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules && + (show_stage || show_deleted || show_others || show_unmerged || + show_killed || show_modified || show_resolve_undo || + show_valid_bit || show_tag || show_eol || with_tree || + (line_terminator == '\0'))) + die("ls-files --recurse-submodules unsupported mode"); + + if (recurse_submodules && error_unmatch) + die("ls-files --recurse-submodules does not support " + "--error-unmatch"); + + if (recurse_submodules && argc) + die("ls-files --recurse-submodules does not support pathspec"); + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD | PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP, diff --git a/git.c b/git.c index 469a83f..df73768 100644 --- a/git.c +++ b/git.c @@ -443,7 +443,7 @@ static struct cmd_struct commands[] = { { "init-db", cmd_init_db }, { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY }, { "log", cmd_log, RUN_SETUP }, - { "ls-files", cmd_ls_files, RUN_SETUP }, + { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX }, { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY }, { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo }, diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh new file mode 100755 index 0000000..b5a53c3 --- /dev/null +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -0,0 +1,100 @@ +#!/bin/sh + +test_description='Test ls-files recurse-submodules feature + +This test verifies the recurse-submodules feature correctly lists files from +submodules. +' + +. ./test-lib.sh + +test_expect_success 'setup directory structure and submodules' ' + echo a >a && + mkdir b && + echo b >b/b && + git add a b && + git commit -m "add a and b" && + git init submodule && + echo c >submodule/c && + git -C submodule add c && + git -C submodule commit -m "add c" && + git submodule add ./submodule && + git commit -m "added submodule" +' + +test_expect_success 'ls-files correctly outputs files in submodule' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files does not output files not added to a repo' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + echo a >not_added && + echo b >b/not_added && + echo c >submodule/not_added && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success 'ls-files recurses more than 1 level' ' + cat >expect <<-\EOF && + .gitmodules + a + b/b + submodule/.gitmodules + submodule/c + submodule/subsub/d + EOF + + git init submodule/subsub && + echo d >submodule/subsub/d && + git -C submodule/subsub add d && + git -C submodule/subsub commit -m "add d" && + git -C submodule submodule add ./subsub && + git -C submodule commit -m "added subsub" && + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules does not support using path arguments' ' + test_must_fail git ls-files --recurse-submodules b 2>actual && + test_i18ngrep "does not support pathspec" actual +' + +test_expect_success '--recurse-submodules does not support --error-unmatch' ' + test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual && + test_i18ngrep "does not support --error-unmatch" actual +' + +test_incompatible_with_recurse_submodules () { + test_expect_success "--recurse-submodules and $1 are incompatible" " + test_must_fail git ls-files --recurse-submodules $1 2>actual && + test_i18ngrep 'unsupported mode' actual + " +} + +test_incompatible_with_recurse_submodules -z +test_incompatible_with_recurse_submodules -v +test_incompatible_with_recurse_submodules -t +test_incompatible_with_recurse_submodules --deleted +test_incompatible_with_recurse_submodules --modified +test_incompatible_with_recurse_submodules --others +test_incompatible_with_recurse_submodules --stage +test_incompatible_with_recurse_submodules --killed +test_incompatible_with_recurse_submodules --unmerged +test_incompatible_with_recurse_submodules --eol + +test_done -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams 2016-10-07 18:18 ` [PATCH v7 1/4] git: make super-prefix option Brandon Williams 2016-10-07 18:18 ` [PATCH v7 2/4] ls-files: optionally recurse into submodules Brandon Williams @ 2016-10-07 18:18 ` Brandon Williams 2016-10-07 18:18 ` [PATCH v7 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-10-07 18:34 ` [PATCH v7 0/4] recursive support for ls-files Stefan Beller 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pass through some known-safe options when recursing into submodules. (--cached, -v, -t, -z, --debug, --eol) Signed-off-by: Brandon Williams <bmwill@google.com> --- builtin/ls-files.c | 30 +++++++++++++++++++++++++++--- t/t3007-ls-files-recurse-submodules.sh | 16 ++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 63befed..b6144a5 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -30,6 +30,7 @@ static int line_terminator = '\n'; static int debug_mode; static int show_eol; static int recurse_submodules; +static struct argv_array submodules_options = ARGV_ARRAY_INIT; static const char *prefix; static const char *super_prefix; @@ -168,6 +169,25 @@ static void show_killed_files(struct dir_struct *dir) } } +/* + * Compile an argv_array with all of the options supported by --recurse_submodules + */ +static void compile_submodule_options(const struct dir_struct *dir, int show_tag) +{ + if (line_terminator == '\0') + argv_array_push(&submodules_options, "-z"); + if (show_tag) + argv_array_push(&submodules_options, "-t"); + if (show_valid_bit) + argv_array_push(&submodules_options, "-v"); + if (show_cached) + argv_array_push(&submodules_options, "--cached"); + if (show_eol) + argv_array_push(&submodules_options, "--eol"); + if (debug_mode) + argv_array_push(&submodules_options, "--debug"); +} + /** * Recursively call ls-files on a submodule */ @@ -182,6 +202,9 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_push(&cp.args, "ls-files"); argv_array_push(&cp.args, "--recurse-submodules"); + /* add supported options */ + argv_array_pushv(&cp.args, submodules_options.argv); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -567,11 +590,12 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); + if (recurse_submodules) + compile_submodule_options(&dir, show_tag); + if (recurse_submodules && (show_stage || show_deleted || show_others || show_unmerged || - show_killed || show_modified || show_resolve_undo || - show_valid_bit || show_tag || show_eol || with_tree || - (line_terminator == '\0'))) + show_killed || show_modified || show_resolve_undo || with_tree)) die("ls-files --recurse-submodules unsupported mode"); if (recurse_submodules && error_unmatch) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index b5a53c3..33a2ea7 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' ' test_cmp expect actual ' +test_expect_success 'ls-files correctly outputs files in submodule with -z' ' + lf_to_nul >expect <<-\EOF && + .gitmodules + a + b/b + submodule/c + EOF + + git ls-files --recurse-submodules -z >actual && + test_cmp expect actual +' + test_expect_success 'ls-files does not output files not added to a repo' ' cat >expect <<-\EOF && .gitmodules @@ -86,15 +98,11 @@ test_incompatible_with_recurse_submodules () { " } -test_incompatible_with_recurse_submodules -z -test_incompatible_with_recurse_submodules -v -test_incompatible_with_recurse_submodules -t test_incompatible_with_recurse_submodules --deleted test_incompatible_with_recurse_submodules --modified test_incompatible_with_recurse_submodules --others test_incompatible_with_recurse_submodules --stage test_incompatible_with_recurse_submodules --killed test_incompatible_with_recurse_submodules --unmerged -test_incompatible_with_recurse_submodules --eol test_done -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* [PATCH v7 4/4] ls-files: add pathspec matching for submodules 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams ` (2 preceding siblings ...) 2016-10-07 18:18 ` [PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams @ 2016-10-07 18:18 ` Brandon Williams 2016-10-07 18:34 ` [PATCH v7 0/4] recursive support for ls-files Stefan Beller 4 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw) To: git; +Cc: Brandon Williams, sbeller, peff, gitster Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> --- Documentation/git-ls-files.txt | 3 +- builtin/ls-files.c | 27 +++++++-- dir.c | 46 +++++++++++++- dir.h | 4 ++ t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++- 5 files changed, 175 insertions(+), 13 deletions(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index ea01d45..446209e 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -140,8 +140,7 @@ a space) at the start of each line: --recurse-submodules:: Recursively calls ls-files on each submodule in the repository. - Currently there is only support for the --cached mode without a - pathspec. + Currently there is only support for the --cached mode. --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b6144a5..0f25914 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -195,6 +195,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + int i; argv_array_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? super_prefix : "", @@ -205,6 +206,15 @@ static void show_gitlink(const struct cache_entry *ce) /* add supported options */ argv_array_pushv(&cp.args, submodules_options.argv); + /* + * Pass in the original pathspec args. The submodule will be + * responsible for prepending the 'submodule_prefix' prior to comparing + * against the pathspec for matches. + */ + argv_array_push(&cp.args, "--"); + for (i = 0; i < pathspec.nr; i++) + argv_array_push(&cp.args, pathspec.items[i].original); + cp.git_cmd = 1; cp.dir = ce->name; status = run_command(&cp); @@ -223,7 +233,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) { + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && + submodule_path_match(&pathspec, name.buf, ps_matched)) { show_gitlink(ce); } else if (match_pathspec(&pathspec, name.buf, name.len, len, ps_matched, @@ -602,16 +613,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) die("ls-files --recurse-submodules does not support " "--error-unmatch"); - if (recurse_submodules && argc) - die("ls-files --recurse-submodules does not support pathspec"); - 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); + /* + * Find common prefix for all pathspec's + * This is used as a performance optimization which unfortunately cannot + * be done when recursing into submodules + */ + if (recurse_submodules) + max_prefix = NULL; + else + 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 0ea235f..28e9736 100644 --- a/dir.c +++ b/dir.c @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen, return 1; } -#define DO_MATCH_EXCLUDE 1 -#define DO_MATCH_DIRECTORY 2 +#define DO_MATCH_EXCLUDE (1<<0) +#define DO_MATCH_DIRECTORY (1<<1) +#define DO_MATCH_SUBMODULE (1<<2) /* * Does 'match' match the given name? @@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, item->nowildcard_len - prefix)) return MATCHED_FNMATCH; + /* Perform checks to see if "name" is a super set of the pathspec */ + if (flags & DO_MATCH_SUBMODULE) { + /* name is a literal prefix of the pathspec */ + if ((namelen < matchlen) && + (match[namelen] == '/') && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_RECURSIVELY; + + /* name" doesn't match up to the first wild character */ + if (item->nowildcard_len < item->len && + ps_strncmp(item, match, name, + item->nowildcard_len - prefix)) + return 0; + + /* + * Here is where we would perform a wildmatch to check if + * "name" can be matched as a directory (or a prefix) against + * the pathspec. Since wildmatch doesn't have this capability + * at the present we have to punt and say that it is a match, + * potentially returning a false positive + * The submodules themselves will be able to perform more + * accurate matching to determine if the pathspec matches. + */ + return MATCHED_RECURSIVELY; + } + return 0; } @@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps, return negative ? 0 : positive; } +/** + * Check if a submodule is a superset of the pathspec + */ +int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen) +{ + int matched = do_match_pathspec(ps, submodule_name, + strlen(submodule_name), + 0, seen, + DO_MATCH_DIRECTORY | + DO_MATCH_SUBMODULE); + return matched; +} + int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix) diff --git a/dir.h b/dir.h index da1a858..97c83bb 100644 --- a/dir.h +++ b/dir.h @@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +extern int submodule_path_match(const struct pathspec *ps, + const char *submodule_name, + char *seen); + static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index 33a2ea7..a542617 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' -test_expect_success '--recurse-submodules does not support using path arguments' ' - test_must_fail git ls-files --recurse-submodules b 2>actual && - test_i18ngrep "does not support pathspec" actual +test_expect_success '--recurse-submodules and pathspecs setup' ' + echo e >submodule/subsub/e.txt && + git -C submodule/subsub add e.txt && + git -C submodule/subsub commit -m "adding e.txt" && + echo f >submodule/f.TXT && + echo g >submodule/g.txt && + git -C submodule add f.TXT g.txt && + git -C submodule commit -m "add f and g" && + echo h >h.txt && + mkdir sib && + echo sib >sib/file && + git add h.txt sib/file && + git commit -m "add h and sib/file" && + git init sub && + echo sub >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git submodule add ./sub && + git commit -m "added sub" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub/d + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + cat actual && + git ls-files --recurse-submodules "*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules "*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + submodule/subsub/e.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + h.txt + submodule/f.TXT + submodule/g.txt + EOF + + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sub/file + EOF + + git ls-files --recurse-submodules "sub" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "sub/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su*/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "su?/file" >actual && + test_cmp expect actual +' + +test_expect_success '--recurse-submodules and pathspecs' ' + cat >expect <<-\EOF && + sib/file + sub/file + EOF + + git ls-files --recurse-submodules "s??/file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s???file" >actual && + test_cmp expect actual && + git ls-files --recurse-submodules "s*file" >actual && + test_cmp expect actual ' test_expect_success '--recurse-submodules does not support --error-unmatch' ' -- 2.10.0 ^ permalink raw reply related [flat|nested] 67+ messages in thread
* Re: [PATCH v7 0/4] recursive support for ls-files 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams ` (3 preceding siblings ...) 2016-10-07 18:18 ` [PATCH v7 4/4] ls-files: add pathspec matching for submodules Brandon Williams @ 2016-10-07 18:34 ` Stefan Beller 2016-10-07 18:35 ` Stefan Beller 4 siblings, 1 reply; 67+ messages in thread From: Stefan Beller @ 2016-10-07 18:34 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote: > Few minor fixes pointed out Stefan Nit: This is not very informative, so you could provide an "interdiff" to the last patch, (see https://github.com/trast/tbdiff for a sophisticated tool, I usually do a git diff origin/sb/<what-junio-queued>) That helps reviewers as they do not need to review it all again, but they may remember what they annotated and see how you addressed it. Thanks, Stefan ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v7 0/4] recursive support for ls-files 2016-10-07 18:34 ` [PATCH v7 0/4] recursive support for ls-files Stefan Beller @ 2016-10-07 18:35 ` Stefan Beller 2016-10-07 18:45 ` Brandon Williams 0 siblings, 1 reply; 67+ messages in thread From: Stefan Beller @ 2016-10-07 18:35 UTC (permalink / raw) To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller <sbeller@google.com> wrote: > On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote: >> Few minor fixes pointed out Stefan > The series itself looks good though. :) Thanks, Stefan ^ permalink raw reply [flat|nested] 67+ messages in thread
* Re: [PATCH v7 0/4] recursive support for ls-files 2016-10-07 18:35 ` Stefan Beller @ 2016-10-07 18:45 ` Brandon Williams 0 siblings, 0 replies; 67+ messages in thread From: Brandon Williams @ 2016-10-07 18:45 UTC (permalink / raw) To: Stefan Beller; +Cc: git, Jeff King, Junio C Hamano On 10/07, Stefan Beller wrote: > On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller <sbeller@google.com> wrote: > > On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote: > >> Few minor fixes pointed out Stefan > > > > The series itself looks good though. :) > > Thanks, > Stefan Thanks! And I'll keep that in mind for the future. Still lots to learn about contributing to the project. -- Brandon Williams ^ permalink raw reply [flat|nested] 67+ messages in thread
end of thread, other threads:[~2016-10-07 18:45 UTC | newest] Thread overview: 67+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-24 0:13 [PATCH 0/3] recursive support for ls-files Brandon Williams 2016-09-24 0:13 ` [PATCH 1/3 v3] submodules: make submodule-prefix option an envvar Brandon Williams 2016-09-25 23:34 ` Junio C Hamano 2016-09-24 0:13 ` [PATCH 2/3 v3] ls-files: optionally recurse into submodules Brandon Williams 2016-09-24 0:13 ` [PATCH 3/3 v3] ls-files: add pathspec matching for submodules Brandon Williams 2016-09-25 7:17 ` [PATCH 0/3] recursive support for ls-files Jeff King 2016-09-25 16:32 ` Brandon Williams 2016-09-25 18:38 ` Junio C Hamano 2016-09-26 17:04 ` Brandon Williams 2016-09-26 18:17 ` Junio C Hamano 2016-09-26 18:38 ` Brandon Williams 2016-09-26 18:48 ` Junio C Hamano 2016-09-26 22:46 ` [PATCH 0/4 v4] " Brandon Williams 2016-09-26 22:46 ` [PATCH 1/4 v4] submodules: make submodule-prefix option Brandon Williams 2016-09-27 18:17 ` Junio C Hamano 2016-09-27 20:29 ` Brandon Williams 2016-09-27 20:35 ` Junio C Hamano 2016-09-27 20:43 ` Brandon Williams 2016-09-26 22:46 ` [PATCH 2/4 v4] ls-files: optionally recurse into submodules Brandon Williams 2016-09-27 18:29 ` Junio C Hamano 2016-09-27 20:33 ` Brandon Williams 2016-09-26 22:46 ` [PATCH 3/4 v4] ls-files: pass through safe options for --recurse-submodules Brandon Williams 2016-09-27 18:40 ` Junio C Hamano 2016-09-27 20:11 ` Junio C Hamano 2016-09-27 20:52 ` Brandon Williams 2016-09-27 20:58 ` Junio C Hamano 2016-09-27 20:59 ` Stefan Beller 2016-09-28 17:24 ` Brandon Williams 2016-09-28 18:59 ` Junio C Hamano 2016-09-27 20:49 ` Brandon Williams 2016-09-27 18:43 ` Junio C Hamano 2016-09-27 20:44 ` Brandon Williams 2016-09-27 20:59 ` Junio C Hamano 2016-09-26 22:46 ` [PATCH 4/4 v4] ls-files: add pathspec matching for submodules Brandon Williams 2016-09-27 20:01 ` Junio C Hamano 2016-09-27 20:40 ` Brandon Williams 2016-09-28 21:50 ` [PATCH v5 0/4] recursive support for ls-files Brandon Williams 2016-09-28 21:50 ` [PATCH v5 1/4] git: make super-prefix option Brandon Williams 2016-09-28 22:01 ` Stefan Beller 2016-09-28 22:19 ` Junio C Hamano 2016-09-29 18:39 ` Jeff King 2016-09-29 18:44 ` Brandon Williams 2016-09-28 21:50 ` [PATCH v5 2/4] ls-files: optionally recurse into submodules Brandon Williams 2016-09-28 22:11 ` Stefan Beller 2016-09-28 22:22 ` Junio C Hamano 2016-09-28 21:50 ` [PATCH v5 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams 2016-09-28 21:50 ` [PATCH v5 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-09-29 21:48 ` [PATCH v6 0/4] recursive support for ls-files Brandon Williams 2016-09-29 21:48 ` [PATCH v6 1/4] git: make super-prefix option Brandon Williams 2016-10-04 17:31 ` Stefan Beller 2016-10-04 17:35 ` Junio C Hamano 2016-10-04 17:39 ` Jeff King 2016-09-29 21:48 ` [PATCH v6 2/4] ls-files: optionally recurse into submodules Brandon Williams 2016-09-29 21:48 ` [PATCH v6 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams 2016-09-30 0:14 ` Junio C Hamano 2016-09-30 16:33 ` Brandon Williams 2016-09-30 17:01 ` Brandon Williams 2016-09-29 21:48 ` [PATCH v6 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-10-04 17:56 ` Stefan Beller 2016-10-07 18:18 ` [PATCH v7 0/4] recursive support for ls-files Brandon Williams 2016-10-07 18:18 ` [PATCH v7 1/4] git: make super-prefix option Brandon Williams 2016-10-07 18:18 ` [PATCH v7 2/4] ls-files: optionally recurse into submodules Brandon Williams 2016-10-07 18:18 ` [PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules Brandon Williams 2016-10-07 18:18 ` [PATCH v7 4/4] ls-files: add pathspec matching for submodules Brandon Williams 2016-10-07 18:34 ` [PATCH v7 0/4] recursive support for ls-files Stefan Beller 2016-10-07 18:35 ` Stefan Beller 2016-10-07 18:45 ` Brandon Williams
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.