On Tue, Mar 14, 2017 at 03:03:34PM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > We can improve this by skipping "ls-files" completely, and > > just feeding the original pathspecs to the diff commands. > > This solution was discussed in 2010: > > > > http://public-inbox.org/git/20100105041438.GB12574@xxxxxxxxxxxxxxxxxxxxxxx/ > > > > but at the time the diff code's pathspecs were more > > primitive than those used by ls-files (e.g., they did not > > support globs). Making the change would have caused a > > user-visible regression, so we didn't. > > Heh. The change and the reasoning are both obviously correct, but > how did you find this? Do you have a pile of "old patches that > should be resurrected when time is right" and this was picked out of > it, or did you see somebody else hit the same thing recently and > then went back to the archive? The latter. Mislav reported it to me off-list earlier today, and I generated the patch. But after scratching my head at the familiarity and especially wondering if there was some reason to use "ls-files" here, I dug up the linked thread. The fact that the patches are identical is just coincidence (though it's such a simple change that it seems highly likely). > > So this takes us one step closer to working correctly > > with files whose names contain wildcard characters, but > > it's likely that others remain (e.g., if "git add -i" > > feeds the selected paths to "git add"). > > We didn't run with --literal-pathspecs which was a bug, but I > suspect that it didn't exist back then ;-). Yep. I think judiciously inserting "--literal-pathspecs" is probably the correct fix. In an earlier thread (that I linked elsewhere from the discussion here) I suggested just setting $GIT_LITERAL_PATHSPECS to 1. But that is probably a bad idea. As this patch shows, we do still sometimes feed the original non-expanded pathspec to some commands. I left that as potential low-hanging fruit for somebody who cares more (the trickiest part is probably not the fix, but coming up with a test case). -Peff