[PATCH 45/45] Rename field "raw" to "_raw" in struct pathspec
This patch is essentially no-op. It helps catching new use of this
field though. This field is introduced as an intermediate step for the
pathspec conversion and will be removed eventually. At this stage no
more access sites should be introduced.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/ls-tree.c | 2 +-
dir.c | 4 ++--
pathspec.c| 6 +++---
pathspec.h| 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 1056634..7882352 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -36,7 +36,7 @@ static int show_recursive(const char *base, int baselen,
const char *pathname)
if (ls_options & LS_RECURSIVE)
return 1;
- s = pathspec.raw;
+ s = pathspec._raw;
if (!s)
return 0;
diff --git a/dir.c b/dir.c
index 7e931af..0a79929 100644
--- a/dir.c
+++ b/dir.c
@@ -158,7 +158,7 @@ int fill_directory(struct dir_struct *dir, const struct
pathspec *pathspec)
len = common_prefix_len(pathspec);
/* Read the directory and prune it */
- read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len,
pathspec);
+ read_directory(dir, pathspec->nr ? pathspec->_raw[0] : "", len,
pathspec);
return len;
}
@@ -1308,7 +1308,7 @@ int read_directory(struct dir_struct *dir, const char
*path, int len, const stru
if (has_symlink_leading_path(path, len))
return dir->nr;
- simplify = create_simplify(pathspec ? pathspec->raw : NULL);
+ simplify = create_simplify(pathspec ? pathspec->_raw : NULL);
if (!len || treat_leading_path(dir, path, len, simplify))
read_directory_recursive(dir, path, len, 0, simplify);
free_simplify(simplify);
diff --git a/pathspec.c b/pathspec.c
index e3581da..8ae1f8c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -376,7 +376,7 @@ void parse_pathspec(struct pathspec *pathspec,
raw[0] = prefix;
raw[1] = NULL;
pathspec->nr = 1;
- pathspec->raw = raw;
+ pathspec->_raw = raw;
return;
}
@@ -386,7 +386,7 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->nr = n;
pathspec->items = item = xmalloc(sizeof(*item) * n);
- pathspec->raw = argv;
+ pathspec->_raw = argv;
prefixlen = prefix ? strlen(prefix) : 0;
for (i = 0; i < n; i++) {
@@ -447,7 +447,7 @@ const char **get_pathspec(const char *prefix, const char
**pathspec)
~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
PATHSPEC_PREFER_CWD,
prefix, pathspec);
- return ps.raw;
+ return ps._raw;
}
void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
diff --git a/pathspec.h b/pathspec.h
index 7608172..5ba42e5 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -15,7 +15,7 @@
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR
*/
struct pathspec {
- const char **raw; /* get_pathspec() result, not freed by
free_pathspec() */
+ const char **_raw; /* get_pathspec() result, not freed by
free_pathspec() */
int nr;
unsigned int has_wildcard:1;
unsigned int recursive:1;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 41/45] Kill limit_pathspec_to_literal() as it's only used by parse_pathspec()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
dir.c | 8
pathspec.c | 8 ++--
pathspec.h | 2 --
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/dir.c b/dir.c
index 1098133..76a2e1a 100644
--- a/dir.c
+++ b/dir.c
@@ -1475,14 +1475,6 @@ int remove_path(const char *name)
return 0;
}
-int limit_pathspec_to_literal(void)
-{
- static int flag = -1;
- if (flag < 0)
- flag = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
- return flag;
-}
-
/*
* Frees memory within dir which was allocated for exclude lists and
* the exclude_stack. Does not free dir itself.
diff --git a/pathspec.c b/pathspec.c
index 2bd400a..69adaba 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -91,11 +91,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
const char *prefix, int prefixlen,
const char *elt)
{
+ static int literal_global = -1;
unsigned magic = 0, short_magic = 0;
const char *copyfrom = elt, *long_magic_end = NULL;
char *match;
int i, pathspec_prefix = -1;
+ if (literal_global < 0)
+ literal_global =
git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
+
if (elt[0] != ':') {
; /* nothing to do */
} else if (elt[1] == '(') {
@@ -184,7 +188,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
if (flags & PATHSPEC_PREFIX_ORIGIN) {
struct strbuf sb = STRBUF_INIT;
const char *start = elt;
- if (prefixlen && !limit_pathspec_to_literal()) {
+ if (prefixlen && !literal_global) {
/* Preserve the actual prefix length of each pattern */
if (long_magic_end) {
strbuf_add(&sb, start, long_magic_end - start);
@@ -232,7 +236,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
elt, ce_len, ce->name);
}
- if (limit_pathspec_to_literal())
+ if (literal_global)
item->nowildcard_len = item->len;
else {
item->nowildcard_len = simple_length(item->match);
diff --git a/pathspec.h b/pathspec.h
index bbcfa74..4ebaadc 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -61,8 +61,6 @@ extern void parse_pathspec(struct pathspec *pathspec,
extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
extern void free_pathspec(struct pathspec *);
-extern int limit_pathspec_to_literal(void);
-
extern char *find_pathspecs_matching_against_index(const struct pathspec
*pathspec);
extern void add_pathspec_matches_against_index(const struct pathspec
*pathspec, char *seen);
extern const char *check_path_for_gitlink(const char *path);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 40/45] parse_pathspec: preserve prefix length via PATHSPEC_PREFIX_ORIGIN
The prefix length is passed from one command to another via the new
magic 'prefix'. The magic is for parse_pathspec's internal use only,
not visible to parse_pathspec's callers.
Prefix length is not preserved across commands when --literal-pathspecs
is specified (no magic is allowed, including 'prefix'). That's OK
because we know all paths are literal. No magic, no special treatment
regarding prefix. (This may be no longer true if we make :(glob)
default)
Other options to preserve the prefix include saving it to env variable
or quoting. Env var way (at least _one_ env var) is not suitable
because the prefix is not the same for all pathspecs. Pathspecs
starting with "../" will eat into the prefix part.
We could also preserve 'prefix' across commands by quoting the prefix
part, then dequoting on receiving. But it may not be 100% accurate, we
may dequote longer than the original prefix part, for example. That
may be good or not, but it's not the purpose.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 41 -
1 file changed, 36 insertions(+), 5 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index e2ee268..2bd400a 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -92,9 +92,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
const char *elt)
{
unsigned magic = 0, short_magic = 0;
- const char *copyfrom = elt;
+ const char *copyfrom = elt, *long_magic_end = NULL;
char *match;
- int i;
+ int i, pathspec_prefix = -1;
if (elt[0] != ':') {
; /* nothing to do */
@@ -112,18 +112,29 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
nextat = copyfrom + len;
if (!len)
continue;
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
+ for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom,
len)) {
magic |= pathspec_magic[i].bit;
break;
}
+ if (!prefixcmp(copyfrom, "prefix:")) {
+ char *endptr;
+ pathspec_prefix = strtol(copyfrom + 7,
+&endptr, 10);
+ if (endptr - copyfrom != len)
+ die(_("invalid parameter for
pathspec magic 'prefix'"));
+ /* "i" would be wrong, but it does not
matter */
+ break;
+ }
+ }
if (ARRAY_SIZE(pathspec_magic) <= i)
die(_("Invalid pathspec magic '%.*s' in '%s'"),
(int) len, copyfrom, elt);
}
if (*copyfrom != ')')
die(_("Missing ')' at the end of pathspec magic in
'%s'"), elt);
+ long_magic_end = copyfrom;
copyfrom++;
} else {
/* shorthand */
@@ -150,7 +161,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
magic |= short_magic;
*p_short_magic = short_magic;
- if (magic & PATHSPEC_FROMTOP) {
+ if (pathspec_prefix >= 0 &&
+ (prefixlen || (prefix && *prefix)))
+ die("BUG: 'prefix' magic is supposed to be used at worktree's
root");
+
+ if (pathspec_prefix >= 0) {
+ match = xstrdup(copyfrom);
+ prefixlen = pathspec_prefix;
+ } else if (magic & PATHSPEC_FROMTOP) {
match = xstrdup(copyfrom);
prefixlen = 0;
} else {
@@ -165,7 +183,20 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
*/
if (flags & PATHSPEC_PREFIX_ORIGIN) {
struct strbuf sb = STRBUF_INIT;
- strbuf_add(&sb, elt, copyfrom - elt);
+ const char *start = elt;
+ if (prefixlen && !limit_pathspec_to_literal()) {
+ /* Preserve the actual prefix length of each pattern */
+ if (long_magic_end) {
+ strbuf_add(&sb, start, long_magic_end - start);
+ strbuf_addf(&sb, ",prefix:%d", prefixlen);
+ start = long_magic_end;
+ } else {
+ if (*start == ':')
+ start++;
+ strbuf_addf(&sb, ":(prefix:%d)", prefixlen);
+ }
+
[PATCH 42/45] pathspec: support :(literal) syntax for noglob pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/glossary-content.txt | 20
builtin/add.c | 2 +-
builtin/diff.c | 2 +-
dir.c | 15 ---
pathspec.c | 11 ---
pathspec.h | 4 +++-
t/t6130-pathspec-noglob.sh | 18 ++
tree-diff.c| 2 +-
tree-walk.c| 5 -
9 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index db2a74d..186b566 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -322,10 +322,22 @@ and a close parentheses `)`, and the remainder is the
pattern to match
against the path.
+
The "magic signature" consists of an ASCII symbol that is not
-alphanumeric. Currently only the slash `/` is recognized as a
-"magic signature": it makes the pattern match from the root of
-the working tree, even when you are running the command from
-inside a subdirectory.
+alphanumeric.
++
+--
+top `/`;;
+ The magic word `top` (mnemonic: `/`) makes the pattern match
+ from the root of the working tree, even when you are running
+ the command from inside a subdirectory.
+
+literal;;
+ Wildcards in the pattern such as `*` or `?` are treated
+ as literal characters.
+--
++
+Currently only the slash `/` is recognized as the "magic signature",
+but it is envisioned that we will support more types of magic in later
+versions of Git.
+
A pathspec with only a colon means "there is no pathspec". This form
should not be combined with other pathspec.
diff --git a/builtin/add.c b/builtin/add.c
index 0b80fa8..663ddd1 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -541,7 +541,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
/*
* file_exists() assumes exact match
*/
- GUARD_PATHSPEC(&pathspec, PATHSPEC_FROMTOP);
+ GUARD_PATHSPEC(&pathspec, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
for (i = 0; i < pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
diff --git a/builtin/diff.c b/builtin/diff.c
index 6b4e3f9..b78435f 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -372,7 +372,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
}
if (rev.prune_data.nr) {
/* builtin_diff_b_f() */
- GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
+ GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP |
PATHSPEC_LITERAL);
if (!path)
path = rev.prune_data.items[0].match;
paths += rev.prune_data.nr;
diff --git a/dir.c b/dir.c
index 76a2e1a..65cac36 100644
--- a/dir.c
+++ b/dir.c
@@ -108,7 +108,10 @@ static size_t common_prefix_len(const struct pathspec
*pathspec)
int n;
size_t max = 0;
- GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+ GUARD_PATHSPEC(pathspec,
+ PATHSPEC_FROMTOP |
+ PATHSPEC_MAXDEPTH |
+ PATHSPEC_LITERAL);
for (n = 0; n < pathspec->nr; n++) {
size_t i = 0, len = 0;
@@ -232,7 +235,10 @@ int match_pathspec_depth(const struct pathspec *ps,
{
int i, retval = 0;
- GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+ GUARD_PATHSPEC(ps,
+ PATHSPEC_FROMTOP |
+ PATHSPEC_MAXDEPTH |
+ PATHSPEC_LITERAL);
if (!ps->nr) {
if (!ps->recursive ||
@@ -1290,7 +1296,10 @@ int read_directory(struct dir_struct *dir, const char
*path, int len, const stru
* Check out create_simplify()
*/
if (pathspec)
- GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+ GUARD_PATHSPEC(pathspec,
+ PATHSPEC_FROMTOP |
+ PATHSPEC_MAXDEPTH |
+ PATHSPEC_LITERAL);
if (has_symlink_leading_path(path, len))
return dir->nr;
diff --git a/pathspec.c b/pathspec.c
index 69adaba..cc6545f 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -70,6 +70,7 @@ static struct pathspec_magic {
const char *name;
} pathspec_magic[] = {
{ PATHSPEC_FROMTOP, '/', "top" },
+ { PATHSPEC_LITERAL, 0, "literal" },
};
/*
@@ -92,13 +93,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
const char *elt)
{
static int literal_global = -1;
- unsigned magic = 0, short_magic = 0;
+ unsigned magic = 0, short_magic = 0, global_magic = 0;
const char *copyfrom = elt, *long_magic_end = NULL;
char *match;
int i, pat
[PATCH 38/45] tree-diff: remove the use of pathspec's raw[] in follow-rename codepath
Put a checkpoint to guard unsupported pathspec features in future.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
tree-diff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index e1145c6..21a50d8 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -224,7 +224,7 @@ static void try_to_follow_renames(struct tree_desc *t1,
struct tree_desc *t2, co
DIFF_OPT_SET(&diff_opts, RECURSIVE);
DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
- diff_opts.single_follow = opt->pathspec.raw[0];
+ diff_opts.single_follow = opt->pathspec.items[0].match;
diff_opts.break_opt = opt->break_opt;
diff_opts.rename_score = opt->rename_score;
diff_setup_done(&diff_opts);
@@ -243,7 +243,7 @@ static void try_to_follow_renames(struct tree_desc *t1,
struct tree_desc *t2, co
* the future!
*/
if ((p->status == 'R' || p->status == 'C') &&
- !strcmp(p->two->path, opt->pathspec.raw[0])) {
+ !strcmp(p->two->path, opt->pathspec.items[0].match)) {
const char *path[2];
/* Switch the file-pairs around */
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 43/45] pathspec: make --literal-pathspecs disable pathspec magic
--literal-pathspecs and its equivalent environment variable are
probably used for scripting. In that setting, pathspec magic may be
unwanted. Disabling globbing in individual pathspec can be done via
:(literal) magic.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git.txt | 4 ++--
pathspec.c | 2 +-
t/t6130-pathspec-noglob.sh | 6 ++
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 68f1ee6..a3fbc59 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -448,8 +448,8 @@ help ...`.
linkgit:git-replace[1] for more information.
--literal-pathspecs::
- Treat pathspecs literally, rather than as glob patterns. This is
- equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
+ Treat pathspecs literally (i.e. no globbing, no pathspec magic).
+ This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
variable to `1`.
diff --git a/pathspec.c b/pathspec.c
index cc6545f..9802829 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -103,7 +103,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
if (literal_global)
global_magic |= PATHSPEC_LITERAL;
- if (elt[0] != ':') {
+ if (elt[0] != ':' || literal_global) {
; /* nothing to do */
} else if (elt[1] == '(') {
/* longhand */
diff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh
index 49c148e..8551b02 100755
--- a/t/t6130-pathspec-noglob.sh
+++ b/t/t6130-pathspec-noglob.sh
@@ -77,6 +77,12 @@ test_expect_success 'no-glob option matches literally
(bracket)' '
test_cmp expect actual
'
+test_expect_success 'no-glob option disables :(literal)' '
+ : >expect &&
+ git --literal-pathspecs log --format=%s -- ":(literal)foo" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'no-glob environment variable works' '
echo star >expect &&
GIT_LITERAL_PATHSPECS=1 git log --format=%s -- "f*" >actual &&
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 39/45] parse_pathspec: make sure the prefix part is wildcard-free
Prepending prefix to pathspec is a trick to workaround the fact that
commands can be executed in a subdirectory, but all git commands run
at worktree's root. The prefix part should always be treated as
literal string. Make it so.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h| 2 ++
path.c | 15 ++-
pathspec.c | 21 +
pathspec.h | 2 +-
setup.c| 24
5 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/cache.h b/cache.h
index b3d8b29..276330e 100644
--- a/cache.h
+++ b/cache.h
@@ -408,6 +408,7 @@ extern void setup_work_tree(void);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
extern char *prefix_path(const char *prefix, int len, const char *path);
+extern char *prefix_path_gently(const char *prefix, int len, int *remaining,
const char *path);
extern const char *prefix_filename(const char *prefix, int len, const char
*path);
extern int check_filename(const char *prefix, const char *name);
extern void verify_filename(const char *prefix,
@@ -720,6 +721,7 @@ const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
const char *relative_path(const char *abs, const char *base);
+int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
diff --git a/path.c b/path.c
index 04ff148..f4b49d6 100644
--- a/path.c
+++ b/path.c
@@ -492,8 +492,14 @@ const char *relative_path(const char *abs, const char
*base)
*
* Note that this function is purely textual. It does not follow symlinks,
* verify the existence of the path, or make any system calls.
+ *
+ * prefix_len != NULL is for a specific case of prefix_pathspec():
+ * assume that src == dst and src[0..prefix_len-1] is already
+ * normalized, any time "../" eats up to the prefix_len part,
+ * prefix_len is reduced. In the end prefix_len is the remaining
+ * prefix that has not been overridden by user pathspec.
*/
-int normalize_path_copy(char *dst, const char *src)
+int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)
{
char *dst0;
@@ -568,11 +574,18 @@ int normalize_path_copy(char *dst, const char *src)
/* Windows: dst[-1] cannot be backslash anymore */
while (dst0 < dst && dst[-1] != '/')
dst--;
+ if (prefix_len && *prefix_len > dst - dst0)
+ *prefix_len = dst - dst0;
}
*dst = '\0';
return 0;
}
+int normalize_path_copy(char *dst, const char *src)
+{
+ return normalize_path_copy_len(dst, src, NULL);
+}
+
/*
* path = Canonical absolute path
* prefixes = string_list containing normalized, absolute paths without
diff --git a/pathspec.c b/pathspec.c
index 17f25d6..e2ee268 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -150,10 +150,14 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
magic |= short_magic;
*p_short_magic = short_magic;
- if (magic & PATHSPEC_FROMTOP)
+ if (magic & PATHSPEC_FROMTOP) {
match = xstrdup(copyfrom);
- else
- match = prefix_path(prefix, prefixlen, copyfrom);
+ prefixlen = 0;
+ } else {
+ match = prefix_path_gently(prefix, prefixlen, &prefixlen,
copyfrom);
+ if (!match)
+ die(_("%s: '%s' is outside repository"), elt, copyfrom);
+ }
*raw = item->match = match;
/*
* Prefix the pathspec (keep all magic) and put to
@@ -167,6 +171,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
} else
item->original = elt;
item->len = strlen(item->match);
+ item->prefix = prefixlen;
if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
(item->len >= 1 && item->match[item->len - 1] == '/') &&
@@ -198,13 +203,20 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
if (limit_pathspec_to_literal())
item->nowildcard_len = item->len;
- else
+ else {
item->nowildcard_len = simple_length(item->match);
+ if (item->nowildcard_len < prefixlen)
+ item->nowildcard_len = prefixlen;
+ }
item->flags = 0;
if (item->nowildcard_len < item->len &&
item->match[item->nowildcard_len] == '*' &&
no_wildcard(item->match + item->nowildcard_len + 1))
item->flags |= PATHSPEC_ONESTAR;
+
+ /* sanity checks, pathspec matchers assume these are sane */
+ assert(item->nowildcard_len <= item->len &&
+ item->prefix <= item->len);
return magic;
}
@@ -284,6
[PATCH 44/45] pathspec: support :(glob) syntax
:(glob)path differs from plain pathspec that it uses wildmatch with
WM_PATHNAME while the other uses fnmatch without FNM_PATHNAME. The
difference lies in how '*' (and '**') is processed.
With the introduction of :(glob) and :(literal) and their global
options --[no]glob-pathspecs, the user can:
- make everything literal by default via --noglob-pathspecs
--literal-pathspecs cannot be used for this purpose as it
disables _all_ pathspec magic.
- individually turn on globbing with :(glob)
- make everything globbing by default via --glob-pathspecs
- individually turn off globbing with :(literal)
The implication behind this is, there is no way to gain the default
matching behavior (i.e. fnmatch without FNM_PATHNAME). You either get
new globbing or literal. The old fnmatch behavior is considered
deprecated and discouraged to use.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git.txt | 19
Documentation/glossary-content.txt | 29 ++
builtin/add.c | 9 --
builtin/ls-tree.c | 2 +-
cache.h| 2 ++
dir.c | 28 +
dir.h | 9 +++---
git.c | 8 +
pathspec.c | 47 +---
pathspec.h | 4 ++-
t/t6130-pathspec-noglob.sh | 63 ++
tree-walk.c| 9 +++---
12 files changed, 198 insertions(+), 31 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index a3fbc59..52b85b6 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -452,6 +452,17 @@ help ...`.
This is equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
variable to `1`.
+--glob-pathspecs:
+ Add "glob" magic to all pathspec. This is equivalent to setting
+ the `GIT_GLOB_PATHSPECS` environment variable to `1`. Disabling
+ globbing on individual pathspecs can be done using pathspec
+ magic ":(literal)"
+
+--noglob-pathspecs:
+ Add "literal" magic to all pathspec. This is equivalent to setting
+ the `GIT_NOGLOB_PATHSPECS` environment variable to `1`. Enabling
+ globbing on individual pathspecs can be done using pathspec
+ magic ":(glob)"
GIT COMMANDS
@@ -847,6 +858,14 @@ GIT_LITERAL_PATHSPECS::
literal paths to Git (e.g., paths previously given to you by
`git ls-tree`, `--raw` diff output, etc).
+GIT_GLOB_PATHSPECS::
+ Setting this variable to `1` will cause Git to treat all
+ pathspecs as glob patterns (aka "glob" magic).
+
+GIT_NOGLOB_PATHSPECS::
+ Setting this variable to `1` will cause Git to treat all
+ pathspecs as literal (aka "literal" magic).
+
Discussion[[Discussion]]
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index 186b566..5c8da50 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -333,6 +333,35 @@ top `/`;;
literal;;
Wildcards in the pattern such as `*` or `?` are treated
as literal characters.
+
+glob;;
+ Git treats the pattern as a shell glob suitable for
+ consumption by fnmatch(3) with the FNM_PATHNAME flag:
+ wildcards in the pattern will not match a / in the pathname.
+ For example, "Documentation/{asterisk}.html" matches
+ "Documentation/git.html" but not "Documentation/ppc/ppc.html"
+ or "tools/perf/Documentation/perf.html".
++
+Two consecutive asterisks ("`**`") in patterns matched against
+full pathname may have special meaning:
+
+ - A leading "`**`" followed by a slash means match in all
+ directories. For example, "`**/foo`" matches file or directory
+ "`foo`" anywhere, the same as pattern "`foo`". "**/foo/bar"
+ matches file or directory "`bar`" anywhere that is directly
+ under directory "`foo`".
+
+ - A trailing "/**" matches everything inside. For example,
+ "abc/**" matches all files inside directory "abc", relative
+ to the location of the `.gitignore` file, with infinite depth.
+
+ - A slash followed by two consecutive asterisks then a slash
+ matches zero or more directories. For example, "`a/**/b`"
+ matches "`a/b`", "`a/x/b`", "`a/x/y/b`" and so on.
+
+ - Other consecutive asterisks are considered invalid.
++
+Glob magic is incompatible with literal magic.
--
+
Currently only the slash `/` is recognized as the "magic signature",
diff --git a/builtin/add.c b/builtin/add.c
index 663ddd1..1dab246 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -541,11 +541,16 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
/*
* file_exists() assumes exact match
*/
- GUARD_PATHSPEC(&pathspec, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
+ GUARD_P
[PATCH 31/45] Convert refresh_index to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c| 15 +++
builtin/commit.c | 2 +-
builtin/rm.c | 2 +-
cache.h | 2 +-
read-cache.c | 5 +++--
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index d039fc9..f5d6a33 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -226,19 +226,18 @@ static char *prune_directory(struct dir_struct *dir,
const char **pathspec,
return seen;
}
-static void refresh(int verbose, const char **pathspec)
+static void refresh(int verbose, const struct pathspec *pathspec)
{
char *seen;
- int i, specs;
+ int i;
- for (specs = 0; pathspec[specs]; specs++)
- /* nothing */;
- seen = xcalloc(specs, 1);
+ seen = xcalloc(pathspec->nr, 1);
refresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN :
REFRESH_QUIET,
pathspec, seen, _("Unstaged changes after refreshing the
index:"));
- for (i = 0; i < specs; i++) {
+ for (i = 0; i < pathspec->nr; i++) {
if (!seen[i])
- die(_("pathspec '%s' did not match any files"),
pathspec[i]);
+ die(_("pathspec '%s' did not match any files"),
+ pathspec->items[i].match);
}
free(seen);
}
@@ -524,7 +523,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
if (refresh_only) {
- refresh(verbose, pathspec.raw);
+ refresh(verbose, &pathspec);
goto finish;
}
if (implicit_dot && prefix)
diff --git a/builtin/commit.c b/builtin/commit.c
index e5a3bb5..56bc7be 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1200,7 +1200,7 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
prefix, argv);
read_cache_preload(&s.pathspec);
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
s.pathspec.raw, NULL, NULL);
+ refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &s.pathspec,
NULL, NULL);
fd = hold_locked_index(&index_lock, 0);
if (0 <= fd)
diff --git a/builtin/rm.c b/builtin/rm.c
index 0b38de3..9afc1c5 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -251,7 +251,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
}
parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
- refresh_index(&the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL);
+ refresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);
seen = NULL;
seen = xcalloc(pathspec.nr, 1);
diff --git a/cache.h b/cache.h
index f978e88..25410b9 100644
--- a/cache.h
+++ b/cache.h
@@ -499,7 +499,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce,
struct stat *st);
#define REFRESH_IGNORE_MISSING 0x0008 /* ignore non-existent */
#define REFRESH_IGNORE_SUBMODULES 0x0010 /* ignore submodules */
#define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not "needs
update" */
-extern int refresh_index(struct index_state *, unsigned int flags, const char
**pathspec, char *seen, const char *header_msg);
+extern int refresh_index(struct index_state *, unsigned int flags, const
struct pathspec *pathspec, char *seen, const char *header_msg);
struct lock_file {
struct lock_file *next;
diff --git a/read-cache.c b/read-cache.c
index 04ed561..27bbc35 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1097,7 +1097,8 @@ static void show_file(const char * fmt, const char *
name, int in_porcelain,
printf(fmt, name);
}
-int refresh_index(struct index_state *istate, unsigned int flags, const char
**pathspec,
+int refresh_index(struct index_state *istate, unsigned int flags,
+ const struct pathspec *pathspec,
char *seen, const char *header_msg)
{
int i;
@@ -1132,7 +1133,7 @@ int refresh_index(struct index_state *istate, unsigned
int flags, const char **p
continue;
if (pathspec &&
- !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0,
seen))
+ !match_pathspec_depth(pathspec, ce->name, ce_namelen(ce),
0, seen))
filtered = 1;
if (ce_stage(ce)) {
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 32/45] Convert {read,fill}_directory to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c | 4 +++-
builtin/clean.c| 2 +-
builtin/grep.c | 2 +-
builtin/ls-files.c | 2 +-
dir.c | 16 +++-
dir.h | 4 ++--
wt-status.c| 2 +-
7 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index f5d6a33..34c9358 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -507,6 +507,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (add_new_files) {
int baselen;
+ struct pathspec empty_pathspec;
/* Set up the default git porcelain excludes */
memset(&dir, 0, sizeof(dir));
@@ -515,8 +516,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
setup_standard_excludes(&dir);
}
+ memset(&empty_pathspec, 0, sizeof(empty_pathspec));
/* This picks up the paths that are not tracked */
- baselen = fill_directory(&dir, implicit_dot ? NULL :
pathspec.raw);
+ baselen = fill_directory(&dir, implicit_dot ? &empty_pathspec :
&pathspec);
if (pathspec.nr)
seen = prune_directory(&dir, pathspec.raw, baselen,
implicit_dot ? WARN_IMPLICIT_DOT : 0);
diff --git a/builtin/clean.c b/builtin/clean.c
index fdd4980..d540ca4 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -214,7 +214,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
PATHSPEC_PREFER_CWD,
prefix, argv);
- fill_directory(&dir, pathspec.raw);
+ fill_directory(&dir, &pathspec);
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
diff --git a/builtin/grep.c b/builtin/grep.c
index 4bc0754..76a6a60 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -523,7 +523,7 @@ static int grep_directory(struct grep_opt *opt, const
struct pathspec *pathspec,
if (exc_std)
setup_standard_excludes(&dir);
- fill_directory(&dir, pathspec->raw);
+ fill_directory(&dir, pathspec);
for (i = 0; i < dir.nr; i++) {
const char *name = dir.entries[i]->name;
int namelen = strlen(name);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index f6db498..7feedba 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -214,7 +214,7 @@ static void show_files(struct dir_struct *dir)
/* For cached/deleted files we don't need to even do the readdir */
if (show_others || show_killed) {
- fill_directory(dir, pathspec.raw);
+ fill_directory(dir, &pathspec);
if (show_others)
show_other_files(dir);
if (show_killed)
diff --git a/dir.c b/dir.c
index fcc0b97..863ee39 100644
--- a/dir.c
+++ b/dir.c
@@ -142,7 +142,7 @@ char *common_prefix(const char **pathspec)
return len ? xmemdupz(*pathspec, len) : NULL;
}
-int fill_directory(struct dir_struct *dir, const char **pathspec)
+int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
{
size_t len;
@@ -150,10 +150,10 @@ int fill_directory(struct dir_struct *dir, const char
**pathspec)
* Calculate common prefix for the pathspec, and
* use that to optimize the directory walk
*/
- len = common_prefix_len(pathspec);
+ len = common_prefix_len(pathspec->raw);
/* Read the directory and prune it */
- read_directory(dir, pathspec ? *pathspec : "", len, pathspec);
+ read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len,
pathspec);
return len;
}
@@ -1390,14 +1390,20 @@ static int treat_leading_path(struct dir_struct *dir,
return rc;
}
-int read_directory(struct dir_struct *dir, const char *path, int len, const
char **pathspec)
+int read_directory(struct dir_struct *dir, const char *path, int len, const
struct pathspec *pathspec)
{
struct path_simplify *simplify;
+ /*
+* Check out create_simplify()
+*/
+ if (pathspec)
+ GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+
if (has_symlink_leading_path(path, len))
return dir->nr;
- simplify = create_simplify(pathspec);
+ simplify = create_simplify(pathspec ? pathspec->raw : NULL);
if (!len || treat_leading_path(dir, path, len, simplify))
read_directory_recursive(dir, path, len, 0, simplify);
free_simplify(simplify);
diff --git a/dir.h b/dir.h
index 229ccc8..076dd96 100644
--- a/dir.h
+++ b/dir.h
@@ -137,8 +137,8 @@ extern int match_pathspec_depth(const struct pathspec
*pathspec,
int prefix, char *seen);
extern int within_depth(const char *name, int namelen, int depth, int
max_depth);
-extern int
[PATCH 37/45] Remove match_pathspec() in favor of match_pathspec_depth()
match_pathspec_depth was created to replace match_pathspec (see
61cf282 (pathspec: add match_pathspec_depth() - 2010-12-15). It took
more than two years, but the replacement finally happens :-)
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c | 30 +++---
builtin/check-ignore.c | 4 +-
dir.c | 107 -
dir.h | 1 -
pathspec.c | 19 -
pathspec.h | 4 +-
6 files changed, 25 insertions(+), 140 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index a47aeb4..0b80fa8 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -195,23 +195,21 @@ int add_files_to_cache(const char *prefix,
}
#define WARN_IMPLICIT_DOT (1u << 0)
-static char *prune_directory(struct dir_struct *dir, const char **pathspec,
+static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,
int prefix, unsigned flag)
{
char *seen;
- int i, specs;
+ int i;
struct dir_entry **src, **dst;
- for (specs = 0; pathspec[specs]; specs++)
- /* nothing */;
- seen = xcalloc(specs, 1);
+ seen = xcalloc(pathspec->nr, 1);
src = dst = dir->entries;
i = dir->nr;
while (--i >= 0) {
struct dir_entry *entry = *src++;
- if (match_pathspec(pathspec, entry->name, entry->len,
- prefix, seen))
+ if (match_pathspec_depth(pathspec, entry->name, entry->len,
+prefix, seen))
*dst++ = entry;
else if (flag & WARN_IMPLICIT_DOT)
/*
@@ -225,7 +223,7 @@ static char *prune_directory(struct dir_struct *dir, const
char **pathspec,
warn_pathless_add();
}
dir->nr = dst - dir->entries;
- add_pathspec_matches_against_index(pathspec, seen, specs);
+ add_pathspec_matches_against_index(pathspec, seen);
return seen;
}
@@ -523,7 +521,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
/* This picks up the paths that are not tracked */
baselen = fill_directory(&dir, implicit_dot ? &empty_pathspec :
&pathspec);
if (pathspec.nr)
- seen = prune_directory(&dir, pathspec.raw, baselen,
+ seen = prune_directory(&dir, &pathspec, baselen,
implicit_dot ? WARN_IMPLICIT_DOT : 0);
}
@@ -538,23 +536,23 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
int i;
if (!seen)
- seen =
find_pathspecs_matching_against_index(pathspec.raw);
+ seen = find_pathspecs_matching_against_index(&pathspec);
/*
* file_exists() assumes exact match
*/
GUARD_PATHSPEC(&pathspec, PATHSPEC_FROMTOP);
- for (i = 0; pathspec.raw[i]; i++) {
- if (!seen[i] && pathspec.raw[i][0]
- && !file_exists(pathspec.raw[i])) {
+ for (i = 0; i < pathspec.nr; i++) {
+ const char *path = pathspec.items[i].match;
+ if (!seen[i] && !file_exists(path)) {
if (ignore_missing) {
int dtype = DT_UNKNOWN;
- if (is_excluded(&dir, pathspec.raw[i],
&dtype))
- dir_add_ignored(&dir,
pathspec.raw[i], strlen(pathspec.raw[i]));
+ if (is_excluded(&dir, path, &dtype))
+ dir_add_ignored(&dir, path,
pathspec.items[i].len);
} else
die(_("pathspec '%s' did not match any
files"),
- pathspec.raw[i]);
+ pathspec.items[i].original);
}
}
free(seen);
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index d49c083..70537c8 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -94,9 +94,9 @@ static int check_ignore(struct dir_struct *dir,
* should not be ignored, in order to be consistent with
* 'git status', 'git add' etc.
*/
- seen = find_pathspecs_matching_against_index(pathspec.raw);
+ seen = find_pathspecs_matching_against_index(&pathspec);
for (i = 0; i < pathspec.nr; i++) {
- full_path = pathspec.raw[i];
+ full_path = pathspec.items[i].match;
exclude = NULL;
if (!seen[i]) {
exclude = last_exclude_matching(dir, full_path,
[PATCH 36/45] Remove init_pathspec() in favor of parse_pathspec()
While at there, move free_pathspec() to pathspec.c
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/blame.c| 8 +---
builtin/log.c | 2 +-
builtin/ls-files.c | 11 +--
diff-lib.c | 2 +-
dir.c | 58 --
merge-recursive.c | 2 +-
pathspec.c | 6 ++
pathspec.h | 1 -
revision.c | 2 +-
tree-diff.c| 10 +-
10 files changed, 21 insertions(+), 81 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 5bd721d..56e3d6b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -408,7 +408,7 @@ static struct origin *find_origin(struct scoreboard *sb,
paths[0] = origin->path;
paths[1] = NULL;
- init_pathspec(&diff_opts.pathspec, paths);
+ parse_pathspec(&diff_opts.pathspec, PATHSPEC_ALL_MAGIC, 0, "", paths);
diff_setup_done(&diff_opts);
if (is_null_sha1(origin->commit->object.sha1))
@@ -486,15 +486,12 @@ static struct origin *find_rename(struct scoreboard *sb,
struct origin *porigin = NULL;
struct diff_options diff_opts;
int i;
- const char *paths[2];
diff_setup(&diff_opts);
DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.detect_rename = DIFF_DETECT_RENAME;
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_opts.single_follow = origin->path;
- paths[0] = NULL;
- init_pathspec(&diff_opts.pathspec, paths);
diff_setup_done(&diff_opts);
if (is_null_sha1(origin->commit->object.sha1))
@@ -1064,7 +1061,6 @@ static int find_copy_in_parent(struct scoreboard *sb,
int opt)
{
struct diff_options diff_opts;
- const char *paths[1];
int i, j;
int retval;
struct blame_list *blame_list;
@@ -1078,8 +1074,6 @@ static int find_copy_in_parent(struct scoreboard *sb,
DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
- paths[0] = NULL;
- init_pathspec(&diff_opts.pathspec, paths);
diff_setup_done(&diff_opts);
/* Try "find copies harder" on new path if requested;
diff --git a/builtin/log.c b/builtin/log.c
index 9e21232..3798955 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -503,7 +503,7 @@ int cmd_show(int argc, const char **argv, const char
*prefix)
init_grep_defaults();
git_config(git_log_config, NULL);
- init_pathspec(&match_all, NULL);
+ memset(&match_all, 0, sizeof(match_all));
init_revisions(&rev, prefix);
rev.diff = 1;
rev.always_show_header = 1;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 67feda6..7dbd23d 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -313,13 +313,12 @@ void overlay_tree_on_cache(const char *tree_name, const
char *prefix)
}
if (prefix) {
- static const char *(matchbuf[2]);
- matchbuf[0] = prefix;
- matchbuf[1] = NULL;
- init_pathspec(&pathspec, matchbuf);
- pathspec.items[0].nowildcard_len = pathspec.items[0].len;
+ static const char *(matchbuf[1]);
+ matchbuf[0] = NULL;
+ parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC,
+ PATHSPEC_PREFER_CWD, prefix, matchbuf);
} else
- init_pathspec(&pathspec, NULL);
+ memset(&pathspec, 0, sizeof(pathspec));
if (read_tree(tree, 1, &pathspec))
die("unable to read tree entries %s", tree_name);
diff --git a/diff-lib.c b/diff-lib.c
index 4729157..68fc991 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -499,7 +499,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct
diff_options *opt)
struct rev_info revs;
init_revisions(&revs, NULL);
- init_pathspec(&revs.prune_data, opt->pathspec.raw);
+ copy_pathspec(&revs.prune_data, &opt->pathspec);
revs.diffopt = *opt;
if (diff_cache(&revs, tree_sha1, NULL, 1))
diff --git a/dir.c b/dir.c
index 9d20691..269ff85 100644
--- a/dir.c
+++ b/dir.c
@@ -1582,64 +1582,6 @@ int remove_path(const char *name)
return 0;
}
-static int pathspec_item_cmp(const void *a_, const void *b_)
-{
- struct pathspec_item *a, *b;
-
- a = (struct pathspec_item *)a_;
- b = (struct pathspec_item *)b_;
- return strcmp(a->match, b->match);
-}
-
-int init_pathspec(struct pathspec *pathspec, const char **paths)
-{
- const char **p = paths;
- int i;
-
- memset(pathspec, 0, sizeof(*pathspec));
- if (!p)
- return 0;
- while (*p)
- p++;
- pathspec->raw = paths;
- pathspec->nr = p - paths;
- if (!pathspec->nr)
- return 0;
-
- pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
- for (i = 0; i < pathspec->nr; i++) {
-
[PATCH 33/45] Convert add_files_to_cache to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c| 11 +++
builtin/commit.c | 2 +-
cache.h | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 34c9358..a47aeb4 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -166,14 +166,16 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-static void update_files_in_cache(const char *prefix, const char **pathspec,
+static void update_files_in_cache(const char *prefix,
+ const struct pathspec *pathspec,
struct update_callback_data *data)
{
struct rev_info rev;
init_revisions(&rev, prefix);
setup_revisions(0, NULL, &rev, NULL);
- init_pathspec(&rev.prune_data, pathspec);
+ if (pathspec)
+ copy_pathspec(&rev.prune_data, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = data;
@@ -181,7 +183,8 @@ static void update_files_in_cache(const char *prefix, const
char **pathspec,
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
}
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+int add_files_to_cache(const char *prefix,
+ const struct pathspec *pathspec, int flags)
{
struct update_callback_data data;
@@ -571,7 +574,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
memset(&pathspec, 0, sizeof(pathspec));
}
update_data.flags = flags & ~ADD_CACHE_IMPLICIT_DOT;
- update_files_in_cache(prefix, pathspec.raw, &update_data);
+ update_files_in_cache(prefix, &pathspec, &update_data);
exit_status |= !!update_data.add_errors;
if (add_new_files)
diff --git a/builtin/commit.c b/builtin/commit.c
index 56bc7be..2ddff4e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -331,7 +331,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
*/
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0);
+ add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
diff --git a/cache.h b/cache.h
index 25410b9..b3d8b29 100644
--- a/cache.h
+++ b/cache.h
@@ -1260,7 +1260,7 @@ void packet_trace_identity(const char *prog);
* return 0 if success, 1 - if addition of a file failed and
* ADD_FILES_IGNORE_ERRORS was specified in flags
*/
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags);
+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,
int flags);
/* diff.c */
extern int diff_auto_refresh_index;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 27/45] Convert run_add_interactive to use struct pathspec
This passes the pathspec, more or less unmodified, to
git-add--interactive. The command itself does not process pathspec. It
simply passes the pathspec to other builtin commands. So if all those
commands support pathspec, we're good.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c | 26 ++
builtin/checkout.c | 9 -
builtin/reset.c| 8
commit.h | 2 +-
4 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 9a7235e..d039fc9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -244,16 +244,12 @@ static void refresh(int verbose, const char **pathspec)
}
int run_add_interactive(const char *revision, const char *patch_mode,
- const char **pathspec)
+ const struct pathspec *pathspec)
{
- int status, ac, pc = 0;
+ int status, ac, i;
const char **args;
- if (pathspec)
- while (pathspec[pc])
- pc++;
-
- args = xcalloc(sizeof(const char *), (pc + 5));
+ args = xcalloc(sizeof(const char *), (pathspec->nr + 6));
ac = 0;
args[ac++] = "add--interactive";
if (patch_mode)
@@ -261,11 +257,9 @@ int run_add_interactive(const char *revision, const char
*patch_mode,
if (revision)
args[ac++] = revision;
args[ac++] = "--";
- if (pc) {
- memcpy(&(args[ac]), pathspec, sizeof(const char *) * pc);
- ac += pc;
- }
- args[ac] = NULL;
+ for (i = 0; i < pathspec->nr; i++)
+ /* pass original pathspec, to be re-parsed */
+ args[ac++] = pathspec->items[i].original;
status = run_command_v_opt(args, RUN_GIT_CMD);
free(args);
@@ -280,17 +274,17 @@ int interactive_add(int argc, const char **argv, const
char *prefix, int patch)
* git-add--interactive itself does not parse pathspec. It
* simply passes the pathspec to other builtin commands. Let's
* hope all of them support all magic, or we'll need to limit
-* the magic here. There is still a problem with prefix. But
-* that'll be worked on later on.
+* the magic here.
*/
parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
PATHSPEC_PREFER_FULL |
- PATHSPEC_SYMLINK_LEADING_PATH,
+ PATHSPEC_SYMLINK_LEADING_PATH |
+ PATHSPEC_PREFIX_ORIGIN,
prefix, argv);
return run_add_interactive(NULL,
patch ? "--patch" : NULL,
- pathspec.raw);
+ &pathspec);
}
static int edit_patch(int argc, const char **argv, const char *prefix)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d7a65e4..0e93af0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -257,7 +257,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (opts->patch_mode)
return run_add_interactive(revision, "--patch=checkout",
- opts->pathspec.raw);
+ &opts->pathspec);
lock_file = xcalloc(1, sizeof(struct lock_file));
@@ -1151,10 +1151,9 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
* cannot handle. Magic mask is pretty safe to be
* lifted for new magic when opts.patch_mode == 0.
*/
- parse_pathspec(&opts.pathspec,
- opts.patch_mode == 0 ? 0 :
- (PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP),
- 0, prefix, argv);
+ parse_pathspec(&opts.pathspec, 0,
+ opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
+ prefix, argv);
if (!opts.pathspec.nr)
die(_("invalid path specification"));
diff --git a/builtin/reset.c b/builtin/reset.c
index da7282e..1c8e4a5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -216,9 +216,9 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
- parse_pathspec(pathspec,
- patch_mode ? PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP : 0,
- PATHSPEC_PREFER_FULL,
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_FULL |
+ (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0),
prefix, argv);
}
@@ -296,7 +296,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with
--{hard,mixed,soft}"));
- return run_add_interactive(sha1_to_hex(sha1),
[PATCH 34/45] Convert common_prefix() to use struct pathspec
The code now takes advantage of nowildcard_len field.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 2 +-
builtin/ls-files.c | 2 +-
dir.c | 31 +++
dir.h | 2 +-
4 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2ddff4e..5b7adb0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -194,7 +194,7 @@ static int list_paths(struct string_list *list, const char
*with_tree,
m = xcalloc(1, pattern->nr);
if (with_tree) {
- char *max_prefix = common_prefix(pattern->raw);
+ char *max_prefix = common_prefix(pattern);
overlay_tree_on_cache(with_tree, max_prefix ? max_prefix :
prefix);
free(max_prefix);
}
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7feedba..67feda6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -544,7 +544,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
prefix, argv);
/* Find common prefix for all pathspec's */
- max_prefix = common_prefix(pathspec.raw);
+ max_prefix = common_prefix(&pathspec);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
/* Treat unmatching pathspec elements as errors */
diff --git a/dir.c b/dir.c
index 863ee39..9d20691 100644
--- a/dir.c
+++ b/dir.c
@@ -103,26 +103,25 @@ static int fnmatch_icase_mem(const char *pattern, int
patternlen,
return match_status;
}
-static size_t common_prefix_len(const char **pathspec)
+static size_t common_prefix_len(const struct pathspec *pathspec)
{
- const char *n, *first;
+ int n;
size_t max = 0;
- int literal = limit_pathspec_to_literal();
- if (!pathspec)
- return max;
-
- first = *pathspec;
- while ((n = *pathspec++)) {
- size_t i, len = 0;
- for (i = 0; first == n || i < max; i++) {
- char c = n[i];
- if (!c || c != first[i] || (!literal &&
is_glob_special(c)))
+ GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+
+ for (n = 0; n < pathspec->nr; n++) {
+ size_t i = 0, len = 0;
+ while (i < pathspec->items[n].nowildcard_len &&
+ (n == 0 || i < max)) {
+ char c = pathspec->items[n].match[i];
+ if (c != pathspec->items[0].match[i])
break;
if (c == '/')
len = i + 1;
+ i++;
}
- if (first == n || len < max) {
+ if (n == 0 || len < max) {
max = len;
if (!max)
break;
@@ -135,11 +134,11 @@ static size_t common_prefix_len(const char **pathspec)
* Returns a copy of the longest leading path common among all
* pathspecs.
*/
-char *common_prefix(const char **pathspec)
+char *common_prefix(const struct pathspec *pathspec)
{
unsigned long len = common_prefix_len(pathspec);
- return len ? xmemdupz(*pathspec, len) : NULL;
+ return len ? xmemdupz(pathspec->items[0].match, len) : NULL;
}
int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
@@ -150,7 +149,7 @@ int fill_directory(struct dir_struct *dir, const struct
pathspec *pathspec)
* Calculate common prefix for the pathspec, and
* use that to optimize the directory walk
*/
- len = common_prefix_len(pathspec->raw);
+ len = common_prefix_len(pathspec);
/* Read the directory and prune it */
read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len,
pathspec);
diff --git a/dir.h b/dir.h
index 076dd96..ba40e69 100644
--- a/dir.h
+++ b/dir.h
@@ -130,7 +130,7 @@ struct dir_struct {
#define MATCHED_EXACTLY 3
extern int simple_length(const char *match);
extern int no_wildcard(const char *string);
-extern char *common_prefix(const char **pathspec);
+extern char *common_prefix(const struct pathspec *pathspec);
extern int match_pathspec(const char **pathspec, const char *name, int
namelen, int prefix, char *seen);
extern int match_pathspec_depth(const struct pathspec *pathspec,
const char *name, int namelen,
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 28/45] Convert unmerge_cache to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
rerere.c | 2 +-
resolve-undo.c | 4 ++--
resolve-undo.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/rerere.c b/rerere.c
index 27afbfe..4105bca 100644
--- a/rerere.c
+++ b/rerere.c
@@ -668,7 +668,7 @@ int rerere_forget(struct pathspec *pathspec)
fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
- unmerge_cache(pathspec->raw);
+ unmerge_cache(pathspec);
find_conflict(&conflict);
for (i = 0; i < conflict.nr; i++) {
struct string_list_item *it = &conflict.items[i];
diff --git a/resolve-undo.c b/resolve-undo.c
index 639eb9c..4b78e6f 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -173,7 +173,7 @@ void unmerge_marked_index(struct index_state *istate)
}
}
-void unmerge_index(struct index_state *istate, const char **pathspec)
+void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)
{
int i;
@@ -182,7 +182,7 @@ void unmerge_index(struct index_state *istate, const char
**pathspec)
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
- if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0,
NULL))
+ if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce),
0, NULL))
continue;
i = unmerge_index_entry_at(istate, i);
}
diff --git a/resolve-undo.h b/resolve-undo.h
index 7a30206..4630645 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -11,7 +11,7 @@ extern void resolve_undo_write(struct strbuf *, struct
string_list *);
extern struct string_list *resolve_undo_read(const char *, unsigned long);
extern void resolve_undo_clear_index(struct index_state *);
extern int unmerge_index_entry_at(struct index_state *, int);
-extern void unmerge_index(struct index_state *, const char **);
+extern void unmerge_index(struct index_state *, const struct pathspec *);
extern void unmerge_marked_index(struct index_state *);
#endif
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 35/45] Remove diff_tree_{setup,release}_paths
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/blame.c | 12 ++--
builtin/reset.c | 9 +
diff.h | 2 --
notes-merge.c | 4 ++--
revision.c | 5 +++--
tree-diff.c | 18 --
6 files changed, 20 insertions(+), 30 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 079dcd3..5bd721d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -408,7 +408,7 @@ static struct origin *find_origin(struct scoreboard *sb,
paths[0] = origin->path;
paths[1] = NULL;
- diff_tree_setup_paths(paths, &diff_opts);
+ init_pathspec(&diff_opts.pathspec, paths);
diff_setup_done(&diff_opts);
if (is_null_sha1(origin->commit->object.sha1))
@@ -458,7 +458,7 @@ static struct origin *find_origin(struct scoreboard *sb,
}
}
diff_flush(&diff_opts);
- diff_tree_release_paths(&diff_opts);
+ free_pathspec(&diff_opts.pathspec);
if (porigin) {
/*
* Create a freestanding copy that is not part of
@@ -494,7 +494,7 @@ static struct origin *find_rename(struct scoreboard *sb,
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_opts.single_follow = origin->path;
paths[0] = NULL;
- diff_tree_setup_paths(paths, &diff_opts);
+ init_pathspec(&diff_opts.pathspec, paths);
diff_setup_done(&diff_opts);
if (is_null_sha1(origin->commit->object.sha1))
@@ -516,7 +516,7 @@ static struct origin *find_rename(struct scoreboard *sb,
}
}
diff_flush(&diff_opts);
- diff_tree_release_paths(&diff_opts);
+ free_pathspec(&diff_opts.pathspec);
return porigin;
}
@@ -1079,7 +1079,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
paths[0] = NULL;
- diff_tree_setup_paths(paths, &diff_opts);
+ init_pathspec(&diff_opts.pathspec, paths);
diff_setup_done(&diff_opts);
/* Try "find copies harder" on new path if requested;
@@ -1162,7 +1162,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
}
reset_scanned_flag(sb);
diff_flush(&diff_opts);
- diff_tree_release_paths(&diff_opts);
+ free_pathspec(&diff_opts.pathspec);
return retval;
}
diff --git a/builtin/reset.c b/builtin/reset.c
index 1c8e4a5..b665218 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -130,12 +130,13 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
+static int read_from_tree(const struct pathspec *pathspec,
+ unsigned char *tree_sha1)
{
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
- diff_tree_setup_paths(pathspec, &opt);
+ copy_pathspec(&opt.pathspec, pathspec);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -144,7 +145,7 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1)
return 1;
diffcore_std(&opt);
diff_flush(&opt);
- diff_tree_release_paths(&opt);
+ free_pathspec(&opt.pathspec);
return 0;
}
@@ -329,7 +330,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
if (reset_type == MIXED) {
- if (read_from_tree(pathspec.raw, sha1))
+ if (read_from_tree(&pathspec, sha1))
return 1;
} else {
int err = reset_index(sha1, reset_type, quiet);
diff --git a/diff.h b/diff.h
index d1bc914..b8df245 100644
--- a/diff.h
+++ b/diff.h
@@ -180,8 +180,6 @@ const char *diff_line_prefix(struct diff_options *);
extern const char mime_boundary_leader[];
-extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
-extern void diff_tree_release_paths(struct diff_options *);
extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
const char *base, struct diff_options *opt);
extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
diff --git a/notes-merge.c b/notes-merge.c
index 0f67bd3..f1a3b39 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -169,7 +169,7 @@ static struct notes_merge_pair *diff_tree_remote(struct
notes_merge_options *o,
sha1_to_hex(mp->remote));
}
diff_flush(&opt);
- diff_tree_release_paths(&opt);
+ free_pathspec(&opt.pathspec);
*num_changes = len;
return changes;
@@ -255,7 +255,7 @@ static void diff_tree_local(struct notes_merge_options *o,
sha1_to_hex(mp->local));
}
diff_flush(&opt);
-
[PATCH 23/45] add: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/add.c | 103 +-
pathspec.c| 43
2 files changed, 45 insertions(+), 101 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index f45d9d4..9a7235e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -226,21 +226,6 @@ static char *prune_directory(struct dir_struct *dir, const
char **pathspec,
return seen;
}
-/*
- * Checks the index to see whether any path in pathspec refers to
- * something inside a submodule. If so, dies with an error message.
- */
-static void treat_gitlinks(const char **pathspec)
-{
- int i;
-
- if (!pathspec || !*pathspec)
- return;
-
- for (i = 0; pathspec[i]; i++)
- pathspec[i] = check_path_for_gitlink(pathspec[i]);
-}
-
static void refresh(int verbose, const char **pathspec)
{
char *seen;
@@ -258,25 +243,6 @@ static void refresh(int verbose, const char **pathspec)
free(seen);
}
-/*
- * Normalizes argv relative to prefix, via get_pathspec(), and then
- * runs die_if_path_beyond_symlink() on each path in the normalized
- * list.
- */
-static const char **validate_pathspec(const char **argv, const char *prefix)
-{
- const char **pathspec = get_pathspec(prefix, argv);
-
- if (pathspec) {
- const char **p;
- for (p = pathspec; *p; p++) {
- die_if_path_beyond_symlink(*p, prefix);
- }
- }
-
- return pathspec;
-}
-
int run_add_interactive(const char *revision, const char *patch_mode,
const char **pathspec)
{
@@ -308,17 +274,23 @@ int run_add_interactive(const char *revision, const char
*patch_mode,
int interactive_add(int argc, const char **argv, const char *prefix, int patch)
{
- const char **pathspec = NULL;
+ struct pathspec pathspec;
- if (argc) {
- pathspec = validate_pathspec(argv, prefix);
- if (!pathspec)
- return -1;
- }
+ /*
+* git-add--interactive itself does not parse pathspec. It
+* simply passes the pathspec to other builtin commands. Let's
+* hope all of them support all magic, or we'll need to limit
+* the magic here. There is still a problem with prefix. But
+* that'll be worked on later on.
+*/
+ parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
+ PATHSPEC_PREFER_FULL |
+ PATHSPEC_SYMLINK_LEADING_PATH,
+ prefix, argv);
return run_add_interactive(NULL,
patch ? "--patch" : NULL,
- pathspec);
+ pathspec.raw);
}
static int edit_patch(int argc, const char **argv, const char *prefix)
@@ -445,7 +417,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
{
int exit_status = 0;
int newfd;
- const char **pathspec;
+ struct pathspec pathspec;
struct dir_struct dir;
int flags;
int add_new_files;
@@ -526,11 +498,19 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
fprintf(stderr, _("Maybe you wanted to say 'git add .'?\n"));
return 0;
}
- pathspec = validate_pathspec(argv, prefix);
if (read_cache() < 0)
die(_("index file corrupt"));
- treat_gitlinks(pathspec);
+
+ /*
+* Check the "pathspec '%s' did not match any files" block
+* below before enabling new magic.
+*/
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_FULL |
+ PATHSPEC_SYMLINK_LEADING_PATH |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE,
+ prefix, argv);
if (add_new_files) {
int baselen;
@@ -543,34 +523,40 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
}
/* This picks up the paths that are not tracked */
- baselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);
- if (pathspec)
- seen = prune_directory(&dir, pathspec, baselen,
+ baselen = fill_directory(&dir, implicit_dot ? NULL :
pathspec.raw);
+ if (pathspec.nr)
+ seen = prune_directory(&dir, pathspec.raw, baselen,
implicit_dot ? WARN_IMPLICIT_DOT : 0);
}
if (refresh_only) {
- refresh(verbose, pathspec);
+ refresh(verbose, pathspec.raw);
goto finish;
}
if (implicit_dot && prefix)
refresh_cache(REFRESH_QUIET);
- if (pathspec) {
+ if (pathspec.nr) {
int i;
if (!seen)
-
[PATCH 30/45] Convert report_path_error to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 2 +-
builtin/commit.c | 14 ++
builtin/ls-files.c | 19 +++
cache.h| 2 +-
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index be08061..f543533 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -304,7 +304,7 @@ static int checkout_paths(const struct checkout_opts *opts,
ce->ce_flags |= CE_MATCHED;
}
- if (report_path_error(ps_matched, opts->pathspec.raw, opts->prefix)) {
+ if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {
free(ps_matched);
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 171de9f..e5a3bb5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -183,20 +183,18 @@ static int commit_index_files(void)
* and return the paths that match the given pattern in list.
*/
static int list_paths(struct string_list *list, const char *with_tree,
- const char *prefix, const char **pattern)
+ const char *prefix, const struct pathspec *pattern)
{
int i;
char *m;
- if (!pattern)
+ if (!pattern->nr)
return 0;
- for (i = 0; pattern[i]; i++)
- ;
- m = xcalloc(1, i);
+ m = xcalloc(1, pattern->nr);
if (with_tree) {
- char *max_prefix = common_prefix(pattern);
+ char *max_prefix = common_prefix(pattern->raw);
overlay_tree_on_cache(with_tree, max_prefix ? max_prefix :
prefix);
free(max_prefix);
}
@@ -207,7 +205,7 @@ static int list_paths(struct string_list *list, const char
*with_tree,
if (ce->ce_flags & CE_UPDATE)
continue;
- if (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))
+ if (!match_pathspec_depth(pattern, ce->name, ce_namelen(ce), 0,
m))
continue;
item = string_list_insert(list, ce->name);
if (ce_skip_worktree(ce))
@@ -397,7 +395,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
memset(&partial, 0, sizeof(partial));
partial.strdup_strings = 1;
- if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix,
pathspec.raw))
+ if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix,
&pathspec))
exit(1);
discard_cache();
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 1534a39..f6db498 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -344,15 +344,16 @@ void overlay_tree_on_cache(const char *tree_name, const
char *prefix)
}
}
-int report_path_error(const char *ps_matched, const char **pathspec, const
char *prefix)
+int report_path_error(const char *ps_matched,
+ const struct pathspec *pathspec,
+ const char *prefix)
{
/*
* Make sure all pathspec matched; otherwise it is an error.
*/
struct strbuf sb = STRBUF_INIT;
- const char *name;
int num, errors = 0;
- for (num = 0; pathspec[num]; num++) {
+ for (num = 0; num < pathspec->nr; num++) {
int other, found_dup;
if (ps_matched[num])
@@ -360,13 +361,16 @@ int report_path_error(const char *ps_matched, const char
**pathspec, const char
/*
* The caller might have fed identical pathspec
* twice. Do not barf on such a mistake.
+* FIXME: parse_pathspec should have eliminated
+* duplicate pathspec.
*/
for (found_dup = other = 0;
-!found_dup && pathspec[other];
+!found_dup && other < pathspec->nr;
other++) {
if (other == num || !ps_matched[other])
continue;
- if (!strcmp(pathspec[other], pathspec[num]))
+ if (!strcmp(pathspec->items[other].original,
+ pathspec->items[num].original))
/*
* Ok, we have a match already.
*/
@@ -375,9 +379,8 @@ int report_path_error(const char *ps_matched, const char
**pathspec, const char
if (found_dup)
continue;
- name = quote_path_relative(pathspec[num], -1, &sb, prefix);
error("pathspec '%s' did not match any file(s) known to git.",
- name);
+ pathspec->items[num].original);
errors++;
}
strbuf_release(&sb);
@@ -573,7 +576,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
[PATCH 21/45] archive: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive.c | 19 +++
archive.h | 4 +++-
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/archive.c b/archive.c
index c699a2d..99fadc8 100644
--- a/archive.c
+++ b/archive.c
@@ -5,7 +5,6 @@
#include "archive.h"
#include "parse-options.h"
#include "unpack-trees.h"
-#include "pathspec.h"
static char const * const archive_usage[] = {
N_("git archive [options] [...]"),
@@ -152,7 +151,6 @@ int write_archive_entries(struct archiver_args *args,
struct archiver_context context;
struct unpack_trees_options opts;
struct tree_desc t;
- struct pathspec pathspec;
int err;
if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
@@ -187,10 +185,8 @@ int write_archive_entries(struct archiver_args *args,
git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
}
- init_pathspec(&pathspec, args->pathspec);
- err = read_tree_recursive(args->tree, "", 0, 0, &pathspec,
+ err = read_tree_recursive(args->tree, "", 0, 0, &args->pathspec,
write_archive_entry, &context);
- free_pathspec(&pathspec);
if (err == READ_TREE_RECURSIVE)
err = 0;
return err;
@@ -223,7 +219,7 @@ static int path_exists(struct tree *tree, const char *path)
struct pathspec pathspec;
int ret;
- init_pathspec(&pathspec, paths);
+ parse_pathspec(&pathspec, 0, 0, "", paths);
ret = read_tree_recursive(tree, "", 0, 0, &pathspec, reject_entry,
NULL);
free_pathspec(&pathspec);
return ret != 0;
@@ -232,11 +228,18 @@ static int path_exists(struct tree *tree, const char
*path)
static void parse_pathspec_arg(const char **pathspec,
struct archiver_args *ar_args)
{
- ar_args->pathspec = pathspec = get_pathspec("", pathspec);
+ /*
+* must be consistent with parse_pathspec in path_exists()
+* Also if pathspec patterns are dependent, we're in big
+* trouble as we test each one separately
+*/
+ parse_pathspec(&ar_args->pathspec, 0,
+ PATHSPEC_PREFER_FULL,
+ "", pathspec);
if (pathspec) {
while (*pathspec) {
if (**pathspec && !path_exists(ar_args->tree,
*pathspec))
- die("path not found: %s", *pathspec);
+ die(_("pathspec '%s' did not match any files"),
*pathspec);
pathspec++;
}
}
diff --git a/archive.h b/archive.h
index 895afcd..4a791e1 100644
--- a/archive.h
+++ b/archive.h
@@ -1,6 +1,8 @@
#ifndef ARCHIVE_H
#define ARCHIVE_H
+#include "pathspec.h"
+
struct archiver_args {
const char *base;
size_t baselen;
@@ -8,7 +10,7 @@ struct archiver_args {
const unsigned char *commit_sha1;
const struct commit *commit;
time_t time;
- const char **pathspec;
+ struct pathspec pathspec;
unsigned int verbose : 1;
unsigned int worktree_attributes : 1;
unsigned int convert : 1;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 22/45] check-ignore: convert to use parse_pathspec
check-ignore (at least the test suite) seems to rely on the pattern
order. PATHSPEC_KEEP_ORDER is introduced to explictly express this.
The lack of PATHSPEC_MAXDEPTH_VALID is sufficient because it's the
only flag that reorders pathspecs, but it's less obvious that way.
Cc: Adam Spiers
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/check-ignore.c | 35 ++-
pathspec.c | 6 +-
pathspec.h | 1 +
t/t0008-ignores.sh | 8
4 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 4a8fc70..d49c083 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -64,37 +64,45 @@ static void output_exclude(const char *path, struct exclude
*exclude)
}
static int check_ignore(struct dir_struct *dir,
- const char *prefix, const char **pathspec)
+ const char *prefix, int argc, const char **argv)
{
- const char *path, *full_path;
+ const char *full_path;
char *seen;
int num_ignored = 0, dtype = DT_UNKNOWN, i;
struct exclude *exclude;
+ struct pathspec pathspec;
- if (!pathspec || !*pathspec) {
+ if (!argc) {
if (!quiet)
fprintf(stderr, "no pathspec given.\n");
return 0;
}
/*
+* check-ignore just needs paths. Magic beyond :/ is really
+* irrelevant.
+*/
+ parse_pathspec(&pathspec,
+ PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
+ PATHSPEC_SYMLINK_LEADING_PATH |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE |
+ PATHSPEC_KEEP_ORDER,
+ prefix, argv);
+
+ /*
* look for pathspecs matching entries in the index, since these
* should not be ignored, in order to be consistent with
* 'git status', 'git add' etc.
*/
- seen = find_pathspecs_matching_against_index(pathspec);
- for (i = 0; pathspec[i]; i++) {
- path = pathspec[i];
- full_path = prefix_path(prefix, prefix
- ? strlen(prefix) : 0, path);
- full_path = check_path_for_gitlink(full_path);
- die_if_path_beyond_symlink(full_path, prefix);
+ seen = find_pathspecs_matching_against_index(pathspec.raw);
+ for (i = 0; i < pathspec.nr; i++) {
+ full_path = pathspec.raw[i];
exclude = NULL;
if (!seen[i]) {
exclude = last_exclude_matching(dir, full_path, &dtype);
}
if (!quiet && (exclude || show_non_matching))
- output_exclude(path, exclude);
+ output_exclude(pathspec.items[i].original, exclude);
if (exclude)
num_ignored++;
}
@@ -120,7 +128,8 @@ static int check_ignore_stdin_paths(struct dir_struct *dir,
const char *prefix)
strbuf_swap(&buf, &nbuf);
}
pathspec[0] = buf.buf;
- num_ignored += check_ignore(dir, prefix, (const char
**)pathspec);
+ num_ignored += check_ignore(dir, prefix,
+ 1, (const char **)pathspec);
maybe_flush_or_die(stdout, "check-ignore to stdout");
}
strbuf_release(&buf);
@@ -166,7 +175,7 @@ int cmd_check_ignore(int argc, const char **argv, const
char *prefix)
if (stdin_paths) {
num_ignored = check_ignore_stdin_paths(&dir, prefix);
} else {
- num_ignored = check_ignore(&dir, prefix, argv);
+ num_ignored = check_ignore(&dir, prefix, argc, argv);
maybe_flush_or_die(stdout, "ignore to stdout");
}
diff --git a/pathspec.c b/pathspec.c
index ba0a41d..152f9b5 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -370,9 +370,13 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->magic |= item[i].magic;
}
- if (pathspec->magic & PATHSPEC_MAXDEPTH)
+
+ if (pathspec->magic & PATHSPEC_MAXDEPTH) {
+ if (flags & PATHSPEC_KEEP_ORDER)
+ die("BUG: PATHSPEC_MAXDEPTH_VALID and
PATHSPEC_KEEP_ORDER are incompatible");
qsort(pathspec->items, pathspec->nr,
sizeof(struct pathspec_item), pathspec_item_cmp);
+ }
}
/*
diff --git a/pathspec.h b/pathspec.h
index 7068f7d..4f144fd 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -51,6 +51,7 @@ struct pathspec {
*/
#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (1<<5)
#define PATHSPEC_PREFIX_ORIGIN (1<<6)
+#define PATHSPEC_KEEP_ORDER (1<<7)
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathspec *pathspec,
diff --git a/t/t0008-ignores.sh b/t/t0008-
[PATCH 29/45] checkout: convert read_tree_some to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 9 +++--
tree.c | 4 ++--
tree.h | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0e93af0..be08061 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -83,12 +83,9 @@ static int update_some(const unsigned char *sha1, const char
*base, int baselen,
return 0;
}
-static int read_tree_some(struct tree *tree, const char **pathspec)
+static int read_tree_some(struct tree *tree, const struct pathspec *pathspec)
{
- struct pathspec ps;
- init_pathspec(&ps, pathspec);
- read_tree_recursive(tree, "", 0, 0, &ps, update_some, NULL);
- free_pathspec(&ps);
+ read_tree_recursive(tree, "", 0, 0, pathspec, update_some, NULL);
/* update the index with the given tree's info
* for all args, expanding wildcards, and exit
@@ -266,7 +263,7 @@ static int checkout_paths(const struct checkout_opts *opts,
return error(_("corrupt index file"));
if (opts->source_tree)
- read_tree_some(opts->source_tree, opts->pathspec.raw);
+ read_tree_some(opts->source_tree, &opts->pathspec);
ps_matched = xcalloc(1, opts->pathspec.nr);
diff --git a/tree.c b/tree.c
index 62fed63..ff72f67 100644
--- a/tree.c
+++ b/tree.c
@@ -47,7 +47,7 @@ static int read_one_entry_quick(const unsigned char *sha1,
const char *base, int
}
static int read_tree_1(struct tree *tree, struct strbuf *base,
- int stage, struct pathspec *pathspec,
+ int stage, const struct pathspec *pathspec,
read_tree_fn_t fn, void *context)
{
struct tree_desc desc;
@@ -116,7 +116,7 @@ static int read_tree_1(struct tree *tree, struct strbuf
*base,
int read_tree_recursive(struct tree *tree,
const char *base, int baselen,
- int stage, struct pathspec *pathspec,
+ int stage, const struct pathspec *pathspec,
read_tree_fn_t fn, void *context)
{
struct strbuf sb = STRBUF_INIT;
diff --git a/tree.h b/tree.h
index 69bcb5e..9dc90ba 100644
--- a/tree.h
+++ b/tree.h
@@ -25,7 +25,7 @@ typedef int (*read_tree_fn_t)(const unsigned char *, const
char *, int, const ch
extern int read_tree_recursive(struct tree *tree,
const char *base, int baselen,
- int stage, struct pathspec *pathspec,
+ int stage, const struct pathspec *pathspec,
read_tree_fn_t fn, void *context);
extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/45] Convert read_cache_preload() to take struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 2 +-
builtin/commit.c | 4 ++--
builtin/diff-files.c | 2 +-
builtin/diff-index.c | 2 +-
builtin/diff.c | 4 ++--
cache.h | 2 +-
preload-index.c | 20 +++-
7 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 197198b..d7a65e4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -262,7 +262,7 @@ static int checkout_paths(const struct checkout_opts *opts,
lock_file = xcalloc(1, sizeof(struct lock_file));
newfd = hold_locked_index(lock_file, 1);
- if (read_cache_preload(opts->pathspec.raw) < 0)
+ if (read_cache_preload(&opts->pathspec) < 0)
return error(_("corrupt index file"));
if (opts->source_tree)
diff --git a/builtin/commit.c b/builtin/commit.c
index 833c7be..171de9f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -289,7 +289,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
PATHSPEC_PREFER_FULL,
prefix, argv);
- if (read_cache_preload(pathspec.raw) < 0)
+ if (read_cache_preload(&pathspec) < 0)
die(_("index file corrupt"));
if (interactive) {
@@ -1201,7 +1201,7 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
PATHSPEC_PREFER_FULL,
prefix, argv);
- read_cache_preload(s.pathspec.raw);
+ read_cache_preload(&s.pathspec);
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
s.pathspec.raw, NULL, NULL);
fd = hold_locked_index(&index_lock, 0);
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 46085f8..9200069 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -61,7 +61,7 @@ int cmd_diff_files(int argc, const char **argv, const char
*prefix)
(rev.diffopt.output_format & DIFF_FORMAT_PATCH))
rev.combine_merges = rev.dense_combined_merges = 1;
- if (read_cache_preload(rev.diffopt.pathspec.raw) < 0) {
+ if (read_cache_preload(&rev.diffopt.pathspec) < 0) {
perror("read_cache_preload");
return -1;
}
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1c737f7..ce15b23 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -43,7 +43,7 @@ int cmd_diff_index(int argc, const char **argv, const char
*prefix)
usage(diff_cache_usage);
if (!cached) {
setup_work_tree();
- if (read_cache_preload(rev.diffopt.pathspec.raw) < 0) {
+ if (read_cache_preload(&rev.diffopt.pathspec) < 0) {
perror("read_cache_preload");
return -1;
}
diff --git a/builtin/diff.c b/builtin/diff.c
index d237e0a..6b4e3f9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -140,7 +140,7 @@ static int builtin_diff_index(struct rev_info *revs,
usage(builtin_diff_usage);
if (!cached) {
setup_work_tree();
- if (read_cache_preload(revs->diffopt.pathspec.raw) < 0) {
+ if (read_cache_preload(&revs->diffopt.pathspec) < 0) {
perror("read_cache_preload");
return -1;
}
@@ -240,7 +240,7 @@ static int builtin_diff_files(struct rev_info *revs, int
argc, const char **argv
revs->combine_merges = revs->dense_combined_merges = 1;
setup_work_tree();
- if (read_cache_preload(revs->diffopt.pathspec.raw) < 0) {
+ if (read_cache_preload(&revs->diffopt.pathspec) < 0) {
perror("read_cache_preload");
return -1;
}
diff --git a/cache.h b/cache.h
index 5917375..19e0d87 100644
--- a/cache.h
+++ b/cache.h
@@ -443,7 +443,7 @@ extern int init_db(const char *template_dir, unsigned int
flags);
/* Initialize and use the cache information */
extern int read_index(struct index_state *);
-extern int read_index_preload(struct index_state *, const char **pathspec);
+extern int read_index_preload(struct index_state *, const struct pathspec
*pathspec);
extern int read_index_from(struct index_state *, const char *path);
extern int is_index_unborn(struct index_state *);
extern int read_index_unmerged(struct index_state *);
diff --git a/preload-index.c b/preload-index.c
index cddfffa..8c44ceb 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -5,7 +5,8 @@
#include "pathspec.h"
#ifdef NO_PTHREADS
-static void preload_index(struct index_state *index, const char **pathspec)
+static void preload_index(struct index_state *index,
+ const struct pathspec *pathspec)
{
; /* nothing */
}
@@ -25,7 +26,7 @@ static void preload_index(struct index_state *index, const
char **pathspec)
struct thread_data {
pthread_t pthread;
struc
[PATCH 25/45] line-log: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy --- line-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/line-log.c b/line-log.c index 4bbb09b..843a334 100644 --- a/line-log.c +++ b/line-log.c @@ -747,7 +747,7 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list r = r->next; } paths[count] = NULL; - init_pathspec(&rev->diffopt.pathspec, paths); + parse_pathspec(&rev->diffopt.pathspec, 0, 0, "", paths); free(paths); } } -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/45] ls-files: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/ls-files.c | 46 +-
1 file changed, 13 insertions(+), 33 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a0b7e30..1534a39 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -31,7 +31,7 @@ static int debug_mode;
static const char *prefix;
static int max_prefix_len;
static int prefix_len;
-static const char **pathspec;
+static struct pathspec pathspec;
static int error_unmatch;
static char *ps_matched;
static const char *with_tree;
@@ -60,7 +60,7 @@ static void show_dir_entry(const char *tag, struct dir_entry
*ent)
if (len >= ent->len)
die("git ls-files: internal error - directory entry not
superset of prefix");
- if (!match_pathspec(pathspec, ent->name, ent->len, len, ps_matched))
+ if (!match_pathspec_depth(&pathspec, ent->name, ent->len, len,
ps_matched))
return;
fputs(tag, stdout);
@@ -135,7 +135,7 @@ static void show_ce_entry(const char *tag, struct
cache_entry *ce)
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of
prefix");
- if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), len,
ps_matched))
+ if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), len,
ps_matched))
return;
if (tag && *tag && show_valid_bit &&
@@ -189,7 +189,7 @@ static void show_ru_info(void)
len = strlen(path);
if (len < max_prefix_len)
continue; /* outside of the prefix */
- if (!match_pathspec(pathspec, path, len, max_prefix_len,
ps_matched))
+ if (!match_pathspec_depth(&pathspec, path, len, max_prefix_len,
ps_matched))
continue; /* uninterested */
for (i = 0; i < 3; i++) {
if (!ui->mode[i])
@@ -214,7 +214,7 @@ static void show_files(struct dir_struct *dir)
/* For cached/deleted files we don't need to even do the readdir */
if (show_others || show_killed) {
- fill_directory(dir, pathspec);
+ fill_directory(dir, pathspec.raw);
if (show_others)
show_other_files(dir);
if (show_killed)
@@ -282,21 +282,6 @@ static void prune_cache(const char *prefix)
active_nr = last;
}
-static void strip_trailing_slash_from_submodules(void)
-{
- const char **p;
-
- for (p = pathspec; *p != NULL; p++) {
- int len = strlen(*p), pos;
-
- if (len < 1 || (*p)[len - 1] != '/')
- continue;
- pos = cache_name_pos(*p, len - 1);
- if (pos >= 0 && S_ISGITLINK(active_cache[pos]->ce_mode))
- *p = xstrndup(*p, len - 1);
- }
-}
-
/*
* Read the tree specified with --with-tree option
* (typically, HEAD) into stage #1 and then
@@ -550,23 +535,18 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
- pathspec = get_pathspec(prefix, argv);
-
- /* be nice with submodule paths ending in a slash */
- if (pathspec)
- strip_trailing_slash_from_submodules();
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
/* Find common prefix for all pathspec's */
- max_prefix = common_prefix(pathspec);
+ max_prefix = common_prefix(pathspec.raw);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
/* Treat unmatching pathspec elements as errors */
- if (pathspec && error_unmatch) {
- int num;
- for (num = 0; pathspec[num]; num++)
- ;
- ps_matched = xcalloc(1, num);
- }
+ if (pathspec.nr && error_unmatch)
+ ps_matched = xcalloc(1, pathspec.nr);
if ((dir.flags & DIR_SHOW_IGNORED) && !exc_given)
die("ls-files --ignored needs some exclude pattern");
@@ -593,7 +573,7 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
if (ps_matched) {
int bad;
- bad = report_path_error(ps_matched, pathspec, prefix);
+ bad = report_path_error(ps_matched, pathspec.raw, prefix);
if (bad)
fprintf(stderr, "Did you forget to 'git add'?\n");
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 24/45] reset: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/reset.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..da7282e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -171,7 +171,10 @@ static void die_if_unmerged_cache(int reset_type)
}
-static const char **parse_args(const char **argv, const char *prefix, const
char **rev_ret)
+static void parse_args(struct pathspec *pathspec,
+ const char **argv, const char *prefix,
+ int patch_mode,
+ const char **rev_ret)
{
const char *rev = "HEAD";
unsigned char unused[20];
@@ -213,7 +216,10 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
}
}
*rev_ret = rev;
- return argv[0] ? get_pathspec(prefix, argv) : NULL;
+ parse_pathspec(pathspec,
+ patch_mode ? PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP : 0,
+ PATHSPEC_PREFER_FULL,
+ prefix, argv);
}
static int update_refs(const char *rev, const unsigned char *sha1)
@@ -243,7 +249,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
- const char **pathspec = NULL;
+ struct pathspec pathspec;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -263,13 +269,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
- pathspec = parse_args(argv, prefix, &rev);
+ parse_args(&pathspec, argv, prefix, patch_mode, &rev);
unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
if (unborn) {
/* reset on unborn branch: treat as reset to empty tree */
hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
- } else if (!pathspec) {
+ } else if (!pathspec.nr) {
struct commit *commit;
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid revision."),
rev);
@@ -290,13 +296,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with
--{hard,mixed,soft}"));
- return run_add_interactive(sha1_to_hex(sha1), "--patch=reset",
pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), "--patch=reset",
pathspec.raw);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (pathspec) {
+ if (pathspec.nr) {
if (reset_type == MIXED)
warning(_("--mixed with paths is deprecated; use 'git
reset -- ' instead."));
else if (reset_type != NONE)
@@ -323,7 +329,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
if (reset_type == MIXED) {
- if (read_from_tree(pathspec, sha1))
+ if (read_from_tree(pathspec.raw, sha1))
return 1;
} else {
int err = reset_index(sha1, reset_type, quiet);
@@ -344,7 +350,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not write new index file."));
}
- if (!pathspec && !unborn) {
+ if (!pathspec.nr && !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
@@ -352,7 +358,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(lookup_commit_reference(sha1));
}
- if (!pathspec)
+ if (!pathspec.nr)
remove_branch_state();
return update_ref_status;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/45] parse_pathspec: support prefixing original patterns
This makes 'original' suitable for passing to an external command
because all pathspec magic is left in place, provided that the
external command understands pathspec. The prefixing is needed because
we usually launch a subcommand at worktree's top directory and the
subcommand can no longer calculate the prefix itself.
This slightly affects the original purpose of 'original'
(i.e. reporting). We should report without prefixing. So only turn
this flag on when you know you are about to pass the result straight
away to an external command.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 12 +++-
pathspec.h | 1 +
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/pathspec.c b/pathspec.c
index 9aaec36..ba0a41d 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -203,7 +203,17 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
else
match = prefix_path(prefix, prefixlen, copyfrom);
*raw = item->match = match;
- item->original = elt;
+ /*
+* Prefix the pathspec (keep all magic) and put to
+* original. Useful for passing to another command.
+*/
+ if (flags & PATHSPEC_PREFIX_ORIGIN) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_add(&sb, elt, copyfrom - elt);
+ strbuf_addstr(&sb, match);
+ item->original = strbuf_detach(&sb, NULL);
+ } else
+ item->original = elt;
item->len = strlen(item->match);
if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
diff --git a/pathspec.h b/pathspec.h
index b631514..3ca6636 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -43,6 +43,7 @@ struct pathspec {
* safer than _SLASH_CHEAP and also more expensive.
*/
#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (1<<5)
+#define PATHSPEC_PREFIX_ORIGIN (1<<6)
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathspec *pathspec,
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/45] checkout: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout.c | 34 +-
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..197198b 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -46,7 +46,7 @@ struct checkout_opts {
int branch_exists;
const char *prefix;
- const char **pathspec;
+ struct pathspec pathspec;
struct tree *source_tree;
};
@@ -257,20 +257,18 @@ static int checkout_paths(const struct checkout_opts
*opts,
if (opts->patch_mode)
return run_add_interactive(revision, "--patch=checkout",
- opts->pathspec);
+ opts->pathspec.raw);
lock_file = xcalloc(1, sizeof(struct lock_file));
newfd = hold_locked_index(lock_file, 1);
- if (read_cache_preload(opts->pathspec) < 0)
+ if (read_cache_preload(opts->pathspec.raw) < 0)
return error(_("corrupt index file"));
if (opts->source_tree)
- read_tree_some(opts->source_tree, opts->pathspec);
+ read_tree_some(opts->source_tree, opts->pathspec.raw);
- for (pos = 0; opts->pathspec[pos]; pos++)
- ;
- ps_matched = xcalloc(1, pos);
+ ps_matched = xcalloc(1, opts->pathspec.nr);
/*
* Make sure all pathspecs participated in locating the paths
@@ -304,12 +302,12 @@ static int checkout_paths(const struct checkout_opts
*opts,
* match_pathspec() for _all_ entries when
* opts->source_tree != NULL.
*/
- if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce),
+ if (match_pathspec_depth(&opts->pathspec, ce->name,
ce_namelen(ce),
0, ps_matched))
ce->ce_flags |= CE_MATCHED;
}
- if (report_path_error(ps_matched, opts->pathspec, opts->prefix)) {
+ if (report_path_error(ps_matched, opts->pathspec.raw, opts->prefix)) {
free(ps_matched);
return 1;
}
@@ -994,7 +992,7 @@ static int switch_unborn_to_new_branch(const struct
checkout_opts *opts)
static int checkout_branch(struct checkout_opts *opts,
struct branch_info *new)
{
- if (opts->pathspec)
+ if (opts->pathspec.nr)
die(_("paths cannot be used with switching branches"));
if (opts->patch_mode)
@@ -1146,9 +1144,19 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
}
if (argc) {
- opts.pathspec = get_pathspec(prefix, argv);
+ /*
+* In patch mode (opts.patch_mode != 0), we pass the
+* pathspec to an external program, git-add--interactive.
+* Do not accept any kind of magic that that program
+* cannot handle. Magic mask is pretty safe to be
+* lifted for new magic when opts.patch_mode == 0.
+*/
+ parse_pathspec(&opts.pathspec,
+ opts.patch_mode == 0 ? 0 :
+ (PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP),
+ 0, prefix, argv);
- if (!opts.pathspec)
+ if (!opts.pathspec.nr)
die(_("invalid path specification"));
/*
@@ -1180,7 +1188,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
strbuf_release(&buf);
}
- if (opts.patch_mode || opts.pathspec)
+ if (opts.patch_mode || opts.pathspec.nr)
return checkout_paths(&opts, new.name);
else
return checkout_branch(&opts, &new);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/45] clean: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/clean.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index f955a40..fdd4980 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,6 +13,7 @@
#include "refs.h"
#include "string-list.h"
#include "quote.h"
+#include "pathspec.h"
static int force = -1; /* unset */
@@ -150,7 +151,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
struct strbuf directory = STRBUF_INIT;
struct dir_struct dir;
- static const char **pathspec;
+ struct pathspec pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct exclude_list *el;
@@ -209,9 +210,11 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
for (i = 0; i < exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
- pathspec = get_pathspec(prefix, argv);
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_CWD,
+ prefix, argv);
- fill_directory(&dir, pathspec);
+ fill_directory(&dir, pathspec.raw);
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
@@ -246,9 +249,9 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (lstat(ent->name, &st))
continue;
- if (pathspec)
- matches = match_pathspec(pathspec, ent->name, len,
-0, NULL);
+ if (pathspec.nr)
+ matches = match_pathspec_depth(&pathspec, ent->name,
+ len, 0, NULL);
if (S_ISDIR(st.st_mode)) {
strbuf_addstr(&directory, ent->name);
@@ -262,7 +265,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
}
strbuf_reset(&directory);
} else {
- if (pathspec && !matches)
+ if (pathspec.nr && !matches)
continue;
res = dry_run ? 0 : unlink(ent->name);
if (res) {
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/45] Guard against new pathspec magic in pathspec matching code
GUARD_PATHSPEC() marks pathspec-sensitive code, basically all those
that touch anything in 'struct pathspec' except fields "nr" and
"original". GUARD_PATHSPEC() is not supposed to fail. It's mainly to
help the designers to catch unsupported codepaths.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/technical/api-setup.txt | 19 +++
builtin/diff.c| 2 ++
dir.c | 2 ++
pathspec.h| 7 +++
tree-diff.c | 19 +++
tree-walk.c | 2 ++
6 files changed, 51 insertions(+)
diff --git a/Documentation/technical/api-setup.txt
b/Documentation/technical/api-setup.txt
index 90d1aff..540e455 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt
@@ -28,3 +28,22 @@ parse_pathspec(). This function takes several arguments:
- prefix and args come from cmd_* functions
get_pathspec() is obsolete and should never be used in new code.
+
+parse_pathspec() helps catch unsupported features and reject them
+politely. At a lower level, different pathspec-related functions may
+not support the same set of features. Such pathspec-sensitive
+functions are guarded with GUARD_PATHSPEC(), which will die in an
+unfriendly way when an unsupported feature is requested.
+
+The command designers are supposed to make sure that GUARD_PATHSPEC()
+never dies. They have to make sure all unsupported features are caught
+by parse_pathspec(), not by GUARD_PATHSPEC. grepping GUARD_PATHSPEC()
+should give the designers all pathspec-sensitive codepaths and what
+features they support.
+
+A similar process is applied when a new pathspec magic is added. The
+designer lifts the GUARD_PATHSPEC restriction in the functions that
+support the new magic. At the same time (s)he has to make sure this
+new feature will be caught at parse_pathspec() in commands that cannot
+handle the new magic in some cases. grepping parse_pathspec() should
+help.
diff --git a/builtin/diff.c b/builtin/diff.c
index 8c2af6c..d237e0a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -371,6 +371,8 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
die(_("unhandled object '%s' given."), name);
}
if (rev.prune_data.nr) {
+ /* builtin_diff_b_f() */
+ GUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP);
if (!path)
path = rev.prune_data.items[0].match;
paths += rev.prune_data.nr;
diff --git a/dir.c b/dir.c
index cfcdda5..fcc0b97 100644
--- a/dir.c
+++ b/dir.c
@@ -340,6 +340,8 @@ int match_pathspec_depth(const struct pathspec *ps,
{
int i, retval = 0;
+ GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | PATHSPEC_MAXDEPTH);
+
if (!ps->nr) {
if (!ps->recursive ||
!(ps->magic & PATHSPEC_MAXDEPTH) ||
diff --git a/pathspec.h b/pathspec.h
index 3ca6636..7068f7d 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -27,6 +27,13 @@ struct pathspec {
} *items;
};
+#define GUARD_PATHSPEC(ps, mask) \
+ do { \
+ if ((ps)->magic & ~(mask)) \
+ die("BUG:%s:%d: unsupported magic %x", \
+ __FILE__, __LINE__, (ps)->magic & ~(mask)); \
+ } while (0)
+
/* parse_pathspec flags */
#define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
#define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
diff --git a/tree-diff.c b/tree-diff.c
index 826512e..5a87614 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -198,6 +198,25 @@ static void try_to_follow_renames(struct tree_desc *t1,
struct tree_desc *t2, co
const char *paths[1];
int i;
+ /*
+* follow-rename code is very specific, we need exactly one
+* path. Magic that matches more than one path is not
+* supported.
+*/
+ GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP);
+#if 0
+ /*
+* We should reject wildcards as well. Unfortunately we
+* haven't got a reliable way to detect that 'foo\*bar' in
+* fact has no wildcards. nowildcard_len is merely a hint for
+* optimization. Let it slip for now until wildmatch is taught
+* about dry-run mode and returns wildcard info.
+*/
+ if (opt->pathspec.has_wildcard)
+ die("BUG:%s:%d: wildcards are not supported",
+ __FILE__, __LINE__);
+#endif
+
/* Remove the file creation entry from the diff queue, and remember it
*/
choice = q->queue[0];
q->nr = 0;
diff --git a/tree-walk.c b/tree-walk.c
index d399ca9..37b157e 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -636,6 +636,8 @@ enum interesting tree_entry_interesting(const struct
name_entry *entry,
enum interesting never_interesting = ps->has_wildcard ?
entry_not_interesting : all_
[PATCH 15/45] commit: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..0efe269 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -279,17 +279,17 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
{
int fd;
struct string_list partial;
- const char **pathspec = NULL;
+ struct pathspec pathspec;
char *old_index_env = NULL;
int refresh_flags = REFRESH_QUIET;
if (is_status)
refresh_flags |= REFRESH_UNMERGED;
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_FULL,
+ prefix, argv);
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- if (read_cache_preload(pathspec) < 0)
+ if (read_cache_preload(pathspec.raw) < 0)
die(_("index file corrupt"));
if (interactive) {
@@ -331,9 +331,9 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
* (A) if all goes well, commit the real index;
* (B) on failure, rollback the real index.
*/
- if (all || (also && pathspec && *pathspec)) {
+ if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, pathspec, 0);
+ add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -352,7 +352,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
* and create commit from the_index.
* We still need to refresh the index here.
*/
- if (!only && (!pathspec || !*pathspec)) {
+ if (!only && !pathspec.nr) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
@@ -397,7 +397,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
memset(&partial, 0, sizeof(partial));
partial.strdup_strings = 1;
- if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix,
pathspec))
+ if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix,
pathspec.raw))
exit(1);
discard_cache();
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/45] status: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 9 +
wt-status.c | 16 +++-
wt-status.h | 2 +-
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 0efe269..833c7be 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1197,11 +1197,12 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
handle_untracked_files_arg(&s);
if (show_ignored_in_status)
s.show_ignored_files = 1;
- if (*argv)
- s.pathspec = get_pathspec(prefix, argv);
+ parse_pathspec(&s.pathspec, 0,
+ PATHSPEC_PREFER_FULL,
+ prefix, argv);
- read_cache_preload(s.pathspec);
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec,
NULL, NULL);
+ read_cache_preload(s.pathspec.raw);
+ refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
s.pathspec.raw, NULL, NULL);
fd = hold_locked_index(&index_lock, 0);
if (0 <= fd)
diff --git a/wt-status.c b/wt-status.c
index bf84a86..99302e6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "pathspec.h"
#include "wt-status.h"
#include "object.h"
#include "dir.h"
@@ -437,7 +438,7 @@ static void wt_status_collect_changes_worktree(struct
wt_status *s)
}
rev.diffopt.format_callback = wt_status_collect_changed_cb;
rev.diffopt.format_callback_data = s;
- init_pathspec(&rev.prune_data, s->pathspec);
+ copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_files(&rev, 0);
}
@@ -462,22 +463,20 @@ static void wt_status_collect_changes_index(struct
wt_status *s)
rev.diffopt.detect_rename = 1;
rev.diffopt.rename_limit = 200;
rev.diffopt.break_opt = 0;
- init_pathspec(&rev.prune_data, s->pathspec);
+ copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_index(&rev, 1);
}
static void wt_status_collect_changes_initial(struct wt_status *s)
{
- struct pathspec pathspec;
int i;
- init_pathspec(&pathspec, s->pathspec);
for (i = 0; i < active_nr; i++) {
struct string_list_item *it;
struct wt_status_change_data *d;
struct cache_entry *ce = active_cache[i];
- if (!ce_path_match(ce, &pathspec))
+ if (!ce_path_match(ce, &s->pathspec))
continue;
it = string_list_insert(&s->change, ce->name);
d = it->util;
@@ -492,7 +491,6 @@ static void wt_status_collect_changes_initial(struct
wt_status *s)
else
d->index_status = DIFF_STATUS_ADDED;
}
- free_pathspec(&pathspec);
}
static void wt_status_collect_untracked(struct wt_status *s)
@@ -515,12 +513,12 @@ static void wt_status_collect_untracked(struct wt_status
*s)
dir.flags |= DIR_SHOW_IGNORED_TOO;
setup_standard_excludes(&dir);
- fill_directory(&dir, s->pathspec);
+ fill_directory(&dir, s->pathspec.raw);
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
if (cache_name_is_other(ent->name, ent->len) &&
- match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+ match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0,
NULL))
string_list_insert(&s->untracked, ent->name);
free(ent);
}
@@ -528,7 +526,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
for (i = 0; i < dir.ignored_nr; i++) {
struct dir_entry *ent = dir.ignored[i];
if (cache_name_is_other(ent->name, ent->len) &&
- match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+ match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0,
NULL))
string_list_insert(&s->ignored, ent->name);
free(ent);
}
diff --git a/wt-status.h b/wt-status.h
index 4121bc2..8463672 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -44,7 +44,7 @@ struct wt_status {
int is_initial;
char *branch;
const char *reference;
- const char **pathspec;
+ struct pathspec pathspec;
int verbose;
int amend;
enum commit_whence whence;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/45] rm: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/rm.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..0b38de3 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -10,6 +10,7 @@
#include "tree-walk.h"
#include "parse-options.h"
#include "submodule.h"
+#include "pathspec.h"
static const char * const builtin_rm_usage[] = {
N_("git rm [options] [--] ..."),
@@ -216,7 +217,7 @@ static struct option builtin_rm_options[] = {
int cmd_rm(int argc, const char **argv, const char *prefix)
{
int i, newfd;
- const char **pathspec;
+ struct pathspec pathspec;
char *seen;
git_config(git_default_config, NULL);
@@ -249,31 +250,30 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
}
}
- pathspec = get_pathspec(prefix, argv);
- refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
+ parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ refresh_index(&the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL);
seen = NULL;
- for (i = 0; pathspec[i] ; i++)
- /* nothing */;
- seen = xcalloc(i, 1);
+ seen = xcalloc(pathspec.nr, 1);
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
- if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0,
seen))
+ if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce),
0, seen))
continue;
ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
list.entry[list.nr].name = ce->name;
list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
}
- if (pathspec) {
- const char *match;
+ if (pathspec.nr) {
+ const char *original;
int seen_any = 0;
- for (i = 0; (match = pathspec[i]) != NULL ; i++) {
+ for (i = 0; i < pathspec.nr; i++) {
+ original = pathspec.items[i].original;
if (!seen[i]) {
if (!ignore_unmatch) {
die(_("pathspec '%s' did not match any
files"),
- match);
+ original);
}
}
else {
@@ -281,7 +281,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
}
if (!recursive && seen[i] == MATCHED_RECURSIVELY)
die(_("not removing '%s' recursively without
-r"),
- *match ? match : ".");
+ *original ? original : ".");
}
if (! seen_any)
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/45] rerere: convert to use parse_pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/rerere.c | 8 +---
rerere.c | 9 +
rerere.h | 4 +++-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/builtin/rerere.c b/builtin/rerere.c
index dc1708e..4e51add 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -6,6 +6,7 @@
#include "rerere.h"
#include "xdiff/xdiff.h"
#include "xdiff-interface.h"
+#include "pathspec.h"
static const char * const rerere_usage[] = {
N_("git rerere [clear | forget path... | status | remaining | diff |
gc]"),
@@ -68,11 +69,12 @@ int cmd_rerere(int argc, const char **argv, const char
*prefix)
return rerere(flags);
if (!strcmp(argv[0], "forget")) {
- const char **pathspec;
+ struct pathspec pathspec;
if (argc < 2)
warning("'git rerere forget' without paths is
deprecated");
- pathspec = get_pathspec(prefix, argv + 1);
- return rerere_forget(pathspec);
+ parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,
+ prefix, argv + 1);
+ return rerere_forget(&pathspec);
}
fd = setup_rerere(&merge_rr, flags);
diff --git a/rerere.c b/rerere.c
index 98e3e29..27afbfe 100644
--- a/rerere.c
+++ b/rerere.c
@@ -6,6 +6,7 @@
#include "resolve-undo.h"
#include "ll-merge.h"
#include "attr.h"
+#include "pathspec.h"
#define RESOLVED 0
#define PUNTED 1
@@ -656,7 +657,7 @@ static int rerere_forget_one_path(const char *path, struct
string_list *rr)
return 0;
}
-int rerere_forget(const char **pathspec)
+int rerere_forget(struct pathspec *pathspec)
{
int i, fd;
struct string_list conflict = STRING_LIST_INIT_DUP;
@@ -667,12 +668,12 @@ int rerere_forget(const char **pathspec)
fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
- unmerge_cache(pathspec);
+ unmerge_cache(pathspec->raw);
find_conflict(&conflict);
for (i = 0; i < conflict.nr; i++) {
struct string_list_item *it = &conflict.items[i];
- if (!match_pathspec(pathspec, it->string, strlen(it->string),
- 0, NULL))
+ if (!match_pathspec_depth(pathspec, it->string,
strlen(it->string),
+ 0, NULL))
continue;
rerere_forget_one_path(it->string, &merge_rr);
}
diff --git a/rerere.h b/rerere.h
index 156d2aa..4aa06c9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -3,6 +3,8 @@
#include "string-list.h"
+struct pathspec;
+
#define RERERE_AUTOUPDATE 01
#define RERERE_NOAUTOUPDATE 02
@@ -16,7 +18,7 @@ extern void *RERERE_RESOLVED;
extern int setup_rerere(struct string_list *, int);
extern int rerere(int);
extern const char *rerere_path(const char *hex, const char *file);
-extern int rerere_forget(const char **);
+extern int rerere_forget(struct pathspec *);
extern int rerere_remaining(struct string_list *);
extern void rerere_clear(struct string_list *);
extern void rerere_gc(struct string_list *);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/45] Convert some get_pathspec() calls to parse_pathspec()
These call sites follow the pattern:
paths = get_pathspec(prefix, argv);
init_pathspec(&pathspec, paths);
which can be converted into a single parse_pathspec() call.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/grep.c | 6 +++---
builtin/ls-tree.c | 10 +-
builtin/update-index.c | 5 +++--
revision.c | 4 ++--
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 4de49df..1a6c028 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -631,7 +631,6 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
const char *show_in_pager = NULL, *default_pager = "dummy";
struct grep_opt opt;
struct object_array list = OBJECT_ARRAY_INIT;
- const char **paths = NULL;
struct pathspec pathspec;
struct string_list path_list = STRING_LIST_INIT_NODUP;
int i;
@@ -858,8 +857,9 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
verify_filename(prefix, argv[j], j == i);
}
- paths = get_pathspec(prefix, argv + i);
- init_pathspec(&pathspec, paths);
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_CWD,
+ prefix, argv + i);
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 93fc3a0..bdb03f3 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -167,7 +167,15 @@ int cmd_ls_tree(int argc, const char **argv, const char
*prefix)
if (get_sha1(argv[0], sha1))
die("Not a valid object name %s", argv[0]);
- init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
+ /*
+* show_recursive() rolls its own matching code and is
+* generally ignorant of 'struct pathspec'. The magic mask
+* cannot be lifted until it is converted to use
+* match_pathspec_depth() or tree_entry_interesting()
+*/
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_CWD,
+ prefix, argv + 1);
for (i = 0; i < pathspec.nr; i++)
pathspec.items[i].nowildcard_len = pathspec.items[i].len;
pathspec.has_wildcard = 0;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b9c2bd0..e795818 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -547,10 +547,11 @@ static int do_reupdate(int ac, const char **av,
*/
int pos;
int has_head = 1;
- const char **paths = get_pathspec(prefix, av + 1);
struct pathspec pathspec;
- init_pathspec(&pathspec, paths);
+ parse_pathspec(&pathspec, 0,
+ PATHSPEC_PREFER_CWD,
+ prefix, av + 1);
if (read_ref("HEAD", head_sha1))
/* If there is no HEAD, that means it is an initial
diff --git a/revision.c b/revision.c
index 518cd08..f9185d4 100644
--- a/revision.c
+++ b/revision.c
@@ -1871,8 +1871,8 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
*/
ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
prune_data.path[prune_data.nr++] = NULL;
- init_pathspec(&revs->prune_data,
- get_pathspec(revs->prefix, prune_data.path));
+ parse_pathspec(&revs->prune_data, 0, 0,
+ revs->prefix, prune_data.path);
}
if (revs->def == NULL)
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/45] parse_pathspec: a special flag for max_depth feature
match_pathspec_depth() and tree_entry_interesting() check max_depth
field in order to support "git grep --max-depth". The feature
activation is tied to "recursive" field, which led to some unwanted
activation, e.g. 5c8eeb8 (diff-index: enable recursive pathspec
matching in unpack_trees - 2012-01-15).
This patch decouples the activation from "recursive" field, puts it in
"magic" field instead. This makes sure that only "git grep" can
activate this feature. And because parse_pathspec knows when the
feature is not used, it does not need to sort pathspec (required for
max_depth to work correctly). A small win for non-grep cases.
Even though a new magic flag is introduced, no magic syntax is. The
magic can be only enabled by parse_pathspec() caller. We might someday
want to support ":(maxdepth:10)src." It all depends on actual use
cases.
max_depth feature cannot be enabled via init_pathspec() anymore. But
that's ok because init_pathspec() is on its way to /dev/null.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/grep.c | 3 ++-
diff-lib.c | 1 -
dir.c | 8 ++--
pathspec.c | 8 ++--
pathspec.h | 6 +-
tree-diff.c| 1 -
tree-walk.c| 8 ++--
7 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 1a6c028..4bc0754 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -858,7 +858,8 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
}
parse_pathspec(&pathspec, 0,
- PATHSPEC_PREFER_CWD,
+ PATHSPEC_PREFER_CWD |
+ (opt.max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0),
prefix, argv + i);
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..4729157 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -474,7 +474,6 @@ static int diff_cache(struct rev_info *revs,
opts.dst_index = NULL;
opts.pathspec = &revs->diffopt.pathspec;
opts.pathspec->recursive = 1;
- opts.pathspec->max_depth = -1;
init_tree_desc(&t, tree->buffer, tree->size);
return unpack_trees(1, &t, &opts);
diff --git a/dir.c b/dir.c
index c0b284f..cfcdda5 100644
--- a/dir.c
+++ b/dir.c
@@ -341,7 +341,9 @@ int match_pathspec_depth(const struct pathspec *ps,
int i, retval = 0;
if (!ps->nr) {
- if (!ps->recursive || ps->max_depth == -1)
+ if (!ps->recursive ||
+ !(ps->magic & PATHSPEC_MAXDEPTH) ||
+ ps->max_depth == -1)
return MATCHED_RECURSIVELY;
if (within_depth(name, namelen, 0, ps->max_depth))
@@ -358,7 +360,9 @@ int match_pathspec_depth(const struct pathspec *ps,
if (seen && seen[i] == MATCHED_EXACTLY)
continue;
how = match_pathspec_item(ps->items+i, prefix, name, namelen);
- if (ps->recursive && ps->max_depth != -1 &&
+ if (ps->recursive &&
+ (ps->magic & PATHSPEC_MAXDEPTH) &&
+ ps->max_depth != -1 &&
how && how != MATCHED_FNMATCH) {
int len = ps->items[i].len;
if (name[len] == '/')
diff --git a/pathspec.c b/pathspec.c
index 9e68321..a352ef1 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -267,6 +267,9 @@ void parse_pathspec(struct pathspec *pathspec,
memset(pathspec, 0, sizeof(*pathspec));
+ if (flags & PATHSPEC_MAXDEPTH_VALID)
+ pathspec->magic |= PATHSPEC_MAXDEPTH;
+
/* No arguments, no prefix -> no pathspec */
if (!entry && !prefix)
return;
@@ -322,8 +325,9 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->magic |= item[i].magic;
}
- qsort(pathspec->items, pathspec->nr,
- sizeof(struct pathspec_item), pathspec_item_cmp);
+ if (pathspec->magic & PATHSPEC_MAXDEPTH)
+ qsort(pathspec->items, pathspec->nr,
+ sizeof(struct pathspec_item), pathspec_item_cmp);
}
/*
diff --git a/pathspec.h b/pathspec.h
index d630e8b..aa98597 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -3,7 +3,10 @@
/* Pathspec magic */
#define PATHSPEC_FROMTOP (1<<0)
-#define PATHSPEC_ALL_MAGIC PATHSPEC_FROMTOP
+#define PATHSPEC_MAXDEPTH (1<<1)
+#define PATHSPEC_ALL_MAGIC \
+ (PATHSPEC_FROMTOP | \
+PATHSPEC_MAXDEPTH)
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR
*/
@@ -27,6 +30,7 @@ struct pathspec {
/* parse_pathspec flags */
#define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
#define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
+#define PATHSPEC_MAXDEPTH_VALID (1<<2) /* max_depth field is valid */
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathsp
[PATCH 07/45] parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
We have two ways of dealing with empty pathspec:
1. limit it to current prefix
2. match the entire working directory
Some commands go with #1, some #2. get_pathspec() and parse_pathspec()
only support #1. Make parse_pathspec() reject empty pathspec by
default. #1 and #2 can be specified via new flags. This makes it more
expressive about default behavior at command level.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 13 -
pathspec.h | 4
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/pathspec.c b/pathspec.c
index 89bdc7f..9e68321 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -271,10 +271,20 @@ void parse_pathspec(struct pathspec *pathspec,
if (!entry && !prefix)
return;
+ if ((flags & PATHSPEC_PREFER_CWD) &&
+ (flags & PATHSPEC_PREFER_FULL))
+ die("BUG: PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are
incompatible");
+
/* No arguments with prefix -> prefix pathspec */
if (!entry) {
static const char *raw[2];
+ if (flags & PATHSPEC_PREFER_FULL)
+ return;
+
+ if (!(flags & PATHSPEC_PREFER_CWD))
+ die("BUG: parse_pathspec cannot take no arguments in
this case");
+
pathspec->items = item = xmalloc(sizeof(*item));
memset(item, 0, sizeof(*item));
item->match = prefix;
@@ -340,7 +350,8 @@ const char **get_pathspec(const char *prefix, const char
**pathspec)
struct pathspec ps;
parse_pathspec(&ps,
PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
- 0, prefix, pathspec);
+ PATHSPEC_PREFER_CWD,
+ prefix, pathspec);
return ps.raw;
}
diff --git a/pathspec.h b/pathspec.h
index cc5841b..d630e8b 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -24,6 +24,10 @@ struct pathspec {
} *items;
};
+/* parse_pathspec flags */
+#define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
+#define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
+
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathspec *pathspec,
unsigned magic_mask,
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/45] parse_pathspec: support stripping/checking submodule paths
PATHSPEC_SYMLINK_LEADING_PATH and _STRIP_SUBMODULE_SLASH_EXPENSIVE are
respectively the alternate implementation of
pathspec.c:die_if_path_beyond_symlink() and
pathspec.c:check_path_for_gitlink(). They are intended to replace
those functions when builtin/add.c and builtin/check-ignore.c are
converted to use parse_pathspec.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 26 ++
pathspec.h | 10 ++
2 files changed, 36 insertions(+)
diff --git a/pathspec.c b/pathspec.c
index e3c8339..9aaec36 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -214,6 +214,26 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
match[item->len] = '\0';
}
+ if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
+ for (i = 0; i < active_nr; i++) {
+ struct cache_entry *ce = active_cache[i];
+ int ce_len = ce_namelen(ce);
+
+ if (!S_ISGITLINK(ce->ce_mode))
+ continue;
+
+ if (item->len <= ce_len || match[ce_len] != '/' ||
+ memcmp(ce->name, match, ce_len))
+ continue;
+ if (item->len == ce_len + 1) {
+ /* strip trailing slash */
+ item->len--;
+ match[item->len] = '\0';
+ } else
+ die (_("Pathspec '%s' is in submodule '%.*s'"),
+elt, ce_len, ce->name);
+ }
+
if (limit_pathspec_to_literal())
item->nowildcard_len = item->len;
else
@@ -329,6 +349,12 @@ void parse_pathspec(struct pathspec *pathspec,
unsupported_magic(entry,
item[i].magic & magic_mask,
short_magic);
+
+ if ((flags & PATHSPEC_SYMLINK_LEADING_PATH) &&
+ has_symlink_leading_path(item[i].match, item[i].len)) {
+ die(_("pathspec '%s' is beyond a symbolic link"),
entry);
+ }
+
if (item[i].nowildcard_len < item[i].len)
pathspec->has_wildcard = 1;
pathspec->magic |= item[i].magic;
diff --git a/pathspec.h b/pathspec.h
index 7935b26..b631514 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -33,6 +33,16 @@ struct pathspec {
#define PATHSPEC_MAXDEPTH_VALID (1<<2) /* max_depth field is valid */
/* stripping the trailing slash if the given path is a gitlink */
#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<3)
+/* die if a symlink is part of the given path's directory */
+#define PATHSPEC_SYMLINK_LEADING_PATH (1<<4)
+/*
+ * This is like a combination of ..LEADING_PATH and .._SLASH_CHEAP
+ * (but not the same): it strips the trailing slash if the given path
+ * is a gitlink but also checks and dies if gitlink is part of the
+ * leading path (i.e. the given path goes beyond a submodule). It's
+ * safer than _SLASH_CHEAP and also more expensive.
+ */
+#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (1<<5)
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathspec *pathspec,
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/45] parse_pathspec: save original pathspec for reporting
We usually use pathspec_item's match field for pathspec error
reporting. However "match" (or "raw") does not show the magic part,
which will play more important role later on. Preserve exact user
input for reporting.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
dir.c | 1 +
pathspec.c | 2 ++
pathspec.h | 1 +
3 files changed, 4 insertions(+)
diff --git a/dir.c b/dir.c
index b8e68c2..c0b284f 100644
--- a/dir.c
+++ b/dir.c
@@ -1601,6 +1601,7 @@ int init_pathspec(struct pathspec *pathspec, const char
**paths)
const char *path = paths[i];
item->match = path;
+ item->original = path;
item->len = strlen(path);
item->flags = 0;
if (limit_pathspec_to_literal()) {
diff --git a/pathspec.c b/pathspec.c
index b49bd51..89bdc7f 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -203,6 +203,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
else
match = prefix_path(prefix, prefixlen, copyfrom);
*raw = item->match = match;
+ item->original = elt;
item->len = strlen(item->match);
if (limit_pathspec_to_literal())
item->nowildcard_len = item->len;
@@ -277,6 +278,7 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->items = item = xmalloc(sizeof(*item));
memset(item, 0, sizeof(*item));
item->match = prefix;
+ item->original = prefix;
item->nowildcard_len = item->len = strlen(prefix);
raw[0] = prefix;
raw[1] = NULL;
diff --git a/pathspec.h b/pathspec.h
index 937ec91..cc5841b 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -16,6 +16,7 @@ struct pathspec {
int max_depth;
struct pathspec_item {
const char *match;
+ const char *original;
unsigned magic;
int len;
int nowildcard_len;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/45] parse_pathspec: support stripping submodule trailing slashes
This flag is equivalent to builtin/ls-files.c:strip_trailing_slashes()
and is intended to replace that function when ls-files is converted to
use parse_pathspec.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 9 +
pathspec.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/pathspec.c b/pathspec.c
index a352ef1..e3c8339 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -205,6 +205,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
*raw = item->match = match;
item->original = elt;
item->len = strlen(item->match);
+
+ if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
+ (item->len >= 1 && item->match[item->len - 1] == '/') &&
+ (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
+ S_ISGITLINK(active_cache[i]->ce_mode)) {
+ item->len--;
+ match[item->len] = '\0';
+ }
+
if (limit_pathspec_to_literal())
item->nowildcard_len = item->len;
else
diff --git a/pathspec.h b/pathspec.h
index aa98597..7935b26 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -31,6 +31,8 @@ struct pathspec {
#define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */
#define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */
#define PATHSPEC_MAXDEPTH_VALID (1<<2) /* max_depth field is valid */
+/* stripping the trailing slash if the given path is a gitlink */
+#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<3)
extern int init_pathspec(struct pathspec *, const char **);
extern void parse_pathspec(struct pathspec *pathspec,
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/45] Add parse_pathspec() that converts cmdline args to struct pathspec
Currently to fill a struct pathspec, we do:
const char **paths;
paths = get_pathspec(prefix, argv);
...
init_pathspec(&pathspec, paths);
"paths" can only carry bare strings, which loses information from
command line arguments such as pathspec magic or the prefix part's
length for each argument.
parse_pathspec() is introduced to combine the two calls into one. The
plan is gradually replace all get_pathspec() and init_pathspec() with
parse_pathspec(). get_pathspec() now becomes a thin wrapper of
parse_pathspec().
parse_pathspec() allows the caller to reject the pathspec magics that
it does not support. When a new pathspec magic is introduced, we can
enable it per command after making sure that all underlying code has no
problem with the new magic.
"flags" parameter is currently unused. But it would allow callers to
pass certain instructions to parse_pathspec, for example forcing
literal pathspec when no magic is used.
With the introduction of parse_pathspec, there are now two functions
that can initialize struct pathspec: init_pathspec and
parse_pathspec. Any semantic changes in struct pathspec must be
reflected in both functions. init_pathspec() will be phased out in
favor of parse_pathspec().
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/technical/api-setup.txt | 19 +++-
dir.c | 4 +-
dir.h | 2 +
pathspec.c| 168 ++
pathspec.h| 11 +++
5 files changed, 163 insertions(+), 41 deletions(-)
diff --git a/Documentation/technical/api-setup.txt
b/Documentation/technical/api-setup.txt
index 4f63a04..90d1aff 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt
@@ -8,6 +8,23 @@ Talk about
* is_inside_git_dir()
* is_inside_work_tree()
* setup_work_tree()
-* get_pathspec()
(Dscho)
+
+Pathspec
+
+
+See glossary-context.txt for the syntax of pathspec. In memory, a
+pathspec set is represented by "struct pathspec" and is prepared by
+parse_pathspec(). This function takes several arguments:
+
+- magic_mask specifies what features that are NOT supported by the
+ following code. If a user attempts to use such a feature,
+ parse_pathspec() can reject it early.
+
+- flags specifies other things that the caller wants parse_pathspec to
+ perform.
+
+- prefix and args come from cmd_* functions
+
+get_pathspec() is obsolete and should never be used in new code.
diff --git a/dir.c b/dir.c
index 9291c9a..b8e68c2 100644
--- a/dir.c
+++ b/dir.c
@@ -381,7 +381,7 @@ int match_pathspec_depth(const struct pathspec *ps,
/*
* Return the length of the "simple" part of a path match limiter.
*/
-static int simple_length(const char *match)
+int simple_length(const char *match)
{
int len = -1;
@@ -393,7 +393,7 @@ static int simple_length(const char *match)
}
}
-static int no_wildcard(const char *string)
+int no_wildcard(const char *string)
{
return string[simple_length(string)] == '\0';
}
diff --git a/dir.h b/dir.h
index 3d6b80c..229ccc8 100644
--- a/dir.h
+++ b/dir.h
@@ -128,6 +128,8 @@ struct dir_struct {
#define MATCHED_RECURSIVELY 1
#define MATCHED_FNMATCH 2
#define MATCHED_EXACTLY 3
+extern int simple_length(const char *match);
+extern int no_wildcard(const char *string);
extern char *common_prefix(const char **pathspec);
extern int match_pathspec(const char **pathspec, const char *name, int
namelen, int prefix, char *seen);
extern int match_pathspec_depth(const struct pathspec *pathspec,
diff --git a/pathspec.c b/pathspec.c
index 8fe56cd..b49bd51 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -103,10 +103,6 @@ void die_if_path_beyond_symlink(const char *path, const
char *prefix)
/*
* Magic pathspec
*
- * NEEDSWORK: These need to be moved to dir.h or even to a new
- * pathspec.h when we restructure get_pathspec() users to use the
- * "struct pathspec" interface.
- *
* Possible future magic semantics include stuff like:
*
* { PATHSPEC_NOGLOB, '!', "noglob" },
@@ -115,7 +111,6 @@ void die_if_path_beyond_symlink(const char *path, const
char *prefix)
* { PATHSPEC_REGEXP, '\0', "regexp" },
*
*/
-#define PATHSPEC_FROMTOP(1<<0)
static struct pathspec_magic {
unsigned bit;
@@ -127,7 +122,7 @@ static struct pathspec_magic {
/*
* Take an element of a pathspec and check for magic signatures.
- * Append the result to the prefix.
+ * Append the result to the prefix. Return the magic bitmap.
*
* For now, we only parse the syntax and throw out anything other than
* "top" magic.
@@ -138,10 +133,15 @@ static struct pathspec_magic {
* the prefix part must always match literally, and a single stupid
* string cannot express such a case.
*/
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const
char *elt)
+static unsigned prefix_pathspec(struct pathspec_item *item,
+
[PATCH 01/45] clean: remove unused variable "seen"
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/clean.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e396b..f955a40 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -155,7 +155,6 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct exclude_list *el;
const char *qname;
- char *seen = NULL;
struct option options[] = {
OPT__QUIET(&quiet, N_("do not print names of files removed")),
OPT__DRY_RUN(&dry_run, N_("dry run")),
@@ -214,9 +213,6 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
fill_directory(&dir, pathspec);
- if (pathspec)
- seen = xmalloc(argc > 0 ? argc : 1);
-
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
int len, pos;
@@ -250,11 +246,9 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (lstat(ent->name, &st))
continue;
- if (pathspec) {
- memset(seen, 0, argc > 0 ? argc : 1);
+ if (pathspec)
matches = match_pathspec(pathspec, ent->name, len,
-0, seen);
- }
+0, NULL);
if (S_ISDIR(st.st_mode)) {
strbuf_addstr(&directory, ent->name);
@@ -281,7 +275,6 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
}
}
}
- free(seen);
strbuf_release(&directory);
string_list_clear(&exclude_list, 0);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/45] pathspec: add copy_pathspec
The function is made to use with free_pathspec() because a simple
struct assignment is not enough (free_pathspec wants to free "items"
pointer).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/mv.c | 13 +++--
pathspec.c | 8
pathspec.h | 1 +
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index 034fec9..16ce99b 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -15,8 +15,9 @@ static const char * const builtin_mv_usage[] = {
NULL
};
-static const char **copy_pathspec(const char *prefix, const char **pathspec,
- int count, int base_name)
+static const char **internal_copy_pathspec(const char *prefix,
+ const char **pathspec,
+ int count, int base_name)
{
int i;
const char **result = xmalloc((count + 1) * sizeof(const char *));
@@ -81,17 +82,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (read_cache() < 0)
die(_("index file corrupt"));
- source = copy_pathspec(prefix, argv, argc, 0);
+ source = internal_copy_pathspec(prefix, argv, argc, 0);
modes = xcalloc(argc, sizeof(enum update_mode));
- dest_path = copy_pathspec(prefix, argv + argc, 1, 0);
+ dest_path = internal_copy_pathspec(prefix, argv + argc, 1, 0);
if (dest_path[0][0] == '\0')
/* special case: "." was normalized to "" */
- destination = copy_pathspec(dest_path[0], argv, argc, 1);
+ destination = internal_copy_pathspec(dest_path[0], argv, argc,
1);
else if (!lstat(dest_path[0], &st) &&
S_ISDIR(st.st_mode)) {
dest_path[0] = add_slash(dest_path[0]);
- destination = copy_pathspec(dest_path[0], argv, argc, 1);
+ destination = internal_copy_pathspec(dest_path[0], argv, argc,
1);
} else {
if (argc != 1)
die("destination '%s' is not a directory",
dest_path[0]);
diff --git a/pathspec.c b/pathspec.c
index 403095b..8fe56cd 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -249,3 +249,11 @@ const char **get_pathspec(const char *prefix, const char
**pathspec)
return NULL;
return pathspec;
}
+
+void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
+{
+ *dst = *src;
+ dst->items = xmalloc(sizeof(struct pathspec_item) * dst->nr);
+ memcpy(dst->items, src->items,
+ sizeof(struct pathspec_item) * dst->nr);
+}
diff --git a/pathspec.h b/pathspec.h
index 7884068..a621676 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -18,6 +18,7 @@ struct pathspec {
};
extern int init_pathspec(struct pathspec *, const char **);
+extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
extern void free_pathspec(struct pathspec *);
extern int limit_pathspec_to_literal(void);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/45] pathspec: i18n-ize error strings in pathspec parsing code
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pathspec.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 133f8be..403095b 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -167,11 +167,11 @@ static const char *prefix_pathspec(const char *prefix,
int prefixlen, const char
break;
}
if (ARRAY_SIZE(pathspec_magic) <= i)
- die("Invalid pathspec magic '%.*s' in '%s'",
+ die(_("Invalid pathspec magic '%.*s' in '%s'"),
(int) len, copyfrom, elt);
}
if (*copyfrom != ')')
- die("Missing ')' at the end of pathspec magic in '%s'",
elt);
+ die(_("Missing ')' at the end of pathspec magic in
'%s'"), elt);
copyfrom++;
} else {
/* shorthand */
@@ -188,7 +188,7 @@ static const char *prefix_pathspec(const char *prefix, int
prefixlen, const char
break;
}
if (ARRAY_SIZE(pathspec_magic) <= i)
- die("Unimplemented pathspec magic '%c' in '%s'",
+ die(_("Unimplemented pathspec magic '%c' in
'%s'"),
ch, elt);
}
if (*copyfrom == ':')
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/45] Move struct pathspec and related functions to pathspec.[ch]
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive.c | 1 +
builtin/grep.c | 1 +
builtin/ls-files.c | 1 +
builtin/ls-tree.c | 1 +
builtin/update-index.c | 1 +
cache.h| 22 +---
diff.h | 1 +
dir.c | 1 +
pathspec.c | 150 +
pathspec.h | 21 +++
preload-index.c| 1 +
setup.c| 149
tree-walk.c| 1 +
13 files changed, 182 insertions(+), 169 deletions(-)
diff --git a/archive.c b/archive.c
index d254fa5..c699a2d 100644
--- a/archive.c
+++ b/archive.c
@@ -5,6 +5,7 @@
#include "archive.h"
#include "parse-options.h"
#include "unpack-trees.h"
+#include "pathspec.h"
static char const * const archive_usage[] = {
N_("git archive [options] [...]"),
diff --git a/builtin/grep.c b/builtin/grep.c
index 159e65d..4de49df 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -17,6 +17,7 @@
#include "grep.h"
#include "quote.h"
#include "dir.h"
+#include "pathspec.h"
static char const * const grep_usage[] = {
N_("git grep [options] [-e] [...] [[--] ...]"),
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2202072..a0b7e30 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -13,6 +13,7 @@
#include "parse-options.h"
#include "resolve-undo.h"
#include "string-list.h"
+#include "pathspec.h"
static int abbrev;
static int show_deleted;
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fb76e38..93fc3a0 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -10,6 +10,7 @@
#include "quote.h"
#include "builtin.h"
#include "parse-options.h"
+#include "pathspec.h"
static int line_termination = '\n';
#define LS_RECURSIVE 1
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 5c7762e..b9c2bd0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -11,6 +11,7 @@
#include "refs.h"
#include "resolve-undo.h"
#include "parse-options.h"
+#include "pathspec.h"
/*
* Default to not allowing changes to the list of files. The
diff --git a/cache.h b/cache.h
index df532f8..5917375 100644
--- a/cache.h
+++ b/cache.h
@@ -185,6 +185,8 @@ struct cache_entry {
#error "CE_EXTENDED_FLAGS out of range"
#endif
+struct pathspec;
+
/*
* Copy the sha1 and stat state of a cache entry from one to
* another. But we never change the name, or the hash state!
@@ -483,28 +485,8 @@ extern void *read_blob_data_from_index(struct index_state
*, const char *, unsig
extern int ie_match_stat(const struct index_state *, struct cache_entry *,
struct stat *, unsigned int);
extern int ie_modified(const struct index_state *, struct cache_entry *,
struct stat *, unsigned int);
-#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR
*/
-
-struct pathspec {
- const char **raw; /* get_pathspec() result, not freed by
free_pathspec() */
- int nr;
- unsigned int has_wildcard:1;
- unsigned int recursive:1;
- int max_depth;
- struct pathspec_item {
- const char *match;
- int len;
- int nowildcard_len;
- int flags;
- } *items;
-};
-
-extern int init_pathspec(struct pathspec *, const char **);
-extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec
*pathspec);
-extern int limit_pathspec_to_literal(void);
-
#define HASH_WRITE_OBJECT 1
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum
object_type type, const char *path, unsigned flags);
diff --git a/diff.h b/diff.h
index 78b4091..d1bc914 100644
--- a/diff.h
+++ b/diff.h
@@ -5,6 +5,7 @@
#define DIFF_H
#include "tree-walk.h"
+#include "pathspec.h"
struct rev_info;
struct diff_options;
diff --git a/dir.c b/dir.c
index 897c874..9291c9a 100644
--- a/dir.c
+++ b/dir.c
@@ -11,6 +11,7 @@
#include "dir.h"
#include "refs.h"
#include "wildmatch.h"
+#include "pathspec.h"
struct path_simplify {
int len;
diff --git a/pathspec.c b/pathspec.c
index 284f397..133f8be 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -99,3 +99,153 @@ void die_if_path_beyond_symlink(const char *path, const
char *prefix)
die(_("'%s' is beyond a symbolic link"), path + len);
}
}
+
+/*
+ * Magic pathspec
+ *
+ * NEEDSWORK: These need to be moved to dir.h or even to a new
+ * pathspec.h when we restructure get_pathspec() users to use the
+ * "struct pathspec" interface.
+ *
+ * Possible future magic semantics include stuff like:
+ *
+ * { PATHSPEC_NOGLOB, '!', "noglob" },
+ * { PATHSPEC_ICASE, '\0', "icase" },
+ * { PATHSPEC_RECURSIVE, '*', "recursive" },
+ * { PATHSPEC_REGEXP, '\0', "regexp" },
+ *
+ */
+#define PATHSPEC_FROMTOP(1<<0)
+
+static struct pathspec_magic {
+ unsigned bit;
+ ch
[PATCH 00/45] "struct pathspec" conversion and :(glob)
This series finishes off "struct pathspec" conversion that was started
2 years ago when this struct was added. In the end we can pass more
information down the callchain than simply a series of strings. This
makes it possible to add more pathspec "magic", the :(glob) in the end
of this series is an example. This also changes --literal-pathspecs
slightly, to not just make the pathspec literal, but to disable all
magic.
Next step: introduction of :(icase) magic for case-insensitive
matching (definitely won't take another 2 years as the groundwork is
pretty much done).
Nguyễn Thái Ngọc Duy (45):
clean: remove unused variable "seen"
Move struct pathspec and related functions to pathspec.[ch]
pathspec: i18n-ize error strings in pathspec parsing code
pathspec: add copy_pathspec
Add parse_pathspec() that converts cmdline args to struct pathspec
parse_pathspec: save original pathspec for reporting
parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
Convert some get_pathspec() calls to parse_pathspec()
parse_pathspec: a special flag for max_depth feature
parse_pathspec: support stripping submodule trailing slashes
parse_pathspec: support stripping/checking submodule paths
parse_pathspec: support prefixing original patterns
Guard against new pathspec magic in pathspec matching code
clean: convert to use parse_pathspec
commit: convert to use parse_pathspec
status: convert to use parse_pathspec
rerere: convert to use parse_pathspec
checkout: convert to use parse_pathspec
rm: convert to use parse_pathspec
ls-files: convert to use parse_pathspec
archive: convert to use parse_pathspec
check-ignore: convert to use parse_pathspec
add: convert to use parse_pathspec
reset: convert to use parse_pathspec
line-log: convert to use parse_pathspec
Convert read_cache_preload() to take struct pathspec
Convert run_add_interactive to use struct pathspec
Convert unmerge_cache to take struct pathspec
checkout: convert read_tree_some to take struct pathspec
Convert report_path_error to take struct pathspec
Convert refresh_index to take struct pathspec
Convert {read,fill}_directory to take struct pathspec
Convert add_files_to_cache to take struct pathspec
Convert common_prefix() to use struct pathspec
Remove diff_tree_{setup,release}_paths
Remove init_pathspec() in favor of parse_pathspec()
Remove match_pathspec() in favor of match_pathspec_depth()
tree-diff: remove the use of pathspec's raw[] in follow-rename codepath
parse_pathspec: make sure the prefix part is wildcard-free
parse_pathspec: preserve prefix length via PATHSPEC_PREFIX_ORIGIN
Kill limit_pathspec_to_literal() as it's only used by parse_pathspec()
pathspec: support :(literal) syntax for noglob pathspec
pathspec: make --literal-pathspecs disable pathspec magic
pathspec: support :(glob) syntax
Rename field "raw" to "_raw" in struct pathspec
Documentation/git.txt | 23 +-
Documentation/glossary-content.txt| 49 +++-
Documentation/technical/api-setup.txt | 38 ++-
archive.c | 18 +-
archive.h | 4 +-
builtin/add.c | 164 ++---
builtin/blame.c | 14 +-
builtin/check-ignore.c| 35 ++-
builtin/checkout.c| 40 +--
builtin/clean.c | 24 +-
builtin/commit.c | 37 ++-
builtin/diff-files.c | 2 +-
builtin/diff-index.c | 2 +-
builtin/diff.c| 6 +-
builtin/grep.c| 10 +-
builtin/log.c | 2 +-
builtin/ls-files.c| 75 +++---
builtin/ls-tree.c | 13 +-
builtin/mv.c | 13 +-
builtin/rerere.c | 8 +-
builtin/reset.c | 33 ++-
builtin/rm.c | 24 +-
builtin/update-index.c| 6 +-
cache.h | 34 +--
commit.h | 2 +-
diff-lib.c| 3 +-
diff.h| 3 +-
dir.c | 261 +---
dir.h | 18 +-
git.c | 8 +
line-log.c| 2 +-
merge-recursive.c | 2 +-
notes-merge.c | 4 +-
path.c| 15 +-
pathspec.c| 442 +++---
pathspec.h| 68 +-
preload-index.c | 21 +-
read-cache.c | 5 +-
rerere.c | 7 +-
rerere.h | 4 +-
resolve-undo.c| 4 +-
resolve-undo.h| 2 +
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On Sat, Jun 08, 2013 at 09:00:30PM +0200, Matthieu Moy wrote:
> > + # Auto-loading in browser
> > + if ($autoload) {
> > + open(my $browser, "-|:encoding(UTF-8)", "xdg-open
> > ".$preview_file_name);
>
> That could be read from Git's configuration, and default to xdg-open.
> But you don't want to hardcode it in the middle of the code.
In fact, I think we provide the "git-web--browse" tool for just this
purpose. It takes care of the trickiness of looking at the "web.browser"
config, resolving custom browser tools defined by "browser..*",
and handling backgrounding of the browser (which you want to do for
graphical browsers but not for terminal ones).
-Peff
PS I agreed with all of the other comments in your review. :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On Fri, Jun 07, 2013 at 11:50:31PM +0200, benoit.per...@ensimag.fr wrote:
> The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
> preview content without pushing would be a nice thing to have.
Sounds like a useful goal.
> The default behaviour for the `preview` subcommand is:
> 1- Find the remote name of the current branch's upstream and check if it's a
> wiki one with its url (ie: mediawiki://)
> 2- Parse the content of the local file (given as argument) using the distant
> wiki's API.
Makes sense.
> 3- Retrieve the current page on the distant mediawiki.
> 4- Merge those those contents.
I'm not sure what these steps are for. You are trying to preview not
just your local version, but pulling in any changes that have happened
upstream since the work you built on top of?
It seems like that is a separate, orthogonal issue, and git would be the
right place to do that merge. IOW, as a user, your workflow would be
something like:
1. git fetch, pulling down the latest copy from the server
2. make your changes on top
3. use this command to preview your changes
4. use git fetch to check whether anything else happened on the
server.
a. If not, go ahead and push.
b. If upstream changed, use "git diff" to inspect the changes to
the wiki source. Merge or rebase your changes with respect to
what's on the server. Goto step 3 to make sure they look good.
I also wonder if it would be useful to be able to specify not only files
in the filesystem, but also arbitrary blobs. So in 4b above, you could
"git mw preview origin:page.mw" to see the rendered version of what
upstream has done.
> It works but a couple of points trouble me:
> 1- I had to copy two functions from `git-remote-mediawiki.perl`, I don't
> really know how we could "factorize" those things ? I don't think it
> makes
> much sense to create a package just for them ?
You could make a Git::MediaWiki.pm module, but installing that would
significantly complicate the build procedure, and potentially be
annoying for users. One trick I have done in the past is to concatenate
bits of perl script together in the Makefile, like this:
foo: common.pl foo.pl
{ \
echo '$(PERL_PATH_SQ)' && \
for i in $^; do \
echo "#line 1 $src" && \
cat $src \
done \
} >$@+
mv $@+ $@
That would conflict a bit with the way we chain to git's Makefile,
though. I suspect you could do something complicated like build "foo.pl"
from "common.pl" and "foo-main.pl", then chain to git's Makefile to
build "foo" from "foo.pl".
> 2- The current behavior is to crash if the current branch do not have an
> upstream branch on a valid mediawiki remote. To find that specific
> remote,
> it runs `git rev-parse --symbolic-full-name @{upstream}` which will
> return
> something like `/refs/remotes/$remote_name/master`.
> 2a- maybe there is a better way to find that remote name ?
If you just care about the remote name and not the name of the local
branch, you can just ask for
my $curr_branch = `git symbolic-ref HEAD`;
my $remote = `git config "branch.$curr_branch.remote"`;
my $url = `git config "remote.$remote.url"`;
Of course you would want some error checks and probably some chomp()s in
there, too.
> 2b- would it be useful to add a fallback if that search fails ? searching
>for a valid mediawiki remote url in all the remotes returned by
>`git remote` for instance ?
That is probably OK as long as there is only one such remote, and it
would help the case where you have branched off of a local branch (so
your upstream remote is "."). If there are two mediawiki remotes,
though, it would make sense to simply fail, as you don't know which to
use. But I'd expect the common case by far to be that you simply have
one such remote.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git.txt: document GIT_TRACE_PACKET
On Sun, Jun 09, 2013 at 12:53:30PM +0700, Nguyen Thai Ngoc Duy wrote: > "This can help with debugging object negotiation or other protocol > issues." > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > >> +'GIT_TRACE_PACKET':: > >> + If this variable is set, it shows a trace of all packets > >> + coming in or out of a given program. This can help with > >> + debugging object negotiation or other protocol issues. > > > > This is not quite true. It stops showing packets after it sees a packet > > starting with "PACK" (optionally with a sideband prefix). So you would > > miss, for example, a sideband error that came after the pack had > > started. So it is really only useful for looking at the ref and object > > negotiation phases. > > I blindly copied the first paragraph from bbc30f9 (add packet tracing > debug code - 2011-02-24) and missed the "PACK" bit in the second one. > How about this? Hmm, what do you know, I sort-of documented it already. I think I just never guessed that anybody but people reading "git log" would actually need to use TRACE_PACKET. :) Thanks, the patch looks fine to me. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git.txt: document GIT_TRACE_PACKET
"This can help with debugging object negotiation or other protocol issues." Signed-off-by: Nguyễn Thái Ngọc Duy --- >> +'GIT_TRACE_PACKET':: >> + If this variable is set, it shows a trace of all packets >> + coming in or out of a given program. This can help with >> + debugging object negotiation or other protocol issues. > > This is not quite true. It stops showing packets after it sees a packet > starting with "PACK" (optionally with a sideband prefix). So you would > miss, for example, a sideband error that came after the pack had > started. So it is really only useful for looking at the ref and object > negotiation phases. I blindly copied the first paragraph from bbc30f9 (add packet tracing debug code - 2011-02-24) and missed the "PACK" bit in the second one. How about this? Documentation/git.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index c760918..c10b647 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -845,6 +845,12 @@ for further details. recorded. This may be helpful for troubleshooting some pack-related performance problems. +'GIT_TRACE_PACKET':: + If this variable is set, it shows a trace of all packets + coming in or out of a given program. This can help with + debugging object negotiation or other protocol issues. Tracing + is turned off at a packet starting with "PACK". + GIT_LITERAL_PATHSPECS:: Setting this variable to `1` will cause Git to treat all pathspecs literally, rather than as glob patterns. For example, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git.txt: document GIT_TRACE_PACKET
On Sun, Jun 09, 2013 at 12:22:49PM +0700, Nguyen Thai Ngoc Duy wrote: > diff --git a/Documentation/git.txt b/Documentation/git.txt > index c760918..72e9045 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -845,6 +845,11 @@ for further details. > recorded. This may be helpful for troubleshooting some > pack-related performance problems. > > +'GIT_TRACE_PACKET':: > + If this variable is set, it shows a trace of all packets > + coming in or out of a given program. This can help with > + debugging object negotiation or other protocol issues. This is not quite true. It stops showing packets after it sees a packet starting with "PACK" (optionally with a sideband prefix). So you would miss, for example, a sideband error that came after the pack had started. So it is really only useful for looking at the ref and object negotiation phases. I know that probably sounds a bit nit-picky, but it might be worth making the distinction in case somebody is trying to track down such messages. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 08, 2013 at 09:17:56PM -0500, Felipe Contreras wrote: > > Definitely, yes. Even if you look at the impact on code alone and > > don't care about the people, destroying a collegial work environment > > is harmful enough to the code to outweigh the (admittedly often > > useful) patches. > > A collegial work environment is overrated, and proof of that the Linux > kernel, where honest and straight talk is the bread and butter of the > mailing list. And the Linux kernel is the most successful software > project in history by far. It's code that speaks. Sorry, but I don't agree, and I want to publicly state my opinion so that Jonathan (and other bystanders on the list) knows that he is not alone in his opinions. I have consistently found your demeanor on the list to be very unfriendly and difficult to work with. It is one thing to have honest and straight talk, and another thing to be obstinate, unmindful of feedback (both with respect to technical details, as well as to communication styles), and disrespectful of other people. You have accused others of assuming you make comments in bad faith. Perhaps it is true that you are very pleasant and easy to work with in person, but in my opinion that is not the case, at least by email. I may be wrong, of course, and I certainly do not claim to be perfect myself. But I find it telling that many of the list participants seem to have had conflicts with you, and not with anyone else. So perhaps you may want to reconsider your style of communication. Unlike Jonathan, I would not ban you from the list. I do not believe in censoring anybody who is not a direct and constant nuisance (like a spammer). But personally I have a limited capacity for discussion with you, as it seems to have a knack for going back and forth, consuming a lot of time, and ending nowhere productive. It is certainly your choice about how you will communicate. But likewise it is the choice of readers and reviewers to choose how much of their time to give to your writings. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] core: use env variable instead of config var to turn on logging pack access
5f44324 (core: log offset pack data accesses happened - 2011-07-06)
provides a way to observe pack access patterns via a config
switch. Setting an environment variable looks more obvious than a
config var, especially when you just need to _observe_, and more
inline with other tracing knobs we have.
Document it as it may be useful for remote troubleshooting.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git.txt | 7 +++
cache.h | 3 ---
config.c | 3 ---
environment.c | 1 -
sha1_file.c | 14 --
5 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 68f1ee6..c760918 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -838,6 +838,13 @@ for further details.
as a file path and will try to write the trace messages
into it.
+'GIT_TRACE_PACK_ACCESS'::
+ If this variable is set to a path, a file will be created at
+ the given path logging all accesses to any packs. For each
+ access, the pack file name and an offset in the pack is
+ recorded. This may be helpful for troubleshooting some
+ pack-related performance problems.
+
GIT_LITERAL_PATHSPECS::
Setting this variable to `1` will cause Git to treat all
pathspecs literally, rather than as glob patterns. For example,
diff --git a/cache.h b/cache.h
index df532f8..4f41606 100644
--- a/cache.h
+++ b/cache.h
@@ -772,9 +772,6 @@ extern int parse_sha1_header(const char *hdr, unsigned long
*sizep);
/* global flag to enable extra checks when accessing packed objects */
extern int do_check_packed_object_crc;
-/* for development: log offset of pack access */
-extern const char *log_pack_access;
-
extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned
long size, const char *type);
extern int move_temp_to_file(const char *tmpfile, const char *filename);
diff --git a/config.c b/config.c
index 7a85ebd..d04e815 100644
--- a/config.c
+++ b/config.c
@@ -688,9 +688,6 @@ static int git_default_core_config(const char *var, const
char *value)
return 0;
}
- if (!strcmp(var, "core.logpackaccess"))
- return git_config_string(&log_pack_access, var, value);
-
if (!strcmp(var, "core.autocrlf")) {
if (value && !strcasecmp(value, "input")) {
if (core_eol == EOL_CRLF)
diff --git a/environment.c b/environment.c
index e2e75c1..0cb67b2 100644
--- a/environment.c
+++ b/environment.c
@@ -37,7 +37,6 @@ size_t packed_git_window_size =
DEFAULT_PACKED_GIT_WINDOW_SIZE;
size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 16 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
-const char *log_pack_access;
const char *pager_program;
int pager_use_color = 1;
const char *editor_program;
diff --git a/sha1_file.c b/sha1_file.c
index b114cc9..4b23bb8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -36,6 +36,9 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
const unsigned char null_sha1[20];
+static const char *no_log_pack_access = "no_log_pack_access";
+static const char *log_pack_access;
+
/*
* This is meant to hold a *small* number of objects that you would
* want read_sha1_file() to be able to return, but yet you do not want
@@ -1956,12 +1959,19 @@ static void write_pack_access_log(struct packed_git *p,
off_t obj_offset)
{
static FILE *log_file;
+ if (!log_pack_access)
+ log_pack_access = getenv("GIT_TRACE_PACK_ACCESS");
+ if (!log_pack_access)
+ log_pack_access = no_log_pack_access;
+ if (log_pack_access == no_log_pack_access)
+ return;
+
if (!log_file) {
log_file = fopen(log_pack_access, "w");
if (!log_file) {
error("cannot open pack access log '%s' for writing:
%s",
log_pack_access, strerror(errno));
- log_pack_access = NULL;
+ log_pack_access = no_log_pack_access;
return;
}
}
@@ -1992,7 +2002,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;
int base_from_cache = 0;
- if (log_pack_access)
+ if (log_pack_access != no_log_pack_access)
write_pack_access_log(p, obj_offset);
/* PHASE 1: drill down to the innermost base object */
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp
On Sat, Jun 8, 2013 at 10:57 PM, Jeff King wrote: > On Sat, Jun 08, 2013 at 08:38:56PM +0200, Matthieu Moy wrote: > >> Célestin Matte writes: >> >> > In Perl, '\n' is not a newline, but instead a literal backslash followed >> > by an >> > "n". As the output of "rev-list --first-parent" is line-oriented, what we >> > want >> > here is a newline. >> >> This is right, but the code actually worked the way it was. I'm not >> sure, but my understanding is that '\n' is the string "backslash >> followed by n", but interpreted as a regexp, it is a newline. > > Yes, the relevant doc (from "perldoc -f split") is: > > The pattern "/PATTERN/" may be replaced with an expression to specify > patterns that vary at runtime. (To do runtime compilation only once, > use "/$variable/o".) > > So it is treating "\n" as an expression and compiling the regex each > time through ... I read this as saying only that static /PATTERN/ can also be a composed /$PATTERN/. It does not indicate how string 'PATTERN' is treated, nor does any other part of "perldoc -f split" make special mention of string 'PATTERN'. In fact, the only explanation I found regarding string 'PATTERN' is in my Camel book (3rd edition, page 796) in a parenthesized comment: (... if you supply a string instead of a regular expression, it'll be interpreted as a regular expression anyway.) >> The new code looks better than the old one, but the log message may be >> improved. > > Agreed. I think the best explanation is something like: > > Perl's split function takes a regex pattern argument. You can also > feed it an expression, which is then compiled into a regex at runtime. > It therefore works to pass your pattern via single quotes, but it is > much less obvious to a reader that the argument is meant to be a > regex, not a static string. Using the traditional slash-delimiters > makes this easier to read. Sounds good to me. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git.txt: document GIT_TRACE_PACKET
"This can help with debugging object negotiation or other protocol issues." Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index c760918..72e9045 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -845,6 +845,11 @@ for further details. recorded. This may be helpful for troubleshooting some pack-related performance problems. +'GIT_TRACE_PACKET':: + If this variable is set, it shows a trace of all packets + coming in or out of a given program. This can help with + debugging object negotiation or other protocol issues. + GIT_LITERAL_PATHSPECS:: Setting this variable to `1` will cause Git to treat all pathspecs literally, rather than as glob patterns. For example, -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp
On Sat, Jun 8, 2013 at 9:35 AM, Célestin Matte
wrote:
> In Perl, '\n' is not a newline, but instead a literal backslash followed by an
> "n". As the output of "rev-list --first-parent" is line-oriented, what we want
> here is a newline.
>
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index 7af202f..a06bc31 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -1190,7 +1190,7 @@ sub mw_push_revision {
> # history (linearized with --first-parent)
> print STDERR "Warning: no common ancestor, pushing complete
> history\n";
> my $history = run_git("rev-list --first-parent --children
> $local");
> - my @history = split('\n', $history);
> + my @history = split(/\n/, $history);
> @history = @history[1..$#history];
> foreach my $line (reverse @history) {
> my @commit_info_split = split(/ |\n/, $line);
It would be quite acceptable to include this patch in your existing
patch series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
On Sat, Jun 8, 2013 at 2:45 PM, Célestin Matte wrote: > Le 08/06/2013 20:41, Matthieu Moy a écrit : >> Célestin Matte writes: >> >>> Le 08/06/2013 02:14, Eric Sunshine a écrit : These two changes are unrelated and could be split into distinct patches (IMHO, though others may disagree). >>> >>> Two distinct patches or two distinct commits? >> >> That's the same. You write commits locally, send them as patches, and >> Junio uses "git am" to turn the patches back into commits. >> > > Oh, I thought a part of a patch was called a commit. So the question was > rather: "Should it be sent to this list independently from this series > of patches?". Terms "patch" and "commit" tend to be used interchangeably, as Matthieu notes. There is no need to send the split up patches to the list independently from this series. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 16/22] git-remote-mediawiki: Modify strings for a better coding-style
On Sat, Jun 8, 2013 at 4:32 PM, Célestin Matte
wrote:
> Le 08/06/2013 02:39, Eric Sunshine a écrit :
>> On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
>> wrote:
>>> - strings which don't need interpolation are single-quoted for more clarity
>>> and
>>> slight gain of performance
>>> - interpolation is preferred over concatenation in many cases, for more
>>> clarity
>>> - variables are always used with the ${} operator inside strings
>>> - strings including double-quotes are written with qq() so that the quotes
>>> do
>>> not have to be escaped
>>
>> Distinct changes could (IMHO) be split into separate patches for easier
>> review.
>
> This commit is a real pain to cut into 3 distinctive ones. Is this
> really necessary?
> I will do it if it is, of course.
[I think you meant that it would be split into 4 patches.]
The final decision is up to the submitter (you) and the people signing
off on and accepting the patch (Matthieu and Junio).
Speaking merely as a person reviewing the patch series, I can say that
mixing conceptually unrelated changes into a single patch makes review
more onerous since it requires repeatedly switching mental gears
(often from line to line or even within a single line). Patches
involving simple "mechanical" changes typically are easy to review,
even when the patches are lengthy. In this case, however, that length
coupled with the mental gear switching, makes the review process more
burdensome that it need be.
Even if you decide ultimately not to bother splitting the patch,
ease-of-review and one-conceptual-change-per-patch are useful notions
to keep in mind for future submissions.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 08, 2013 at 09:20:54AM -0500, Felipe Contreras wrote: > Let the code speak. Show me a script in any language that does > something useful using libgit2, doing the equivalent to at least a > couple of 'git foo' commands. Sorry that I cannot show you the source code, but you may interested to know that libgit2 powers: 1. Microsoft's "Visual Studio Tools for Git" plugin 2. GitHub's native Mac and Windows clients (using Objective C and C# bindings); some operations still shell out to git where the functionality is not yet implemented in libgit2. 3. Parts of the web view of GitHub.com via Ruby bindings It is definitely not feature-complete when compared with git.git. But I do think it is in a state that is usable for quite a few tasks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 10:21 PM, Jonathan Nieder wrote: > Felipe Contreras wrote: > >> A collegial work environment is overrated, and proof of that the Linux >> kernel, where honest and straight talk is the bread and butter of the >> mailing list. > > An aside, since it doesn't bear too much on the topic at hand: > > For what it's worth, in my experience the people working on the kernel > are quite sensible and friendly on-list. They are professional. When they need to be straight-forward, they are, even if that hurts the feelings of the colleagues. > Probably you are referring to some high-profile cases of flames, No, I'm not. Heated discussions happen all the time, specially when the issue at hand is important. > I do not think the way the list works normally is a > counterexample to common decency being useful. Of course you wouldn't, but you are purposely ignoring the facts. The Linux kernel mailing lists concentrates on *the code*; he who writes the code has a voice, he who only has words doesn't. Personal beefs are not relevant. When there's something horribly wrong with the code, so are the responses. > So no, I don't find "But they are mean, and look how well they are > doing!" to be a compelling argument here. Because you dismiss the premise a priori. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Felipe Contreras wrote: > A collegial work environment is overrated, and proof of that the Linux > kernel, where honest and straight talk is the bread and butter of the > mailing list. An aside, since it doesn't bear too much on the topic at hand: For what it's worth, in my experience the people working on the kernel are quite sensible and friendly on-list. Probably you are referring to some high-profile cases of flames, which perhaps I have just been lucky to avoid. I do not think the way the list works normally is a counterexample to common decency being useful. So no, I don't find "But they are mean, and look how well they are doing!" to be a compelling argument here. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
Hi Duy, On Sat, 8 Jun 2013, Duy Nguyen wrote: > On Thu, Jun 6, 2013 at 11:22 PM, Johannes Schindelin > wrote: > > Hi Greg, > > > > On Thu, 6 Jun 2013, Greg Troxel wrote: > > > >> As one of the people who helps maintain git packages in pkgsrc, my > >> initial reaction is negative to adding a ruby dependency. > > > > My initial reaction, too. It was hard enough to get Perl included with Git > > for Windows (because of that pesky Subversion dependency). > > > > As you can see from the commit history, I was the primary force behind > > trying to get everything "core" in Git away from requiring scripting > > languages (I think it is an awesome thing to provide APIs for as many > > languages as possible, but a not-so-cool thing to use more than one > > language in the core code). It does not seem that anybody picked up that > > task when I left, though. > > Nobody seems to mention it yet. There's another reason behind the C > rewrite effort: fork is costly on Windows. The C rewrite allows us to > run with one process (most of the time). This applies for shell, perl > and even ruby scripts because libgit.a is never meant to be used > outside git.c context (unlike libgit2). In this regard, ruby is just > as bad as currently supported non-C languages. I think you should have said "on Windows" when you said "fork is costly". Oh wait, you did. It seems that at least some people participating in this discussion are not overly keen on supporting the platform that -- according to statistics, i.e. facts -- is the most prevalent. I am glad that Junio still seems to be interested in giving us poor folks trying to make the Git experience on Windows a less sucky one enough credit, even if we only got a little over 900 commits into git.git. But that does not count because the commits are older than one year. That makes them useless to some people, apparently. Hopefully Junio will have more mercy on us poor Windows folks than others would, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
Hi Ram, On Sat, 8 Jun 2013, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: > > While at it, why not re-evaluate the whole msysgit approach? I bet we > > don't need a whole separate project just to create a Windows > > installer. I've written Windows installers before, it's very easy to > > do from Linux. > > Yeah, taking the pain out of msysgit packaging would be a great way to > counter this new-dependency-fud. The main problem, as mm pointed out > is subversion + perlxs. Yeah, this is the main problem, and you probably will end up with a much better (Linux-based) solution than the people who contributed to the Git for Windows project in all those years since August 2007. Surprise me, with code, J -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp
On Sat, Jun 08, 2013 at 08:38:56PM +0200, Matthieu Moy wrote: > Célestin Matte writes: > > > In Perl, '\n' is not a newline, but instead a literal backslash followed by > > an > > "n". As the output of "rev-list --first-parent" is line-oriented, what we > > want > > here is a newline. > > This is right, but the code actually worked the way it was. I'm not > sure, but my understanding is that '\n' is the string "backslash > followed by n", but interpreted as a regexp, it is a newline. Yes, the relevant doc (from "perldoc -f split") is: The pattern "/PATTERN/" may be replaced with an expression to specify patterns that vary at runtime. (To do runtime compilation only once, use "/$variable/o".) So it is treating "\n" as an expression and compiling the regex each time through (though I think modern perl may be smart enough to realize it is a constant expression and compile the regex only once). You would get the same behavior with this: split $arg, $data; if $arg contained '\n'. Of course, you _also_ get the same thing if you use a literal newline (either "\n" or if $arg contained a literal newline), because they function the same in a regex. In other words, it does not matter which you use because perl's interpolation of "\n" and the regex expansion of "\n" are identical: t hey both mean a newline. A more subtle example that shows what is going on is this: split '.', $data; If you feed that "foo.bar.baz", it does not split it into three words; each character is a delimiter, because the dot is compiled to a regex. > The new code looks better than the old one, but the log message may be > improved. Agreed. I think the best explanation is something like: Perl's split function takes a regex pattern argument. You can also feed it an expression, which is then compiled into a regex at runtime. It therefore works to pass your pattern via single quotes, but it is much less obvious to a reader that the argument is meant to be a regex, not a static string. Using the traditional slash-delimiters makes this easier to read. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
Hi Ram, On Sat, 8 Jun 2013, Ramkumar Ramachandra wrote: > Felipe Contreras wrote: > >> Also we heard from no regular/high-value reviewers that they feel > >> comfortable reviewing additions in Ruby. > > > > Correction; *current* regular/high-value reviewers. > > Correct. The opinions of inactive community members and > non-contributors are less useful. I humbly suggest to treat other people's contribution with the same respect you want yours' to be treated. Just a suggestion, J -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 8, 2013 at 9:23 PM, Jeff King wrote: > On Sat, Jun 08, 2013 at 08:17:08PM -0500, Felipe Contreras wrote: > >> > No, I didn't say that at all. >> >> Then you truly think libgit2 will ever reach the point where it can >> replace libgit.a? > > I don't know. It may. Or something like it may. It is certainly not > ready to do so yet, but perhaps one day it will be. Perhaps one day we would end poverty and hunger, and perhaps one day we will live in peace, but I wouldn't hold on my breath. I fact, I'll do the opposite, I bet it won't happen anytime soon. >> It won't. > > Oh, I see, you were not actually interested in my answer and were just > being rhetorical. > >> But decreeing that both projects should remain isolated, and >> that libgit.a should never be a library, you are effectively >> condemning the effort to fail, knowingly or not. > > Huh? When did I decree anything? When you said in your opinion we should wait until libgit2 is ready, and not improve libgit.a. >> How many years has libgit2 been brewing? Do you think it's closer for >> merging so it can be used by Git's core? No, it doesn't, and it will >> not in the future, because it was never meant for that. > > There has been about 2 years of active development, and there's been > quite a lot of improvement in that time. Closer than what? Than it was 2 > years ago? Yes, I think it is. But it still has a ways to go. Why is it closer? In what ways is it a better fit now than 2 years ago? What is missing before merging to be used in Git's core? > I do not think there will be a flag day where we throw away git.git's > code and start using libgit2. But we could slowly start replacing > underlying bits with libgit2 bits, if that implementation proves to be > robust and clean enough to do so. And what are we waiting for then? Shouldn't we copy the whole libgit2 code and start migrating? >> > But hey, you don't need to listen to me. If you think it would be easier >> > to make the git.git code into a library, go ahead and work on it. But I >> > think you will find that there are a large number of hard-to-find bugs >> > caused by implicit assumptions about global state, how file descriptors >> > are used, and so forth. >> >> That's impossible. Specially since moving irrelevant code out of >> libgit.a is not permitted. > > I'm not even sure what your second sentence means. It means this: http://article.gmane.org/gmane.comp.version-control.git/226752 I move code that doesn't belong in a libgit library out of libgit.a, and the change gets rejected. > But it seems to me that the first step would be cleaning up the internal > code so that it is more friendly to library callers (both in interface > and in being re-entrant), That is the second step. It doesn't make sense to make code re-entrant, if that code will only be used by builtin commands. First step is to move irrelevant code out of libgit.a. >> >> There's a reason why the Git project doesn't use libgit2, and for the >> >> same reason the official Ruby scripts should not use it. >> > >> > What reason is that? >> >> You tell me. Why isn't Git using libgit2? > > Wait, you indicated you had such a reason in mind, but now you won't > tell me? Is it a secret? I did not. I made the assumption that there was a reason, if there's no reason to stay clear of libgit2, then let's merge it already. >> > I think it is a matter of critical mass. If you were to start linking >> > against libgit.a and 90% of it worked, you might have a reason to fix >> > the other 10%. But I suspect it is more the other way around. >> >> It doesn't matter if it's 90% or 10%, it's the only thing we have. >> >> Unless you are in favor of including libgit2 and start using it for >> Git's core *right now*, the only way forward is to improve libgit.a. > > That seems like a false choice to me. You obviously feel that libgit2 is > some kind of dead end. I don't agree. Whatever. I never said so. It is a dead end *if* we don't do an effort to have a proper libgit library, which is the path we are taking. > I have very little interest in discussing this further with you, as it > is not leading in a productive direction. In my opinion, the productive > things to do would be one (or both) of: > > 1. Work on libgit2. > > 2. Clean up non-reentrant bits of git.git, hopefully making the code > more readable and more modular (and taking care not to make it > worse in other ways, like maintainability or performance). You forgot the first step of 2.: move irrelevant code out of libgit.a. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] read-cache: plug a few leaks
On Sat, Jun 8, 2013 at 9:11 PM, René Scharfe wrote: > Am 08.06.2013 19:27, schrieb Felipe Contreras: > >> On Sat, Jun 8, 2013 at 12:22 PM, René Scharfe >> wrote: >> >>> Let's find and fix those leaks by freeing memory in the right places. >>> Freeing memory just in case in places where we can show that no leak is >>> triggered by our test suite doesn't help. >> >> >> It helps; it prevents leaks. The real culprit is the bogus API, but I >> don't see that changing anytime soon, so there are two options when >> somebody makes a mistake the API allows; leak or don't leak. And you >> seem to prefer the leak, even though it provides absolutely no >> advantage. > > It covers up bugs, It doesn't. I thought you already silently agreed that nobody would ever find that leak, as they haven't found the hundreds of leaks that plague Git's code. > What would be a better API? Making discard_index free the array is a good > first step; what else is bogus? 'initialized' for starters; it should be renamed to 'loaded' or removed, but removing it would require many more changes to make sure we don't load twice. Also, when loading cache entries, it might make sense to check if there's already entries that have not been previously discarded properly. In the meantime, just in case, the only sane thing to do is free the entries rather than leak. That being said I'm not interested in this patch any more. The patch is good yet after three tries and countless arguments it's still not applied, nor is there any sign of getting there. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 08, 2013 at 08:17:08PM -0500, Felipe Contreras wrote: > > No, I didn't say that at all. > > Then you truly think libgit2 will ever reach the point where it can > replace libgit.a? I don't know. It may. Or something like it may. It is certainly not ready to do so yet, but perhaps one day it will be. > It won't. Oh, I see, you were not actually interested in my answer and were just being rhetorical. > But decreeing that both projects should remain isolated, and > that libgit.a should never be a library, you are effectively > condemning the effort to fail, knowingly or not. Huh? When did I decree anything? You asked Duy what kinds of problems you would run into with running multiple git commands in the same process space. I answered with concrete examples, and gave my opinions on what the path of least work would be to reach a re-entrant library. You don't have to agree (didn't I even say "you don't have to listen to me" in the last email?). > How many years has libgit2 been brewing? Do you think it's closer for > merging so it can be used by Git's core? No, it doesn't, and it will > not in the future, because it was never meant for that. There has been about 2 years of active development, and there's been quite a lot of improvement in that time. Closer than what? Than it was 2 years ago? Yes, I think it is. But it still has a ways to go. I do not think there will be a flag day where we throw away git.git's code and start using libgit2. But we could slowly start replacing underlying bits with libgit2 bits, if that implementation proves to be robust and clean enough to do so. > > But hey, you don't need to listen to me. If you think it would be easier > > to make the git.git code into a library, go ahead and work on it. But I > > think you will find that there are a large number of hard-to-find bugs > > caused by implicit assumptions about global state, how file descriptors > > are used, and so forth. > > That's impossible. Specially since moving irrelevant code out of > libgit.a is not permitted. I'm not even sure what your second sentence means. But it seems to me that the first step would be cleaning up the internal code so that it is more friendly to library callers (both in interface and in being re-entrant), with those first sets of callers being the existing code in git.git. Such cleanups would be a good thing for the modularity of the code, even without an intended library step. And then you can start to pull out individual interfaces that are known to be safe for library use, and think about making a coherent library out of them. And please don't tell me about "not permitted". You are free to fork and work on this. But do not expect people who have already said "that does not seem like a fruitful path to me" to jump into it with you. If you think it is worth doing and that you can come up with initial results to convince others, go for it. > >> There's a reason why the Git project doesn't use libgit2, and for the > >> same reason the official Ruby scripts should not use it. > > > > What reason is that? > > You tell me. Why isn't Git using libgit2? Wait, you indicated you had such a reason in mind, but now you won't tell me? Is it a secret? > > I think it is a matter of critical mass. If you were to start linking > > against libgit.a and 90% of it worked, you might have a reason to fix > > the other 10%. But I suspect it is more the other way around. > > It doesn't matter if it's 90% or 10%, it's the only thing we have. > > Unless you are in favor of including libgit2 and start using it for > Git's core *right now*, the only way forward is to improve libgit.a. That seems like a false choice to me. You obviously feel that libgit2 is some kind of dead end. I don't agree. Whatever. I have very little interest in discussing this further with you, as it is not leading in a productive direction. In my opinion, the productive things to do would be one (or both) of: 1. Work on libgit2. 2. Clean up non-reentrant bits of git.git, hopefully making the code more readable and more modular (and taking care not to make it worse in other ways, like maintainability or performance). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 8:40 PM, Jonathan Nieder wrote: > Felipe Contreras wrote: > >> Just the past three months I've probably done more work than anybody >> else[1], and you would ban me because you don't like my words? > > Definitely, yes. Even if you look at the impact on code alone and > don't care about the people, destroying a collegial work environment > is harmful enough to the code to outweigh the (admittedly often > useful) patches. A collegial work environment is overrated, and proof of that the Linux kernel, where honest and straight talk is the bread and butter of the mailing list. And the Linux kernel is the most successful software project in history by far. It's code that speaks. And I have not destroyed anything, except maybe your sense of fairness and balance when reviewing my patches, but that is not my fault. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] read-cache: plug a few leaks
Am 08.06.2013 19:27, schrieb Felipe Contreras: On Sat, Jun 8, 2013 at 12:22 PM, René Scharfe wrote: Let's find and fix those leaks by freeing memory in the right places. Freeing memory just in case in places where we can show that no leak is triggered by our test suite doesn't help. It helps; it prevents leaks. The real culprit is the bogus API, but I don't see that changing anytime soon, so there are two options when somebody makes a mistake the API allows; leak or don't leak. And you seem to prefer the leak, even though it provides absolutely no advantage. It covers up bugs, which may seem helpful, but isn't, because it's better to fix the actual mistake. What would be a better API? Making discard_index free the array is a good first step; what else is bogus? René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Felipe Contreras wrote: > Just the past three months I've probably done more work than anybody > else[1], and you would ban me because you don't like my words? Definitely, yes. Even if you look at the impact on code alone and don't care about the people, destroying a collegial work environment is harmful enough to the code to outweigh the (admittedly often useful) patches. But I am not the mailing list owner, so what I would do is not too important. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 8, 2013 at 7:10 PM, Jeff King wrote: > On Sat, Jun 08, 2013 at 12:40:19PM -0500, Felipe Contreras wrote: > >> > These are problems that can be solved. But there is a lot of work >> > involved in finding these subtle bugs and coming up with fixes. I think >> > you would be better off working on an implementation of git that was >> > designed from scratch to work in-process, like libgit2. >> >> So you are in favor of never ever having an official Git library. Got it. > > No, I didn't say that at all. Then you truly think libgit2 will ever reach the point where it can replace libgit.a? It won't. But decreeing that both projects should remain isolated, and that libgit.a should never be a library, you are effectively condemning the effort to fail, knowingly or not. How many years has libgit2 been brewing? Do you think it's closer for merging so it can be used by Git's core? No, it doesn't, and it will not in the future, because it was never meant for that. > I do think that it would be more work to try to slowly massage the git > code into a library-ready form than it would be to simply start with > more library-friendly code and pull in bits of git.git as appropriate. It might be more effort, but the results are guaranteed by our extensive testing infrastructure and huge user-base. Slowly but steadily we'll get there. Waiting for libgit2 to switch directions and reach some hypothetical point is waiting for hell to freeze. It won't happen. There's no incentive nor reason for it to happen. > That is what the libgit2 project is doing. Perhaps one day that project > will reach a point where we start building git.git commands off of it or > sometihng like it (for that matter, there is no reason you could not > build external commands off of libgit2 right now). Would it be the > "official" Git library then? I don't know. It is not clear to me what > that even means. It means 'make install' installs a shared library with a clearly defined and stable API that other projects can depend on, and it can be used for all sort of purposes, including the git binary, and it's builtins. > In the meantime, I think it cannot be a bad thing for libgit2 to proceed > along its path, and I don't see a good reason for people not to use it. Its path will never end as an official Git library, not unless we do something. > But hey, you don't need to listen to me. If you think it would be easier > to make the git.git code into a library, go ahead and work on it. But I > think you will find that there are a large number of hard-to-find bugs > caused by implicit assumptions about global state, how file descriptors > are used, and so forth. That's impossible. Specially since moving irrelevant code out of libgit.a is not permitted. >> There's a reason why the Git project doesn't use libgit2, and for the >> same reason the official Ruby scripts should not use it. > > What reason is that? You tell me. Why isn't Git using libgit2? >> As history indicates, the Git project will never have any pressure to >> fix it's re-entrancy and re-run issues, so these issues will remain >> there forever. >> >> Only if you allow code that exposes those issues will there ever be >> any pressure to fix them. > > I think it is a matter of critical mass. If you were to start linking > against libgit.a and 90% of it worked, you might have a reason to fix > the other 10%. But I suspect it is more the other way around. It doesn't matter if it's 90% or 10%, it's the only thing we have. Unless you are in favor of including libgit2 and start using it for Git's core *right now*, the only way forward is to improve libgit.a. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
On Fri, Jun 07, 2013 at 12:12:52PM +0200, Erik Faye-Lund wrote: > > Yeah, if it were mingw_raise responsible for this, I would suggest using > > the POSIX shell "128+sig" instead. We could potentially check for > > SIG_DFL[1] mingw_raise and intercept and exit there. I don't know if > > that would create headaches or confusion for other msys programs, > > though. I'd leave that up to the msysgit people to decide whether it is > > worth the trouble. > > > > ...and here's the code to do just that: > [...] > @@ -1715,6 +1720,13 @@ int mingw_raise(int sig) > sigint_fn(SIGINT); > return 0; > > + case SIGTERM: > + if (sigterm_fn == SIG_DFL) > + exit(128 + SIGTERM); > + else if (sigterm_fn != SIG_IGN) > + sigterm_fn(SIGTERM); > + return 0; I'm a little negative on handling just SIGTERM. That would make the test pass, but does it really address the overall issue? To me, the usefulness is having exit values with consistent meanings. Imagine I run a very large git hosting site, and I want to log exceptional conditions (e.g., a git sub-process crashes). What exit code do I get from a SIGSEGV or SIGBUS (or GPF, or whatever Windows calls these)? On Unix systems, this is pretty easy. To be honest, I do not care that much about Windows systems because I would not host a large site on it. :) But IMHO, the point of such a scheme is to be consistent across all signals. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 08, 2013 at 12:40:19PM -0500, Felipe Contreras wrote: > > These are problems that can be solved. But there is a lot of work > > involved in finding these subtle bugs and coming up with fixes. I think > > you would be better off working on an implementation of git that was > > designed from scratch to work in-process, like libgit2. > > So you are in favor of never ever having an official Git library. Got it. No, I didn't say that at all. I do think that it would be more work to try to slowly massage the git code into a library-ready form than it would be to simply start with more library-friendly code and pull in bits of git.git as appropriate. That is what the libgit2 project is doing. Perhaps one day that project will reach a point where we start building git.git commands off of it or sometihng like it (for that matter, there is no reason you could not build external commands off of libgit2 right now). Would it be the "official" Git library then? I don't know. It is not clear to me what that even means. In the meantime, I think it cannot be a bad thing for libgit2 to proceed along its path, and I don't see a good reason for people not to use it. But hey, you don't need to listen to me. If you think it would be easier to make the git.git code into a library, go ahead and work on it. But I think you will find that there are a large number of hard-to-find bugs caused by implicit assumptions about global state, how file descriptors are used, and so forth. > There's a reason why the Git project doesn't use libgit2, and for the > same reason the official Ruby scripts should not use it. What reason is that? > As history indicates, the Git project will never have any pressure to > fix it's re-entrancy and re-run issues, so these issues will remain > there forever. > > Only if you allow code that exposes those issues will there ever be > any pressure to fix them. I think it is a matter of critical mass. If you were to start linking against libgit.a and 90% of it worked, you might have a reason to fix the other 10%. But I suspect it is more the other way around. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] build: trivial cleanup
There's no need to list again the prerequisites. Signed-off-by: Felipe Contreras --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 03524d0..fda98d6 100644 --- a/Makefile +++ b/Makefile @@ -2067,13 +2067,13 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIB_FILE): $(LIB_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $^ $(XDIFF_LIB): $(XDIFF_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $^ $(VCSSVN_LIB): $(VCSSVN_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(VCSSVN_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $^ export DEFAULT_EDITOR DEFAULT_PAGER -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diff: add --ignore-blank-lines option
The goal of the patch is to introduce the GNU diff -B/--ignore-blank-lines as closely as possible. The short option is not available because it's already used for "break-rewrites". When this option is used, git-diff will not create hunks that simply adds or removes empty lines, but will still show empty lines addition/suppression if they are close enough to "valuable" changes. There are two differences between this option and GNU diff -B option: - GNU diff doesn't have "--inter-hunk-context", so this must be handled - The following sequence looks like a bug (context is displayed twice): $ seq 5 >file1 $ catOK. Thanks. > > I think the logic would be more like: > > 1. Start from xscr, find the first xchp that is !xchp->ignore; > if there is none, we are done. There is no more to show. > > 2. Remember the xchp as the beginning. > > 3. Tangle ->next pointer to find the next xch that is !xch->ignore; > if there is none, we are also done. xdchanges between the > beginning you remembered in the step 2. and your current xchp > are the only things we want to show. > > 4. Measure the distance between the end of xchp and the beginning > of xch. > > - If it is larger than max_common, xdchanges between the > beginning you remembered in the step 2. and your current xchp > are the only things we want to show. The next iteration will > start by skipping the blank-only changes between xchp and xch. > > - If it is short enough, assign xchp = xch and go back to 3. to > find more interesting hunks (that is why we remembered the > real "beginning" in step 2.). Actually it doesn't quite work like that because we don't totally ignore "blank lines". We want to keep them if they are close enough to other changes. I've tried to improve the number of tests as it helped me during implementation, and to give a better description of the feature. The initial reroll was meant to simplify xdl_get_hunk() but I'm afraid it became kind of "voodoo code". I'm not sure if I should provide some more comments about it and where. Please let me know if something is not working as expected. Cheers, Antoine Documentation/diff-options.txt |3 + diff.c |2 + t/t4015-diff-whitespace.sh | 282 xdiff/xdiff.h |2 + xdiff/xdiffi.c | 29 - xdiff/xdiffi.h |1 + xdiff/xemit.c | 47 ++- xdiff/xemit.h |2 +- xdiff/xutils.c | 13 ++ xdiff/xutils.h |1 + 10 files changed, 374 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index b8a9b86..4e042d9 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -439,6 +439,9 @@ endif::git-format-patch[] differences even if one line has whitespace where the other line has none. +--ignore-blank-lines:: + Ignore changes whose lines are all blank. + --inter-hunk-context=:: Show the context between diff hunks, up to the specified number of lines, thereby fusing hunks that are close to each other. diff --git a/diff.c b/diff.c index f0b3e7c..208094f 100644 --- a/diff.c +++ b/diff.c @@ -3593,6 +3593,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) DIFF_XDL_SET(options, IGNORE_WHITESPACE_CHANGE); else if (!strcmp(arg, "--ignore-space-at-eol")) DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL); + else if (!strcmp(arg, "--ignore-blank-lines")) + DIFF_XDL_SET(options, IGNORE_BLANK_LINES); else if (!strcmp(arg, "--patience")) options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF); else if (!strcmp(arg, "--histogram")) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index cc3db13..fc77713 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -142,6 +142,288 @@ EOF git d
Re: [PATCH v2 16/22] git-remote-mediawiki: Modify strings for a better coding-style
Le 08/06/2013 02:39, Eric Sunshine a écrit :
> On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
> wrote:
>> - strings which don't need interpolation are single-quoted for more clarity
>> and
>> slight gain of performance
>> - interpolation is preferred over concatenation in many cases, for more
>> clarity
>> - variables are always used with the ${} operator inside strings
>> - strings including double-quotes are written with qq() so that the quotes do
>> not have to be escaped
>
> Distinct changes could (IMHO) be split into separate patches for easier
> review.
This commit is a real pain to cut into 3 distinctive ones. Is this
really necessary?
I will do it if it is, of course.
--
Célestin Matte
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp
Le 08/06/2013 20:38, Matthieu Moy a écrit :> This is right, but the code actually worked the way it was. I'm not > sure, but my understanding is that '\n' is the string "backslash > followed by n", but interpreted as a regexp, it is a newline. > > The new code looks better than the old one, but the log message may be > improved. Is this better? " In Perl, '\n' is not a newline, but instead the string composed of a backslash followed by an "n". To match newlines, one has to use the /\n/ regexp. As the output of "rev-list --first-parent" is line-oriented, what we want here is to match newlines, and not the "\n" string. " -- Célestin Matte -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 12:34 PM, Jonathan Nieder wrote: > If I were managing this list, I would ban mails from you, since this > discussion style does more harm than good. There is a nice motto around: "Talk is cheap. Show me the code." Just the past three months I've probably done more work than anybody else[1], and you would ban me because you don't like my words? At the end of the day the project has benefited from my patches, and a wise maintainer would do what is best for the project. If you don't like my words, ignore them. Taking things personal is more often than not the wrong thing to do. Specially when they were not even directed to you. [1] https://www.ohloh.net/p/git/contributors?query=&sort=commits_12_mo -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
Célestin Matte writes: > Oh, I thought a part of a patch was called a commit. 1 patch = 1 commit 1 patch series = several commits sent together. Will normally end-up in a branch in Junio's repo (named with /) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
benoit.per...@ensimag.fr writes:
> From: Benoit Person
>
> The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
> preview content without pushing would be a nice thing to have.
>
> This commit is a first attempt to achieve it. It adds a new git command,
> named `git mw`. This command accepts the subcommands `help` and `preview`
> for now.
Review could be easier if you have PATCH 1/2 introducing the command
with only "help" (essentially to check that the build system works), and
then focus on "preview".
I won't insist in splitting this particular commit, just take it as an
advice for future work.
> --- a/contrib/mw-to-git/Makefile
> +++ b/contrib/mw-to-git/Makefile
> @@ -5,13 +5,17 @@
> ## Build git-remote-mediawiki
>
> SCRIPT_PERL=git-remote-mediawiki.perl
> +SCRIPT_PERL_MW=git-mw.perl
Why do you need another variable? Just adding git-mw.perl to SCRIPT_PERL
should do it, no? (Well, probably the make SCRIPT_PERL=$(SCRIPT_PERL)
lacks quotes around the argument, but then you should fix that).
> +# Constants
> +# Mediawiki filenames can contain forward slashes. This variable decides by
> which pattern they should be replaced
> +Readonly::scalar $SLASH_REPLACEMENT => "%2F";
This one is copied from git-remote-mediawiki.perl, and is a
git-mediawiki specific thing, so it would not make sense to put it into
Git.pm. This starts convincing me that we should have a GitMediawiki.pm
or so, and both scripts should use it.
Another option would be to put everything in git-remote-mediawiki.perl,
and probably to have "git mw" as a wrapper around it (to avoid having to
type "git remote-mediawiki" which is a bit long).
But the script starts being a bit long, so I think it makes more sense
to split it. So I'd say
PATCH 1/3 : introduce "git mw"
PATCH 2/3 : move sharable code to a new module (and make sure it's
installed properly by "make install")
PATCH 3/3 : actually implement the preview feature
Perhaps others will have other/better advices.
> + # Auto-loading in browser
> + if ($autoload) {
> + open(my $browser, "-|:encoding(UTF-8)", "xdg-open
> ".$preview_file_name);
That could be read from Git's configuration, and default to xdg-open.
But you don't want to hardcode it in the middle of the code.
Also, why use open if you don't want to communicate with the process?
system would be sufficient, and won't need close.
Prefer passing an array of arguments, instead of concatenating strings
and then rely on command-line parsing to re-split it.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
Le 08/06/2013 20:41, Matthieu Moy a écrit : > Célestin Matte writes: > >> Le 08/06/2013 02:14, Eric Sunshine a écrit : >>> These two changes are unrelated and could be split into distinct >>> patches (IMHO, though others may disagree). >> >> Two distinct patches or two distinct commits? > > That's the same. You write commits locally, send them as patches, and > Junio uses "git am" to turn the patches back into commits. > Oh, I thought a part of a patch was called a commit. So the question was rather: "Should it be sent to this list independently from this series of patches?". -- Célestin Matte -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
Célestin Matte writes: > Le 08/06/2013 02:14, Eric Sunshine a écrit : >> These two changes are unrelated and could be split into distinct >> patches (IMHO, though others may disagree). > > Two distinct patches or two distinct commits? That's the same. You write commits locally, send them as patches, and Junio uses "git am" to turn the patches back into commits. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Ramkumar Ramachandra writes: > Yes, please merge. The commit message looks good as well: it doesn't > say anything about waiting for 2.0. The commit message doesn't, but the doc does. I'll resend without the 2.0 mention. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp
Célestin Matte writes: > In Perl, '\n' is not a newline, but instead a literal backslash followed by an > "n". As the output of "rev-list --first-parent" is line-oriented, what we want > here is a newline. This is right, but the code actually worked the way it was. I'm not sure, but my understanding is that '\n' is the string "backslash followed by n", but interpreted as a regexp, it is a newline. The new code looks better than the old one, but the log message may be improved. In any case, Acked-by: Matthieu Moy -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
Jeff King writes: > Thanks both for the explanation. I don't see us using that to our > advantage anywhere in the patch. So I think this is purely a style > issue, which to me indicates that the extra dependency is not worth it, > and the patch should be dropped. Same here: I'd rather keep the dependency list small. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sat, Jun 8, 2013 at 1:02 PM, Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
>> There's no libgit, and there will never be, every object file in Git is
>> the same, and there's wish to organize them in any way; they are *all*
>> for the 'git' binary and its builtin commands.
>
> Nice joke patch to illustrate your point ;)
It's not a joke. This is seriously the direction the others say is the
correct one.
One direction or the other, the problem that top-level objects can't
access code from builtin objects must be fixed. And if the others
don't want to fix it by properly splitting code between library-like
objects, and builtin objects, there's only one other way to fix it;
this way.
> On a more serious note, please be a little more patient while everyone
> copes with what you're attempting.
I don't think patience will help. What do you suggest? Wait until the
problem fixes itself? (I'll be waiting until the end times). Wait
until somebody changed their opinion by themselves? (I don't see that
happening).
> I've already made it clear that
> I'm in favor of moving forward with your plan to lib'ify git.
Unfortunately you are the only one.
> The
> problem is that you're sending your changes in fragmented comments and
> diffs, and nobody is able to piece together what the big picture is.
>
> Please write one cogent email (preferably with code included)
> explaining your plan.
The plan is simple; make libgit.a a proper library, starting by
clarifying what goes into libgit.a, and what doesn't. If there's any
hopes of ever having a public library, it's clear what code doesn't
belong in libgit.a; code that is meant for builtins, that code belongs
in builtins/lib.a, or similar.
But to be honest, I don't really care, all I want is the problem of
the bogus split to be solved. One way to solve it is going the proper
library way, but the other is to stash everything together into git.a.
Both ways solve the problem.
Give this a try:
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,6 +14,7 @@
#include "merge-recursive.h"
#include "refs.h"
#include "argv-array.h"
+#include "builtin.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -102,6 +103,17 @@ static int has_conforming_footer(struct strbuf
*sb, struct strbuf *sob,
return 1;
}
+static void copy_notes(const char *name)
+{
+ struct notes_rewrite_cfg *cfg;
+
+ cfg = init_copy_notes_for_rewrite(name);
+ if (!cfg)
+ return;
+
+ finish_copy_notes_for_rewrite(cfg);
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -997,6 +1009,8 @@ static int pick_commits(struct commit_list
*todo_list, struct replay_opts *opts)
return res;
}
+ copy_notes("cherry-pick");
+
/*
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
What happens?
libgit.a(sequencer.o): In function `copy_notes':
/home/felipec/dev/git/sequencer.c:110: undefined reference to
`init_copy_notes_for_rewrite'
/home/felipec/dev/git/sequencer.c:114: undefined reference to
`finish_copy_notes_for_rewrite'
It is not the first time, nor the last that top-level code needs
builtin code, and the solution is easy; organize the code. Alas, this
simple solution reject on the basis that we shouldn't organize the
code, because the code is not meant to be organized.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
Felipe Contreras wrote: > There's no libgit, and there will never be, every object file in Git is > the same, and there's wish to organize them in any way; they are *all* > for the 'git' binary and its builtin commands. Nice joke patch to illustrate your point ;) On a more serious note, please be a little more patient while everyone copes with what you're attempting. I've already made it clear that I'm in favor of moving forward with your plan to lib'ify git. The problem is that you're sending your changes in fragmented comments and diffs, and nobody is able to piece together what the big picture is. Please write one cogent email (preferably with code included) explaining your plan. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 12:34 PM, Jonathan Nieder wrote: > Felipe Contreras wrote: > >> This has nothing to do with better strategy, it has everything to do >> with gut feelings and tradition. Not reasons. > > I try to help you, and you insult me. I don't think this is worth it. I didn't direct that comment to you; you took the pellet and threw it at yourself. Moreover, following gut feelings and traditions without reason is not an insult, that's what human beings do. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 8, 2013 at 12:15 PM, Jeff King wrote: > On Sat, Jun 08, 2013 at 08:20:28AM -0500, Felipe Contreras wrote: > >> > There are a lot of static variables in builtin/ (and outside too), >> > which make it non-entrant, or at least not safe. >> >> So? >> >> > fork provides a process space isolation, some depend on that. >> >> Process space isolation from what? > > Manipulation of global variables. Here are a few examples off the top of > my head: > > Try running "git diff" from your Ruby hook, then try running "git > diff-files" within the same process. I believe the latter will start > respecting porcelain diff config like diff.mnemonicprefix. To clear > state you need to reset a list of global variables back to their initial > states (some of which are the BSS-default zero, but some of which are > not). > > Try running "git log" followed by another "git log". The log family of > commands does not clear its marks from the commit objects, since it > expects to exit after the traversal. The second log will sometimes give > wrong answers if its traversal overlaps with the first (e.g., commits > marked SEEN or UNINTERESTING that should not be). You can add a call to > clear them at the end of the process, but that does not cover any cases > where we die(). > > These are problems that can be solved. But there is a lot of work > involved in finding these subtle bugs and coming up with fixes. I think > you would be better off working on an implementation of git that was > designed from scratch to work in-process, like libgit2. So you are in favor of never ever having an official Git library. Got it. > libgit2 doesn't have feature parity with regular git yet, but there are > many clients based around it that use the library internally for speed, > and then exec regular git to fill in the gaps. There's a reason why the Git project doesn't use libgit2, and for the same reason the official Ruby scripts should not use it. As history indicates, the Git project will never have any pressure to fix it's re-entrancy and re-run issues, so these issues will remain there forever. Only if you allow code that exposes those issues will there ever be any pressure to fix them. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Felipe Contreras wrote: > This has nothing to do with better strategy, it has everything to do > with gut feelings and tradition. Not reasons. I try to help you, and you insult me. I don't think this is worth it. If I were managing this list, I would ban mails from you, since this discussion style does more harm than good. If I were maintaining git, I'd still accept your contributions, waiting until times when I had more patience to read them and sending them to the list when appropriate to get more feedback. Of course I am neither managing the list nor maintaining git, but I thought I should put that out there... Annoyed, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
On Sat, Jun 08, 2013 at 03:01:03PM +0200, Célestin Perdu wrote: > Oh yes, part of this commit went into the previous one, which was not > formated as an email when I did my git-format-patch. I should check my > patches more carefully before sending them. Sorry for this. No problem. It is easy to make simple mistakes like that with our workflow, but it is also easy to fix them and repost. :) > > What advantage does this have over "use constant"? I do not mind > > following guidelines from perlcritic if they are a matter of style, but > > in this case there is a cost: we now depend on the "Readonly" module, > > which is not part of the standard distribution. I.e., users now have to > > deal with installing an extra dependency. Is it worth it? > > Like Benoit said, the problem is that they sometimes don't interpolate. > I don't know if we should keep this commit or not. Thanks both for the explanation. I don't see us using that to our advantage anywhere in the patch. So I think this is purely a style issue, which to me indicates that the extra dependency is not worth it, and the patch should be dropped. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] build: get rid of the notion of a git library
There's no libgit, and there will never be, every object file in Git is the same, and there's wish to organize them in any way; they are *all* for the 'git' binary and its builtin commands. So let's shatter any hopes of ever having a library, and be clear about it; both the top-level objects (./*.o) and the builtin objects (./builtin/*.o) go into git.a, which is not a library, merely a convenient way to stash objects together. This way there will not be linking issues when top-level objects try to access functions of builtin objects. LIB_OBJS and LIB_H imply a library, but there isn't one, and never will be; so give them proper names; just a bunch of headers and objects. Signed-off-by: Felipe Contreras --- Makefile | 564 --- 1 file changed, 283 insertions(+), 281 deletions(-) diff --git a/Makefile b/Makefile index 03524d0..63451b1 100644 --- a/Makefile +++ b/Makefile @@ -435,8 +435,8 @@ XDIFF_OBJS = VCSSVN_OBJS = GENERATED_H = EXTRA_CPPFLAGS = -LIB_H = -LIB_OBJS = +HEADERS = +OBJS = PROGRAM_OBJS = PROGRAMS = SCRIPT_PERL = @@ -629,270 +629,270 @@ endif export PERL_PATH export PYTHON_PATH -LIB_FILE = libgit.a +GIT_LIB = git.a XDIFF_LIB = xdiff/lib.a VCSSVN_LIB = vcs-svn/lib.a GENERATED_H += common-cmds.h -LIB_H += advice.h -LIB_H += archive.h -LIB_H += argv-array.h -LIB_H += attr.h -LIB_H += bisect.h -LIB_H += blob.h -LIB_H += branch.h -LIB_H += builtin.h -LIB_H += bulk-checkin.h -LIB_H += bundle.h -LIB_H += cache-tree.h -LIB_H += cache.h -LIB_H += color.h -LIB_H += column.h -LIB_H += commit.h -LIB_H += compat/bswap.h -LIB_H += compat/cygwin.h -LIB_H += compat/mingw.h -LIB_H += compat/obstack.h -LIB_H += compat/poll/poll.h -LIB_H += compat/precompose_utf8.h -LIB_H += compat/terminal.h -LIB_H += compat/win32/dirent.h -LIB_H += compat/win32/pthread.h -LIB_H += compat/win32/syslog.h -LIB_H += connected.h -LIB_H += convert.h -LIB_H += credential.h -LIB_H += csum-file.h -LIB_H += decorate.h -LIB_H += delta.h -LIB_H += diff.h -LIB_H += diffcore.h -LIB_H += dir.h -LIB_H += exec_cmd.h -LIB_H += fetch-pack.h -LIB_H += fmt-merge-msg.h -LIB_H += fsck.h -LIB_H += gettext.h -LIB_H += git-compat-util.h -LIB_H += gpg-interface.h -LIB_H += graph.h -LIB_H += grep.h -LIB_H += hash.h -LIB_H += help.h -LIB_H += http.h -LIB_H += kwset.h -LIB_H += levenshtein.h -LIB_H += line-log.h -LIB_H += line-range.h -LIB_H += list-objects.h -LIB_H += ll-merge.h -LIB_H += log-tree.h -LIB_H += mailmap.h -LIB_H += merge-blobs.h -LIB_H += merge-recursive.h -LIB_H += mergesort.h -LIB_H += notes-cache.h -LIB_H += notes-merge.h -LIB_H += notes.h -LIB_H += object.h -LIB_H += pack-revindex.h -LIB_H += pack.h -LIB_H += parse-options.h -LIB_H += patch-ids.h -LIB_H += pathspec.h -LIB_H += pkt-line.h -LIB_H += progress.h -LIB_H += prompt.h -LIB_H += quote.h -LIB_H += reachable.h -LIB_H += reflog-walk.h -LIB_H += refs.h -LIB_H += remote.h -LIB_H += rerere.h -LIB_H += resolve-undo.h -LIB_H += revision.h -LIB_H += run-command.h -LIB_H += send-pack.h -LIB_H += sequencer.h -LIB_H += sha1-array.h -LIB_H += sha1-lookup.h -LIB_H += shortlog.h -LIB_H += sideband.h -LIB_H += sigchain.h -LIB_H += strbuf.h -LIB_H += streaming.h -LIB_H += string-list.h -LIB_H += submodule.h -LIB_H += tag.h -LIB_H += tar.h -LIB_H += thread-utils.h -LIB_H += transport.h -LIB_H += tree-walk.h -LIB_H += tree.h -LIB_H += unpack-trees.h -LIB_H += url.h -LIB_H += userdiff.h -LIB_H += utf8.h -LIB_H += varint.h -LIB_H += vcs-svn/fast_export.h -LIB_H += vcs-svn/line_buffer.h -LIB_H += vcs-svn/repo_tree.h -LIB_H += vcs-svn/sliding_window.h -LIB_H += vcs-svn/svndiff.h -LIB_H += vcs-svn/svndump.h -LIB_H += walker.h -LIB_H += wildmatch.h -LIB_H += wt-status.h -LIB_H += xdiff-interface.h -LIB_H += xdiff/xdiff.h -LIB_H += xdiff/xdiffi.h -LIB_H += xdiff/xemit.h -LIB_H += xdiff/xinclude.h -LIB_H += xdiff/xmacros.h -LIB_H += xdiff/xprepare.h -LIB_H += xdiff/xtypes.h -LIB_H += xdiff/xutils.h - -LIB_OBJS += abspath.o -LIB_OBJS += advice.o -LIB_OBJS += alias.o -LIB_OBJS += alloc.o -LIB_OBJS += archive.o -LIB_OBJS += archive-tar.o -LIB_OBJS += archive-zip.o -LIB_OBJS += argv-array.o -LIB_OBJS += attr.o -LIB_OBJS += base85.o -LIB_OBJS += bisect.o -LIB_OBJS += blob.o -LIB_OBJS += branch.o -LIB_OBJS += bulk-checkin.o -LIB_OBJS += bundle.o -LIB_OBJS += cache-tree.o -LIB_OBJS += color.o -LIB_OBJS += column.o -LIB_OBJS += combine-diff.o -LIB_OBJS += commit.o -LIB_OBJS += compat/obstack.o -LIB_OBJS += compat/terminal.o -LIB_OBJS += config.o -LIB_OBJS += connect.o -LIB_OBJS += connected.o -LIB_OBJS += convert.o -LIB_OBJS += copy.o -LIB_OBJS += credential.o -LIB_OBJS += csum-file.o -LIB_OBJS += ctype.o -LIB_OBJS += date.o -LIB_OBJS += decorate.o -LIB_OBJS += diffcore-break.o -LIB_OBJS += diffcore-delta.o -LIB_OBJS += diffcore-order.o -LIB_OBJS += diffcore-pickaxe.o -LIB_OBJS += diffcore-rename.o -LIB_OBJS += diff-delta.o -LIB_OBJS += diff-lib.o -LIB_OBJS += diff-no-index.o -LIB_OBJS += diff.o -LIB_OBJS += dir.o -LIB_OBJS += editor.
Re: [PATCH v3 2/2] read-cache: plug a few leaks
On Sat, Jun 8, 2013 at 12:22 PM, René Scharfe wrote: > Let's find and fix those leaks by freeing memory in the right places. > Freeing memory just in case in places where we can show that no leak is > triggered by our test suite doesn't help. It helps; it prevents leaks. The real culprit is the bogus API, but I don't see that changing anytime soon, so there are two options when somebody makes a mistake the API allows; leak or don't leak. And you seem to prefer the leak, even though it provides absolutely no advantage. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] read-cache: plug a few leaks
Am 08.06.2013 18:53, schrieb Felipe Contreras: On Sat, Jun 8, 2013 at 10:56 AM, René Scharfe wrote: Am 08.06.2013 16:04, schrieb Felipe Contreras: On Sat, Jun 8, 2013 at 8:22 AM, René Scharfe wrote: Am 08.06.2013 14:15, schrieb Felipe Contreras: Why leave it out? If somebody makes the mistake of doing the above sequence, would you prefer that we leak? Leaking is better than silently cleaning up after a buggy caller because it still allows the underlying bug to be found. No, it doesn't. The pointer is replaced and forever lost. How is leaking memory helping anyone to find the bug? Valgrind can tell you where leaked memory was allocated, but not if you free it in the "wrong" place. It is the correct place to free it. And nobody will ever find it with valgrind, just like nobody has ever tracked down the hundreds of leaks in Git that happen reliably 100% of the time, much less the ones that happen rarely. We could argue about freeing it here or adding a discard_index call somewhere else (which seems more natural to me) once we had a call sequence that actually causes such a leak. The test suite contains none, so I'd say we need more tests first. Lots of the existing leaks were not worth plugging because the process would end right after freeing the memory. Leaving clean-up to the OS was a conscious design choice, simplifying the code. When such code is reused in a library or run multiple times while it was originally meant to be run only a single time (like with cherry-pick --stdin) the original assumption doesn't hold any more and we have a problem. Let's find and fix those leaks by freeing memory in the right places. Freeing memory just in case in places where we can show that no leak is triggered by our test suite doesn't help. René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Sat, Jun 08, 2013 at 08:20:28AM -0500, Felipe Contreras wrote: > > There are a lot of static variables in builtin/ (and outside too), > > which make it non-entrant, or at least not safe. > > So? > > > fork provides a process space isolation, some depend on that. > > Process space isolation from what? Manipulation of global variables. Here are a few examples off the top of my head: Try running "git diff" from your Ruby hook, then try running "git diff-files" within the same process. I believe the latter will start respecting porcelain diff config like diff.mnemonicprefix. To clear state you need to reset a list of global variables back to their initial states (some of which are the BSS-default zero, but some of which are not). Try running "git log" followed by another "git log". The log family of commands does not clear its marks from the commit objects, since it expects to exit after the traversal. The second log will sometimes give wrong answers if its traversal overlaps with the first (e.g., commits marked SEEN or UNINTERESTING that should not be). You can add a call to clear them at the end of the process, but that does not cover any cases where we die(). These are problems that can be solved. But there is a lot of work involved in finding these subtle bugs and coming up with fixes. I think you would be better off working on an implementation of git that was designed from scratch to work in-process, like libgit2. And it even has an actively developed and maintained Ruby binding[1]. libgit2 doesn't have feature parity with regular git yet, but there are many clients based around it that use the library internally for speed, and then exec regular git to fill in the gaps. -Peff [1] https://github.com/libgit2/rugged -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 11:49 AM, Jonathan Nieder wrote: > Duy Nguyen wrote: > >> libgit.a is just a way of grouping a bunch of objects together, not a >> real library and not meant to be. If you aim something more organized, >> please show at least a roadmap what to move where. > > Exactly. There are some rough plans I would like to help with in the > direction of a more organized source tree (so "ls" output can be less > intimidating --- see Nico Pitre's mail on this a while ago for more > hints), but randomly moving files one at a time to builtin/ destroys > consistency and just makes things *worse*. So if you'd like to work > on this, you'll need to start with a description of the endpoint, to > help people work with you to ensure it is something consistent and > usable. So lets stash everything together. --- a/Makefile +++ b/Makefile @@ -990,6 +990,8 @@ BUILTIN_OBJS += builtin/verify-pack.o BUILTIN_OBJS += builtin/verify-tag.o BUILTIN_OBJS += builtin/write-tree.o +LIB_OBJS += $(BUILTIN_OBJS) + GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = @@ -1712,9 +1714,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \ '-DGIT_INFO_PATH="$(infodir_relative_SQ)"' -git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) +git$X: git.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \ - $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) + $(ALL_LDFLAGS) $(LIBS) help.sp help.s help.o: common-cmds.h @@ -1892,7 +1894,7 @@ VCSSVN_OBJS += vcs-svn/svndiff.o VCSSVN_OBJS += vcs-svn/svndump.o TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) -OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ +OBJECTS := $(LIB_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ $(XDIFF_OBJS) \ $(VCSSVN_OBJS) \ git.o And stop any delusions that libgit.a has any meaning at all, and along the way get rid of any hopes of ever having an official public library similar to libgit2. > Actually, Felipe, I doubt that would work well. This project requires > understanding how a variety of people use the git source code, which > requires listening carefully to them and not alienating them so you > can find out what they need. My patch covers every need. Nobody has come forward with a reason not to organize the object files. Everything works after my patch the same way it has worked before. > Someone good at moderating a discussion > could do that on-list, but based on my experience of how threads with > you go, a better strategy might be to cultivate a wiki page somewhere > with a plan and give it some time (a month, maybe) to collect input. This has nothing to do with better strategy, it has everything to do with gut feelings and tradition. Not reasons. > NAK to changing the meaning of builtin/ to "built-in commands, plus > sequencer", which seriously hurts consistency. Then apply the patch above and stop wasting our time with a "library". Git is nothing but a bunch of disorganized object files, all squashed together, there's no library, nor will ever be; libgit.a is a misnomer. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: fix autostash
Ramkumar Ramachandra wrote: > How else am I supposed to write them? If there is a stale state from > the previous test, there isn't too much I can do. Or should I be > cleaning up state at the beginning of each test, instead of at the > end? That's one strategy. "test_when_finished" to restore the set-up state is another. Making tests skippable unless labelled otherwise is currently an aspirational goal rather than a practical one. Hopefully some day we'll get there and the test harness can start checking it. :) It makes reorganizing test scripts, for example by reordering tests, much easier. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] read-cache: plug a few leaks
On Sat, Jun 8, 2013 at 10:56 AM, René Scharfe wrote: > Am 08.06.2013 16:04, schrieb Felipe Contreras: > >> On Sat, Jun 8, 2013 at 8:22 AM, René Scharfe >> wrote: >>> >>> Am 08.06.2013 14:15, schrieb Felipe Contreras: >> >> Why leave it out? If somebody makes the mistake of doing the above sequence, would you prefer that we leak? >>> >>> >>> Leaking is better than silently cleaning up after a buggy caller because >>> it >>> still allows the underlying bug to be found. >> >> >> No, it doesn't. The pointer is replaced and forever lost. How is >> leaking memory helping anyone to find the bug? > > Valgrind can tell you where leaked memory was allocated, but not if you free > it in the "wrong" place. It is the correct place to free it. And nobody will ever find it with valgrind, just like nobody has ever tracked down the hundreds of leaks in Git that happen reliably 100% of the time, much less the ones that happen rarely. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Duy Nguyen wrote: > libgit.a is just a way of grouping a bunch of objects together, not a > real library and not meant to be. If you aim something more organized, > please show at least a roadmap what to move where. Exactly. There are some rough plans I would like to help with in the direction of a more organized source tree (so "ls" output can be less intimidating --- see Nico Pitre's mail on this a while ago for more hints), but randomly moving files one at a time to builtin/ destroys consistency and just makes things *worse*. So if you'd like to work on this, you'll need to start with a description of the endpoint, to help people work with you to ensure it is something consistent and usable. Actually, Felipe, I doubt that would work well. This project requires understanding how a variety of people use the git source code, which requires listening carefully to them and not alienating them so you can find out what they need. Someone good at moderating a discussion could do that on-list, but based on my experience of how threads with you go, a better strategy might be to cultivate a wiki page somewhere with a plan and give it some time (a month, maybe) to collect input. NAK to changing the meaning of builtin/ to "built-in commands, plus sequencer", which seriously hurts consistency. Sincerely, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
