On Tue, Sep 04, 2018 at 06:13:36PM +0200, Duy Nguyen wrote: > > > Doh, of course. I even thought about this issue and dug all the way into > > > reopen_lock_file(), but for some reason temporarily forgot that O_WRONLY > > > does not imply O_TRUNC. > > > > > > Arguably this should be the default for reopen_lockfile(), as getting a > > > write pointer into an existing file is not ever going to be useful for > > > the way Git uses lockfiles. Opening with O_APPEND could conceivably be > > > useful, but it's pretty unlikely (and certainly not helpful here, and > > > this is the only caller). Alternatively, the function should just take > > > open(2) flags. > > > > > > At any rate, I think this perfectly explains the behavior we're seeing. > > > > Thanks all for digging this down (I am a bit jealous to see that I > > seem to have missed all this fun over the weekend X-<). > > And just to be clear I'm looking forward to a patch from Jeff to fix > this since he clearly put more thoughts on this than me. With commit.c > being the only user of reopen_lock_file() I guess it's even ok to just > stick O_TRUNC in there and worry about O_APPEND when a new caller > needs that. That's the way I'm leaning to. The fix is obviously a one-liner, but I was hoping to construct a minimal test case. I just haven't gotten around to it yet. The bug is ancient, so I don't think it's important for v2.19. -Peff
