On Thu, Sep 01, 2016 at 09:07:00PM +0100, Thomas Gummerer wrote: > > Related problem: `t3700-add.sh` currently fails in 2.9.3. I can > > provide more debug information if you don't already know this problem. > > I noticed this problem as well, when I'm compiling with USE_NSEC = 1 > in my config.mak. I can replicate this even without USE_NSEC with my stress-tester[1]. That makes sense why it would show up with the profiling run; git runs slower and therefore increases the chances of crossing the 1-second boundary and losing the race. [1] https://github.com/peff/git/blob/meta/stress > Tracking this problem down a bit, it happens because the --chmod=[+-]x > option introduced in 4e55ed32 ("add: add --chmod=+x / --chmod=-x > options") only works if the file on disk is modified. When the test > was changed to work on one single file, instead of doing chmod=+x on > one file and chmod=-x on another file in b38ab197c ("t3700: merge two > tests into one"), this test started breaking when the mtime of the > file and the index file weren't the same (in other words, if the file > was not racily clean and thus was not smudged). That certainly sounds buggy. A less racy way of verifying this is just: # guarantee not-racy state echo content >file test-chmtime -60 file git add file # now check --chmod; file will still be 100644! git add --chmod=+x file git ls-files -s > One possible fix for the test is to smudge the entry as showed below, > though I'm not sure it's the right fix. The other way I can think of > is to change the file in the index regardless of whether the file was > changed in some other way before issuing the git add command, as that > might fit the user expectation better. Thoughts? Yeah, I think we should _always_ act on the --chmod, no matter if the file is racy or not, or whether it has a content change or not. I.e., the race is not the problem, but rather the behavior of 4e55ed32. Your second proposal there sounds more like the right approach. -Peff