* [PATCH 4/8] wildmatch: support "no FNM_PATHNAME" mode
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
So far, wildmatch() has always honoured directory boundary and there
was no way to turn it off. Make it behave more like fnmatch() by
requiring all callers that want the FNM_PATHNAME behaviour to pass
that in the equivalent flag WM_PATHNAME. Callers that do not specify
WM_PATHNAME will get wildcards like ? and * in their patterns matched
against '/', just like not passing FNM_PATHNAME to fnmatch().
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 2 +-
t/t3070-wildmatch.sh | 27 +++++++++++++++++++++++++++
test-wildmatch.c | 6 ++++--
wildmatch.c | 13 +++++++++----
wildmatch.h | 1 +
5 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/dir.c b/dir.c
index 175a182..6ef0396 100644
--- a/dir.c
+++ b/dir.c
@@ -595,7 +595,7 @@ int match_pathname(const char *pathname, int pathlen,
}
return wildmatch(pattern, name,
- ignore_case ? WM_CASEFOLD : 0,
+ WM_PATHNAME | (ignore_case ? WM_CASEFOLD : 0),
NULL) == 0;
}
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index d5bafef..dbfa903 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -29,6 +29,18 @@ match() {
fi
}
+pathmatch() {
+ if [ $1 = 1 ]; then
+ test_expect_success "pathmatch: match '$2' '$3'" "
+ test-wildmatch pathmatch '$2' '$3'
+ "
+ else
+ test_expect_success "pathmatch: no match '$2' '$3'" "
+ ! test-wildmatch pathmatch '$2' '$3'
+ "
+ fi
+}
+
# Basic wildmat features
match 1 1 foo foo
match 0 0 foo bar
@@ -192,4 +204,19 @@ match 0 0 'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/
match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
+pathmatch 1 foo foo
+pathmatch 0 foo fo
+pathmatch 1 foo/bar foo/bar
+pathmatch 1 foo/bar 'foo/*'
+pathmatch 1 foo/bba/arr 'foo/*'
+pathmatch 1 foo/bba/arr 'foo/**'
+pathmatch 1 foo/bba/arr 'foo*'
+pathmatch 1 foo/bba/arr 'foo**'
+pathmatch 1 foo/bba/arr 'foo/*arr'
+pathmatch 1 foo/bba/arr 'foo/**arr'
+pathmatch 0 foo/bba/arr 'foo/*z'
+pathmatch 0 foo/bba/arr 'foo/**z'
+pathmatch 1 foo/bar 'foo?bar'
+pathmatch 1 foo/bar 'foo[/]bar'
+
test_done
diff --git a/test-wildmatch.c b/test-wildmatch.c
index 4bb23b4..a5f4833 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -12,9 +12,11 @@ int main(int argc, char **argv)
argv[i] += 3;
}
if (!strcmp(argv[1], "wildmatch"))
- return !!wildmatch(argv[3], argv[2], 0, NULL);
+ return !!wildmatch(argv[3], argv[2], WM_PATHNAME, NULL);
else if (!strcmp(argv[1], "iwildmatch"))
- return !!wildmatch(argv[3], argv[2], WM_CASEFOLD, NULL);
+ return !!wildmatch(argv[3], argv[2], WM_PATHNAME | WM_CASEFOLD, NULL);
+ else if (!strcmp(argv[1], "pathmatch"))
+ return !!wildmatch(argv[3], argv[2], 0, NULL);
else if (!strcmp(argv[1], "fnmatch"))
return !!fnmatch(argv[3], argv[2], FNM_PATHNAME);
else
diff --git a/wildmatch.c b/wildmatch.c
index a79f97e..4fe1d65 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -77,14 +77,17 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
continue;
case '?':
/* Match anything but '/'. */
- if (t_ch == '/')
+ if ((flags & WM_PATHNAME) && t_ch == '/')
return WM_NOMATCH;
continue;
case '*':
if (*++p == '*') {
const uchar *prev_p = p - 2;
while (*++p == '*') {}
- if ((prev_p == text || *prev_p == '/') ||
+ if (!(flags & WM_PATHNAME))
+ /* without WM_PATHNAME, '*' == '**' */
+ special = 1;
+ else if ((prev_p == text || *prev_p == '/') ||
(*p == '\0' || *p == '/' ||
(p[0] == '\\' && p[1] == '/'))) {
/*
@@ -103,7 +106,8 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
} else
return WM_ABORT_MALFORMED;
} else
- special = 0;
+ /* without WM_PATHNAME, '*' == '**' */
+ special = flags & WM_PATHNAME ? 0 : 1;
if (*p == '\0') {
/* Trailing "**" matches everything. Trailing "*" matches
* only if there are no more slash characters. */
@@ -214,7 +218,8 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
} else if (t_ch == p_ch)
matched = 1;
} while (prev_ch = p_ch, (p_ch = *++p) != ']');
- if (matched == special || t_ch == '/')
+ if (matched == special ||
+ ((flags & WM_PATHNAME) && t_ch == '/'))
return WM_NOMATCH;
continue;
}
diff --git a/wildmatch.h b/wildmatch.h
index 1c814fd..4090c8f 100644
--- a/wildmatch.h
+++ b/wildmatch.h
@@ -2,6 +2,7 @@
#define WILDMATCH_H
#define WM_CASEFOLD 1
+#define WM_PATHNAME 2
#define WM_ABORT_MALFORMED 2
#define WM_NOMATCH 1
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 2/8] wildmatch: rename constants and update prototype
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
- All exported constants now have a prefix WM_
- Do not rely on FNM_* constants, use the WM_ counterparts
- Remove TRUE and FALSE to follow Git's coding style
- While at it, turn flags type from int to unsigned int
- Add an (unused yet) argument to carry extra information
so that we don't have to change the prototype again later
when we need to pass other stuff to wildmatch
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 3 +-
test-wildmatch.c | 4 +--
wildmatch.c | 86 +++++++++++++++++++++++++++-----------------------------
wildmatch.h | 22 ++++++++++-----
4 files changed, 61 insertions(+), 54 deletions(-)
diff --git a/dir.c b/dir.c
index cb7328b..175a182 100644
--- a/dir.c
+++ b/dir.c
@@ -595,7 +595,8 @@ int match_pathname(const char *pathname, int pathlen,
}
return wildmatch(pattern, name,
- ignore_case ? FNM_CASEFOLD : 0) == 0;
+ ignore_case ? WM_CASEFOLD : 0,
+ NULL) == 0;
}
/* Scan the list and let the last match determine the fate.
diff --git a/test-wildmatch.c b/test-wildmatch.c
index e384c8e..4bb23b4 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -12,9 +12,9 @@ int main(int argc, char **argv)
argv[i] += 3;
}
if (!strcmp(argv[1], "wildmatch"))
- return !!wildmatch(argv[3], argv[2], 0);
+ return !!wildmatch(argv[3], argv[2], 0, NULL);
else if (!strcmp(argv[1], "iwildmatch"))
- return !!wildmatch(argv[3], argv[2], FNM_CASEFOLD);
+ return !!wildmatch(argv[3], argv[2], WM_CASEFOLD, NULL);
else if (!strcmp(argv[1], "fnmatch"))
return !!fnmatch(argv[3], argv[2], FNM_PATHNAME);
else
diff --git a/wildmatch.c b/wildmatch.c
index 3972e26..6ee1b09 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -18,9 +18,6 @@ typedef unsigned char uchar;
#define NEGATE_CLASS '!'
#define NEGATE_CLASS2 '^'
-#define FALSE 0
-#define TRUE 1
-
#define CC_EQ(class, len, litmatch) ((len) == sizeof (litmatch)-1 \
&& *(class) == *(litmatch) \
&& strncmp((char*)class, litmatch, len) == 0)
@@ -63,7 +60,7 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
int matched, special;
uchar t_ch, prev_ch;
if ((t_ch = *text) == '\0' && p_ch != '*')
- return ABORT_ALL;
+ return WM_ABORT_ALL;
if (force_lower_case && ISUPPER(t_ch))
t_ch = tolower(t_ch);
if (force_lower_case && ISUPPER(p_ch))
@@ -76,12 +73,12 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
/* FALLTHROUGH */
default:
if (t_ch != p_ch)
- return NOMATCH;
+ return WM_NOMATCH;
continue;
case '?':
/* Match anything but '/'. */
if (t_ch == '/')
- return NOMATCH;
+ return WM_NOMATCH;
continue;
case '*':
if (*++p == '*') {
@@ -100,33 +97,33 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
* both foo/bar and foo/a/bar.
*/
if (p[0] == '/' &&
- dowild(p + 1, text, force_lower_case) == MATCH)
- return MATCH;
- special = TRUE;
+ dowild(p + 1, text, force_lower_case) == WM_MATCH)
+ return WM_MATCH;
+ special = 1;
} else
- return ABORT_MALFORMED;
+ return WM_ABORT_MALFORMED;
} else
- special = FALSE;
+ special = 0;
if (*p == '\0') {
/* Trailing "**" matches everything. Trailing "*" matches
* only if there are no more slash characters. */
if (!special) {
if (strchr((char*)text, '/') != NULL)
- return NOMATCH;
+ return WM_NOMATCH;
}
- return MATCH;
+ return WM_MATCH;
}
while (1) {
if (t_ch == '\0')
break;
- if ((matched = dowild(p, text, force_lower_case)) != NOMATCH) {
- if (!special || matched != ABORT_TO_STARSTAR)
+ if ((matched = dowild(p, text, force_lower_case)) != WM_NOMATCH) {
+ if (!special || matched != WM_ABORT_TO_STARSTAR)
return matched;
} else if (!special && t_ch == '/')
- return ABORT_TO_STARSTAR;
+ return WM_ABORT_TO_STARSTAR;
t_ch = *++text;
}
- return ABORT_ALL;
+ return WM_ABORT_ALL;
case '[':
p_ch = *++p;
#ifdef NEGATE_CLASS2
@@ -134,101 +131,102 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
p_ch = NEGATE_CLASS;
#endif
/* Assign literal TRUE/FALSE because of "matched" comparison. */
- special = p_ch == NEGATE_CLASS? TRUE : FALSE;
+ special = p_ch == NEGATE_CLASS;
if (special) {
/* Inverted character class. */
p_ch = *++p;
}
prev_ch = 0;
- matched = FALSE;
+ matched = 0;
do {
if (!p_ch)
- return ABORT_ALL;
+ return WM_ABORT_ALL;
if (p_ch == '\\') {
p_ch = *++p;
if (!p_ch)
- return ABORT_ALL;
+ return WM_ABORT_ALL;
if (t_ch == p_ch)
- matched = TRUE;
+ matched = 1;
} else if (p_ch == '-' && prev_ch && p[1] && p[1] != ']') {
p_ch = *++p;
if (p_ch == '\\') {
p_ch = *++p;
if (!p_ch)
- return ABORT_ALL;
+ return WM_ABORT_ALL;
}
if (t_ch <= p_ch && t_ch >= prev_ch)
- matched = TRUE;
+ matched = 1;
p_ch = 0; /* This makes "prev_ch" get set to 0. */
} else if (p_ch == '[' && p[1] == ':') {
const uchar *s;
int i;
for (s = p += 2; (p_ch = *p) && p_ch != ']'; p++) {} /*SHARED ITERATOR*/
if (!p_ch)
- return ABORT_ALL;
+ return WM_ABORT_ALL;
i = p - s - 1;
if (i < 0 || p[-1] != ':') {
/* Didn't find ":]", so treat like a normal set. */
p = s - 2;
p_ch = '[';
if (t_ch == p_ch)
- matched = TRUE;
+ matched = 1;
continue;
}
if (CC_EQ(s,i, "alnum")) {
if (ISALNUM(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "alpha")) {
if (ISALPHA(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "blank")) {
if (ISBLANK(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "cntrl")) {
if (ISCNTRL(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "digit")) {
if (ISDIGIT(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "graph")) {
if (ISGRAPH(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "lower")) {
if (ISLOWER(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "print")) {
if (ISPRINT(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "punct")) {
if (ISPUNCT(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "space")) {
if (ISSPACE(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "upper")) {
if (ISUPPER(t_ch))
- matched = TRUE;
+ matched = 1;
} else if (CC_EQ(s,i, "xdigit")) {
if (ISXDIGIT(t_ch))
- matched = TRUE;
+ matched = 1;
} else /* malformed [:class:] string */
- return ABORT_ALL;
+ return WM_ABORT_ALL;
p_ch = 0; /* This makes "prev_ch" get set to 0. */
} else if (t_ch == p_ch)
- matched = TRUE;
+ matched = 1;
} while (prev_ch = p_ch, (p_ch = *++p) != ']');
if (matched == special || t_ch == '/')
- return NOMATCH;
+ return WM_NOMATCH;
continue;
}
}
- return *text ? NOMATCH : MATCH;
+ return *text ? WM_NOMATCH : WM_MATCH;
}
/* Match the "pattern" against the "text" string. */
-int wildmatch(const char *pattern, const char *text, int flags)
+int wildmatch(const char *pattern, const char *text,
+ unsigned int flags, struct wildopts *wo)
{
return dowild((const uchar*)pattern, (const uchar*)text,
- flags & FNM_CASEFOLD ? 1 :0);
+ flags & WM_CASEFOLD ? 1 :0);
}
diff --git a/wildmatch.h b/wildmatch.h
index 984a38c..1c814fd 100644
--- a/wildmatch.h
+++ b/wildmatch.h
@@ -1,9 +1,17 @@
-/* wildmatch.h */
+#ifndef WILDMATCH_H
+#define WILDMATCH_H
-#define ABORT_MALFORMED 2
-#define NOMATCH 1
-#define MATCH 0
-#define ABORT_ALL -1
-#define ABORT_TO_STARSTAR -2
+#define WM_CASEFOLD 1
-int wildmatch(const char *pattern, const char *text, int flags);
+#define WM_ABORT_MALFORMED 2
+#define WM_NOMATCH 1
+#define WM_MATCH 0
+#define WM_ABORT_ALL -1
+#define WM_ABORT_TO_STARSTAR -2
+
+struct wildopts;
+
+int wildmatch(const char *pattern, const char *text,
+ unsigned int flags,
+ struct wildopts *wo);
+#endif
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 3/8] wildmatch: make dowild() take arbitrary flags
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
wildmatch.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/wildmatch.c b/wildmatch.c
index 6ee1b09..a79f97e 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -52,7 +52,7 @@ typedef unsigned char uchar;
#define ISXDIGIT(c) (ISASCII(c) && isxdigit(c))
/* Match pattern "p" against "text" */
-static int dowild(const uchar *p, const uchar *text, int force_lower_case)
+static int dowild(const uchar *p, const uchar *text, unsigned int flags)
{
uchar p_ch;
@@ -61,9 +61,9 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
uchar t_ch, prev_ch;
if ((t_ch = *text) == '\0' && p_ch != '*')
return WM_ABORT_ALL;
- if (force_lower_case && ISUPPER(t_ch))
+ if ((flags & WM_CASEFOLD) && ISUPPER(t_ch))
t_ch = tolower(t_ch);
- if (force_lower_case && ISUPPER(p_ch))
+ if ((flags & WM_CASEFOLD) && ISUPPER(p_ch))
p_ch = tolower(p_ch);
switch (p_ch) {
case '\\':
@@ -97,7 +97,7 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
* both foo/bar and foo/a/bar.
*/
if (p[0] == '/' &&
- dowild(p + 1, text, force_lower_case) == WM_MATCH)
+ dowild(p + 1, text, flags) == WM_MATCH)
return WM_MATCH;
special = 1;
} else
@@ -116,7 +116,7 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
while (1) {
if (t_ch == '\0')
break;
- if ((matched = dowild(p, text, force_lower_case)) != WM_NOMATCH) {
+ if ((matched = dowild(p, text, flags)) != WM_NOMATCH) {
if (!special || matched != WM_ABORT_TO_STARSTAR)
return matched;
} else if (!special && t_ch == '/')
@@ -227,6 +227,5 @@ static int dowild(const uchar *p, const uchar *text, int force_lower_case)
int wildmatch(const char *pattern, const char *text,
unsigned int flags, struct wildopts *wo)
{
- return dowild((const uchar*)pattern, (const uchar*)text,
- flags & WM_CASEFOLD ? 1 :0);
+ return dowild((const uchar*)pattern, (const uchar*)text, flags);
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 5/8] test-wildmatch: add "perf" command to compare wildmatch and fnmatch
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
It takes a text file, a pattern, a number <n> and pathname flag. Each
line in the text file is matched against the pattern <n> times. If
"pathname" is given, FNM_PATHNAME is used.
test-wildmatch is built with -O2 and tested against glibc 2.14.1 (also
-O2) and compat/fnmatch. The input file is linux-2.6.git file list.
<n> is 2000. The complete command list is at the end.
wildmatch is beaten in the following cases. Apparently it needs some
improvement in FNM_PATHNAME case:
glibc, '*/*/*' with FNM_PATHNAME:
wildmatch 8s 1559us
fnmatch 1s 11877us or 12.65% faster
compat, '*/*/*' with FNM_PATHNAME:
wildmatch 7s 922458us
fnmatch 2s 905111us or 36.67% faster
compat, '*/*/*' without FNM_PATHNAME:
wildmatch 7s 264201us
fnmatch 2s 1897us or 27.56% faster
compat, '[a-z]*/[a-z]*/[a-z]*' with FNM_PATHNAME:
wildmatch 8s 742827us
fnmatch 0s 922943us or 10.56% faster
compat, '[a-z]*/[a-z]*/[a-z]*' without FNM_PATHNAME:
wildmatch 8s 284520us
fnmatch 0s 6936us or 0.08% faster
The rest of glibc numbers
-------------------------
'Documentation/*'
wildmatch 1s 529479us
fnmatch 1s 98263us or 71.81% slower
'drivers/*'
wildmatch 1s 988288us
fnmatch 1s 192049us or 59.95% slower
'Documentation/*' pathname
wildmatch 1s 557507us
fnmatch 1s 93696us or 70.22% slower
'drivers/*' pathname
wildmatch 2s 161626us
fnmatch 1s 230372us or 56.92% slower
'[Dd]ocu[Mn]entation/*'
wildmatch 1s 776581us
fnmatch 1s 471693us or 82.84% slower
'[Dd]o?u[Mn]en?ati?n/*'
wildmatch 1s 770770us
fnmatch 1s 555727us or 87.86% slower
'[Dd]o?u[Mn]en?ati?n/*' pathname
wildmatch 1s 783507us
fnmatch 1s 537029us or 86.18% slower
'[A-Za-z][A-Za-z]??*'
wildmatch 4s 110386us
fnmatch 4s 926306us or 119.85% slower
'[A-Za-z][A-Za-z]??'
wildmatch 3s 918114us
fnmatch 3s 686175us or 94.08% slower
'[A-Za-z][A-Za-z]??*' pathname
wildmatch 4s 453746us
fnmatch 4s 955856us or 111.27% slower
'[A-Za-z][A-Za-z]??' pathname
wildmatch 3s 896646us
fnmatch 3s 733828us or 95.82% slower
'*/*/*'
wildmatch 7s 287985us
fnmatch 1s 74083us or 14.74% slower
'[a-z]*/[a-z]*/[a-z]*' pathname
wildmatch 8s 796659us
fnmatch 1s 568409us or 17.83% slower
'[a-z]*/[a-z]*/[a-z]*'
wildmatch 8s 316559us
fnmatch 3s 430652us or 41.25% slower
The rest of compat numbers
--------------------------
'Documentation/*'
wildmatch 1s 520389us
fnmatch 0s 62579us or 4.12% slower
'drivers/*'
wildmatch 1s 955354us
fnmatch 0s 190109us or 9.72% slower
'Documentation/*' pathname
wildmatch 1s 561675us
fnmatch 0s 55336us or 3.54% slower
'drivers/*' pathname
wildmatch 2s 106100us
fnmatch 0s 219680us or 10.43% slower
'[Dd]ocu[Mn]entation/*'
wildmatch 1s 750810us
fnmatch 0s 542721us or 31.00% slower
'[Dd]o?u[Mn]en?ati?n/*'
wildmatch 1s 724791us
fnmatch 0s 538948us or 31.25% slower
'[Dd]o?u[Mn]en?ati?n/*' pathname
wildmatch 1s 731403us
fnmatch 0s 537474us or 31.04% slower
'[A-Za-z][A-Za-z]??*'
wildmatch 4s 28555us
fnmatch 1s 67297us or 26.49% slower
'[A-Za-z][A-Za-z]??'
wildmatch 3s 838279us
fnmatch 0s 880005us or 22.93% slower
'[A-Za-z][A-Za-z]??*' pathname
wildmatch 4s 379476us
fnmatch 1s 55643us or 24.10% slower
'[A-Za-z][A-Za-z]??' pathname
wildmatch 3s 830910us
fnmatch 0s 849699us or 22.18% slower
The following commands are used:
LANG=C ./test-wildmatch perf /tmp/filelist.txt 'Documentation/*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt 'drivers/*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt 'Documentation/*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt 'drivers/*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[Dd]ocu[Mn]entation/*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[A-Za-z][A-Za-z]??*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[A-Za-z][A-Za-z]??' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[A-Za-z][A-Za-z]??*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[A-Za-z][A-Za-z]??' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '*/*/*' 2000
LANG=C ./test-wildmatch perf /tmp/filelist.txt '*/*/*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[a-z]*/[a-z]*/[a-z]*' 2000 pathname
LANG=C ./test-wildmatch perf /tmp/filelist.txt '[a-z]*/[a-z]*/[a-z]*' 2000
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
test-wildmatch.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/test-wildmatch.c b/test-wildmatch.c
index a5f4833..ac86800 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -1,9 +1,82 @@
#include "cache.h"
#include "wildmatch.h"
+static int perf(int ac, char **av)
+{
+ struct timeval tv1, tv2;
+ struct stat st;
+ int fd, i, n, flags1 = 0, flags2 = 0;
+ char *buffer, *p;
+ uint32_t usec1, usec2;
+ const char *lang;
+ const char *file = av[0];
+ const char *pattern = av[1];
+
+ lang = getenv("LANG");
+ if (lang && strcmp(lang, "C"))
+ die("Please test it on C locale.");
+
+ if ((fd = open(file, O_RDONLY)) == -1 || fstat(fd, &st))
+ die_errno("file open");
+
+ buffer = xmalloc(st.st_size + 2);
+ if (read(fd, buffer, st.st_size) != st.st_size)
+ die_errno("read");
+
+ buffer[st.st_size] = '\0';
+ buffer[st.st_size + 1] = '\0';
+ for (i = 0; i < st.st_size; i++)
+ if (buffer[i] == '\n')
+ buffer[i] = '\0';
+
+ n = atoi(av[2]);
+ if (av[3] && !strcmp(av[3], "pathname")) {
+ flags1 = WM_PATHNAME;
+ flags2 = FNM_PATHNAME;
+ }
+
+ gettimeofday(&tv1, NULL);
+ for (i = 0; i < n; i++) {
+ for (p = buffer; *p; p += strlen(p) + 1)
+ wildmatch(pattern, p, flags1, NULL);
+ }
+ gettimeofday(&tv2, NULL);
+
+ usec1 = (uint32_t)tv2.tv_sec * 1000000 + tv2.tv_usec;
+ usec1 -= (uint32_t)tv1.tv_sec * 1000000 + tv1.tv_usec;
+ printf("wildmatch %ds %dus\n",
+ (int)(usec1 / 1000000),
+ (int)(usec1 % 1000000));
+
+ gettimeofday(&tv1, NULL);
+ for (i = 0; i < n; i++) {
+ for (p = buffer; *p; p += strlen(p) + 1)
+ fnmatch(pattern, p, flags2);
+ }
+ gettimeofday(&tv2, NULL);
+
+ usec2 = (uint32_t)tv2.tv_sec * 1000000 + tv2.tv_usec;
+ usec2 -= (uint32_t)tv1.tv_sec * 1000000 + tv1.tv_usec;
+ if (usec2 > usec1)
+ printf("fnmatch %ds %dus or %.2f%% slower\n",
+ (int)((usec2 - usec1) / 1000000),
+ (int)((usec2 - usec1) % 1000000),
+ (float)(usec2 - usec1) / usec1 * 100);
+ else
+ printf("fnmatch %ds %dus or %.2f%% faster\n",
+ (int)((usec1 - usec2) / 1000000),
+ (int)((usec1 - usec2) % 1000000),
+ (float)(usec1 - usec2) / usec1 * 100);
+ return 0;
+}
+
int main(int argc, char **argv)
{
int i;
+
+ if (!strcmp(argv[1], "perf"))
+ return perf(argc - 2, argv + 2);
+
for (i = 2; i < argc; i++) {
if (argv[i][0] == '/')
die("Forward slash is not allowed at the beginning of the\n"
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 6/8] Makefile: add USE_WILDMATCH to use wildmatch as fnmatch
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
This is similar to NO_FNMATCH but it uses wildmatch instead of
compat/fnmatch. This is an intermediate step to let wildmatch be used
as fnmatch replacement for wider audience before it replaces fnmatch
completely and compat/fnmatch is removed.
fnmatch in test-wildmatch is not impacted by this and is the only
place that NO_FNMATCH or NO_FNMATCH_CASEFOLD remain active when
USE_WILDMATCH is set.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Makefile | 6 ++++++
git-compat-util.h | 13 +++++++++++++
test-wildmatch.c | 3 +++
3 files changed, 22 insertions(+)
diff --git a/Makefile b/Makefile
index bc868d1..24e2774 100644
--- a/Makefile
+++ b/Makefile
@@ -99,6 +99,9 @@ all::
# Define NO_FNMATCH_CASEFOLD if your fnmatch function doesn't have the
# FNM_CASEFOLD GNU extension.
#
+# Define USE_WILDMATCH if you want to use Git's wildmatch
+# implementation as fnmatch
+#
# Define NO_GECOS_IN_PWENT if you don't have pw_gecos in struct passwd
# in the C library.
#
@@ -1625,6 +1628,9 @@ ifdef NO_FNMATCH_CASEFOLD
COMPAT_OBJS += compat/fnmatch/fnmatch.o
endif
endif
+ifdef USE_WILDMATCH
+ COMPAT_CFLAGS += -DUSE_WILDMATCH
+endif
ifdef NO_SETENV
COMPAT_CFLAGS += -DNO_SETENV
COMPAT_OBJS += compat/setenv.o
diff --git a/git-compat-util.h b/git-compat-util.h
index 02f48f6..b2c7638 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -106,7 +106,9 @@
#include <sys/time.h>
#include <time.h>
#include <signal.h>
+#ifndef USE_WILDMATCH
#include <fnmatch.h>
+#endif
#include <assert.h>
#include <regex.h>
#include <utime.h>
@@ -238,6 +240,17 @@ extern char *gitbasename(char *);
#include "compat/bswap.h"
+#ifdef USE_WILDMATCH
+#include "wildmatch.h"
+#define FNM_PATHNAME WM_PATHNAME
+#define FNM_CASEFOLD WM_CASEFOLD
+#define FNM_NOMATCH WM_NOMATCH
+static inline int fnmatch(const char *pattern, const char *string, int flags)
+{
+ return wildmatch(pattern, string, flags, NULL);
+}
+#endif
+
/* General helper functions */
extern void vreportf(const char *prefix, const char *err, va_list params);
extern void vwritef(int fd, const char *prefix, const char *err, va_list params);
diff --git a/test-wildmatch.c b/test-wildmatch.c
index ac86800..a3e2643 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -1,3 +1,6 @@
+#ifdef USE_WILDMATCH
+#undef USE_WILDMATCH /* We need real fnmatch implementation here */
+#endif
#include "cache.h"
#include "wildmatch.h"
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 7/8] wildmatch: make a special case for "*/" with FNM_PATHNAME
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
Normally we need recursion for "*". In this case we know that it
matches everything until "/" so we can skip the recursion.
glibc, '*/*/*' on linux-2.6.git file list 2000 times
before:
wildmatch 8s 74513us
fnmatch 1s 97042us or 13.59% faster
after:
wildmatch 3s 521862us
fnmatch 3s 488616us or 99.06% slower
Same test with compat/fnmatch:
wildmatch 8s 110763us
fnmatch 2s 980845us or 36.75% faster
wildmatch 3s 522156us
fnmatch 1s 544487us or 43.85% slower
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
wildmatch.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/wildmatch.c b/wildmatch.c
index 4fe1d65..3794c4d 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -116,6 +116,18 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
return WM_NOMATCH;
}
return WM_MATCH;
+ } else if (*p == '/' && (flags & WM_PATHNAME) && !special) {
+ /*
+ * an asterisk followed by a slash
+ * with WM_PATHNAME matches the next
+ * directory
+ */
+ const char *slash = strchr((char*)text, '/');
+ if (!slash)
+ return WM_NOMATCH;
+ text = (const uchar*)slash;
+ /* the slash is consumed by the top-level for loop */
+ break;
}
while (1) {
if (t_ch == '\0')
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 8/8] wildmatch: advance faster in <asterisk> + <literal> patterns
From: Nguyễn Thái Ngọc Duy @ 2012-12-22 7:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1356163028-29967-1-git-send-email-pclouds@gmail.com>
compat, '*/*/*' on linux-2.6.git file list 2000 times, before:
wildmatch 7s 985049us
fnmatch 2s 735541us or 34.26% faster
and after:
wildmatch 4s 492549us
fnmatch 0s 888263us or 19.77% slower
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
wildmatch.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/wildmatch.c b/wildmatch.c
index 3794c4d..68b02e4 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -132,6 +132,27 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
while (1) {
if (t_ch == '\0')
break;
+ /*
+ * Try to advance faster when an asterisk is
+ * followed by a literal. We know in this case
+ * that the the string before the literal
+ * must belong to "*".
+ */
+ if (!is_glob_special(*p)) {
+ p_ch = *p;
+ if ((flags & WM_CASEFOLD) && ISUPPER(p_ch))
+ p_ch = tolower(p_ch);
+ while ((t_ch = *text) != '\0' &&
+ (!(flags & WM_PATHNAME) || t_ch != '/')) {
+ if ((flags & WM_CASEFOLD) && ISUPPER(t_ch))
+ t_ch = tolower(t_ch);
+ if (t_ch == p_ch)
+ break;
+ text++;
+ }
+ if (t_ch != p_ch)
+ return WM_NOMATCH;
+ }
if ((matched = dowild(p, text, flags)) != WM_NOMATCH) {
if (!special || matched != WM_ABORT_TO_STARSTAR)
return matched;
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 0/7] format-patch --reroll-count
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
The --reroll-count=$N option, when given a positive integer:
- Adds " v$N" to the subject prefix specified. As the default
subject prefix string is "PATCH", --reroll-count=2 makes it
"PATCH v2".
- Prefixes "v$N-" to the names used for output files. The cover
letter, whose name is usually 0000-cover-letter.patch, becomes
v2-0000-cover-letter.patch when given --reroll-count=2.
This allows users to use the same --output-directory for multiple
iterations of the same series, without letting the output for a
newer round overwrite output files from the earlier rounds. The
user can incorporate materials from earlier rounds to update the
newly minted iteration, and use "send-email v2-*.patch" to send out
the patches belonging to the second iteration easily.
The early patches of this series are all preparatory clean-ups. I
think reopen_stdout() also can be cleaned up, but I'll leave it to
a future reroll.
Junio C Hamano (7):
builtin/log.c: drop unused "numbered" parameter from
make_cover_letter()
builtin/log.c: drop redundant "numbered_files" parameter from
make_cover_letter()
builtin/log.c: stop using global patch_suffix
get_patch_filename(): simplify function signature
get_patch_filename(): drop "just-numbers" hack
get_patch_filename(): split into two functions
format-patch: add --reroll-count=$N option
builtin/log.c | 33 ++++++++++++++++++++++++---------
log-tree.c | 52 +++++++++++++++++++++++++++++++---------------------
log-tree.h | 4 ++--
revision.h | 1 +
4 files changed, 58 insertions(+), 32 deletions(-)
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply
* [PATCH 1/7] builtin/log.c: drop unused "numbered" parameter from make_cover_letter()
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e..28d9063 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -768,7 +768,7 @@ static void add_branch_description(struct strbuf *buf, const char *branch_name)
}
static void make_cover_letter(struct rev_info *rev, int use_stdout,
- int numbered, int numbered_files,
+ int numbered_files,
struct commit *origin,
int nr, struct commit **list, struct commit *head,
const char *branch_name,
@@ -1343,7 +1343,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
if (cover_letter) {
if (thread)
gen_message_id(&rev, "cover");
- make_cover_letter(&rev, use_stdout, numbered, numbered_files,
+ make_cover_letter(&rev, use_stdout, numbered_files,
origin, nr, list, head, branch_name, quiet);
total++;
start_number--;
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 2/7] builtin/log.c: drop redundant "numbered_files" parameter from make_cover_letter()
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 28d9063..f1d086e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -768,7 +768,6 @@ static void add_branch_description(struct strbuf *buf, const char *branch_name)
}
static void make_cover_letter(struct rev_info *rev, int use_stdout,
- int numbered_files,
struct commit *origin,
int nr, struct commit **list, struct commit *head,
const char *branch_name,
@@ -791,7 +790,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
committer = git_committer_info(0);
if (!use_stdout &&
- reopen_stdout(NULL, numbered_files ? NULL : "cover-letter", rev, quiet))
+ reopen_stdout(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet))
return;
log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,
@@ -1045,7 +1044,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
int nr = 0, total, i;
int use_stdout = 0;
int start_number = -1;
- int numbered_files = 0; /* _just_ numbers */
+ int just_numbers = 0;
int ignore_if_in_upstream = 0;
int cover_letter = 0;
int boundary_count = 0;
@@ -1070,7 +1069,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
N_("print patches to standard out")),
OPT_BOOLEAN(0, "cover-letter", &cover_letter,
N_("generate a cover letter")),
- OPT_BOOLEAN(0, "numbered-files", &numbered_files,
+ OPT_BOOLEAN(0, "numbered-files", &just_numbers,
N_("use simple number sequence for output file names")),
OPT_STRING(0, "suffix", &fmt_patch_suffix, N_("sfx"),
N_("use <sfx> instead of '.patch'")),
@@ -1338,12 +1337,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
const char *msgid = clean_message_id(in_reply_to);
string_list_append(rev.ref_message_ids, msgid);
}
- rev.numbered_files = numbered_files;
+ rev.numbered_files = just_numbers;
rev.patch_suffix = fmt_patch_suffix;
if (cover_letter) {
if (thread)
gen_message_id(&rev, "cover");
- make_cover_letter(&rev, use_stdout, numbered_files,
+ make_cover_letter(&rev, use_stdout,
origin, nr, list, head, branch_name, quiet);
total++;
start_number--;
@@ -1390,7 +1389,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
}
if (!use_stdout &&
- reopen_stdout(numbered_files ? NULL : commit, NULL, &rev, quiet))
+ reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
die(_("Failed to create output files"));
shown = log_tree_commit(&rev, commit);
free(commit->buffer);
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 3/7] builtin/log.c: stop using global patch_suffix
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
The suffix for the output filename is found in rev->patch_suffix; do
not keep using the global that is only used to parse the command
line and configuration.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index f1d086e..7cf157e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -673,7 +673,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,
struct rev_info *rev, int quiet)
{
struct strbuf filename = STRBUF_INIT;
- int suffix_len = strlen(fmt_patch_suffix) + 1;
+ int suffix_len = strlen(rev->patch_suffix) + 1;
if (output_directory) {
strbuf_addstr(&filename, output_directory);
@@ -684,7 +684,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,
strbuf_addch(&filename, '/');
}
- get_patch_filename(commit, subject, rev->nr, fmt_patch_suffix, &filename);
+ get_patch_filename(commit, subject, rev->nr, rev->patch_suffix, &filename);
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 4/7] get_patch_filename(): simplify function signature
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
Most functions that emit to a strbuf take the strbuf as their first
parameter; make this function follow suit.
The serial number of the patch being emitted (nr) and suffix used
for patch filename (suffix) are both recorded in rev_info; drop
these separate parameters and pass the rev_info directly.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 2 +-
log-tree.c | 12 ++++++++----
log-tree.h | 5 +++--
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 7cf157e..d9946ec 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -684,7 +684,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,
strbuf_addch(&filename, '/');
}
- get_patch_filename(commit, subject, rev->nr, rev->patch_suffix, &filename);
+ get_patch_filename(&filename, commit, subject, rev);
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
diff --git a/log-tree.c b/log-tree.c
index c894930..ceed6b6 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -299,9 +299,12 @@ static unsigned int digits_in_number(unsigned int number)
return result;
}
-void get_patch_filename(struct commit *commit, const char *subject, int nr,
- const char *suffix, struct strbuf *buf)
+void get_patch_filename(struct strbuf *buf,
+ struct commit *commit, const char *subject,
+ struct rev_info *info)
{
+ const char *suffix = info->patch_suffix;
+ int nr = info->nr;
int suffix_len = strlen(suffix) + 1;
int start_len = buf->len;
@@ -387,8 +390,9 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
mime_boundary_leader, opt->mime_boundary);
extra_headers = subject_buffer;
- get_patch_filename(opt->numbered_files ? NULL : commit, NULL,
- opt->nr, opt->patch_suffix, &filename);
+ get_patch_filename(&filename,
+ opt->numbered_files ? NULL : commit, NULL,
+ opt);
snprintf(buffer, sizeof(buffer) - 1,
"\n--%s%s\n"
"Content-Type: text/x-patch;"
diff --git a/log-tree.h b/log-tree.h
index f5ac238..c6a944a 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -21,7 +21,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
void load_ref_decorations(int flags);
#define FORMAT_PATCH_NAME_MAX 64
-void get_patch_filename(struct commit *commit, const char *subject, int nr,
- const char *suffix, struct strbuf *buf);
+void get_patch_filename(struct strbuf *buf,
+ struct commit *commit, const char *subject,
+ struct rev_info *);
#endif
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 6/7] get_patch_filename(): split into two functions
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
The function switched between two operating modes depending on the
NULL-ness of its two parameters, as a hacky way to share small part
of implementation, sacrificing cleanliness of the API.
Implement "fmt_output_subject()" function that takes a subject
string and gives the name for the output file, and on top of it,
implement "fmt_output_commit()" function that takes a commit and
gives the name for the output file.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 4 +++-
log-tree.c | 41 +++++++++++++++++++++++------------------
log-tree.h | 5 ++---
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 3c6f20a..8cfb4da 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -686,8 +686,10 @@ static int reopen_stdout(struct commit *commit, const char *subject,
if (rev->numbered_files)
strbuf_addf(&filename, "%d", rev->nr);
+ else if (commit)
+ fmt_output_commit(&filename, commit, rev);
else
- get_patch_filename(&filename, commit, subject, rev);
+ fmt_output_subject(&filename, subject, rev);
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
diff --git a/log-tree.c b/log-tree.c
index d9f86ce..670beae 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -299,26 +299,32 @@ static unsigned int digits_in_number(unsigned int number)
return result;
}
-void get_patch_filename(struct strbuf *buf,
- struct commit *commit, const char *subject,
+void fmt_output_subject(struct strbuf *filename,
+ const char *subject,
struct rev_info *info)
{
const char *suffix = info->patch_suffix;
int nr = info->nr;
- int suffix_len = strlen(suffix) + 1;
- int start_len = buf->len;
- int max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
-
- strbuf_addf(buf, "%04d-", nr);
- if (subject)
- strbuf_addstr(buf, subject);
- else if (commit) {
- struct pretty_print_context ctx = {0};
- format_commit_message(commit, "%f", buf, &ctx);
- }
- if (max_len < buf->len)
- strbuf_setlen(buf, max_len);
- strbuf_addstr(buf, suffix);
+ int start_len = filename->len;
+ int max_len = start_len + FORMAT_PATCH_NAME_MAX - (strlen(suffix) + 1);
+
+ strbuf_addf(filename, "%04d-%s", nr, subject);
+
+ if (max_len < filename->len)
+ strbuf_setlen(filename, max_len);
+ strbuf_addstr(filename, suffix);
+}
+
+void fmt_output_commit(struct strbuf *filename,
+ struct commit *commit,
+ struct rev_info *info)
+{
+ struct pretty_print_context ctx = {0};
+ struct strbuf subject = STRBUF_INIT;
+
+ format_commit_message(commit, "%f", &subject, &ctx);
+ fmt_output_subject(filename, subject.buf, info);
+ strbuf_release(&subject);
}
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
@@ -390,8 +396,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
if (opt->numbered_files)
strbuf_addf(&filename, "%d", opt->nr);
else
- get_patch_filename(&filename, commit, NULL,
- opt);
+ fmt_output_commit(&filename, commit, opt);
snprintf(buffer, sizeof(buffer) - 1,
"\n--%s%s\n"
"Content-Type: text/x-patch;"
diff --git a/log-tree.h b/log-tree.h
index c6a944a..9140f48 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -21,8 +21,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
void load_ref_decorations(int flags);
#define FORMAT_PATCH_NAME_MAX 64
-void get_patch_filename(struct strbuf *buf,
- struct commit *commit, const char *subject,
- struct rev_info *);
+void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);
+void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);
#endif
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 7/7] format-patch: add --reroll-count=$N option
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
The --reroll-count=$N option, when given a positive integer:
- Adds " v$N" to the subject prefix specified. As the default
subject prefix string is "PATCH", --reroll-count=2 makes it
"PATCH v2".
- Prefixes "v$N-" to the names used for output files. The cover
letter, whose name is usually 0000-cover-letter.patch, becomes
v2-0000-cover-letter.patch when given --reroll-count=2.
This allows users to use the same --output-directory for multiple
iterations of the same series, without letting the output for a
newer round overwrite output files from the earlier rounds. The
user can incorporate materials from earlier rounds to update the
newly minted iteration, and use "send-email v2-*.patch" to send out
the patches belonging to the second iteration easily.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 11 +++++++++++
log-tree.c | 2 ++
revision.h | 1 +
3 files changed, 14 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index 8cfb4da..e101498 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1061,6 +1061,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
int quiet = 0;
+ int reroll_count = -1;
char *branch_name = NULL;
const struct option builtin_format_patch_options[] = {
{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1080,6 +1081,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
N_("use <sfx> instead of '.patch'")),
OPT_INTEGER(0, "start-number", &start_number,
N_("start numbering patches at <n> instead of 1")),
+ OPT_INTEGER(0, "reroll-count", &reroll_count,
+ N_("mark the series as Nth re-roll")),
{ OPTION_CALLBACK, 0, "subject-prefix", &rev, N_("prefix"),
N_("Use [<prefix>] instead of [PATCH]"),
PARSE_OPT_NONEG, subject_prefix_callback },
@@ -1152,6 +1155,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
PARSE_OPT_KEEP_DASHDASH);
+ if (0 < reroll_count) {
+ struct strbuf sprefix = STRBUF_INIT;
+ strbuf_addf(&sprefix, "%s v%d",
+ rev.subject_prefix, reroll_count);
+ rev.reroll_count = reroll_count;
+ rev.subject_prefix = strbuf_detach(&sprefix, NULL);
+ }
+
if (do_signoff) {
const char *committer;
const char *endpos;
diff --git a/log-tree.c b/log-tree.c
index 670beae..5dc126b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -308,6 +308,8 @@ void fmt_output_subject(struct strbuf *filename,
int start_len = filename->len;
int max_len = start_len + FORMAT_PATCH_NAME_MAX - (strlen(suffix) + 1);
+ if (0 < info->reroll_count)
+ strbuf_addf(filename, "v%d-", info->reroll_count);
strbuf_addf(filename, "%04d-%s", nr, subject);
if (max_len < filename->len)
diff --git a/revision.h b/revision.h
index a95bd0b..e4a912a 100644
--- a/revision.h
+++ b/revision.h
@@ -134,6 +134,7 @@ struct rev_info {
const char *mime_boundary;
const char *patch_suffix;
int numbered_files;
+ int reroll_count;
char *message_id;
struct string_list *ref_message_ids;
const char *add_signoff;
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* [PATCH 5/7] get_patch_filename(): drop "just-numbers" hack
From: Junio C Hamano @ 2012-12-22 8:33 UTC (permalink / raw)
To: git
In-Reply-To: <1356165212-5611-1-git-send-email-gitster@pobox.com>
The function chooses from three operating modes (format using the
subject, the commit, or just number) based on NULL-ness of two of
its parameters, which is an ugly hack for sharing only a bit of
code.
Separate out the "just numbers" part out to the callers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/log.c | 5 ++++-
log-tree.c | 29 ++++++++++++++---------------
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index d9946ec..3c6f20a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -684,7 +684,10 @@ static int reopen_stdout(struct commit *commit, const char *subject,
strbuf_addch(&filename, '/');
}
- get_patch_filename(&filename, commit, subject, rev);
+ if (rev->numbered_files)
+ strbuf_addf(&filename, "%d", rev->nr);
+ else
+ get_patch_filename(&filename, commit, subject, rev);
if (!quiet)
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
diff --git a/log-tree.c b/log-tree.c
index ceed6b6..d9f86ce 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -307,21 +307,18 @@ void get_patch_filename(struct strbuf *buf,
int nr = info->nr;
int suffix_len = strlen(suffix) + 1;
int start_len = buf->len;
+ int max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
- strbuf_addf(buf, commit || subject ? "%04d-" : "%d", nr);
- if (commit || subject) {
- int max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
+ strbuf_addf(buf, "%04d-", nr);
+ if (subject)
+ strbuf_addstr(buf, subject);
+ else if (commit) {
struct pretty_print_context ctx = {0};
-
- if (subject)
- strbuf_addstr(buf, subject);
- else if (commit)
- format_commit_message(commit, "%f", buf, &ctx);
-
- if (max_len < buf->len)
- strbuf_setlen(buf, max_len);
- strbuf_addstr(buf, suffix);
+ format_commit_message(commit, "%f", buf, &ctx);
}
+ if (max_len < buf->len)
+ strbuf_setlen(buf, max_len);
+ strbuf_addstr(buf, suffix);
}
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
@@ -390,9 +387,11 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
mime_boundary_leader, opt->mime_boundary);
extra_headers = subject_buffer;
- get_patch_filename(&filename,
- opt->numbered_files ? NULL : commit, NULL,
- opt);
+ if (opt->numbered_files)
+ strbuf_addf(&filename, "%d", opt->nr);
+ else
+ get_patch_filename(&filename, commit, NULL,
+ opt);
snprintf(buffer, sizeof(buffer) - 1,
"\n--%s%s\n"
"Content-Type: text/x-patch;"
--
1.8.0.6.gd28b5d4.dirty
^ permalink raw reply related
* Re: Pushing symbolic references to remote repositories?
From: Andreas Schwab @ 2012-12-22 9:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dun Peal, Shawn Pearce, Git ML
In-Reply-To: <7vip7vrof6.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I think that the only one and a half sensible use cases that
> unconditionally make sense to update symrefs across repositories are
> to update bare.git/HEAD symref:
>
> - update bare.git/HEAD of a repository that is a local mirror of a
> more authoritative repository with "git fetch --mirror", in which
> case you do want to follow what branch is designated as the
> primary by the project you are mirroring from;
>
> - update bare.git/HEAD from outside by some means to change which
> branch is the primary one for the project. Only because your
> hosting site does not give you an easy way to do so, pushing from
> another repository that happens to point its HEAD at a different
> branch seems to be one plausible way to do so, but that does not
> have to be the only way.
This is not limited to HEAD, any ref may want to be set up as a symref
at a remote repo. For example, I want to set up a symref master ->
trunk at a repository I have no shell access to. Without this I get
spurious error whenever I fetch from that remote (where master and trunk
are separate refs) into a local mirror which does have the symref:
>From git://repo.or.cz/emacs
f0ae89f..5595931 master -> master
error: Ref refs/heads/trunk is at 559593152b9de5a1c144729e0583fa7968abab22 but expected f0ae89f92326beb3f5a19e90c8f4fe0ab6197926
! f0ae89f..5595931 trunk -> trunk (unable to update local ref)
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 3/3] Makefile: use -Wdeclaration-after-statement if supported
From: Adam Spiers @ 2012-12-22 12:25 UTC (permalink / raw)
To: git list
In-Reply-To: <7v8v8xpazq.fsf@alter.siamese.dyndns.org>
On Mon, Dec 17, 2012 at 4:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> OK; I expect these issues with the implementation are all
>> surmountable. I did not necessarily expect this to be the final
>> implementation anyhow, as indicated by my comments below the divider
>> line. However it's not clear to me what you think about the idea in
>> principle, and whether other compiler flags would merit inclusion.
>
> As different versions of GCC behave differently, and the same GCC
> (mis)detect issues differently depending on the optimization level,
> I do not know if it will be a fruitful exercise to try to come up
> with one expression to come up with the set of flags to suit
> everybody.
Fair enough, but let's not allow perfect to become the enemy of good.
Other flags aside, surely enabling -Wdeclaration-after-statement when
it is available is an improvement on the status quo, if it is done in
a way which doesn't damage the current build process? History shows
quite a few instances of other developers falling into the same trap I
did, e.g.
http://search.gmane.org/search.php?group=gmane.comp.version-control.git&query=decl-after-statement
So if the check was automated in the majority of cases (I guess the
majority of developers use gcc), it would mean less review work for
you and fewer re-rolls. If you agree, I will try to rework the patch
so that it doesn't damage the build.
> One flag I prefer to use is -Werror, but that means the
> other flags must have zero false positive rate.
Personally I'm a fan of -Werror too. How frequent are false
positives, and are any of them ever insurmountable?
> If you are interested, the flags I personally use with the version
> of GCC I happen to have is in the Make script on the 'todo' branch.
Thanks for the info.
^ permalink raw reply
* Re: problem with BOINC repository and CR/LF
From: Toralf Förster @ 2012-12-22 12:31 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, Andrew Ardill, git@vger.kernel.org
In-Reply-To: <20121218164132.GC20122@sigill.intra.peff.net>
On 12/18/2012 05:41 PM, Jeff King wrote:
> I could reproduce it, too, on Linux.
>
> The reason it does not always happen is that git will not re-examine the
> file content unless the timestamp on the file is older than what's in
> the index. So it is a race condition for git to see whether the file is
> stat-dirty.
/me still wonders whether this race condition is a feature or an issue
in GIT - b/c it means that 2 different people cloning the same
repository get different results.
>
> -Peff
>
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply
* Re: Re: Re: Change in cvsps maintainership, abd a --fast-export option
From: Heiko Voigt @ 2012-12-22 13:04 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: Michael Haggerty, git
In-Reply-To: <20121222062118.GA31331@thyrsus.com>
Hi,
On Sat, Dec 22, 2012 at 01:21:18AM -0500, Eric S. Raymond wrote:
> Heiko Voigt <hvoigt@hvoigt.net>:
> > Back then when I was converting some repositories to git and I also
> > wrote a quick testsuite for cvsps in an attempt to fix the bugs but gave
> > up. That was the point when I wrote about cvsimports limitations in the
> > documentation.
> >
> > My commits can be found here:
> >
> > http://repo.or.cz/w/cvsps-hv.git
> >
> > I just quickly checked and it seems that it does not run cleanly on a
> > modern Linux anymore. If it is of interest to you I can try to get it
> > running again.
>
> That would be helpful. Please give it some effort.
Here you go. I have pushed my changes on the master branch there.
You should now be able to run my tests with
make test
from the root directory of the repository. The expected and actual
output can be found in the t[0-9]{4}... directories underneath t/.
Cheers Heiko
^ permalink raw reply
* Re: Re: Change in cvsps maintainership, abd a --fast-export option
From: Eric S. Raymond @ 2012-12-22 14:04 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Michael Haggerty, git
In-Reply-To: <20121222034751.GA11635@book-mint>
Heiko Voigt <hvoigt@hvoigt.net>:
> My commits can be found here:
>
> http://repo.or.cz/w/cvsps-hv.git
>
> I just quickly checked and it seems that it does not run cleanly on a
> modern Linux anymore. If it is of interest to you I can try to get it
> running again.
esr@snark:~/WWW/cvsps/fixrepos$ git clone http://repo.or.cz/w/cvsps-hv.git
Cloning into 'cvsps-hv'...
fatal: http://repo.or.cz/w/cvsps-hv.git/info/refs not valid: is this a git repository?
Doesn't seem to be a good day for cloning - I can't get Yann's repo either,
something about HEAD pointing to an invalid reference.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Re: Re: Change in cvsps maintainership, abd a --fast-export option
From: Eric S. Raymond @ 2012-12-22 14:15 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Michael Haggerty, git
In-Reply-To: <20121222130452.GB13549@book-mint>
Heiko Voigt <hvoigt@hvoigt.net>:
> Hi,
>
> On Sat, Dec 22, 2012 at 01:21:18AM -0500, Eric S. Raymond wrote:
> > Heiko Voigt <hvoigt@hvoigt.net>:
> > > Back then when I was converting some repositories to git and I also
> > > wrote a quick testsuite for cvsps in an attempt to fix the bugs but gave
> > > up. That was the point when I wrote about cvsimports limitations in the
> > > documentation.
> > >
> > > My commits can be found here:
> > >
> > > http://repo.or.cz/w/cvsps-hv.git
> > >
> > > I just quickly checked and it seems that it does not run cleanly on a
> > > modern Linux anymore. If it is of interest to you I can try to get it
> > > running again.
> >
> > That would be helpful. Please give it some effort.
>
> Here you go. I have pushed my changes on the master branch there.
>
> You should now be able to run my tests with
>
> make test
>
> from the root directory of the repository. The expected and actual
> output can be found in the t[0-9]{4}... directories underneath t/.
>
> Cheers Heiko
sr@snark:~/WWW/cvsps/fixrepos$ git clone http://repo.or.cz/w/cvsps-hv.git
Cloning into 'cvsps-hv'...
fatal: http://repo.or.cz/w/cvsps-hv.git/info/refs not valid: is this a git repository?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Re: Change in cvsps maintainership, abd a --fast-export option
From: Eric S. Raymond @ 2012-12-22 14:17 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Heiko Voigt, Michael Haggerty, git
In-Reply-To: <CALWbr2zZFq_9qa+pTx3nYn+KFv61LrSMcNM4N1Xs5cmnr8teXg@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com>:
> > esr@snark:~/WWW/cvsps/fixrepos$ git clone http://repo.or.cz/w/cvsps-hv.git
> > Cloning into 'cvsps-hv'...
> > fatal: http://repo.or.cz/w/cvsps-hv.git/info/refs not valid: is this a git repository?
>
> I guess 'w' means write, and you don't have write access. You should use
>
> http://repo.or.cz/r/cvsps-hv.git
>
> instead. It works for me.
OK, that got it. Looking at the tests now.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Re: Change in cvsps maintainership, abd a --fast-export option
From: Antoine Pelisse @ 2012-12-22 14:10 UTC (permalink / raw)
To: esr; +Cc: Heiko Voigt, Michael Haggerty, git
In-Reply-To: <20121222140453.GB375@thyrsus.com>
> esr@snark:~/WWW/cvsps/fixrepos$ git clone http://repo.or.cz/w/cvsps-hv.git
> Cloning into 'cvsps-hv'...
> fatal: http://repo.or.cz/w/cvsps-hv.git/info/refs not valid: is this a git repository?
I guess 'w' means write, and you don't have write access. You should use
http://repo.or.cz/r/cvsps-hv.git
instead. It works for me.
^ permalink raw reply
* Re: Change in cvsps maintainership, abd a --fast-export option
From: Andreas Schwab @ 2012-12-22 14:21 UTC (permalink / raw)
To: esr; +Cc: Heiko Voigt, Michael Haggerty, git
In-Reply-To: <20121222140453.GB375@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> esr@snark:~/WWW/cvsps/fixrepos$ git clone http://repo.or.cz/w/cvsps-hv.git
>From <http://repo.or.cz/w/cvsps-hv.git>:
URL git://repo.or.cz/cvsps-hv.git
http://repo.or.cz/r/cvsps-hv.git
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH] git-subtree: ignore git-subtree executable
From: Michael Schubert @ 2012-12-22 14:45 UTC (permalink / raw)
To: git
Signed-off-by: Michael Schubert <mschub@elegosoft.com>
---
contrib/subtree/.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
index 7e77c9d..91360a3 100644
--- a/contrib/subtree/.gitignore
+++ b/contrib/subtree/.gitignore
@@ -1,4 +1,5 @@
*~
+git-subtree
git-subtree.xml
git-subtree.1
mainline
--
1.8.1.rc2.333.g912e06f.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox