[PATCH] git-apply: apply submodule changes

Subsystems: the rest

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] git-apply: apply submodule changes

From: Sven Verdoolaege <hidden>
Date: 2016-06-15 22:43:28

Apply "Subproject commit HEX" changes produced by git-diff.
As usual in the current git, only the superproject itself is actually
modified (possibly creating empty directories for new submodules).

Signed-off-by: Sven Verdoolaege <redacted>
---
This also makes rebase handle submodules.

 builtin-apply.c |   49 ++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index da27075..24df282 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1984,6 +1984,26 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
 	return 0;
 }
 
+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,
+				unsigned long *size_p)
+{
+	if (!ce)
+		return 0;
+
+	if (S_ISGITLINK(ntohl(ce->ce_mode))) {
+		*buf_p = xmalloc(100);
+		*size_p = snprintf(*buf_p, 100,
+			"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+	} else {
+		enum object_type type;
+		*buf_p = read_sha1_file(ce->sha1, &type, size_p);
+		if (!*buf_p)
+			return -1;
+	}
+
+	return 0;
+}
+
 static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)
 {
 	char *buf;
@@ -1994,20 +2014,18 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 	alloc = 0;
 	buf = NULL;
 	if (cached) {
-		if (ce) {
-			enum object_type type;
-			buf = read_sha1_file(ce->sha1, &type, &size);
-			if (!buf)
-				return error("read of %s failed",
-					     patch->old_name);
-			alloc = size;
-		}
+		if (read_file_or_gitlink(ce, &buf, &size))
+			return error("read of %s failed", patch->old_name);
+		alloc = size;
 	}
 	else if (patch->old_name) {
 		size = xsize_t(st->st_size);
 		alloc = size + 8192;
 		buf = xmalloc(alloc);
-		if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
+		if (S_ISGITLINK(patch->old_mode))
+			size = snprintf(buf, alloc,
+				"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+		else if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
 			return error("read of %s failed", patch->old_name);
 	}
 
@@ -2098,7 +2116,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
 			}
 			if (!cached)
 				changed = ce_match_stat(ce, &st, 1);
-			if (changed)
+			if (changed && !S_ISGITLINK(patch->old_mode))
 				return error("%s: does not match index",
 					     old_name);
 			if (cached)
@@ -2387,7 +2405,9 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 			die("unable to stat newly created file %s", path);
 		fill_stat_cache_info(ce, &st);
 	}
-	if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
+	if (S_ISGITLINK(mode))
+		get_sha1_hex(buf + strlen("Subproject commit "), ce->sha1);
+	else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
 		die("unable to create backing store for newly created file %s", path);
 	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
 		die("unable to add cache entry for %s", path);
@@ -2398,6 +2418,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
 	int fd;
 	char *nbuf;
 
