* Git 2.7.0 gitignore behaviour regression @ 2016-01-05 14:40 Mike McQuaid 2016-01-05 15:06 ` Jeff King 0 siblings, 1 reply; 13+ messages in thread From: Mike McQuaid @ 2016-01-05 14:40 UTC (permalink / raw) To: git Hi folks, Firstly, thanks for all your hard work on Git. It makes my life much easier. Homebrew has a series of convoluted .gitignore rules due to our special/weird use-case of wanting to ignore everything in a working directory except a select few files/directories. We experienced a bug with our .gitignore file for users using Git 2.7.0. This may well be a valid WONTFIX or intentional behaviour change but I wanted to flag it in case it wasn’t. Here’s a minimal test case: - Create an empty git repository in a directory with `git init` - Create a directory named ‘a' containing a file named ‘b' with `mkdir a && touch a/b` - Create a ‘gitignore’ file with the following contents: ``` */ /a !/a/* ``` - Run `git status --short`. The output with Git 2.6.4 is: ``` ?? .gitignore ``` The output with Git 2.7.0 is: ``` ?? .gitignore ?? a/ ``` Another minimal test case: - Create an empty git repository in a directory with `git init` - Create a directory named ‘a' containing a file named ‘b' with `mkdir a && touch a/b` - Create a ‘gitignore’ file with the following contents: ``` */ /a !/a/ ``` - Run `git status —short`. The output with Git 2.6.4 is: ``` ?? .gitignore ?? a/ ``` The output with Git 2.7.0 is: ``` ?? .gitignore ``` Let me know if you need any more information, thanks! Mike McQuaid http://mikemcquaid.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-05 14:40 Git 2.7.0 gitignore behaviour regression Mike McQuaid @ 2016-01-05 15:06 ` Jeff King 2016-01-06 9:42 ` Duy Nguyen 2016-01-07 23:44 ` brian m. carlson 0 siblings, 2 replies; 13+ messages in thread From: Jeff King @ 2016-01-05 15:06 UTC (permalink / raw) To: Mike McQuaid; +Cc: git On Tue, Jan 05, 2016 at 02:40:16PM +0000, Mike McQuaid wrote: > Homebrew has a series of convoluted .gitignore rules due to our > special/weird use-case of wanting to ignore everything in a working > directory except a select few files/directories. We experienced a bug > with our .gitignore file for users using Git 2.7.0. This may well be a > valid WONTFIX or intentional behaviour change but I wanted to flag it > in case it wasn’t. > > Here’s a minimal test case: > > - Create an empty git repository in a directory with `git init` > - Create a directory named ‘a' containing a file named ‘b' with `mkdir a && touch a/b` > - Create a ‘gitignore’ file with the following contents: > ``` > */ > /a > !/a/* > ``` > - Run `git status --short`. > > The output with Git 2.6.4 is: > ``` > ?? .gitignore > ``` > > The output with Git 2.7.0 is: > ``` > ?? .gitignore > ?? a/ > ``` Thanks for giving a clear example. This bisects to Duy's 57534ee (dir.c: don't exclude whole dir prematurely if neg pattern may match, 2015-09-21). AFAICT (and I don't recall looking over this patch previously), what you are seeing is the intended effect of the patch. Your final line unignores stuff inside of "a", so we're reporting it (if you gave "-uall", you'd see the actual file "a/b"). Older versions of git generally optimized out looking inside "a/" at all. This created a hassle when people wanted to do things like: a/ !a/precious-file in their .gitignore. I'm sympathetic that in making that use-case work, we might have regressed another one, but it's hard to tell from the small example. Can you elaborate on your use case? Why are you both ignoring and unignoring everything in the directory? -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-05 15:06 ` Jeff King @ 2016-01-06 9:42 ` Duy Nguyen 2016-01-06 9:50 ` Mike McQuaid 2016-01-07 23:44 ` brian m. carlson 1 sibling, 1 reply; 13+ messages in thread From: Duy Nguyen @ 2016-01-06 9:42 UTC (permalink / raw) To: Jeff King, Mike McQuaid; +Cc: Git Mailing List On Tue, Jan 5, 2016 at 10:06 PM, Jeff King <peff@peff.net> wrote: > On Tue, Jan 05, 2016 at 02:40:16PM +0000, Mike McQuaid wrote: > >> Homebrew has a series of convoluted .gitignore rules due to our >> special/weird use-case of wanting to ignore everything in a working >> directory except a select few files/directories. We experienced a bug >> with our .gitignore file for users using Git 2.7.0. This may well be a >> valid WONTFIX or intentional behaviour change but I wanted to flag it >> in case it wasn’t. >> >> Here’s a minimal test case: >> >> - Create an empty git repository in a directory with `git init` >> - Create a directory named ‘a' containing a file named ‘b' with `mkdir a && touch a/b` >> - Create a ‘gitignore’ file with the following contents: >> ``` >> */ >> /a >> !/a/* >> ``` >> - Run `git status --short`. >> >> The output with Git 2.6.4 is: >> ``` >> ?? .gitignore >> ``` >> >> The output with Git 2.7.0 is: >> ``` >> ?? .gitignore >> ?? a/ >> ``` > > Thanks for giving a clear example. This bisects to Duy's 57534ee (dir.c: > don't exclude whole dir prematurely if neg pattern may match, > 2015-09-21). AFAICT (and I don't recall looking over this patch > previously), what you are seeing is the intended effect of the patch. Yeah.. I think it's the only relevant commit in 2.7.0 cycle anyway. These patterns "/a" followed by "!/a/*" were wrecking my head. But I finally decided 2.7 output makes more sense. You asked to un-ignore everything inside 'a' so we can't treat 'a' as (entirely) ignored and hide it away. > I'm sympathetic that in making that use-case work, we might have > regressed another one, but it's hard to tell from the small example. Can > you elaborate on your use case? Why are you both ignoring and unignoring > everything in the directory? Also how bad this affects you (widely-used 'wrong' behavior can become 'right', and my change a regression as a result) -- Duy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-06 9:42 ` Duy Nguyen @ 2016-01-06 9:50 ` Mike McQuaid 2016-01-06 10:03 ` Duy Nguyen 0 siblings, 1 reply; 13+ messages in thread From: Mike McQuaid @ 2016-01-06 9:50 UTC (permalink / raw) To: Duy Nguyen; +Cc: Jeff King, Git Mailing List > On 6 Jan 2016, at 09:42, Duy Nguyen <pclouds@gmail.com> wrote: > > Yeah.. I think it's the only relevant commit in 2.7.0 cycle anyway. > These patterns "/a" followed by "!/a/*" were wrecking my head. But I > finally decided 2.7 output makes more sense. You asked to un-ignore > everything inside 'a' so we can't treat 'a' as (entirely) ignored and > hide it away. > >> I'm sympathetic that in making that use-case work, we might have >> regressed another one, but it's hard to tell from the small example. Can >> you elaborate on your use case? Why are you both ignoring and unignoring >> everything in the directory? > > Also how bad this affects you (widely-used 'wrong' behavior can become > 'right', and my change a regression as a result) This doesn’t affect me badly; I was able to work around the original issue before the bug report in a way that was consistent between Git 2.6 and Git 2.7 but wanted to ensure that I filed something upstream just so it was a known issue as it was relatively easy to reproduce. I agree that all the pattern handling stuff like in my example is pretty awful; it’s also a big area where libgit2 was inconsistent with Git’s behaviour on either of those versions too. I’ve played around and now got a .gitignore file that behaves consistently across Git 2.6, Git 2.7 and libgit2 0.23.4 (https://github.com/Homebrew/homebrew/blob/master/.gitignore) so there’s no outstanding issue on my side. Thanks! Mike McQuaid http://mikemcquaid.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-06 9:50 ` Mike McQuaid @ 2016-01-06 10:03 ` Duy Nguyen 2016-01-06 18:58 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Duy Nguyen @ 2016-01-06 10:03 UTC (permalink / raw) To: Mike McQuaid; +Cc: Jeff King, Git Mailing List On Wed, Jan 6, 2016 at 4:50 PM, Mike McQuaid <mike@mikemcquaid.com> wrote: > it’s also a big area where libgit2 was inconsistent with Git’s behaviour on either of those versions too. Yeah.. it looks like libgit2's gitignore support was written new, not imported from C Git, so behavior differences (especially in corner cases) and even lack of some feature ("**" support comes to mind). For isolated features like gitignore, perhaps we can have an option to replace C Git code with libgit2 and therefore can test libgit2 against C Git test suite. It could be a good start for libgit2 to invade C Git. Not sure if anybody's interested in doing it though. -- Duy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-06 10:03 ` Duy Nguyen @ 2016-01-06 18:58 ` Junio C Hamano 2016-01-07 2:04 ` Jeff King 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2016-01-06 18:58 UTC (permalink / raw) To: Duy Nguyen; +Cc: Mike McQuaid, Jeff King, Git Mailing List On Wed, Jan 6, 2016 at 2:03 AM, Duy Nguyen <pclouds@gmail.com> wrote: > > On Wed, Jan 6, 2016 at 4:50 PM, Mike McQuaid <mike@mikemcquaid.com> wrote: > > it’s also a big area where libgit2 was inconsistent with Git’s behaviour on either of those versions too. > > Yeah.. it looks like libgit2's gitignore support was written new, not > imported from C Git, so behavior differences (especially in corner > cases) and even lack of some feature ("**" support comes to mind). For > isolated features like gitignore, perhaps we can have an option to > replace C Git code with libgit2 and therefore can test libgit2 against > C Git test suite. It could be a good start for libgit2 to invade C > Git. Not sure if anybody's interested in doing it though. Yup, an area that is reasonably isolated from the remainder of the system like this might be a good starting point. But I suspect that the invasion needs to happen in the opposite direction in this particular case before it happens. That is, if libgit2's implementation does not behave like how we do, it needs to be fixed, possibly by discarding what they did and instead importing code from us. After the behaviour of libgit2 is fixed, we can talk about the invasion in the opposite direction. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-06 18:58 ` Junio C Hamano @ 2016-01-07 2:04 ` Jeff King 2016-01-07 2:13 ` Duy Nguyen 2016-01-08 1:47 ` Carlos Martín Nieto 0 siblings, 2 replies; 13+ messages in thread From: Jeff King @ 2016-01-07 2:04 UTC (permalink / raw) To: Junio C Hamano Cc: Duy Nguyen, Mike McQuaid, Git Mailing List, Shawn Pearce, Carlos Martín Nieto [+cc Carlos and Shawn for libgit2/JGit talk] On Wed, Jan 06, 2016 at 10:58:37AM -0800, Junio C Hamano wrote: > On Wed, Jan 6, 2016 at 2:03 AM, Duy Nguyen <pclouds@gmail.com> wrote: > > > > Yeah.. it looks like libgit2's gitignore support was written new, not > > imported from C Git, so behavior differences (especially in corner > > cases) and even lack of some feature ("**" support comes to mind). For > > isolated features like gitignore, perhaps we can have an option to > > replace C Git code with libgit2 and therefore can test libgit2 against > > C Git test suite. It could be a good start for libgit2 to invade C > > Git. Not sure if anybody's interested in doing it though. Yeah, libgit2 is in the difficult position of trying to hit a moving target. There's a good chance that it _was_ the same as git's behavior when it was written. :) JGit is in the same boat, and I wouldn't be surprised if they don't handle "**" either (I didn't check). Note that git inherited that feature (and probably some others) by importing a GPLv2 version of wildmatch. That certainly isn't an option for JGit (because it's not pure Java), and probably not for libgit2 (which would need the wildmatch authors to grant the linking exception). > Yup, an area that is reasonably isolated from the remainder of the system like > this might be a good starting point. But I suspect that the invasion needs to > happen in the opposite direction in this particular case before it happens. > That is, if libgit2's implementation does not behave like how we do, it needs to > be fixed, possibly by discarding what they did and instead importing code from > us. After the behaviour of libgit2 is fixed, we can talk about the > invasion in the > opposite direction. Unfortunately, "importing code from us" is not so easy. :( They'll either need to contact the wildmatch authors, or rewrite wildmatch from scratch. -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-07 2:04 ` Jeff King @ 2016-01-07 2:13 ` Duy Nguyen 2016-01-08 1:47 ` Carlos Martín Nieto 1 sibling, 0 replies; 13+ messages in thread From: Duy Nguyen @ 2016-01-07 2:13 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Mike McQuaid, Git Mailing List, Shawn Pearce, Carlos Martín Nieto On Thu, Jan 7, 2016 at 9:04 AM, Jeff King <peff@peff.net> wrote: >> Yup, an area that is reasonably isolated from the remainder of the system like >> this might be a good starting point. But I suspect that the invasion needs to >> happen in the opposite direction in this particular case before it happens. >> That is, if libgit2's implementation does not behave like how we do, it needs to >> be fixed, possibly by discarding what they did and instead importing code from >> us. After the behaviour of libgit2 is fixed, we can talk about the >> invasion in the >> opposite direction. > > Unfortunately, "importing code from us" is not so easy. :( > > They'll either need to contact the wildmatch authors, or rewrite > wildmatch from scratch. wildmatch author relicensed it on request before [1] so he might do it again (hard to say). I'm not sure if there are other authors though. But even with wildmatch good to go, there still a lot of work to do. [1] http://thread.gmane.org/gmane.comp.version-control.git/264312/focus=264328 -- Duy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-07 2:04 ` Jeff King 2016-01-07 2:13 ` Duy Nguyen @ 2016-01-08 1:47 ` Carlos Martín Nieto 1 sibling, 0 replies; 13+ messages in thread From: Carlos Martín Nieto @ 2016-01-08 1:47 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Duy Nguyen, Mike McQuaid, Git Mailing List, Shawn Pearce On 07 Jan 2016, at 03:04, Jeff King <peff@peff.net> wrote: > [+cc Carlos and Shawn for libgit2/JGit talk] > > On Wed, Jan 06, 2016 at 10:58:37AM -0800, Junio C Hamano wrote: > >> On Wed, Jan 6, 2016 at 2:03 AM, Duy Nguyen <pclouds@gmail.com> wrote: >>> >>> Yeah.. it looks like libgit2's gitignore support was written new, not >>> imported from C Git, so behavior differences (especially in corner >>> cases) and even lack of some feature ("**" support comes to mind). For >>> isolated features like gitignore, perhaps we can have an option to >>> replace C Git code with libgit2 and therefore can test libgit2 against >>> C Git test suite. It could be a good start for libgit2 to invade C >>> Git. Not sure if anybody's interested in doing it though. > > Yeah, libgit2 is in the difficult position of trying to hit a moving > target. There's a good chance that it _was_ the same as git's behavior > when it was written. :) We did try to match the rules which git followed at the time when we added the functionality, but sometimes small changes in git mean large changes for us. There’s a couple of recent issues complaining that we don’t match git where *git* doesn’t quite seem to match the rules as I know them. I don’t recall the details of how we brought ignore support in, but when we don’t just copy the code from git, it’s generally because it’s embedded into the rest of the code, or in some other way doesn’t make a lot of sense in a library API. We do (at least I do) want to test libgit2 more against the git test suite; this either involves porting it to our test suite (generally non-trivial since we can’t use shell, making it harder to compare) or creating a command which pretends to be git but uses libgit2. The latter is what I think we’ll have to do, as we keep hitting complex pieces which git keeps changing like crlf and ignore handling. There’s been plans to do this, but it’s never been the top priority. > > JGit is in the same boat, and I wouldn't be surprised if they don't > handle "**" either (I didn't check). Note that git inherited that > feature (and probably some others) by importing a GPLv2 version of > wildmatch. That certainly isn't an option for JGit (because it's not > pure Java), and probably not for libgit2 (which would need the wildmatch > authors to grant the linking exception). > >> Yup, an area that is reasonably isolated from the remainder of the system like >> this might be a good starting point. But I suspect that the invasion needs to >> happen in the opposite direction in this particular case before it happens. >> That is, if libgit2's implementation does not behave like how we do, it needs to >> be fixed, possibly by discarding what they did and instead importing code from >> us. After the behaviour of libgit2 is fixed, we can talk about the >> invasion in the >> opposite direction. > > Unfortunately, "importing code from us" is not so easy. :( > > They'll either need to contact the wildmatch authors, or rewrite > wildmatch from scratch. Yeah, we currently use the fnmatch code from Android IIRC. I’m not looking forward to writing a variant of that for double-star. If the wildmatch authors are receptive to allowing the linking exception, that would be ideal. cmn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-05 15:06 ` Jeff King 2016-01-06 9:42 ` Duy Nguyen @ 2016-01-07 23:44 ` brian m. carlson 2016-01-08 0:38 ` Duy Nguyen 1 sibling, 1 reply; 13+ messages in thread From: brian m. carlson @ 2016-01-07 23:44 UTC (permalink / raw) To: Jeff King; +Cc: Mike McQuaid, git [-- Attachment #1: Type: text/plain, Size: 1329 bytes --] On Tue, Jan 05, 2016 at 10:06:02AM -0500, Jeff King wrote: > Thanks for giving a clear example. This bisects to Duy's 57534ee (dir.c: > don't exclude whole dir prematurely if neg pattern may match, > 2015-09-21). AFAICT (and I don't recall looking over this patch > previously), what you are seeing is the intended effect of the patch. > > Your final line unignores stuff inside of "a", so we're reporting it (if you gave > "-uall", you'd see the actual file "a/b"). Older versions of git > generally optimized out looking inside "a/" at all. This created a > hassle when people wanted to do things like: > > a/ > !a/precious-file > > in their .gitignore. I think there's still a bug in the code here. If you do git init mkdir -p base/a/ printf 'base/a/\n!base/a/b.txt\n' >.gitignore git add .gitignore git commit -m 'Add .gitignore' >base/a/b.txt git add base/a/b.txt git commit -m 'Add base/a/b.txt' >base/a/c.txt git status --porcelain git status outputs base/a/c.txt as unknown, when it should be ignored. We saw this in a repository at $DAYJOB. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-07 23:44 ` brian m. carlson @ 2016-01-08 0:38 ` Duy Nguyen 2016-01-08 2:41 ` brian m. carlson 0 siblings, 1 reply; 13+ messages in thread From: Duy Nguyen @ 2016-01-08 0:38 UTC (permalink / raw) To: brian m. carlson, Jeff King, Mike McQuaid, Git Mailing List On Fri, Jan 8, 2016 at 6:44 AM, brian m. carlson <sandals@crustytoothpaste.net> wrote: > On Tue, Jan 05, 2016 at 10:06:02AM -0500, Jeff King wrote: >> Thanks for giving a clear example. This bisects to Duy's 57534ee (dir.c: >> don't exclude whole dir prematurely if neg pattern may match, >> 2015-09-21). AFAICT (and I don't recall looking over this patch >> previously), what you are seeing is the intended effect of the patch. >> >> Your final line unignores stuff inside of "a", so we're reporting it (if you gave >> "-uall", you'd see the actual file "a/b"). Older versions of git >> generally optimized out looking inside "a/" at all. This created a >> hassle when people wanted to do things like: >> >> a/ >> !a/precious-file >> >> in their .gitignore. > > I think there's still a bug in the code here. If you do > > git init > mkdir -p base/a/ > printf 'base/a/\n!base/a/b.txt\n' >.gitignore Here we have the ignore rule "base/a/", but gitignore.txt, section NOTES mentions this - The rules to exclude the parent directory must not end with a trailing slash. > git add .gitignore > git commit -m 'Add .gitignore' > >base/a/b.txt > git add base/a/b.txt > git commit -m 'Add base/a/b.txt' > >base/a/c.txt > git status --porcelain > > git status outputs base/a/c.txt as unknown, when it should be ignored. > We saw this in a repository at $DAYJOB. If I delete that trailing slash, c.txt is ignored. So it's known limitation. I think we can make trailing slash case work too, but if I remember correctly it would involve a lot more changes, so I didn't do it (there are other conditions to follow anyway to make it work). -- Duy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-08 0:38 ` Duy Nguyen @ 2016-01-08 2:41 ` brian m. carlson 2016-01-08 9:02 ` Duy Nguyen 0 siblings, 1 reply; 13+ messages in thread From: brian m. carlson @ 2016-01-08 2:41 UTC (permalink / raw) To: Duy Nguyen; +Cc: Jeff King, Mike McQuaid, Git Mailing List [-- Attachment #1: Type: text/plain, Size: 2123 bytes --] On Fri, Jan 08, 2016 at 07:38:58AM +0700, Duy Nguyen wrote: > On Fri, Jan 8, 2016 at 6:44 AM, brian m. carlson > <sandals@crustytoothpaste.net> wrote: > > I think there's still a bug in the code here. If you do > > > > git init > > mkdir -p base/a/ > > printf 'base/a/\n!base/a/b.txt\n' >.gitignore > > Here we have the ignore rule "base/a/", but gitignore.txt, section > NOTES mentions this > > - The rules to exclude the parent directory must not end with a > trailing slash. The text here says, "To re-include files or directories when their parent directory is excluded, the following conditions must be met". In other words, the text implies that it's required for re-inclusion to work, not exclusion. > > git add .gitignore > > git commit -m 'Add .gitignore' > > >base/a/b.txt > > git add base/a/b.txt > > git commit -m 'Add base/a/b.txt' > > >base/a/c.txt > > git status --porcelain > > > > git status outputs base/a/c.txt as unknown, when it should be ignored. > > We saw this in a repository at $DAYJOB. > > If I delete that trailing slash, c.txt is ignored. So it's known > limitation. I think we can make trailing slash case work too, but if I > remember correctly it would involve a lot more changes, so I didn't do > it (there are other conditions to follow anyway to make it work). The case I'm seeing is that b.txt was already checked into the repository before being re-added, and c.txt was not. So it didn't affect us that b.txt was ignored (as it was already in the repo), but c.txt not being ignored broke a whole bunch of scripts that checked that the repository was clean, simply because we upgraded Git. I think regardless of whether b.txt is re-included, c.txt should be ignored. If it isn't possible to re-include b.txt, that's fine, since that isn't a regression, but ignored files should remain ignored. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Git 2.7.0 gitignore behaviour regression 2016-01-08 2:41 ` brian m. carlson @ 2016-01-08 9:02 ` Duy Nguyen 0 siblings, 0 replies; 13+ messages in thread From: Duy Nguyen @ 2016-01-08 9:02 UTC (permalink / raw) To: brian m. carlson; +Cc: Jeff King, Mike McQuaid, Git Mailing List On Fri, Jan 08, 2016 at 02:41:25AM +0000, brian m. carlson wrote: > On Fri, Jan 08, 2016 at 07:38:58AM +0700, Duy Nguyen wrote: > > On Fri, Jan 8, 2016 at 6:44 AM, brian m. carlson > > <sandals@crustytoothpaste.net> wrote: > > > I think there's still a bug in the code here. If you do > > > > > > git init > > > mkdir -p base/a/ > > > printf 'base/a/\n!base/a/b.txt\n' >.gitignore > > > > Here we have the ignore rule "base/a/", but gitignore.txt, section > > NOTES mentions this > > > > - The rules to exclude the parent directory must not end with a > > trailing slash. > > The text here says, "To re-include files or directories when their > parent directory is excluded, the following conditions must be met". In > other words, the text implies that it's required for re-inclusion to > work, not exclusion. > > > > git add .gitignore > > > git commit -m 'Add .gitignore' > > > >base/a/b.txt > > > git add base/a/b.txt > > > git commit -m 'Add base/a/b.txt' > > > >base/a/c.txt > > > git status --porcelain > > > > > > git status outputs base/a/c.txt as unknown, when it should be ignored. > > > We saw this in a repository at $DAYJOB. > > > > If I delete that trailing slash, c.txt is ignored. So it's known > > limitation. I think we can make trailing slash case work too, but if I > > remember correctly it would involve a lot more changes, so I didn't do > > it (there are other conditions to follow anyway to make it work). > > The case I'm seeing is that b.txt was already checked into the > repository before being re-added, and c.txt was not. So it didn't > affect us that b.txt was ignored (as it was already in the repo), but > c.txt not being ignored broke a whole bunch of scripts that checked that > the repository was clean, simply because we upgraded Git. > > I think regardless of whether b.txt is re-included, c.txt should be > ignored. If it isn't possible to re-include b.txt, that's fine, since > that isn't a regression, but ignored files should remain ignored. Thanks for clarification. I looked at this the wrong way. I agree it is a regression. The following should fix it. It looks correct (and does fix your test case), but I will have to look harder over the weekend before sending a proper patch. -- 8< -- diff --git a/dir.c b/dir.c index d2a8f06..7934e87 100644 --- a/dir.c +++ b/dir.c @@ -1008,6 +1008,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname, if (exc && !(exc->flags & EXC_FLAG_NEGATIVE) && !(exc->flags & EXC_FLAG_NODIR) && + !(exc->flags & EXC_FLAG_MUSTBEDIR) && matched_negative_path) exc = NULL; return exc; -- 8< -- -- Duy ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-01-08 9:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-05 14:40 Git 2.7.0 gitignore behaviour regression Mike McQuaid 2016-01-05 15:06 ` Jeff King 2016-01-06 9:42 ` Duy Nguyen 2016-01-06 9:50 ` Mike McQuaid 2016-01-06 10:03 ` Duy Nguyen 2016-01-06 18:58 ` Junio C Hamano 2016-01-07 2:04 ` Jeff King 2016-01-07 2:13 ` Duy Nguyen 2016-01-08 1:47 ` Carlos Martín Nieto 2016-01-07 23:44 ` brian m. carlson 2016-01-08 0:38 ` Duy Nguyen 2016-01-08 2:41 ` brian m. carlson 2016-01-08 9:02 ` Duy Nguyen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.