On Fri, 20 Jul 2007, Linus Torvalds wrote: > > As far as I can tell, it would have been exactly the same thing as the > S_IFDIR, just instead of the S_IFDIR check, you'd have had to check the > end of the filename for being '/'. BTW, there is actually one big difference, and the '/' at the end actually has one huge advantage. Why? Because my preliminary patches sort the index entries wrong. A directory should always sort *as*if* it had the '/' at the end. See base_name_compare() for details. And we've never done that for the index, because the index has never had this issue (since it never contained directories). So sit down and compare base_name_compare (for tree entries) with cache_name_compare() (for index entries), and see how the latter doesn't care about the type of names. This was actually something that I hit already with subproject support, and one of my very first patches even had some (aborted) code to start sorting subprojects in the index the way we sort directories. And I *should* have done it that way, but I never did. It now makes the S_ISDIR handling harder, because directories really do have to be sorted as if they had the '/' at the end, or "git-fsck" will complain about bad sorting. Sad, sad, sad. It effectively means that S_IFGITLINK is *not* quite the same as S_IFDIR, because they sort differently. Duh. Of course, it seldom matters, but basically, you should test a directory structure that has the files dir.c dir/test in it, and the "dir" directory should always sort _after_ "dir.c". And yes, having the index entry with a '/' at the end would handle that automatically. As it is, with the "mode" difference, it instead needs to fix up "cache_name_compare()". Admittedly, that would actually be a cleanup (since it would now match base_name_compare() in logic, and could actually use that to do the name comparison!), but it's a damn painful cleanup because we don't even pass in the mode to "cache_name_compare()", since we never needed it. Gaah. cache_name_compare itself isn't used in that many places, but it's used by "index_name_pos()/cache_name_pos()", which *is* used in many places. And again, that one doesn't even have the mode, so it cannot pass it down. So it probably *is* easier to add the '/' at the end of the name instead, to make directories sort the right way in the index. I'd still suggest you *also* make the mode be S_IFDIR, though (and preferably make git-fsck actually verify that the mode and the last character of the name matches!). Linus - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
