Re: Broken adding of cache entries

PB> What about PB> ... suggested changes removed. PB> then? (Completely untested and everything.)

It is clear that it is untested. In the sequence Kay had trouble with:

    $ rm -fr path; date >path
    $ git-update-cache --add path
    $ rm -f path; mkdir path; date >path/file
    $ git-update-cache --add path/file

The cache_name_compare function you are looking at is called with "path" and "path/file", and compares the name strings (up to the length of the shorter one), and when they are the same, then compares the lengths of them. At that point, the if() statements have already answered that "path" comes before "path/file" and your changes will _not_ change the behavior of that function.

That said, I understand the behaviour you would want to see, and I tend to agree that this should be prevented from happening in the first place when git-update-cache happens. Let me think a bit more about it.

PB> I'd still prefer add_cache_entry() to just replace the original entry PB> (as it does otherwise).

I do not think it should just silently replace; instead it should make the user take notice, because this is an unusual situation. In other words, it should refuse until the user makes a conscious decision to tell it that the user is removing it and then placing something completely different.

This implies (I am thinking aloud here):

    $ find fs -type f -print | xargs git-update-cache --add
    $ rm -fr fs
    $ date >fs
    $ git-update-cache --add fs

would refuse to add "fs" because it notices it has some entry whose "name" field starts with "fs/". Likewise, for the opposite sequence:

    $ date >fs
    $ git-update-cache --add fs
    $ rm -f fs; mkdir fs; date >fs/file    
    $ find fs -type f -print | xargs git-update-cache --add

Telling git-update-cache about anything that contains a '/' in its name would first check if an entry whose name is exactly one of the leading path prefix, and refuses the operation if it finds one.

The above logic should work and it would also work if we decide to just replace without refusing as well. I'll code it up and see which one I like; maybe it will end up as an option.