+	if (S_ISGITLINK(mode)) {
+		struct stat st;
+		if (!lstat(path, &st) && S_ISDIR(st.st_mode))
+			return 0;
+		return mkdir(path, 0777);
+	}
+
 	if (has_symlinks && S_ISLNK(mode))
 		/* Although buf:size is counted string, it also is NUL
 		 * terminated.
-- 
1.5.3.rc4.29.g74276-dirty

Re: [PATCH] git-apply: apply submodule changes

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:28

Hi,

On Fri, 10 Aug 2007, Sven Verdoolaege wrote:
Apply "Subproject commit HEX" changes produced by git-diff.
As usual in the current git, only the superproject itself is actually
modified (possibly creating empty directories for new submodules).
For rebase and cherry-pick, it would be nice if git just ignored the 
changes in the submodules, provided that the submodule commit was not 
affected by the to-be-applied patches.

Hmm?

Ciao,
Dscho

Re: [PATCH] git-apply: apply submodule changes

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:28

Hi,

On Fri, 10 Aug 2007, Sven Verdoolaege wrote:
quoted hunk
@@ -2387,7 +2405,9 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 			die("unable to stat newly created file %s", path);
 		fill_stat_cache_info(ce, &st);
 	}
-	if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
+	if (S_ISGITLINK(mode))
+		get_sha1_hex(buf + strlen("Subproject commit "), ce->sha1);
+	else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
 		die("unable to create backing store for newly created file %s", path);
 	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
 		die("unable to add cache entry for %s", path);
I guess that you need to catch an error from get_sha1_hex(), too.

I hope it is not to much to ask for a patch to t7400 to show what this 
patch fixes?

Ciao,
Dscho

[PATCH resend] git-apply: apply submodule changes

From: Sven Verdoolaege <hidden>
Date: 2016-06-15 22:43:28

Apply "Subproject commit HEX" changes produced by git-diff.
As usual in the current git, only the superproject itself is actually
modified (possibly creating empty directories for new submodules).
Any checked-out submodule is left untouched and is not required to
be up-to-date.

Signed-off-by: Sven Verdoolaege <redacted>
---
This second version has a test and an extra sanity check.

I seem to be experiencing some problems receiving emails,
so I'll reply to a message from Dscho here.

Johannes Schindelin <Johannes.Schindelin <at> gmx.de> writes:
For rebase and cherry-pick, it would be nice if git just ignored the 
changes in the submodules, provided that the submodule commit was not 
affected by the to-be-applied patches.
I have no idea what you mean.

The checked out copies of the submodules are ignored completely
(if that is what you were talking about, then I hope this issue
is clarified by the updated commit message).  In the superproject,
the change to the submodule is obviously not ignored, since it's
an integral part of the patch.  However, git-apply will fail if
the original submodule commit does not correspond exactly to the
"from-file" submodule commit.
I don't think there is anything else we can do without a true
recursive git-diff/git-apply.

skimo

 builtin-apply.c            |   50 ++++++++++++++++++++++++++++++++++---------
 t/t7400-submodule-basic.sh |    8 +++++++
 2 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index da27075..a38dbf1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1984,6 +1984,26 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
 	return 0;
 }
 
+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,
+				unsigned long *size_p)
+{
+	if (!ce)
+		return 0;
+
+	if (S_ISGITLINK(ntohl(ce->ce_mode))) {
+		*buf_p = xmalloc(100);
+		*size_p = snprintf(*buf_p, 100,
+			"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+	} else {
+		enum object_type type;
+		*buf_p = read_sha1_file(ce->sha1, &type, size_p);
+		if (!*buf_p)
+			return -1;
+	}
+
+	return 0;
+}
+
 static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)
 {
 	char *buf;
@@ -1994,20 +2014,18 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 	alloc = 0;
 	buf = NULL;
 	if (cached) {
-		if (ce) {
-			enum object_type type;
-			buf = read_sha1_file(ce->sha1, &type, &size);
-			if (!buf)
-				return error("read of %s failed",
-					     patch->old_name);
-			alloc = size;
-		}
+		if (read_file_or_gitlink(ce, &buf, &size))
+			return error("read of %s failed", patch->old_name);
+		alloc = size;
 	}
 	else if (patch->old_name) {
 		size = xsize_t(st->st_size);
 		alloc = size + 8192;
 		buf = xmalloc(alloc);
-		if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
+		if (S_ISGITLINK(patch->old_mode))
+			size = snprintf(buf, alloc,
+				"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+		else if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
 			return error("read of %s failed", patch->old_name);
 	}
 
@@ -2098,7 +2116,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
 			}
 			if (!cached)
 				changed = ce_match_stat(ce, &st, 1);
-			if (changed)
+			if (changed && !S_ISGITLINK(patch->old_mode))
 				return error("%s: does not match index",
 					     old_name);
 			if (cached)
@@ -2387,7 +2405,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 			die("unable to stat newly created file %s", path);
 		fill_stat_cache_info(ce, &st);
 	}
-	if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
+	if (S_ISGITLINK(mode)) {
+		if (get_sha1_hex(buf + strlen("Subproject commit "), ce->sha1))
+			die("corrupt patch for subproject %s", path);
+	} else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
 		die("unable to create backing store for newly created file %s", path);
 	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
 		die("unable to add cache entry for %s", path);
@@ -2398,6 +2419,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
 	int fd;
 	char *nbuf;
 
+	if (S_ISGITLINK(mode)) {
+		struct stat st;
+		if (!lstat(path, &st) && S_ISDIR(st.st_mode))
+			return 0;
+		return mkdir(path, 0777);
+	}
+
 	if (has_symlinks && S_ISLNK(mode))
 		/* Although buf:size is counted string, it also is NUL
 		 * terminated.
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index e8ce7cd..cede2e7 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -175,4 +175,12 @@ test_expect_success 'checkout superproject with subproject already present' '
 	git-checkout master
 '
 
+test_expect_success 'rebase with subproject changes' '
+	git-checkout initial &&
+	echo t > t &&
+	git add t &&
+	git-commit -m "change t" &&
+	git-rebase HEAD master
+'
+
 test_done
-- 
1.5.3.rc4.29.g74276-dirty

[PATCH v3] git-apply: apply submodule changes

From: Sven Verdoolaege <hidden>
Date: 2016-06-15 22:43:28

Apply "Subproject commit HEX" changes produced by git-diff.
As usual in the current git, only the superproject itself is actually
modified (possibly creating empty directories for new submodules).
Any checked-out submodule is left untouched and is not required to
be up-to-date.

Signed-off-by: Sven Verdoolaege <redacted>
---
The third version also works without --index and documents
the behaviour of git-apply in the presence of submodule changes.

skimo

 Documentation/git-apply.txt |   14 +++++++++
 builtin-apply.c             |   69 +++++++++++++++++++++++++++++++++++-------
 t/t7400-submodule-basic.sh  |    8 +++++
 3 files changed, 79 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index f03f661..804fdc3 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -171,6 +171,20 @@ apply.whitespace::
 	When no `--whitespace` flag is given from the command
 	line, this configuration item is used as the default.
 
+Submodules
+----------
+If the patch contains any changes to submodules then gitlink:git-apply[1]
+behaves as follows.
+
+If --index is specified (explicitly or implicitly), then the submodule
+commits must match the index exactly for the patch to apply.  If any
+of the submodules are checked-out, then these check-outs are completely
+ignored, i.e., they are not required to be up-to-date or clean and they
+are not updated.
+
+If --index is not specified, then the submodule commits in the patch
+are ignored and only the absence of presence of the corresponding
+subdirectory is checked and (if possible) updated.
 
 Author
 ------
diff --git a/builtin-apply.c b/builtin-apply.c
index da27075..eef596b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1984,6 +1984,40 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
 	return 0;
 }
 
+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,
+				unsigned long *size_p)
+{
+	if (!ce)
+		return 0;
+
+	if (S_ISGITLINK(ntohl(ce->ce_mode))) {
+		*buf_p = xmalloc(100);
+		*size_p = snprintf(*buf_p, 100,
+			"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+	} else {
+		enum object_type type;
+		*buf_p = read_sha1_file(ce->sha1, &type, size_p);
+		if (!*buf_p)
+			return -1;
+	}
+
+	return 0;
+}
+
+static int read_gitlink_or_skip(struct patch *patch, struct cache_entry *ce,
+				char *buf, unsigned long alloc)
+{
+	if (ce)
+		return snprintf(buf, alloc,
+				"Subproject commit %s\n", sha1_to_hex(ce->sha1));
+
+	/* We can't apply the submodule change without an index, so just
+	 * skip the patch itself and only create/remove directory.
+	 */
+	patch->fragments = NULL;
+	return 0;
+}
+
 static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)
 {
 	char *buf;
@@ -1994,20 +2028,17 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 	alloc = 0;
 	buf = NULL;
 	if (cached) {
-		if (ce) {
-			enum object_type type;
-			buf = read_sha1_file(ce->sha1, &type, &size);
-			if (!buf)
-				return error("read of %s failed",
-					     patch->old_name);
-			alloc = size;
-		}
+		if (read_file_or_gitlink(ce, &buf, &size))
+			return error("read of %s failed", patch->old_name);
+		alloc = size;
 	}
 	else if (patch->old_name) {
 		size = xsize_t(st->st_size);
 		alloc = size + 8192;
 		buf = xmalloc(alloc);
-		if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
+		if (S_ISGITLINK(patch->old_mode))
+			size = read_gitlink_or_skip(patch, ce, buf, alloc);
+		else if (read_old_data(st, patch->old_name, &buf, &alloc, &size))
 			return error("read of %s failed", patch->old_name);
 	}
 
@@ -2098,7 +2129,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
 			}
 			if (!cached)
 				changed = ce_match_stat(ce, &st, 1);
