Re: Possible Solaris problem in 'checkout_entry()'

Linus Torvalds [off-list ref] writes:

On Mon, 17 Mar 2008, Linus Torvalds wrote:

quoted

IOW, the real "checking" is taking place in "create_file()", so if the 
unlinking failed (due to a read-only directory or something), that's where 
we'll do the proper error reporting.

Thinking about this, I'm probably full of sh*t.

My argument is admittedly true in general, but there is one case it is 
*not* true for: if the old entry was a symlink.

IOW, let's imagine that the directory is read-only (or other permission 
issue), and we want to unlink the old symlink, which points somewhere we 
can write to. In that case, the symlink removal is important, because we 
won't necessarily catch the error when we create the file in place later 
(because that will just follow the symlink).

So I retract my statement. We *should* check the result of the unlink.

While I agree we should check the result, I think we are safe against the
un-unlinkable symlink case.  If you have a stale symlink at "dir/file"
where you are checking out a new blob, and the directory "dir" the symlink
is in is unwritable, then our callpath would look like this:

	checkout_entry()
         unlink("dir/file") -- failure silently ignored which is bad
	 write_entry()
          create_file("dir/file")
           open("dir/file", O_WRONLY | O_CREAT | O_EXCL)

which would fail, and we get:

    error: git-checkout-index: unable to create file a/b (File exists)

from around ll.135 in entry.c::write_entry()

So I'll apply the patch purely as "Root on Solaris safety fix".