Re: [PATCH 2/2] Teach "git-read-tree -u" to check out submodules as a directory

On Thu, 12 Apr 2007, Junio C Hamano wrote:

I actually tried to think this through, and I don't *think* we should need 
to do this. It should literally be sufficient to just do the "mkdir()" 
call.

Why? Because we've already done the "create_directories()" calls earlier, 
and if you actually look at "create_file()", that one also just does a 
simple "open(path, O_WRONLY | O_CREAT | O_EXCL, mode)" - so doing just the 
mkdir() for creating a subdirectory should be the logically equivalent 
operation.

Similarly, the S_IFLNK case just does a "symlink()" system call. No "try 
to unlink if there was an old entry there before" code.

Basically, once we are inside "write_entry()", we expect to get a success 
or a failure, not a "needs cleanup". If it needed resolving of other state 
before, we shouldn't even have gotten to that stage in the first place.

That said, looking closer, you're right - the cleanup that is done earlier 
is actually wrong for the gitlink case. So I think the *real* problem is 
the cleanup in "checkout_entry()". In particular, note the

	if (!lstat(path, &st)) {
		...
		if (S_ISDIR(st.st_mode)) {
			...
		}
	}

code. If there was a directory there, we should actually leave it alone 
for the gitlink case, because it is up to the *subproject* checkout (not 
the superproject checkout) to handle any issues within that 
subdirectory!

So I think the *right* patch (on top of the one I send out) is actually 
just this:

		Linus

---

diff --git a/entry.c b/entry.c
index 9545e89..50ffae4 100644
--- a/entry.c
+++ b/entry.c

@@ -195,6 +195,9 @@ int checkout_entry(struct cache_entry *ce, struct checkout *state, char *topath)
 		 */
 		unlink(path);
 		if (S_ISDIR(st.st_mode)) {
+			/* If it is a gitlink, leave it alone! */
+			if (S_ISDIRLNK(ntohl(ce->ce_mode)))
+				return 0;
 			if (!state->force)
 				return error("%s is a directory", path);
 			remove_subtree(path);