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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Apr 13, 2007, 04:54 UTC
Message-ID: <Pine.LNX.4.64.0704122143420.28042@woody.linux-foundation.org>
In-Reply-To: <7v7isghp1r.fsf@assigned-by-dhcp.cox.net>

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

Show 12 quoted lines

> 
> Hmm.  Perhaps something like this on top?
> 
> diff --git a/entry.c b/entry.c
> index 9545e89..0874d61 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -1,6 +1,23 @@
>  #include "cache.h"
>  #include "blob.h"
>  
> +static int make_directory(const char *path, int unlink_as_needed)

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);