threads / patch / 7643

patch, 3 partsresend/cleanup: Fix up subproject support

Subject: [PATCH 0/3] resend/cleanup: Fix up subproject support

## tl;dr

4 messages between Apr 13, 2007 and Apr 13, 2007. Diffs are folded; open one to read it.

replies: 3people: 1as markdown or json

Linus Torvalds· Apr 13, 2007, 16:23 UTC · lore

This series of three patches is a *replacement* for the patch series of two patches (plus one-liner fixup) I sent yesterday.

It fixes the issue I noted with "git status" incorrectly claiming that a non-checked out subproject wasn't clean - that was just a total thinko in the code (we were checking the filesystem mode against S_IFDIRLNK, which obviously cannot work, since S_IFDIRLINK is a git-internal state, not a filesystem state).

It then re-sends the two patches on top of that, with the fix for checking out superprojects (we should *not* mess up any existing subproject directories, certainly not remove them - if we already have a directory in the place where we now want a subproject, we should leave it well alone!)

If you already applied the patches from yesterday (and my small fix), you already have the two latter ones - the only reason I added 1/3 to the front of the patch-queue is that it really is a fix, and it makes the commit commentary about a remaining bug in the patch I sent out yesterday go away.

		Linus
Linus Torvalds· Apr 13, 2007, 16:24 UTC · re: Linus Torvalds · lore

[PATCH 1/3] Fix gitlink index entry filesystem matching

The code to match up index entries with the filesystem was stupidly broken. We shouldn't compare the filesystem stat() information with S_IFDIRLNK, since that's purely a git-internal value, and not what the filesystem uses (on the filesystem, it's just a regular directory).

Also, don't bother to make the stat() time comparisons etc for DIRLNK entries in ce_match_stat_basic(), since we do an exact match for these things, and the hints in the stat data simply doesn't matter.

This fixes "git status" with submodules that haven't been checked out in the supermodule.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 read-cache.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to read-cache.c +4 −4
diff --git a/read-cache.c b/read-cache.c
index 4040fac..72d7d7c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -123,9 +123,9 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
 		if (ce_compare_link(ce, xsize_t(st->st_size)))
 			return DATA_CHANGED;
 		break;
-	case S_IFDIRLNK:
-		/* No need to do anything, we did the exact compare in "match_stat_basic" */
-		break;
+	case S_IFDIR:
+		if (S_ISDIRLNK(ntohl(ce->ce_mode)))
+			return 0;
 	default:
 		return TYPE_CHANGED;
 	}
@@ -156,7 +156,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 			changed |= TYPE_CHANGED;
 		else if (ce_compare_gitlink(ce))
 			changed |= DATA_CHANGED;
-		break;
+		return changed;
 	default:
 		die("internal error: ce_mode is %o", ntohl(ce->ce_mode));
 	}
-- 
1.5.1.1.786.gdc49a-dirty
Linus Torvalds· Apr 13, 2007, 16:25 UTC · re: Linus Torvalds · lore

[PATCH 2/3] Teach git list-objects logic to not follow gitlinks

This allows us to pack superprojects and thus clone them (but not yet check them out on the receiving side.. That's the next patch)

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 list-objects.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)
Show changes to list-objects.c +34 −0
diff --git a/list-objects.c b/list-objects.c
index 2ba2c95..310f8d3 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -25,6 +25,37 @@ static void process_blob(struct rev_info *revs,
 	add_object(obj, p, path, name);
 }
 
