Re: [PATCH] Make !pattern in .gitattributes non-fatal
Duy Nguyen pclo...@gmail.com writes: This return NULL; means we ignore !blah pattern, which is a regression, isn't it? Should we treat '!' as literal here? Probably not. Can you point to a project everybody has heard of that keeps track of a path that begins with an exclamation point? With clarification to the documentation that says you cannot use ! to negate and your die on such an entry, we have been going in the direction that forbids the use of such an entry, and making it mean literal exclamation point is going sideways in the middle of the road. Besides, if you want to match a path that begins with an exclam, you can always say [!], no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Make !pattern in .gitattributes non-fatal
Before 82dce99 (attr: more matching optimizations from .gitignore,
2012-10-15), .gitattributes did not have any special treatment of a
leading '!'. The docs, however, always said
The rules how the pattern matches paths are the same as in
`.gitignore` files; see linkgit:gitignore[5].
By those rules, leading '!' means pattern negation. So 82dce99
correctly determined that this kind of line makes no sense and should
be disallowed.
However, users who actually had a rule for files starting with a '!'
are in a bad position: before 82dce99 '!' matched that literal
character, so it is conceivable that users have .gitattributes with
such lines in them. After 82dce99 the unescaped version was
disallowed in such a way that git outright refuses to run(!) most
commands in the presence of such a .gitattributes. It therefore
becomes very hard to fix, let alone work with, such repositories.
Let's at least allow the users to fix their repos: change the fatal
error into a warning.
Reported-by: maths...@gmail.com
Signed-off-by: Thomas Rast tr...@student.ethz.ch
---
attr.c| 8 +---
t/t0003-attributes.sh | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/attr.c b/attr.c
index 4657cc2..e2f9377 100644
--- a/attr.c
+++ b/attr.c
@@ -255,9 +255,11 @@ static struct match_attr *parse_attr_line(const char
*line, const char *src,
res-u.pat.patternlen,
res-u.pat.flags,
res-u.pat.nowildcardlen);
- if (res-u.pat.flags EXC_FLAG_NEGATIVE)
- die(_(Negative patterns are forbidden in git
attributes\n
- Use '\\!' for literal leading exclamation.));
+ if (res-u.pat.flags EXC_FLAG_NEGATIVE) {
+ warning(_(Negative patterns are ignored in git
attributes\n
+ Use '\\!' for literal leading
exclamation.));
+ return NULL;
+ }
}
res-is_macro = is_macro;
res-num_attr = num_attr;
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 43b2513..0b98b6f 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -198,7 +198,8 @@ test_expect_success 'root subdir attribute test' '
test_expect_success 'negative patterns' '
echo !f test=bar .gitattributes
- test_must_fail git check-attr test -- f
+ git check-attr test -- '''!f''' 2errors
+ test_i18ngrep Negative patterns are ignored errors
'
test_expect_success 'patterns starting with exclamation' '
--
1.8.2.rc1.392.gf57d6b8.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make !pattern in .gitattributes non-fatal
Thomas Rast tr...@student.ethz.ch writes: Before 82dce99 (attr: more matching optimizations from .gitignore, 2012-10-15), .gitattributes did not have any special treatment of a leading '!'. The docs, however, always said The rules how the pattern matches paths are the same as in `.gitignore` files; see linkgit:gitignore[5]. By those rules, leading '!' means pattern negation. So 82dce99 correctly determined that this kind of line makes no sense and should be disallowed. However, users who actually had a rule for files starting with a '!' are in a bad position: before 82dce99 '!' matched that literal character, so it is conceivable that users have .gitattributes with such lines in them. After 82dce99 the unescaped version was disallowed in such a way that git outright refuses to run(!) most commands in the presence of such a .gitattributes. It therefore becomes very hard to fix, let alone work with, such repositories. Fixing the working tree is easy, but when we read from a history that already records such an entry in an attribute file, it would become somewhat cumbersome. I wouldn't use very hard to fix to describe such a case. But the demotion to warning does make sense; let's do that in v1.8.1.5. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make !pattern in .gitattributes non-fatal
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@student.ethz.ch writes:
Before 82dce99 (attr: more matching optimizations from .gitignore,
2012-10-15), .gitattributes did not have any special treatment of a
leading '!'. The docs, however, always said
The rules how the pattern matches paths are the same as in
`.gitignore` files; see linkgit:gitignore[5].
By those rules, leading '!' means pattern negation. So 82dce99
correctly determined that this kind of line makes no sense and should
be disallowed.
However, users who actually had a rule for files starting with a '!'
are in a bad position: before 82dce99 '!' matched that literal
character, so it is conceivable that users have .gitattributes with
such lines in them. After 82dce99 the unescaped version was
disallowed in such a way that git outright refuses to run(!) most
commands in the presence of such a .gitattributes. It therefore
becomes very hard to fix, let alone work with, such repositories.
Fixing the working tree is easy, but when we read from a history
that already records such an entry in an attribute file, it would
become somewhat cumbersome. I wouldn't use very hard to fix to
describe such a case.
Well, I'm sorry if I hurt any feelings there, but...
~/tmp/badattr(master)$ git show bad:.gitattributes
!bad text
~/tmp/badattr(master)$ ~/g/git-checkout bad # a git without my patch
fatal: Negative patterns are forbidden in git attributes
Use '\!' for literal leading exclamation.
~/tmp/badattr(master)$ git status
# On branch master
nothing to commit (use -u to show untracked files)
Notice how it remains on master. I suppose with enough knowledge of the
internals I could manage, but after seeing how hard it was to *build*
such broken history with a git that dies, I don't really want to try.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make !pattern in .gitattributes non-fatal
Thomas Rast tr...@student.ethz.ch writes: Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@student.ethz.ch writes: Before 82dce99 (attr: more matching optimizations from .gitignore, 2012-10-15), .gitattributes did not have any special treatment of a leading '!'. The docs, however, always said The rules how the pattern matches paths are the same as in `.gitignore` files; see linkgit:gitignore[5]. By those rules, leading '!' means pattern negation. So 82dce99 correctly determined that this kind of line makes no sense and should be disallowed. However, users who actually had a rule for files starting with a '!' are in a bad position: before 82dce99 '!' matched that literal character, so it is conceivable that users have .gitattributes with such lines in them. After 82dce99 the unescaped version was disallowed in such a way that git outright refuses to run(!) most commands in the presence of such a .gitattributes. It therefore becomes very hard to fix, let alone work with, such repositories. Fixing the working tree is easy, but when we read from a history that already records such an entry in an attribute file, it would become somewhat cumbersome. I wouldn't use very hard to fix to describe such a case. Well, I'm sorry if I hurt any feelings there, but... ~/tmp/badattr(master)$ git show bad:.gitattributes !bad text ~/tmp/badattr(master)$ ~/g/git-checkout bad # a git without my patch fatal: Negative patterns are forbidden in git attributes Use '\!' for literal leading exclamation. ~/tmp/badattr(master)$ git status # On branch master nothing to commit (use -u to show untracked files) Notice how it remains on master. I suppose with enough knowledge of the internals I could manage,... That would have been nicer to have in the log message. In any case, 1.8.1.5 will go out with this fix (this is v1.8.1 regression IIUC) this afternoon. Thanks for a fix. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make !pattern in .gitattributes non-fatal
On Sat, Mar 2, 2013 at 3:06 AM, Thomas Rast tr...@student.ethz.ch wrote:
Before 82dce99 (attr: more matching optimizations from .gitignore,
2012-10-15), .gitattributes did not have any special treatment of a
leading '!'. The docs, however, always said
The rules how the pattern matches paths are the same as in
`.gitignore` files; see linkgit:gitignore[5].
By those rules, leading '!' means pattern negation. So 82dce99
correctly determined that this kind of line makes no sense and should
be disallowed.
However, users who actually had a rule for files starting with a '!'
are in a bad position: before 82dce99 '!' matched that literal
character, so it is conceivable that users have .gitattributes with
such lines in them. After 82dce99 the unescaped version was
disallowed in such a way that git outright refuses to run(!) most
commands in the presence of such a .gitattributes. It therefore
becomes very hard to fix, let alone work with, such repositories.
Let's at least allow the users to fix their repos: change the fatal
error into a warning.
I agree we should not break existing .gitattributes. So yes, die()ing
here sounds bad. But..
@@ -255,9 +255,11 @@ static struct match_attr *parse_attr_line(const char
*line, const char *src,
res-u.pat.patternlen,
res-u.pat.flags,
res-u.pat.nowildcardlen);
- if (res-u.pat.flags EXC_FLAG_NEGATIVE)
- die(_(Negative patterns are forbidden in git
attributes\n
- Use '\\!' for literal leading exclamation.));
+ if (res-u.pat.flags EXC_FLAG_NEGATIVE) {
+ warning(_(Negative patterns are ignored in git
attributes\n
+ Use '\\!' for literal leading
exclamation.));
+ return NULL;
+ }
This return NULL; means we ignore !blah pattern, which is a
regression, isn't it? Should we treat '!' as literal here?
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