-			if (changed)
+			if (changed && !S_ISGITLINK(patch->old_mode))
 				return error("%s: does not match index",
 					     old_name);
 			if (cached)
@@ -2354,7 +2385,11 @@ static void remove_file(struct patch *patch, int rmdir_empty)
 		cache_tree_invalidate_path(active_cache_tree, patch->old_name);
 	}
 	if (!cached) {
-		if (!unlink(patch->old_name) && rmdir_empty) {
+		if (S_ISGITLINK(patch->old_mode)) {
+			if (rmdir(patch->old_name))
+				warning("unable to remove submodule %s",
+					patch->old_name);
+		} else if (!unlink(patch->old_name) && rmdir_empty) {
 			char *name = xstrdup(patch->old_name);
 			char *end = strrchr(name, '/');
 			while (end) {
@@ -2387,7 +2422,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 			die("unable to stat newly created file %s", path);
 		fill_stat_cache_info(ce, &st);
 	}
-	if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
+	if (S_ISGITLINK(mode)) {
+		if (get_sha1_hex(buf + strlen("Subproject commit "), ce->sha1))
+			die("corrupt patch for subproject %s", path);
+	} else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
 		die("unable to create backing store for newly created file %s", path);
 	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
 		die("unable to add cache entry for %s", path);
@@ -2398,6 +2436,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
 	int fd;
 	char *nbuf;
 
+	if (S_ISGITLINK(mode)) {
+		struct stat st;
+		if (!lstat(path, &st) && S_ISDIR(st.st_mode))
+			return 0;
+		return mkdir(path, 0777);
+	}
+
 	if (has_symlinks && S_ISLNK(mode))
 		/* Although buf:size is counted string, it also is NUL
 		 * terminated.
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index e8ce7cd..cede2e7 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -175,4 +175,12 @@ test_expect_success 'checkout superproject with subproject already present' '
 	git-checkout master
 '
 
+test_expect_success 'rebase with subproject changes' '
+	git-checkout initial &&
+	echo t > t &&
+	git add t &&
+	git-commit -m "change t" &&
+	git-rebase HEAD master
+'
+
 test_done
-- 
1.5.3.rc4.30.gd0c97-dirty
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help