+/*
+ * Processing a gitlink entry currently does nothing, since
+ * we do not recurse into the subproject.
+ *
+ * We *could* eventually add a flag that actually does that,
+ * which would involve:
+ *  - is the subproject actually checked out?
+ *  - if so, see if the subproject has already been added
+ *    to the alternates list, and add it if not.
+ *  - process the commit (or tag) the gitlink points to
+ *    recursively.
+ *
+ * However, it's unclear whether there is really ever any
+ * reason to see superprojects and subprojects as such a
+ * "unified" object pool (potentially resulting in a totally
+ * humongous pack - avoiding which was the whole point of
+ * having gitlinks in the first place!).
+ *
+ * So for now, there is just a note that we *could* follow
+ * the link, and how to do it. Whether it necessarily makes
+ * any sense what-so-ever to ever do that is another issue.
+ */
+static void process_gitlink(struct rev_info *revs,
+			    const unsigned char *sha1,
+			    struct object_array *p,
+			    struct name_path *path,
+			    const char *name)
+{
+	/* Nothing to do */
+}
+
 static void process_tree(struct rev_info *revs,
 			 struct tree *tree,
 			 struct object_array *p,
@@ -56,6 +87,9 @@ static void process_tree(struct rev_info *revs,
 			process_tree(revs,
 				     lookup_tree(entry.sha1),
 				     p, &me, entry.path);
+		else if (S_ISDIRLNK(entry.mode))
+			process_gitlink(revs, entry.sha1,
+					p, &me, entry.path);
 		else
 			process_blob(revs,
 				     lookup_blob(entry.sha1),
-- 
1.5.1.1.786.gdc49a-dirty
Linus Torvalds· Apr 13, 2007, 16:26 UTC · re: Linus Torvalds · lore

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

This actually allows us to check out a supermodule after cloning, although the submodules themselves will obviously not be checked out, and will just be empty directories.

Checking out the submodules will be up to higher levels - we may not even want to!

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 entry.c |   45 ++++++++++++++++++++++++++++++++-------------
 1 files changed, 32 insertions(+), 13 deletions(-)
Show changes to entry.c +32 −13
diff --git a/entry.c b/entry.c
index d72f811..50ffae4 100644
--- a/entry.c
+++ b/entry.c
@@ -62,26 +62,33 @@ static int create_file(const char *path, unsigned int mode)
 	return open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
 }
 
+static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned long *size)
+{
+	enum object_type type;
+	void *new = read_sha1_file(ce->sha1, &type, size);
+
+	if (new) {
+		if (type == OBJ_BLOB)
+			return new;
+		free(new);
+	}
+	return NULL;
+}
+
 static int write_entry(struct cache_entry *ce, char *path, struct checkout *state, int to_tempfile)
 {
 	int fd;
-	void *new;
-	unsigned long size;
 	long wrote;
-	enum object_type type;
 
-	new = read_sha1_file(ce->sha1, &type, &size);
-	if (!new || type != OBJ_BLOB) {
-		if (new)
-			free(new);
-		return error("git-checkout-index: unable to read sha1 file of %s (%s)",
-			path, sha1_to_hex(ce->sha1));
-	}
 	switch (ntohl(ce->ce_mode) & S_IFMT) {
-		char *buf;
-		unsigned long nsize;
+		char *buf, *new;
+		unsigned long size, nsize;
 
 	case S_IFREG:
+		new = read_blob_entry(ce, path, &size);
+		if (!new)
+			return error("git-checkout-index: unable to read sha1 file of %s (%s)",
+				path, sha1_to_hex(ce->sha1));
 		if (to_tempfile) {
 			strcpy(path, ".merge_file_XXXXXX");
 			fd = mkstemp(path);
@@ -111,6 +118,10 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat
 			return error("git-checkout-index: unable to write file %s", path);
 		break;
 	case S_IFLNK:
+		new = read_blob_entry(ce, path, &size);
+		if (!new)
+			return error("git-checkout-index: unable to read sha1 file of %s (%s)",
+				path, sha1_to_hex(ce->sha1));
 		if (to_tempfile || !has_symlinks) {
 			if (to_tempfile) {
 				strcpy(path, ".merge_link_XXXXXX");
@@ -136,8 +147,13 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat
 						 "symlink %s (%s)", path, strerror(errno));
 		}
 		break;
+	case S_IFDIRLNK:
+		if (to_tempfile)
+			return error("git-checkout-index: cannot create temporary subproject %s", path);
+		if (mkdir(path, 0777) < 0)
+			return error("git-checkout-index: cannot create subproject directory %s", path);
+		break;
 	default:
-		free(new);
 		return error("git-checkout-index: unknown file mode for %s", path);
 	}
 
@@ -179,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);
-- 
1.5.1.1.786.gdc49a-dirty

← back to recent threads