* [PATCH v2 5/6] dir.c: pass pathname length to last_exclude_matching
From: Nguyễn Thái Ngọc Duy @ 2013-03-10 6:14 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1362896070-17456-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 7b6a625..880b5e6 100644
--- a/dir.c
+++ b/dir.c
@@ -764,9 +764,9 @@ int is_excluded_from_list(const char *pathname,
*/
static struct exclude *last_exclude_matching(struct dir_struct *dir,
const char *pathname,
+ int pathlen,
int *dtype_p)
{
- int pathlen = strlen(pathname);
int i, j;
struct exclude_list_group *group;
struct exclude *exclude;
@@ -793,10 +793,12 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
* scans all exclude lists to determine whether pathname is excluded.
* Returns 1 if true, otherwise 0.
*/
-static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+static int is_excluded(struct dir_struct *dir,
+ const char *pathname, int pathlen,
+ int *dtype_p)
{
struct exclude *exclude =
- last_exclude_matching(dir, pathname, dtype_p);
+ last_exclude_matching(dir, pathname, pathlen, dtype_p);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -859,7 +861,8 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
if (ch == '/') {
int dt = DT_DIR;
exclude = last_exclude_matching(check->dir,
- path->buf, &dt);
+ path->buf, path->len,
+ &dt);
if (exclude) {
check->exclude = exclude;
return exclude;
@@ -871,7 +874,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
- return last_exclude_matching(check->dir, name, dtype);
+ return last_exclude_matching(check->dir, name, namelen, dtype);
}
/*
@@ -1249,7 +1252,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = is_excluded(dir, path->buf, &dtype);
+ int exclude = is_excluded(dir, path->buf, path->len, &dtype);
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v2 6/6] exclude: filter patterns by directory level
From: Nguyễn Thái Ngọc Duy @ 2013-03-10 6:14 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1362896070-17456-1-git-send-email-pclouds@gmail.com>
A non-basename pattern that does not contain /**/ can't match anything
outside the attached directory. Record its directory level and avoid
matching unless the pathname is also at the same directory level.
This optimization shines when there are a lot of non-basename patterns
are the root .gitignore and big/deep worktree. Due to the cascading
rule of .gitignore, patterns in the root .gitignore are checked for
_all_ entries in the worktree.
before after
user 0m0.424s 0m0.365s
user 0m0.427s 0m0.366s
user 0m0.432s 0m0.374s
user 0m0.435s 0m0.374s
user 0m0.435s 0m0.377s
user 0m0.437s 0m0.381s
user 0m0.439s 0m0.381s
user 0m0.440s 0m0.383s
user 0m0.450s 0m0.384s
user 0m0.454s 0m0.384s
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
attr.c | 3 ++-
dir.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++------------------
dir.h | 9 ++++++++-
3 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/attr.c b/attr.c
index 1818ba5..7764ddd 100644
--- a/attr.c
+++ b/attr.c
@@ -254,7 +254,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
parse_exclude_pattern(&res->u.pat.pattern,
&res->u.pat.patternlen,
&res->u.pat.flags,
- &res->u.pat.nowildcardlen);
+ &res->u.pat.nowildcardlen,
+ NULL);
if (res->u.pat.flags & EXC_FLAG_MUSTBEDIR)
res->u.pat.patternlen++;
if (res->u.pat.flags & EXC_FLAG_NEGATIVE) {
diff --git a/dir.c b/dir.c
index 880b5e6..de7a6ba 100644
--- a/dir.c
+++ b/dir.c
@@ -360,10 +360,12 @@ static int no_wildcard(const char *string)
void parse_exclude_pattern(const char **pattern,
int *patternlen,
int *flags,
- int *nowildcardlen)
+ int *nowildcardlen,
+ int *dirs_p)
{
const char *p = *pattern;
size_t i, len;
+ int dirs;
*flags = 0;
if (*p == '!') {
@@ -375,12 +377,15 @@ void parse_exclude_pattern(const char **pattern,
len--;
*flags |= EXC_FLAG_MUSTBEDIR;
}
- for (i = 0; i < len; i++) {
+ for (i = 0, dirs = 0; i < len; i++) {
if (p[i] == '/')
- break;
+ dirs++;
}
- if (i == len)
+ if (!dirs)
*flags |= EXC_FLAG_NODIR;
+ else if (*p == '/')
+ dirs--;
+
*nowildcardlen = simple_length(p);
/*
* we should have excluded the trailing slash from 'p' too,
@@ -393,6 +398,8 @@ void parse_exclude_pattern(const char **pattern,
*flags |= EXC_FLAG_ENDSWITH;
*pattern = p;
*patternlen = len;
+ if (dirs_p)
+ *dirs_p = dirs;
}
void add_exclude(const char *string, const char *base,
@@ -402,8 +409,9 @@ void add_exclude(const char *string, const char *base,
int patternlen;
int flags;
int nowildcardlen;
+ int dirs;
- parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen);
+ parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen, &dirs);
if (flags & EXC_FLAG_MUSTBEDIR) {
char *s;
x = xmalloc(sizeof(*x) + patternlen + 1);
@@ -415,11 +423,26 @@ void add_exclude(const char *string, const char *base,
x = xmalloc(sizeof(*x));
x->pattern = string;
}
+ /*
+ * TODO: nowildcardlen < patternlen is a stricter than
+ * necessary mainly to exclude "**" that breaks directory
+ * boundary. Patterns like "/foo-*" should be fine.
+ */
+ if ((flags & EXC_FLAG_NODIR) || nowildcardlen < patternlen)
+ dirs = -1;
+ else {
+ int i;
+ for (i = 0; i < baselen; i++) {
+ if (base[i] == '/')
+ dirs++;
+ }
+ }
x->patternlen = patternlen;
x->nowildcardlen = nowildcardlen;
x->base = base;
x->baselen = baselen;
x->flags = flags;
+ x->dirs = dirs;
x->srcpos = srcpos;
ALLOC_GROW(el->excludes, el->nr + 1, el->alloc);
el->excludes[el->nr++] = x;
@@ -701,7 +724,7 @@ int match_pathname(const char *pathname, int pathlen,
* matched, or NULL for undecided.
*/
static struct exclude *last_exclude_matching_from_list(const char *pathname,
- int pathlen,
+ int pathlen, int dirs,
const char *basename,
int *dtype,
struct exclude_list *el)
@@ -732,6 +755,9 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,
continue;
}
+ if (dirs >= 0 && x->dirs >= 0 && x->dirs != dirs)
+ continue;
+
assert(x->baselen == 0 || x->base[x->baselen - 1] == '/');
if (match_pathname(pathname, pathlen,
x->base, x->baselen ? x->baselen - 1 : 0,
@@ -750,7 +776,8 @@ int is_excluded_from_list(const char *pathname,
struct exclude_list *el)
{
struct exclude *exclude;
- exclude = last_exclude_matching_from_list(pathname, pathlen, basename, dtype, el);
+ exclude = last_exclude_matching_from_list(pathname, pathlen, -1,
+ basename, dtype, el);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return -1; /* undecided */
@@ -765,6 +792,7 @@ int is_excluded_from_list(const char *pathname,
static struct exclude *last_exclude_matching(struct dir_struct *dir,
const char *pathname,
int pathlen,
+ int dirs,
int *dtype_p)
{
int i, j;
@@ -779,8 +807,8 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
group = &dir->exclude_list_group[i];
for (j = group->nr - 1; j >= 0; j--) {
exclude = last_exclude_matching_from_list(
- pathname, pathlen, basename, dtype_p,
- &group->el[j]);
+ pathname, pathlen, dir->dir_level,
+ basename, dtype_p, &group->el[j]);
if (exclude)
return exclude;
}
@@ -794,11 +822,11 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
* Returns 1 if true, otherwise 0.
*/
static int is_excluded(struct dir_struct *dir,
- const char *pathname, int pathlen,
+ const char *pathname, int pathlen, int dirs,
int *dtype_p)
{
struct exclude *exclude =
- last_exclude_matching(dir, pathname, pathlen, dtype_p);
+ last_exclude_matching(dir, pathname, pathlen, dirs, dtype_p);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -862,7 +890,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
int dt = DT_DIR;
exclude = last_exclude_matching(check->dir,
path->buf, path->len,
- &dt);
+ -1, &dt);
if (exclude) {
check->exclude = exclude;
return exclude;
@@ -874,7 +902,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
- return last_exclude_matching(check->dir, name, namelen, dtype);
+ return last_exclude_matching(check->dir, name, namelen, -1, dtype);
}
/*
@@ -1248,11 +1276,11 @@ enum path_treatment {
};
static enum path_treatment treat_one_path(struct dir_struct *dir,
- struct strbuf *path,
+ struct strbuf *path, int dirs,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = is_excluded(dir, path->buf, path->len, &dtype);
+ int exclude = is_excluded(dir, path->buf, path->len, dirs, &dtype);
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
@@ -1310,7 +1338,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,
return path_ignored;
dtype = DTYPE(de);
- return treat_one_path(dir, path, simplify, dtype, de);
+ return treat_one_path(dir, path, -1, simplify, dtype, de);
}
/*
@@ -1338,6 +1366,7 @@ static int read_directory_recursive(struct dir_struct *dir,
if (!fdir)
goto out;
+ dir->dir_level++;
while ((de = readdir(fdir)) != NULL) {
switch (treat_path(dir, de, &path, baselen, simplify)) {
case path_recurse:
@@ -1357,6 +1386,7 @@ static int read_directory_recursive(struct dir_struct *dir,
}
closedir(fdir);
out:
+ dir->dir_level--;
strbuf_release(&path);
return contents;
@@ -1427,7 +1457,7 @@ static int treat_leading_path(struct dir_struct *dir,
break;
if (simplify_away(sb.buf, sb.len, simplify))
break;
- if (treat_one_path(dir, &sb, simplify,
+ if (treat_one_path(dir, &sb, -1, simplify,
DT_DIR, NULL) == path_ignored)
break; /* do not recurse into it */
if (len <= baselen) {
@@ -1447,8 +1477,10 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char
return dir->nr;
simplify = create_simplify(pathspec);
- if (!len || treat_leading_path(dir, path, len, simplify))
+ if (!len || treat_leading_path(dir, path, len, simplify)) {
+ dir->dir_level = -1;
read_directory_recursive(dir, path, len, 0, simplify);
+ }
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);
diff --git a/dir.h b/dir.h
index 560ade4..c434f1c 100644
--- a/dir.h
+++ b/dir.h
@@ -45,6 +45,7 @@ struct exclude_list {
const char *base;
int baselen;
int flags;
+ int dirs;
/*
* Counting starts from 1 for line numbers in ignore files,
@@ -87,6 +88,8 @@ struct dir_struct {
/* Exclude info */
const char *exclude_per_dir;
+ int dir_level;
+
/*
* We maintain three groups of exclude pattern lists:
*
@@ -171,7 +174,11 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir,
extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,
struct exclude_list *el, int check_index);
extern void add_excludes_from_file(struct dir_struct *, const char *fname);
-extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);
+extern void parse_exclude_pattern(const char **string,
+ int *patternlen,
+ int *flags,
+ int *nowildcardlen,
+ int *dirs);
extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, int srcpos);
extern void clear_exclude_list(struct exclude_list *el);
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* Re: [PATCH] format-patch: RFC 2047 says multi-octet character may not be split
From: Kirill Smelkov @ 2013-03-10 7:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dmitry Komissarov, git, Jan H. Schönherr, kirr
In-Reply-To: <7vzjyce6jc.fsf@alter.siamese.dyndns.org>
On Sat, Mar 09, 2013 at 11:07:19AM -0800, Junio C Hamano wrote:
> Kirill Smelkov <kirr@navytux.spb.ru> writes:
> > P.S. sorry for the delay - I harmed my arm yesterday.
>
> Ouch. Take care and be well soon.
Thanks, and thanks fr accepting the patch.
^ permalink raw reply
* Re: Memory corruption when rebasing with git version 1.8.1.5 on arch
From: Jeff King @ 2013-03-10 7:05 UTC (permalink / raw)
To: Bernhard Posselt; +Cc: git
In-Reply-To: <513B14EC.4040504@bernhard-posselt.com>
On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:
> >Also, I can almost reproduce here, as PatrickHeller/core.git is public.
> >However, I suspect the problem is particular to your work built on top,
> >which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
> >Is it possible for you to make that commit available on a temporary
> >branch?
> What do you mean exactly by that?
I just meant to push the work from your local repository somewhere where
I could access it to try to replicate the issue. What you did here:
> git clone https://github.com/Raydiation/memorycorruption
> cd memorycorruption
> git pull --rebase https://github.com/Raydiation/core
...should be plenty. Unfortunately, I'm not able to reproduce the
segfault. All of the patches apply fine, both normally and when run
under valgrind.
> Heres the output of the GIT_TRACE file
> [...]
> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'
This confirms my suspicion that the problem is in "git apply".
You had mentioned before that the valgrind log was very long. If you're
still able to reproduce, could you try running it with valgrind like
this:
valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
git pull --rebase https://github.com/Raydiation/core
Logging to a file instead of stderr should mean we still get output for
commands that are invoked with their stderr redirected (which is the
case for the "git apply" in question), and using "-q" should eliminate
the uninteresting cruft from the log.
-Peff
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Junio C Hamano @ 2013-03-10 7:34 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1362896070-17456-4-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> strncmp is provided length information which could be taken advantage
> by the underlying implementation.
I may be missing something fundamental, but I somehow find the above
does not make any sense.
strcmp(a, b) has to pay attention to NUL in these strings and stop
comparison. strncmp(a, b, n) not only has to pay the same attention
to NUL in the strings, but also needs to stop comparing at n bytes.
In what situation can the latter take advantage of that extra thing
that it needs to keep track of and operate faster, when n is the
length of shorter of the two strings?
> diff --git a/dir.c b/dir.c
> index 9960a37..46b24db 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -610,12 +610,14 @@ int match_basename(const char *basename, int basenamelen,
> int flags)
> {
> if (prefix == patternlen) {
> - if (!strcmp_icase(pattern, basename))
> + if (patternlen == basenamelen &&
> + !strncmp_icase(pattern, basename, patternlen))
> return 1;
What happens if you replace this with
if (patternlen == baselen &&
!strcmp_icase(pattern, basename, patternlen))
and drop the other hunk and run the benchmark again?
> } else if (flags & EXC_FLAG_ENDSWITH) {
> if (patternlen - 1 <= basenamelen &&
> - !strcmp_icase(pattern + 1,
> - basename + basenamelen - patternlen + 1))
> + !strncmp_icase(pattern + 1,
> + basename + basenamelen - patternlen + 1,
> + patternlen - 1))
> return 1;
> } else {
> if (fnmatch_icase(pattern, basename, 0) == 0)
^ permalink raw reply
* Re: [PATCH v2 6/6] exclude: filter patterns by directory level
From: Junio C Hamano @ 2013-03-10 8:20 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1362896070-17456-7-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> A non-basename pattern that does not contain /**/ can't match anything
> outside the attached directory. Record its directory level and avoid
> matching unless the pathname is also at the same directory level.
Without defining what a "directory level" is, the above is a bit
hard to grok, but I think you mean an entry "b/c/*.c" that appears
in "a/.gitignore" file will want to match a path that is directly
in "a/b/c" directory (and not in its subdirectories),
"a/b/x.c" at the two levels deep subdirectory or "a/b/c/d/x.c" that is
four levels deep will never match the pattern.
The logic feels sound.
> diff --git a/dir.c b/dir.c
> index 880b5e6..de7a6ba 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -360,10 +360,12 @@ static int no_wildcard(const char *string)
> void parse_exclude_pattern(const char **pattern,
> int *patternlen,
> int *flags,
> - int *nowildcardlen)
> + int *nowildcardlen,
> + int *dirs_p)
> {
> const char *p = *pattern;
> size_t i, len;
> + int dirs;
>
> *flags = 0;
> if (*p == '!') {
> @@ -375,12 +377,15 @@ void parse_exclude_pattern(const char **pattern,
> len--;
> *flags |= EXC_FLAG_MUSTBEDIR;
> }
> - for (i = 0; i < len; i++) {
> + for (i = 0, dirs = 0; i < len; i++) {
> if (p[i] == '/')
> - break;
> + dirs++;
> }
> - if (i == len)
> + if (!dirs)
> *flags |= EXC_FLAG_NODIR;
> + else if (*p == '/')
> + dirs--;
I presume this is to compensate for a pattern like "/pat" whose
leading slash is only to anchor the pattern at the level. Correct?
> @@ -415,11 +423,26 @@ void add_exclude(const char *string, const char *base,
> x = xmalloc(sizeof(*x));
> x->pattern = string;
> }
> + /*
> + * TODO: nowildcardlen < patternlen is a stricter than
> + * necessary mainly to exclude "**" that breaks directory
> + * boundary. Patterns like "/foo-*" should be fine.
> + */
> + if ((flags & EXC_FLAG_NODIR) || nowildcardlen < patternlen)
> + dirs = -1;
OK, so an entry "README" to match README in any subdirectory will
becomes (dirs < 0) and the matcher below will not short-circuit the
comparison. Good.
> + else {
> + int i;
> + for (i = 0; i < baselen; i++) {
> + if (base[i] == '/')
> + dirs++;
> + }
> + }
> x->patternlen = patternlen;
> x->nowildcardlen = nowildcardlen;
> x->base = base;
> x->baselen = baselen;
> x->flags = flags;
> + x->dirs = dirs;
> x->srcpos = srcpos;
> ALLOC_GROW(el->excludes, el->nr + 1, el->alloc);
> el->excludes[el->nr++] = x;
> @@ -701,7 +724,7 @@ int match_pathname(const char *pathname, int pathlen,
> * matched, or NULL for undecided.
> */
> static struct exclude *last_exclude_matching_from_list(const char *pathname,
> - int pathlen,
> + int pathlen, int dirs,
> const char *basename,
> int *dtype,
> struct exclude_list *el)
> @@ -732,6 +755,9 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,
> continue;
> }
>
> + if (dirs >= 0 && x->dirs >= 0 && x->dirs != dirs)
> + continue;
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Ramkumar Ramachandra @ 2013-03-10 8:23 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, Torsten Bögershausen, Robert Zeh, Git List,
finnag
In-Reply-To: <CACsJy8DZm153Tu_3GTOnxF8bFrYPh7_DP6Rn6rr3n6tfuVuv2Q@mail.gmail.com>
Duy Nguyen wrote:
> On Fri, Mar 8, 2013 at 3:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> The possible options are:
>>> +
>>> - - 'no' - Show no untracked files
>>> + - 'no' - Show no untracked files (this is fastest)
>>
>> There is a trade-off around the use of -uno between safety and
>> performance. The default is not to use -uno so that you will not
>> forget to add a file you newly created (i.e safety). You would pay
>> for the safety with the cost to find such untracked files (i.e.
>> performance).
>>
>> I suspect that the documentation was written with the assumption
>> that at least for the people who are reading this part of the
>> documentation, the trade-off is obvious. In order to find more
>> information, you naturally need to spend more cycles.
>>
>> If the trade-off is not so obvious, however, I do not object at all
>> to describing it. But if we are to do so, I do object to mentioning
>> only one side of the trade-off. People who choose "fastest" needs
>> to be made very aware that they are disabling "safety".
>
> On the topic of trading off, I was thinking about new -uauto as
> default that is like -uall if it takes less than a certan amount of
> time (e.g. 0.5 seconds), if it exceeds that limit, the operation is
> aborted (i.e. it turns to -uno). The safety net is still there, "git
> status" advices to use -u to show full information.
Ugh, this is too opaque; the user has no idea whether untracked files
are being counted or not.
> Or a less intrusive approach: measure the time and advice the user to
> (read doc and) use -uno.
I just learnt about -uno myself, from this thread. At best, it's a
stopgap until we get inotify support.
> But it's probably worth waiting for the first cut of inotify support
> from Ram. It's better with inotify anyway.
This is quite urgent in my opinion. One of git's primary tasks is to
quickly tell me what changed in the repository, and inotify is the
perfect way to do this.
I'll try to get the first cut out quickly, so we can immediately
correct any fundamental design flaws.
^ permalink raw reply
* Fwd: mailmap documentation add grave accents
From: 乙酸鋰 @ 2013-03-10 8:34 UTC (permalink / raw)
To: git
In-Reply-To: <CAHtLG6R2mp0aDpxYr5T9V3opY3=FWHYjqJt8bng4VOzyJDLCbQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 42 bytes --]
Hi,
Here is the patch.
Regards,
ch3cooli
[-- Attachment #2: 0001-mailmap-add-grave-accents-around-mailmap.file-and-ma.patch --]
[-- Type: application/octet-stream, Size: 990 bytes --]
From 3cd5a67fbb15de3ae1b94999f88f4921c0028d4b Mon Sep 17 00:00:00 2001
From: Sup Yut Sum <ch3cooli@gmail.com>
Date: Sun, 10 Mar 2013 15:49:10 +0800
Subject: [PATCH] mailmap: add grave accents around mailmap.file and
mailmap.blob configuration options
Signed-off-by: Sup Yut Sum <ch3cooli@gmail.com>
---
Documentation/mailmap.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/mailmap.txt b/Documentation/mailmap.txt
index 4a8c276..a54393c 100644
--- a/Documentation/mailmap.txt
+++ b/Documentation/mailmap.txt
@@ -1,7 +1,7 @@
If the file `.mailmap` exists at the toplevel of the repository, or at
-the location pointed to by the mailmap.file or mailmap.blob
+the location pointed to by the `mailmap.file` or `mailmap.blob`
configuration options, it
is used to map author and committer names and email addresses to
canonical real names and email addresses.
In the simple form, each line in the file consists of the canonical
--
1.8.1.msysgit.1
^ permalink raw reply related
* Re: pt_BR initializaton
From: Jiang Xin @ 2013-03-10 9:12 UTC (permalink / raw)
To: Harring Figueiredo; +Cc: Git List
In-Reply-To: <CAN_hzmot2vHRYeZWaoh=pbeoG3RNvzsKkUv+9jVs4WUU74tkbg@mail.gmail.com>
Hi Harring,
I'm glad to see that there will be another locale for Git.
I reviewed three commit from you. There are some problems:
Because git has a strict and high standard for commit log, so
1. your commit log must have "Signed-off-by: " line(s).
Commit using "commit -s", or using alias such as
"git config --global alias.ci 'commit -s'" may help.
Such line(s) are signatures of reviewers and contributors for
your commit, and these line(s) are ordered by the time of
contributions.
2. 50+72 rule.
The first line of your commit should no longer than 50
characters, and should not contain characters other than
standard ascii characters. This is because when your
commit save as patch file using git format-patch, first line
of your commit will be used as filename, and as email
subject.
Then a blank line to separate subject and other contents
if there are any other descriptions for your commit.
Other lines in your commit log messages should be
wrapped at line 72 or less, and you can use unicode
characters.
3. Add "l10n:" prefix in subject of your commit log messages.
You can find out how other l10n contributors writing
there commit log message, using this command:
git log -- po/
4. Squash trivial commits before send to upstream.
If you find it is hard to write your commit log message,
it may indicate that you have make too many trivial
commits. In this case, you should squash your commit
before send them to upstream using "git rebase -i
--autosquash" command.
Tips: trivial commit (for backup or fixup previous commit)
may commit using "--fixup" or "--squash" option, such as:
git commit --fixup HEAD
and these commits will be squash automatically when:
git rebase -i --autosquash
There is a script in po-helper branch, and I use it to check
commits from other l10n contributors, and it may also help
for you:
https://github.com/git-l10n/git-po/blob/po-helper/po/po-helper.sh
This discussion may also help:
http://article.gmane.org/gmane.comp.version-control.git/198626
2013/3/10 Harring Figueiredo <harringf@gmail.com>:
>
>
> Hello Jiang,
>
> Please pull the changes from https://github.com/harringf/git-po.git to
> initialize the pt_BR translation.
>
> Thank you,
>
> Harring Figueiredo
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH v2 6/6] exclude: filter patterns by directory level
From: Duy Nguyen @ 2013-03-10 10:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtxojd5u7.fsf@alter.siamese.dyndns.org>
On Sun, Mar 10, 2013 at 3:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> + else if (*p == '/')
>> + dirs--;
>
> I presume this is to compensate for a pattern like "/pat" whose
> leading slash is only to anchor the pattern at the level. Correct?
Yes.
Also for the record, we could cut down the number of prep_exclude
calls significantly by only calling it when we switch directories
(i.e. when read_directory_recursive begins or exits), not calling it
for all entries of the same directory. For instance, path/, path/a,
path/b, path/c/, path/c/d, path/e should only call prep_exclude 3
times when we enter "path", "path/c" and leave "path/c" (rather than 6
times currently). Unfortunately, I see no real time savings by this
call reduction. So no patch. Maybe I'll try it again on my slower
laptop and see if it makes any difference.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-10 10:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5dvd7yq.fsf@alter.siamese.dyndns.org>
On Sun, Mar 10, 2013 at 2:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> strncmp is provided length information which could be taken advantage
>> by the underlying implementation.
>
> I may be missing something fundamental, but I somehow find the above
> does not make any sense.
>
> strcmp(a, b) has to pay attention to NUL in these strings and stop
> comparison. strncmp(a, b, n) not only has to pay the same attention
> to NUL in the strings, but also needs to stop comparing at n bytes.
>
> In what situation can the latter take advantage of that extra thing
> that it needs to keep track of and operate faster, when n is the
> length of shorter of the two strings?
glibc's C strncmp version does 4-byte comparison at a time when n >=4,
then fall back to 1-byte for the rest. I don't know if it's faster
than a plain always 1-byte comparison though. There's also the hand
written assembly version that compares n from 1..16, not exactly sure
how this version works yet.
>> diff --git a/dir.c b/dir.c
>> index 9960a37..46b24db 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -610,12 +610,14 @@ int match_basename(const char *basename, int basenamelen,
>> int flags)
>> {
>> if (prefix == patternlen) {
>> - if (!strcmp_icase(pattern, basename))
>> + if (patternlen == basenamelen &&
>> + !strncmp_icase(pattern, basename, patternlen))
>> return 1;
>
> What happens if you replace this with
>
> if (patternlen == baselen &&
> !strcmp_icase(pattern, basename, patternlen))
>
> and drop the other hunk and run the benchmark again?
>
before after
user 0m0.533s 0m0.522s
user 0m0.549s 0m0.530s
user 0m0.550s 0m0.534s
user 0m0.551s 0m0.545s
user 0m0.556s 0m0.550s
user 0m0.557s 0m0.552s
user 0m0.559s 0m0.554s
user 0m0.564s 0m0.561s
user 0m0.567s 0m0.565s
user 0m0.567s 0m0.565s
--
Duy
^ permalink raw reply
* Re: [PATCH 2/2] shell: new no-interactive-login command to print a custom message
From: Ramkumar Ramachandra @ 2013-03-10 10:49 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ethan Reesor, git, Jeff King, Sitaram Chamarty, Junio C Hamano,
Greg Brockman
In-Reply-To: <20130309220011.GC24777@elie.Belkin>
Jonathan Nieder wrote:
> * If the file ~/git-shell-commands/no-interactive-login exists,
> run no-interactive-login to let the server say what it likes,
> then hang up.
>
> * Otherwise, if ~/git-shell-commands/ is present, start an
> interactive read-eval-print loop.
>
> * Otherwise, print the usual configuration hint and hang up.
Excellent. A way to suppress the ugly warning, and replace it with a
nice message in a non-interactive shell. You've chosen
"no-interactive-login" as the name of this special file, which is
reasonable. I'm not too fond of the name "git-shell-commands" in the
first place, but I suspect it's too late to do anything about it now.
> diff --git a/shell.c b/shell.c
> index 84b237fe..1429870a 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -6,6 +6,7 @@
>
> #define COMMAND_DIR "git-shell-commands"
> #define HELP_COMMAND COMMAND_DIR "/help"
> +#define NOLOGIN_COMMAND COMMAND_DIR "/no-interactive-login"
>
> static int do_generic_cmd(const char *me, char *arg)
> {
> @@ -65,6 +66,18 @@ static void run_shell(void)
> {
> int done = 0;
> static const char *help_argv[] = { HELP_COMMAND, NULL };
> +
> + if (!access(NOLOGIN_COMMAND, F_OK)) {
> + /* Interactive login disabled. */
You're just checking for its existence here, not for execute permissions.
> + const char *argv[] = { NOLOGIN_COMMAND, NULL };
> + int status;
> +
> + status = run_command_v_opt(argv, 0);
If "no-interactive-login" doesn't have execute permissions, we'll get
an error from here:
fatal: cannot exec 'git-shell-commands/no-interactive-login':
Permission denied
Would you like to check that the file has execute permission in
advance to prevent some extra processing (in run_command_v_opt,
start_command and friends) before this message is printed?
Looks good otherwise.
^ permalink raw reply
* Re: [PATCH v2 6/6] exclude: filter patterns by directory level
From: Junio C Hamano @ 2013-03-10 10:58 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <7vtxojd5u7.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> A non-basename pattern that does not contain /**/ can't match anything
>> outside the attached directory. Record its directory level and avoid
>> matching unless the pathname is also at the same directory level.
>
> Without defining what a "directory level" is, the above is a bit
> hard to grok, but I think you mean an entry "b/c/*.c" that appears
> in "a/.gitignore" file will want to match a path that is directly
> in "a/b/c" directory (and not in its subdirectories),
> "a/b/x.c" at the two levels deep subdirectory or "a/b/c/d/x.c" that is
> four levels deep will never match the pattern.
>
> The logic feels sound.
Actually, I think you may be able to do a lot more with a simpler
change. If your top-level .gitignore has "a/b/c/*.c" in it, you
certainly want to mark it not to be applied when you are looking at
paths directly in directory a/b/ because they will never match, but
you also know that nothing will match when you are inside a/b/d/,
even though the pattern and the path you are checking are at the
same levels. Your dirlen approach will fail for that case, no?
The idea behind prep_exclude() that organizes the exclode patterns
into a stack structure and update the groups near the leaves by
popping those for the old directory we were in and pushing those for
the new directory we are going into is to give us a place to tweak
the elements on the whole stack for optimization when we notice that
we are looking at paths in different directories. Instead of giving
a "dirlen" member to each element, you could give a "do not look at
me" flag to it, and when you notice that you were in a/b/c/ and now
you are going to look at paths in a/b/d/, you can look at the group
that was read from the .gitignore from the top-level, and mark
entries that cannot be relevant (e.g. "a/b/c/*.c") as such.
The mark does not have to be a boolean. "a/b/*.c" when you are in
"a/b/c/" can be marked as "This never matches, and I do not have to
re-check until I pop one level". When digging deeper to "a/b/c/d",
you add one to that. When switching to "a/b/e", you would first pop
twice ("d" and then "c"), each time decrementing the "I do not have
to re-check" counter by one, and then when pushing "e" down, you
notice that you need to re-check, and mark it again as "no need to
re-check for one pop". So it is not like you have to re-scan all
entries textually every time you switch directories. Most entries
that are level-limited you would increment or decrement its counter
and only the ones at the level boundary need to be re-checked.
Hmm?
^ permalink raw reply
* Re: Memory corruption when rebasing with git version 1.8.1.5 on arch
From: Bernhard Posselt @ 2013-03-10 11:04 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20130310070505.GA15324@sigill.intra.peff.net>
On 03/10/2013 08:05 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:
>
>>> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
>>> However, I suspect the problem is particular to your work built on top,
>>> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
>>> Is it possible for you to make that commit available on a temporary
>>> branch?
>> What do you mean exactly by that?
> I just meant to push the work from your local repository somewhere where
> I could access it to try to replicate the issue. What you did here:
>
>> git clone https://github.com/Raydiation/memorycorruption
>> cd memorycorruption
>> git pull --rebase https://github.com/Raydiation/core
> ...should be plenty. Unfortunately, I'm not able to reproduce the
> segfault. All of the patches apply fine, both normally and when run
> under valgrind.
>
>> Heres the output of the GIT_TRACE file
>> [...]
>> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'
> This confirms my suspicion that the problem is in "git apply".
>
> You had mentioned before that the valgrind log was very long. If you're
> still able to reproduce, could you try running it with valgrind like
> this:
>
> valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
> git pull --rebase https://github.com/Raydiation/core
>
> Logging to a file instead of stderr should mean we still get output for
> commands that are invoked with their stderr redirected (which is the
> case for the "git apply" in question), and using "-q" should eliminate
> the uninteresting cruft from the log.
>
> -Peff
Do you need debug symbols?
==2395== Invalid write of size 1
==2395== at 0x4C2DB93: memcpy@@GLIBC_2.14 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395== by 0x4076B1: ??? (in /usr/lib/git-core/git)
==2395== by 0x40A60F: ??? (in /usr/lib/git-core/git)
==2395== by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395== by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395== by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395== by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395== by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395== by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395== Address 0x5f245c0 is 0 bytes after a block of size 384 alloc'd
==2395== at 0x4C2C04B: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395== by 0x4C2C2FF: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395== by 0x4F057B: ??? (in /usr/lib/git-core/git)
==2395== by 0x4DDF9F: ??? (in /usr/lib/git-core/git)
==2395== by 0x409E9C: ??? (in /usr/lib/git-core/git)
==2395== by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395== by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395== by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395== by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395== by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395== by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395==
==2395== Invalid read of size 1
==2395== at 0x4C2DCB4: memcpy@@GLIBC_2.14 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395== by 0x40B0D5: ??? (in /usr/lib/git-core/git)
==2395== by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395== by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395== by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395== by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395== by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395== by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395== Address 0x5f245e1 is not stack'd, malloc'd or (recently) free'd
==2395==
^ permalink raw reply
* Re: [PATCH v2 6/6] exclude: filter patterns by directory level
From: Duy Nguyen @ 2013-03-10 11:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwubcyii.fsf@alter.siamese.dyndns.org>
On Sun, Mar 10, 2013 at 5:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>>
>>> A non-basename pattern that does not contain /**/ can't match anything
>>> outside the attached directory. Record its directory level and avoid
>>> matching unless the pathname is also at the same directory level.
>>
>> Without defining what a "directory level" is, the above is a bit
>> hard to grok, but I think you mean an entry "b/c/*.c" that appears
>> in "a/.gitignore" file will want to match a path that is directly
>> in "a/b/c" directory (and not in its subdirectories),
>> "a/b/x.c" at the two levels deep subdirectory or "a/b/c/d/x.c" that is
>> four levels deep will never match the pattern.
>>
>> The logic feels sound.
>
> Actually, I think you may be able to do a lot more with a simpler
> change. If your top-level .gitignore has "a/b/c/*.c" in it, you
> certainly want to mark it not to be applied when you are looking at
> paths directly in directory a/b/ because they will never match, but
> you also know that nothing will match when you are inside a/b/d/,
> even though the pattern and the path you are checking are at the
> same levels. Your dirlen approach will fail for that case, no?
>
> The idea behind prep_exclude() that organizes the exclode patterns
> into a stack structure and update the groups near the leaves by
> popping those for the old directory we were in and pushing those for
> the new directory we are going into is to give us a place to tweak
> the elements on the whole stack for optimization when we notice that
> we are looking at paths in different directories. Instead of giving
> a "dirlen" member to each element, you could give a "do not look at
> me" flag to it, and when you notice that you were in a/b/c/ and now
> you are going to look at paths in a/b/d/, you can look at the group
> that was read from the .gitignore from the top-level, and mark
> entries that cannot be relevant (e.g. "a/b/c/*.c") as such.
>
> The mark does not have to be a boolean. "a/b/*.c" when you are in
> "a/b/c/" can be marked as "This never matches, and I do not have to
> re-check until I pop one level". When digging deeper to "a/b/c/d",
> you add one to that. When switching to "a/b/e", you would first pop
> twice ("d" and then "c"), each time decrementing the "I do not have
> to re-check" counter by one, and then when pushing "e" down, you
> notice that you need to re-check, and mark it again as "no need to
> re-check for one pop". So it is not like you have to re-scan all
> entries textually every time you switch directories. Most entries
> that are level-limited you would increment or decrement its counter
> and only the ones at the level boundary need to be re-checked.
A bit confused by "dirlen" (what is it?). I think what you're trying
to say is "mark whether a pattern is applicable for entries in this
directory in prep_exclude, update the marks as we push and pop
directories". It does not sound simpler (and it's actually more
powerful, as you said it could avoid checking "a/b/c/*.c" when
standing in a/b/d). I'll give it a try.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Antoine Pelisse @ 2013-03-10 11:43 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8A_4SqLu5L6P0PJ78Lwy12fjL7T2p-KbVEVLJmKNqhyRw@mail.gmail.com>
On Sun, Mar 10, 2013 at 11:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> glibc's C strncmp version does 4-byte comparison at a time when n >=4,
> then fall back to 1-byte for the rest.
Looking at this
(http://fossies.org/dox/glibc-2.17/strncmp_8c_source.html), it's not
exactly true.
It would rather be while (n >= 4), manually unroll the loop.
^ permalink raw reply
* Re: Memory corruption when rebasing with git version 1.8.1.5 on arch
From: Bernhard Posselt @ 2013-03-10 11:45 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20130310070505.GA15324@sigill.intra.peff.net>
On 03/10/2013 08:05 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:
>
>>> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
>>> However, I suspect the problem is particular to your work built on top,
>>> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
>>> Is it possible for you to make that commit available on a temporary
>>> branch?
>> What do you mean exactly by that?
> I just meant to push the work from your local repository somewhere where
> I could access it to try to replicate the issue. What you did here:
>
>> git clone https://github.com/Raydiation/memorycorruption
>> cd memorycorruption
>> git pull --rebase https://github.com/Raydiation/core
> ...should be plenty. Unfortunately, I'm not able to reproduce the
> segfault. All of the patches apply fine, both normally and when run
> under valgrind.
>
>> Heres the output of the GIT_TRACE file
>> [...]
>> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'
> This confirms my suspicion that the problem is in "git apply".
>
> You had mentioned before that the valgrind log was very long. If you're
> still able to reproduce, could you try running it with valgrind like
> this:
>
> valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
> git pull --rebase https://github.com/Raydiation/core
>
> Logging to a file instead of stderr should mean we still get output for
> commands that are invoked with their stderr redirected (which is the
> case for the "git apply" in question), and using "-q" should eliminate
> the uninteresting cruft from the log.
>
> -Peff
First time I've used Archlinux ABS and build from source :)
The log file was empty and it seemed to apply everything nice when
running valgrind. When i tried to run it without valgrind it failed with
memory corruption.
Heres the output with debug symbols, fetched with tail -f:
==22291== Invalid write of size 1
==22291== at 0x4C2DB93: memcpy@@GLIBC_2.14 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291== by 0x4076B1: update_pre_post_images (in /usr/lib/git-core/git)
==22291== by 0x40A60F: apply_fragments (in /usr/lib/git-core/git)
==22291== by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291== by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291== by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291== by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291== by 0x404DD1: main (in /usr/lib/git-core/git)
==22291== Address 0x5f245c0 is 0 bytes after a block of size 384 alloc'd
==22291== at 0x4C2C04B: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291== by 0x4C2C2FF: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291== by 0x4F057B: xrealloc (in /usr/lib/git-core/git)
==22291== by 0x4DDF9F: strbuf_grow (in /usr/lib/git-core/git)
==22291== by 0x409E9C: apply_fragments (in /usr/lib/git-core/git)
==22291== by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291== by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291== by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291== by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291== by 0x404DD1: main (in /usr/lib/git-core/git)
==22291==
==22291== Invalid read of size 1
==22291== at 0x4C2DCB4: memcpy@@GLIBC_2.14 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291== by 0x40B0D5: apply_fragments (in /usr/lib/git-core/git)
==22291== by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291== by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291== by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291== by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291== by 0x404DD1: main (in /usr/lib/git-core/git)
==22291== Address 0x5f245e1 is not stack'd, malloc'd or (recently) free'd
==22291==
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Antoine Pelisse @ 2013-03-10 11:54 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git
In-Reply-To: <CALWbr2wEJy0p2hcFK_rLtA98koeacE8rS2T=9P130GUFjWKc0Q@mail.gmail.com>
On Sun, Mar 10, 2013 at 12:43 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> On Sun, Mar 10, 2013 at 11:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> glibc's C strncmp version does 4-byte comparison at a time when n >=4,
>> then fall back to 1-byte for the rest.
>
> Looking at this
> (http://fossies.org/dox/glibc-2.17/strncmp_8c_source.html), it's not
> exactly true.
>
> It would rather be while (n >= 4), manually unroll the loop.
By the way, if we know the length of the string, we could use memcmp.
This one is allowed to compare 4-bytes at a time (he doesn't care
about end of string). This is true because the value of the length
parameter is no longer "at most".
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-10 12:06 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Junio C Hamano, git
In-Reply-To: <CALWbr2x6V6TB9g_nQCgG2r9L__a2wxG28Qi5KTXChHxj5JSQ8w@mail.gmail.com>
On Sun, Mar 10, 2013 at 6:54 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> On Sun, Mar 10, 2013 at 12:43 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> On Sun, Mar 10, 2013 at 11:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> glibc's C strncmp version does 4-byte comparison at a time when n >=4,
>>> then fall back to 1-byte for the rest.
>>
>> Looking at this
>> (http://fossies.org/dox/glibc-2.17/strncmp_8c_source.html), it's not
>> exactly true.
>>
>> It would rather be while (n >= 4), manually unroll the loop.
>
> By the way, if we know the length of the string, we could use memcmp.
> This one is allowed to compare 4-bytes at a time (he doesn't care
> about end of string). This is true because the value of the length
> parameter is no longer "at most".
We still need to worry about access violation after NUL when two
strings have different lengths. That could be avoided in this
particular case, but I think it's too fragile.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Antoine Pelisse @ 2013-03-10 12:11 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8A4+2t=PMJ+iSFFz-fafkAHYGvm6G4M-qpiO9674sanEQ@mail.gmail.com>
>> By the way, if we know the length of the string, we could use memcmp.
>> This one is allowed to compare 4-bytes at a time (he doesn't care
>> about end of string). This is true because the value of the length
>> parameter is no longer "at most".
>
> We still need to worry about access violation after NUL when two
> strings have different lengths. That could be avoided in this
> particular case, but I think it's too fragile.
Why would we need to compare if the strings don't have the same length
? We already do that in combine-diff.c:append_lost().
^ permalink raw reply
* Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-10 12:14 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Junio C Hamano, git
In-Reply-To: <CALWbr2w+HdsOJAXF3974=vx+BtxgCC=bKHetE=ptXTwv7_L0pg@mail.gmail.com>
On Sun, Mar 10, 2013 at 7:11 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>>> By the way, if we know the length of the string, we could use memcmp.
>>> This one is allowed to compare 4-bytes at a time (he doesn't care
>>> about end of string). This is true because the value of the length
>>> parameter is no longer "at most".
>>
>> We still need to worry about access violation after NUL when two
>> strings have different lengths. That could be avoided in this
>> particular case, but I think it's too fragile.
>
> Why would we need to compare if the strings don't have the same length
> ? We already do that in combine-diff.c:append_lost().
Watching movie and replying to git@ don't mix. You're right we don't
need to compare if lengths are different. What was I thinking..
--
Duy
^ permalink raw reply
* Re: rebase: strange failures to apply patc 3-way
From: Max Horn @ 2013-03-10 13:22 UTC (permalink / raw)
To: Andrew Wong; +Cc: git@vger.kernel.org
In-Reply-To: <513B8037.7060107@gmail.com>
Sorry for taking so long to reply... :-/
On 09.03.2013, at 19:32, Andrew Wong wrote:
> On 03/09/13 06:26, Max Horn wrote:
>> It tends to fail in separate places, but eventually "stabilizes". E.g. I just did a couple test rebases, and it failed twice in commit 14, then the third time in commit 15 (which underlines once more that the failures are inappropriate).
>>
>> The fourth time, something new and weird happened:
>>
>> $ git rebase --abort
>> $ git rebase NEW-PARENT
>> Cannot rebase: You have unstaged changes.
>> Please commit or stash them.
>> $
>>
>> This is quite suspicious. It appears that git for some reason things a file is dirty when it isn't. That could explain the other rebase failures too, couldn't it? But what might cause such a thing?
> Yea, that's really suspicious. This could mean there's an issue with
> when git is checking the index. Try running these a couple times in a
> clean work tree:
> $ git update-index --refresh
> $ git diff-files
>
> In a clean work tree, these commands should print nothing. But in your
> case, these might print random files that git thinks have been modified...
I did run
touch lib/*.* src/*.* && git update-index --refresh && git diff-files
a couple dozen times (the "problematic" files where in src/ and lib), but nothing happened. I just re-checked, and the rebase still fails in the same why...
Perhaps I should add some printfs into the git source to figure out what exactly it thinks is not right about those files... i.e. how does it come to the conclusion that I have local changes, exactly. I don't know how Git does that -- does it take the mtime from (l)stat into account? Perhaps problems with my machine's clock could be responsible?
>
> If the commands do print out some files, check the timestamp from the
> git index and the filesystem:
> $ git ls-files --debug file1 file2
> $ stat -f "%N %m %c" file1 file2
>
> Is this repo on a network drive? Or is it local drive in your Mac?
Local (some more details also described in my first email on this thread, but I'll happily provide more data if I can).
Thanks again,
Max
^ permalink raw reply
* [PATCH] Translate git_more_info_string consistently
From: Kevin Bracey @ 2013-03-10 15:10 UTC (permalink / raw)
To: git; +Cc: Kevin Bracey
"git help" translated the "See 'git help <command>' for more
information..." message, but "git" didn't.
Signed-off-by: Kevin Bracey <kevin@bracey.fi>
---
git.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git.c b/git.c
index d33f9b3..e484644 100644
--- a/git.c
+++ b/git.c
@@ -536,7 +536,7 @@ int main(int argc, const char **argv)
commit_pager_choice();
printf("usage: %s\n\n", git_usage_string);
list_common_cmds_help();
- printf("\n%s\n", git_more_info_string);
+ printf("\n%s\n", _(git_more_info_string));
exit(1);
}
cmd = argv[0];
--
1.8.2.rc3.8.g96befb6.dirty
^ permalink raw reply related
* Re: [PATCH 1/2] require pathspec for "git add -u/-A"
From: Matthieu Moy @ 2013-03-10 15:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <1362786889-28688-2-git-send-email-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without
> pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec
> in a subdirectory will stop working sometime before Git 2.0, to wean
> users off of the old default, in preparation for adopting the new
> default in Git 2.0.
I originally thought this step was necessary, but I changed my mind. The
warning is big enough and doesn't need to be turned into an error.
If this step is applied, it should be applied at 2.0, not before, as
this is the big incompatible change. Re-introducing a new behavior won't
harm users OTOH.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 4/4] teach config parsing to read from strbuf
From: Heiko Voigt @ 2013-03-10 16:39 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, git, Jens Lehmann, Jeff King
In-Reply-To: <5138DFA3.8040308@ramsay1.demon.co.uk>
Hi,
On Thu, Mar 07, 2013 at 06:42:43PM +0000, Ramsay Jones wrote:
> Heiko Voigt wrote:
> > +int git_config_from_strbuf(config_fn_t fn, struct strbuf *strbuf, void *data)
> > +{
> > + struct config top;
> > + struct config_strbuf str;
> > +
> > + str.strbuf = strbuf;
> > + str.pos = 0;
> > +
> > + top.data = &str;
>
> You will definitely want to initialise 'top.name' here, rather
> than let it take whatever value happens to be at that position
> on the stack. In your editor, search for 'cf->name' and contemplate
> each such occurrence.
Good catch, thanks. The initialization seems to got lost during
refactoring. In the codepaths we call with the new strbuf function it is
only used for error reporting so I think we need to get the name from
the user of this function so the error messages are useful.
I have extended the test to demonstrate how I imagine this name could
look like.
> Does the 'include' facility work from a strbuf? Should it?
No the 'include' facility does not work here. A special handling would
need to be implemented by the caller. For us 'include' values have no
special meaning and are parsed like normal values.
AFAICS when a config file is given to git config we do not currently
respect includes. So we can just do the same behavior here for the
moment. There is no roadblock against adding it later.
> Are you happy with the error handling/reporting?
Good point, while adding the name for strbuf I noticed that we currently
die on some parsing errors. We should probably make these errors
handleable for strbufs. Currently the main usecase is to read submodule
configurations from the database and in case someone commits a broken
configuration we should be able to continue with that. Otherwise the
repository might render into an unuseable state. I will look into that.
> Do the above additions to the test-suite give you confidence
> that the code works as you intend?
Well, I am reusing most of the existing infrastructure which I assume is
tested using config files. So what I want to test here is the
integration of this new function. As you mentioned the name variable I
have now added a test for the parsing error case as well. I have
refactored the test binary to read configs from stdin so its easiert to
implement further tests from the bash part of the testsuite.
I will send out another iteration shortly.
Cheers Heiko
^ permalink raw reply
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