Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Junio C Hamano
Jeff King writes: > I'd be very surprised if this works in practice on most of our current > test scripts. There are often subtle dependencies on the state left over > from previous tests. Running the script below up through t3800 (at which > point I lost patience) reveals 37 test scripts that ar

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Jeff King
On Mon, Aug 26, 2013 at 09:02:46AM -0700, Junio C Hamano wrote: > > There's one subtle thing I didn't mention in the "it is already on stack > > overflow". If you have a version of git which complains about the null > > sha1, then the SO advice is already broken. But if the SO works, then > > you

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Jeff King
On Mon, Aug 26, 2013 at 10:35:01AM -0700, Jonathan Nieder wrote: > > I don't see that splitting it up more hurts this. If we wanted more > > automatic rearranging or skipping of tests, we would need tests to > > declare dependencies on their setup. And we would need to be able to > > declare depen

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Jonathan Nieder
Jeff King wrote: > On Sun, Aug 25, 2013 at 12:54:12PM -0700, Jonathan Nieder wrote: >> [setup split across three tests] >> >> This is kind of an old-fashioned test, since each step of the setup is >> treated as a separate test assertion. I don't really mind until we >> get better automation to ma

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Junio C Hamano
Jeff King writes: >> I found this version more readable than Peff's (albeit slightly). > > OK. Do you want to apply with Jonathan's wording, then? I can do that, as it seems all of us are in agreement. > There's one subtle thing I didn't mention in the "it is already on stack > overflow". If yo

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Jeff King
On Sun, Aug 25, 2013 at 11:03:54PM -0700, Junio C Hamano wrote: > Jonathan Nieder writes: > > > In other words, why not use something like this? > > > > write_index: optionally allow broken null sha1s > > > > Commit 4337b58 (do not write null sha1s to on-disk index, 2012-07-28) > > a

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-26 Thread Jeff King
On Sun, Aug 25, 2013 at 12:54:12PM -0700, Jonathan Nieder wrote: > [setup split across three tests] > > This is kind of an old-fashioned test, since each step of the setup is > treated as a separate test assertion. I don't really mind until we > get better automation to make it easy to skip or re

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-25 Thread Junio C Hamano
Jonathan Nieder writes: > In other words, why not use something like this? > > write_index: optionally allow broken null sha1s > > Commit 4337b58 (do not write null sha1s to on-disk index, 2012-07-28) > added a safety check preventing git from writing null sha1s into the >

Re: [PATCHv2] write_index: optionally allow broken null sha1s

2013-08-25 Thread Jonathan Nieder
On Sun, Aug 25, 2013 at 05:58:18AM -0400, Jeff King wrote: > On Sat, Aug 24, 2013 at 11:15:00PM -0700, Jonathan Nieder wrote: >>> I was tempted to not involve filter-branch in this commit at all, and >>> instead require the user to manually invoke >>> >>> GIT_ALLOW_NULL_SHA1=1 git filter-branch