Re: [RFC PATCH] Re: Empty directories...

Linus Torvalds [off-list ref] writes:

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.

Personally, I am not much in favor of using different names in index
and repository.

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,

dir.c and readcache.c

but it's used by "index_name_pos()/cache_name_pos()", which *is*
used in many places.

cache_name_pos:
builtin-apply.c
builtin-blame.c
builtin-checkout-index.c
builtin-ls-files.c
builtin-mv.c
builtin-read-tree.c
builtin-rm.c
builtin-update-index.c
diff.c
diff-lib.c
dir.c
merge-index.c
sha1_name.c
unpack-trees.c
wt-status.c

index_name_pos:
read-cache.c

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!).

Actually, pretty much all of the above files are likely to get touched
by directory support one way or another anyway.  One really should aim
for the cleanest solution in the long run, and this for me more or
less means that it makes no sense to have different names in index and
repository.  Putting that slash in always would probably simplify some
logic in the repository as well, but I don't really like something as
marker-like as "/" in the data structures.  Putting a slash there
would involve a three-phase plan:

a) make fsck and the other code deal gracefully with either slash or
   no slash.
Wait until everybody uses this code.

b) make the code actually _put_ slashes there.
Wait until everybody has used this code.

c) deal with it for all eternity, oops: since rewriting the
   cryptographic history of existing repositories is pretty much out
   as far as I understand (which might be insufficient), one has to
   navigate around slash/noslash all the time when accessing
   repositories, including the sorting.  The index, however, can at
   one point of time phase out the slash-specific sorting.  There is
   no such thing as prehistoric indexes we would need to mind.

I guess that looks like not being worth the pain.  Double the code or
no money back.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum