{"thread":{"id":"9470","subject":"[PATCH] git-apply: apply submodule changes","startedAt":"2007-08-10T09:30:49Z","lastAt":"2007-08-16T00:02:58Z","messageCount":19,"participants":["Sven Verdoolaege","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50395","messageId":"20070810093049.GA868MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":null,"subject":"[PATCH] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-10T09:30:49Z","receivedAt":"2007-08-10T09:30:49Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"Apply \"Subproject commit HEX\" changes produced by git-diff.\nAs usual in the current git, only the superproject itself is actually\nmodified (possibly creating empty directories for new submodules).\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n---\nThis also makes rebase handle submodules.\n\n builtin-apply.c |   49 ++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 38 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex da27075..24df282 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1984,6 +1984,26 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n+\t\t\t\tunsigned long *size_p)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\t*buf_p = xmalloc(100);\n+\t\t*size_p = snprintf(*buf_p, 100,\n+\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t} else {\n+\t\tenum object_type type;\n+\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n+\t\tif (!*buf_p)\n+\t\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tchar *buf;\n@@ -1994,20 +2014,18 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \talloc = 0;\n \tbuf = NULL;\n \tif (cached) {\n-\t\tif (ce) {\n-\t\t\tenum object_type type;\n-\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n-\t\t\talloc = size;\n-\t\t}\n+\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\t\treturn error(\"read of %s failed\", patch->old_name);\n+\t\talloc = size;\n \t}\n \telse if (patch->old_name) {\n \t\tsize = xsize_t(st->st_size);\n \t\talloc = size + 8192;\n \t\tbuf = xmalloc(alloc);\n-\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n+\t\tif (S_ISGITLINK(patch->old_mode))\n+\t\t\tsize = snprintf(buf, alloc,\n+\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t}\n \n@@ -2098,7 +2116,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t}\n \t\t\tif (!cached)\n \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n-\t\t\tif (changed)\n+\t\t\tif (changed && !S_ISGITLINK(patch->old_mode))\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n \t\t\tif (cached)\n@@ -2387,7 +2405,9 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \t\t\tdie(\"unable to stat newly created file %s\", path);\n \t\tfill_stat_cache_info(ce, &st);\n \t}\n-\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n+\tif (S_ISGITLINK(mode))\n+\t\tget_sha1_hex(buf + strlen(\"Subproject commit \"), ce->sha1);\n+\telse if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n \t\tdie(\"unable to create backing store for newly created file %s\", path);\n \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n \t\tdie(\"unable to add cache entry for %s\", path);\n@@ -2398,6 +2418,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tint fd;\n \tchar *nbuf;\n \n+\tif (S_ISGITLINK(mode)) {\n+\t\tstruct stat st;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\treturn 0;\n+\t\treturn mkdir(path, 0777);\n+\t}\n+\n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\n-- \n1.5.3.rc4.29.g74276-dirty\n"},{"id":"50404","messageId":"Pine.LNX.4.64.0708101332240.21857@racer.site","threadId":"9470","inReplyTo":"20070810093049.GA868MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH] git-apply: apply submodule changes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-08-10T12:34:26Z","receivedAt":"2007-08-10T12:34:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 10 Aug 2007, Sven Verdoolaege wrote:\n\n> Apply \"Subproject commit HEX\" changes produced by git-diff.\n> As usual in the current git, only the superproject itself is actually\n> modified (possibly creating empty directories for new submodules).\n\nFor rebase and cherry-pick, it would be nice if git just ignored the \nchanges in the submodules, provided that the submodule commit was not \naffected by the to-be-applied patches.\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"50405","messageId":"Pine.LNX.4.64.0708101337510.21857@racer.site","threadId":"9470","inReplyTo":"20070810093049.GA868MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH] git-apply: apply submodule changes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-08-10T12:39:21Z","receivedAt":"2007-08-10T12:39:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 10 Aug 2007, Sven Verdoolaege wrote:\n\n> @@ -2387,7 +2405,9 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n>  \t\t\tdie(\"unable to stat newly created file %s\", path);\n>  \t\tfill_stat_cache_info(ce, &st);\n>  \t}\n> -\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n> +\tif (S_ISGITLINK(mode))\n> +\t\tget_sha1_hex(buf + strlen(\"Subproject commit \"), ce->sha1);\n> +\telse if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n>  \t\tdie(\"unable to create backing store for newly created file %s\", path);\n>  \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n>  \t\tdie(\"unable to add cache entry for %s\", path);\n\nI guess that you need to catch an error from get_sha1_hex(), too.\n\nI hope it is not to much to ask for a patch to t7400 to show what this \npatch fixes?\n\nCiao,\nDscho\n"},{"id":"50410","messageId":"20070810135744.GA29243MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"20070810093049.GA868MdfPADPa@greensroom.kotnet.org","subject":"[PATCH resend] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-10T13:57:44Z","receivedAt":"2007-08-10T13:57:44Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"Apply \"Subproject commit HEX\" changes produced by git-diff.\nAs usual in the current git, only the superproject itself is actually\nmodified (possibly creating empty directories for new submodules).\nAny checked-out submodule is left untouched and is not required to\nbe up-to-date.\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n---\nThis second version has a test and an extra sanity check.\n\nI seem to be experiencing some problems receiving emails,\nso I'll reply to a message from Dscho here.\n\nJohannes Schindelin <Johannes.Schindelin <at> gmx.de> writes:\n> For rebase and cherry-pick, it would be nice if git just ignored the \n> changes in the submodules, provided that the submodule commit was not \n> affected by the to-be-applied patches.\n\nI have no idea what you mean.\n\nThe checked out copies of the submodules are ignored completely\n(if that is what you were talking about, then I hope this issue\nis clarified by the updated commit message).  In the superproject,\nthe change to the submodule is obviously not ignored, since it's\nan integral part of the patch.  However, git-apply will fail if\nthe original submodule commit does not correspond exactly to the\n\"from-file\" submodule commit.\nI don't think there is anything else we can do without a true\nrecursive git-diff/git-apply.\n\nskimo\n\n builtin-apply.c            |   50 ++++++++++++++++++++++++++++++++++---------\n t/t7400-submodule-basic.sh |    8 +++++++\n 2 files changed, 47 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex da27075..a38dbf1 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1984,6 +1984,26 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n+\t\t\t\tunsigned long *size_p)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\t*buf_p = xmalloc(100);\n+\t\t*size_p = snprintf(*buf_p, 100,\n+\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t} else {\n+\t\tenum object_type type;\n+\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n+\t\tif (!*buf_p)\n+\t\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tchar *buf;\n@@ -1994,20 +2014,18 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \talloc = 0;\n \tbuf = NULL;\n \tif (cached) {\n-\t\tif (ce) {\n-\t\t\tenum object_type type;\n-\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n-\t\t\talloc = size;\n-\t\t}\n+\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\t\treturn error(\"read of %s failed\", patch->old_name);\n+\t\talloc = size;\n \t}\n \telse if (patch->old_name) {\n \t\tsize = xsize_t(st->st_size);\n \t\talloc = size + 8192;\n \t\tbuf = xmalloc(alloc);\n-\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n+\t\tif (S_ISGITLINK(patch->old_mode))\n+\t\t\tsize = snprintf(buf, alloc,\n+\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t}\n \n@@ -2098,7 +2116,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t}\n \t\t\tif (!cached)\n \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n-\t\t\tif (changed)\n+\t\t\tif (changed && !S_ISGITLINK(patch->old_mode))\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n \t\t\tif (cached)\n@@ -2387,7 +2405,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \t\t\tdie(\"unable to stat newly created file %s\", path);\n \t\tfill_stat_cache_info(ce, &st);\n \t}\n-\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n+\tif (S_ISGITLINK(mode)) {\n+\t\tif (get_sha1_hex(buf + strlen(\"Subproject commit \"), ce->sha1))\n+\t\t\tdie(\"corrupt patch for subproject %s\", path);\n+\t} else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n \t\tdie(\"unable to create backing store for newly created file %s\", path);\n \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n \t\tdie(\"unable to add cache entry for %s\", path);\n@@ -2398,6 +2419,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tint fd;\n \tchar *nbuf;\n \n+\tif (S_ISGITLINK(mode)) {\n+\t\tstruct stat st;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\treturn 0;\n+\t\treturn mkdir(path, 0777);\n+\t}\n+\n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e8ce7cd..cede2e7 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -175,4 +175,12 @@ test_expect_success 'checkout superproject with subproject already present' '\n \tgit-checkout master\n '\n \n+test_expect_success 'rebase with subproject changes' '\n+\tgit-checkout initial &&\n+\techo t > t &&\n+\tgit add t &&\n+\tgit-commit -m \"change t\" &&\n+\tgit-rebase HEAD master\n+'\n+\n test_done\n-- \n1.5.3.rc4.29.g74276-dirty\n"},{"id":"50458","messageId":"7vd4xupqwh.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070810135744.GA29243MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH resend] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-11T05:43:42Z","receivedAt":"2007-08-11T05:43:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> @@ -1994,20 +2014,18 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n>  \talloc = 0;\n>  \tbuf = NULL;\n>  \tif (cached) {\n> -\t\tif (ce) {\n> -\t\t\tenum object_type type;\n> -\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n> -\t\t\tif (!buf)\n> -\t\t\t\treturn error(\"read of %s failed\",\n> -\t\t\t\t\t     patch->old_name);\n> -\t\t\talloc = size;\n> -\t\t}\n> +\t\tif (read_file_or_gitlink(ce, &buf, &size))\n> +\t\t\treturn error(\"read of %s failed\", patch->old_name);\n> +\t\talloc = size;\n>  \t}\n>  \telse if (patch->old_name) {\n>  \t\tsize = xsize_t(st->st_size);\n>  \t\talloc = size + 8192;\n>  \t\tbuf = xmalloc(alloc);\n> -\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> +\t\tif (S_ISGITLINK(patch->old_mode))\n> +\t\t\tsize = snprintf(buf, alloc,\n> +\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n> +\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n\nWho guarantees that ce is given to apply_data() in this codepath?\n"},{"id":"50461","messageId":"20070811064555.GC29996@liacs.nl","threadId":"9470","inReplyTo":"7vd4xupqwh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH resend] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@liacs.nl","sentAt":"2007-08-11T06:45:55Z","receivedAt":"2007-08-11T06:45:55Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Fri, Aug 10, 2007 at 10:43:42PM -0700, Junio C Hamano wrote:\n> Sven Verdoolaege <skimo@kotnet.org> writes:\n> >  \telse if (patch->old_name) {\n> >  \t\tsize = xsize_t(st->st_size);\n> >  \t\talloc = size + 8192;\n> >  \t\tbuf = xmalloc(alloc);\n> > -\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> > +\t\tif (S_ISGITLINK(patch->old_mode))\n> > +\t\t\tsize = snprintf(buf, alloc,\n> > +\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n> > +\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> \n> Who guarantees that ce is given to apply_data() in this codepath?\n\nOops.  I guess it shows that I only tested it through git-rebase.\nWould changing\n\n\t\tif (check_index) {\n\non line 2093 to\n\n\t\tif (check_index || S_ISGITLINK(patch->old_mode)) {\n\nbe acceptable?  Adding a conditional call to read_cache(), of course.\nWe're not going to be able to apply submodule patches without\nan index, anyway.\nOr should we just refuse to apply submodule patches if --index\nhas not been specified?\n\nskimo\n"},{"id":"50462","messageId":"7vvebmo8sr.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070811064555.GC29996@liacs.nl","subject":"Re: [PATCH resend] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-11T07:00:04Z","receivedAt":"2007-08-11T07:00:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@liacs.nl> writes:\n\n> Or should we just refuse to apply submodule patches if --index\n> has not been specified?\n\nIf somebody sends you a patch that adds a new gitlink, I think\nit is very natural to create a directory (or make sure a\ndirectory exists) there when applying without --index.\n\nMy gut feeling is that it would probably be the most user\nfriendly to just warn but ignore if you do not have submodule\nthere and a patch wants to modify that path.  After all, the\nfundamental idea behind our submodule support is that you as a\nsuperproject person should not even have to have the submodule\nchecked out.\n"},{"id":"50558","messageId":"20070812142340.GA10399MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"20070810093049.GA868MdfPADPa@greensroom.kotnet.org","subject":"[PATCH v3] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-12T14:23:40Z","receivedAt":"2007-08-12T14:23:40Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"Apply \"Subproject commit HEX\" changes produced by git-diff.\nAs usual in the current git, only the superproject itself is actually\nmodified (possibly creating empty directories for new submodules).\nAny checked-out submodule is left untouched and is not required to\nbe up-to-date.\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n---\nThe third version also works without --index and documents\nthe behaviour of git-apply in the presence of submodule changes.\n\nskimo\n\n Documentation/git-apply.txt |   14 +++++++++\n builtin-apply.c             |   69 +++++++++++++++++++++++++++++++++++-------\n t/t7400-submodule-basic.sh  |    8 +++++\n 3 files changed, 79 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex f03f661..804fdc3 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -171,6 +171,20 @@ apply.whitespace::\n \tWhen no `--whitespace` flag is given from the command\n \tline, this configuration item is used as the default.\n \n+Submodules\n+----------\n+If the patch contains any changes to submodules then gitlink:git-apply[1]\n+behaves as follows.\n+\n+If --index is specified (explicitly or implicitly), then the submodule\n+commits must match the index exactly for the patch to apply.  If any\n+of the submodules are checked-out, then these check-outs are completely\n+ignored, i.e., they are not required to be up-to-date or clean and they\n+are not updated.\n+\n+If --index is not specified, then the submodule commits in the patch\n+are ignored and only the absence of presence of the corresponding\n+subdirectory is checked and (if possible) updated.\n \n Author\n ------\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex da27075..eef596b 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1984,6 +1984,40 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n+\t\t\t\tunsigned long *size_p)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\t*buf_p = xmalloc(100);\n+\t\t*size_p = snprintf(*buf_p, 100,\n+\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t} else {\n+\t\tenum object_type type;\n+\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n+\t\tif (!*buf_p)\n+\t\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int read_gitlink_or_skip(struct patch *patch, struct cache_entry *ce,\n+\t\t\t\tchar *buf, unsigned long alloc)\n+{\n+\tif (ce)\n+\t\treturn snprintf(buf, alloc,\n+\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\n+\t/* We can't apply the submodule change without an index, so just\n+\t * skip the patch itself and only create/remove directory.\n+\t */\n+\tpatch->fragments = NULL;\n+\treturn 0;\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tchar *buf;\n@@ -1994,20 +2028,17 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \talloc = 0;\n \tbuf = NULL;\n \tif (cached) {\n-\t\tif (ce) {\n-\t\t\tenum object_type type;\n-\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n-\t\t\talloc = size;\n-\t\t}\n+\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\t\treturn error(\"read of %s failed\", patch->old_name);\n+\t\talloc = size;\n \t}\n \telse if (patch->old_name) {\n \t\tsize = xsize_t(st->st_size);\n \t\talloc = size + 8192;\n \t\tbuf = xmalloc(alloc);\n-\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n+\t\tif (S_ISGITLINK(patch->old_mode))\n+\t\t\tsize = read_gitlink_or_skip(patch, ce, buf, alloc);\n+\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t}\n \n@@ -2098,7 +2129,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t}\n \t\t\tif (!cached)\n \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n-\t\t\tif (changed)\n+\t\t\tif (changed && !S_ISGITLINK(patch->old_mode))\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n \t\t\tif (cached)\n@@ -2354,7 +2385,11 @@ static void remove_file(struct patch *patch, int rmdir_empty)\n \t\tcache_tree_invalidate_path(active_cache_tree, patch->old_name);\n \t}\n \tif (!cached) {\n-\t\tif (!unlink(patch->old_name) && rmdir_empty) {\n+\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\tif (rmdir(patch->old_name))\n+\t\t\t\twarning(\"unable to remove submodule %s\",\n+\t\t\t\t\tpatch->old_name);\n+\t\t} else if (!unlink(patch->old_name) && rmdir_empty) {\n \t\t\tchar *name = xstrdup(patch->old_name);\n \t\t\tchar *end = strrchr(name, '/');\n \t\t\twhile (end) {\n@@ -2387,7 +2422,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \t\t\tdie(\"unable to stat newly created file %s\", path);\n \t\tfill_stat_cache_info(ce, &st);\n \t}\n-\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n+\tif (S_ISGITLINK(mode)) {\n+\t\tif (get_sha1_hex(buf + strlen(\"Subproject commit \"), ce->sha1))\n+\t\t\tdie(\"corrupt patch for subproject %s\", path);\n+\t} else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n \t\tdie(\"unable to create backing store for newly created file %s\", path);\n \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n \t\tdie(\"unable to add cache entry for %s\", path);\n@@ -2398,6 +2436,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tint fd;\n \tchar *nbuf;\n \n+\tif (S_ISGITLINK(mode)) {\n+\t\tstruct stat st;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\treturn 0;\n+\t\treturn mkdir(path, 0777);\n+\t}\n+\n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e8ce7cd..cede2e7 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -175,4 +175,12 @@ test_expect_success 'checkout superproject with subproject already present' '\n \tgit-checkout master\n '\n \n+test_expect_success 'rebase with subproject changes' '\n+\tgit-checkout initial &&\n+\techo t > t &&\n+\tgit add t &&\n+\tgit-commit -m \"change t\" &&\n+\tgit-rebase HEAD master\n+'\n+\n test_done\n-- \n1.5.3.rc4.30.gd0c97-dirty\n"},{"id":"50566","messageId":"7vwsw0ipp2.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070812142340.GA10399MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v3] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-12T18:16:09Z","receivedAt":"2007-08-12T18:16:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n> index f03f661..804fdc3 100644\n> --- a/Documentation/git-apply.txt\n> +++ b/Documentation/git-apply.txt\n> @@ -171,6 +171,20 @@ apply.whitespace::\n>  \tWhen no `--whitespace` flag is given from the command\n>  \tline, this configuration item is used as the default.\n>  \n> +Submodules\n> +----------\n> +If the patch contains any changes to submodules then gitlink:git-apply[1]\n> +behaves as follows.\n\nperhaps \"as follows wrt the submodules\"...\n\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index da27075..eef596b 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -1984,6 +1984,40 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n>  \treturn 0;\n>  }\n>  \n> +static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n> +\t\t\t\tunsigned long *size_p)\n> +{\n> +\tif (!ce)\n> +\t\treturn 0;\n> +\n> +\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n> +\t\t*buf_p = xmalloc(100);\n> +\t\t*size_p = snprintf(*buf_p, 100,\n> +\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n> +\t} else {\n> +\t\tenum object_type type;\n> +\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n> +\t\tif (!*buf_p)\n> +\t\t\treturn -1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n\nOk, read_file_or_gitlink() expects ce taken from the current\nindex and fills *buf_p with the preimage to be patched from it.\n\n> +static int read_gitlink_or_skip(struct patch *patch, struct cache_entry *ce,\n> +\t\t\t\tchar *buf, unsigned long alloc)\n> +{\n> +\tif (ce)\n> +\t\treturn snprintf(buf, alloc,\n> +\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n> +\n> +\t/* We can't apply the submodule change without an index, so just\n> +\t * skip the patch itself and only create/remove directory.\n> +\t */\n> +\tpatch->fragments = NULL;\n> +\treturn 0;\n> +}\n\nHmmmm...  see below.\n\n>  static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n>  {\n>  \tchar *buf;\n> @@ -1994,20 +2028,17 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n>  \talloc = 0;\n>  \tbuf = NULL;\n>  \tif (cached) {\n> +\t\tif (read_file_or_gitlink(ce, &buf, &size))\n> +\t\t\treturn error(\"read of %s failed\", patch->old_name);\n> +\t\talloc = size;\n>  \t}\n\nThis part is consistent with the read_file_or_gitlink()\nsemantics above...\n\n>  \telse if (patch->old_name) {\n>  \t\tsize = xsize_t(st->st_size);\n>  \t\talloc = size + 8192;\n>  \t\tbuf = xmalloc(alloc);\n> -\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> +\t\tif (S_ISGITLINK(patch->old_mode))\n> +\t\t\tsize = read_gitlink_or_skip(patch, ce, buf, alloc);\n> +\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n>  \t\t\treturn error(\"read of %s failed\", patch->old_name);\n>  \t}\n\nread_old_data() gets the lstat information from the current\nfilesystem data at old_name, and gives the preimage to be\npatched, and naturally it bombs out if it is a directory, but\nwhen we are applying a change to gitlink, the patch expects\nold_name to be a directory.\n\nSo you introduced read_gitlink_or_skip() to work it around.  But\nthis makes me wonder...\n\n - what does ce have to do in this codepath?  read_old_data()\n   does not care about what is in the index (in fact, in the\n   index the entry can be a symlink when the path on the\n   filesystem is a regular file, and it reads from the regular\n   file as asked--it does not even look at ce by design).  \n   if you have a regular file there in the current version, ce\n   would say it is a regular file blob and you would not want\n   read_gitlink_or_skip() to say \"Subproject commit xyz...\".\n\n - what is alloc at this point?  it is based on the size of\n   directory st->st_size.\n\nI think dropping fragments for a patch that tries to modify a\ngitlink here is fine, but that can be done regardless of what ce\nis.\n\nThe type-mismatch case to attempt to apply gitlink patch to a\nregular blob is covered much earlier in check_patch().  It\ncomplains if st_mode does not match patch->old_mode; I think you\nneed to adjust it a bit to:\n\n - allow gitlink patch to a path that currently has nothing (no\n   submodule checked out) or a directory that has \".git/\"\n   (i.e. submodule checked out).\n\n - reject gitlink patch otherwise.\n"},{"id":"50572","messageId":"20070812185006.GG999MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"7vwsw0ipp2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v3] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-12T18:50:06Z","receivedAt":"2007-08-12T18:50:06Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Sun, Aug 12, 2007 at 11:16:09AM -0700, Junio C Hamano wrote:\n> Sven Verdoolaege <skimo@kotnet.org> writes:\n> >  \telse if (patch->old_name) {\n> >  \t\tsize = xsize_t(st->st_size);\n> >  \t\talloc = size + 8192;\n> >  \t\tbuf = xmalloc(alloc);\n> > -\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> > +\t\tif (S_ISGITLINK(patch->old_mode))\n> > +\t\t\tsize = read_gitlink_or_skip(patch, ce, buf, alloc);\n> > +\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> >  \t\t\treturn error(\"read of %s failed\", patch->old_name);\n> >  \t}\n> \n> read_old_data() gets the lstat information from the current\n> filesystem data at old_name, and gives the preimage to be\n> patched, and naturally it bombs out if it is a directory, but\n> when we are applying a change to gitlink, the patch expects\n> old_name to be a directory.\n> \n> So you introduced read_gitlink_or_skip() to work it around.  But\n> this makes me wonder...\n> \n>  - what does ce have to do in this codepath?  read_old_data()\n>    does not care about what is in the index (in fact, in the\n>    index the entry can be a symlink when the path on the\n>    filesystem is a regular file, and it reads from the regular\n>    file as asked--it does not even look at ce by design).  \n>    if you have a regular file there in the current version, ce\n>    would say it is a regular file blob and you would not want\n>    read_gitlink_or_skip() to say \"Subproject commit xyz...\".\n\nHmmm... the documentation says that if --index is in effect\nthen the file to be patched in the work tree is supposed to be \nup-to-date.  Then for files it shouldn't matter if the data\ncomes from the index or not.  For submodules it's crucial to\nlook at the index (if it is available), since that's the\nonly place you can find the current state of the submodule.\n(Remember that git currently doesn't automatically update submodules.)\n\nIf I specify --index (e.g., through git rebase), then I really\nwouldn't want the patch to apply if the old submodule commit\nin the patch doesn't match the submodule commit in the index.\nWould you?\n\n>  - what is alloc at this point?  it is based on the size of\n>    directory st->st_size.\n\nI assume you mean \"size\".  For submodules, the value isn't used,\nexcept to test that the \"file\" is empty (size==0) on delete.\nSo it seems safe to set it to zero for submodules in any case.\n\n> I think dropping fragments for a patch that tries to modify a\n> gitlink here is fine, but that can be done regardless of what ce\n> is.\n\nAs explained above, I don't think it would a good idea to\njust drop gitlink patches if --index is specified.\n\n> The type-mismatch case to attempt to apply gitlink patch to a\n> regular blob is covered much earlier in check_patch().  It\n> complains if st_mode does not match patch->old_mode; I think you\n> need to adjust it a bit to:\n> \n>  - allow gitlink patch to a path that currently has nothing (no\n>    submodule checked out) or a directory that has \".git/\"\n>    (i.e. submodule checked out).\n> \n>  - reject gitlink patch otherwise.\n\nAre you talking about the case where --index is specified?\nOtherwise, I don't think we can make any assumptions on\nwhat's inside the subdirectory.\n\nskimo\n"},{"id":"50575","messageId":"7vr6m8imj6.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070812185006.GG999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v3] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-12T19:24:29Z","receivedAt":"2007-08-12T19:24:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n>>  - what does ce have to do in this codepath?  read_old_data()\n>>    does not care about what is in the index (in fact, in the\n>>    index the entry can be a symlink when the path on the\n>>    filesystem is a regular file, and it reads from the regular\n>>    file as asked--it does not even look at ce by design).  \n>>    if you have a regular file there in the current version, ce\n>>    would say it is a regular file blob and you would not want\n>>    read_gitlink_or_skip() to say \"Subproject commit xyz...\".\n>\n> Hmmm... the documentation says that if --index is in effect\n> then the file to be patched in the work tree is supposed to be \n> up-to-date.\n\nBut that is the job of check_patch(), not this function, isn't it?\n\n>> The type-mismatch case to attempt to apply gitlink patch to a\n>> regular blob is covered much earlier in check_patch().  It\n>> complains if st_mode does not match patch->old_mode; I think you\n>> need to adjust it a bit to:\n>> \n>>  - allow gitlink patch to a path that currently has nothing (no\n>>    submodule checked out) or a directory that has \".git/\"\n>>    (i.e. submodule checked out).\n>> \n>>  - reject gitlink patch otherwise.\n>\n> Are you talking about the case where --index is specified?\n\nTalking about both cases, and the division of responsibility\nbetween check_patch() and apply_data().\n"},{"id":"50625","messageId":"20070813093740.GA4684@liacs.nl","threadId":"9470","inReplyTo":"7vr6m8imj6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v3] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@liacs.nl","sentAt":"2007-08-13T09:37:40Z","receivedAt":"2007-08-13T09:37:40Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Sun, Aug 12, 2007 at 12:24:29PM -0700, Junio C Hamano wrote:\n> Sven Verdoolaege <skimo@kotnet.org> writes:\n> \n> >>  - what does ce have to do in this codepath?  read_old_data()\n> >>    does not care about what is in the index (in fact, in the\n> >>    index the entry can be a symlink when the path on the\n> >>    filesystem is a regular file, and it reads from the regular\n> >>    file as asked--it does not even look at ce by design).  \n> >>    if you have a regular file there in the current version, ce\n> >>    would say it is a regular file blob and you would not want\n> >>    read_gitlink_or_skip() to say \"Subproject commit xyz...\".\n> >\n> > Hmmm... the documentation says that if --index is in effect\n> > then the file to be patched in the work tree is supposed to be \n> > up-to-date.\n> \n> But that is the job of check_patch(), not this function, isn't it?\n\nAh... so you want me to check that there has been no type change\nto the submodule in check_patch (and then I can use my\nread_gitlink_or_skip as is).  Is that right?\n\n> >> The type-mismatch case to attempt to apply gitlink patch to a\n> >> regular blob is covered much earlier in check_patch().  It\n> >> complains if st_mode does not match patch->old_mode; I think you\n> >> need to adjust it a bit to:\n> >> \n> >>  - allow gitlink patch to a path that currently has nothing (no\n> >>    submodule checked out) or a directory that has \".git/\"\n> >>    (i.e. submodule checked out).\n> >> \n> >>  - reject gitlink patch otherwise.\n> >\n> > Are you talking about the case where --index is specified?\n> \n> Talking about both cases, and the division of responsibility\n> between check_patch() and apply_data().\n\nI'll do that for the --index case, but I really think it doesn't\nmake sense for the other case.  If we're not in a git repo,\nthen the submodule, which may very well be present, is not\ngoing to be a git repo either.\n\nWhat could make sense is to enforce that in the --index case,\nthe submodule directory is either empty or a git repo and\nthat in the non --index case it is empty.\n\nskimo\n"},{"id":"50643","messageId":"20070813171349.GL999MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"20070813093740.GA4684@liacs.nl","subject":"[PATCH v4] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-13T17:13:49Z","receivedAt":"2007-08-13T17:13:49Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"Apply \"Subproject commit HEX\" changes produced by git-diff.\nAs usual in the current git, only the superproject itself is actually\nmodified (possibly creating empty directories for new submodules).\nAny checked-out submodule is left untouched and is not required to\nbe up-to-date.\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n---\nThis version adds an extra check to verify that a submodule\nstill looks like a submodule in the work tree.\n\n Documentation/git-apply.txt |   14 +++++++\n builtin-apply.c             |   91 +++++++++++++++++++++++++++++++++++++------\n t/t7400-submodule-basic.sh  |    8 ++++\n 3 files changed, 101 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex f03f661..4c7e3a2 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -171,6 +171,20 @@ apply.whitespace::\n \tWhen no `--whitespace` flag is given from the command\n \tline, this configuration item is used as the default.\n \n+Submodules\n+----------\n+If the patch contains any changes to submodules then gitlink:git-apply[1]\n+treats these changes as follows.\n+\n+If --index is specified (explicitly or implicitly), then the submodule\n+commits must match the index exactly for the patch to apply.  If any\n+of the submodules are checked-out, then these check-outs are completely\n+ignored, i.e., they are not required to be up-to-date or clean and they\n+are not updated.\n+\n+If --index is not specified, then the submodule commits in the patch\n+are ignored and only the absence of presence of the corresponding\n+subdirectory is checked and (if possible) updated.\n \n Author\n ------\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex da27075..8ba20a6 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -12,6 +12,7 @@\n #include \"blob.h\"\n #include \"delta.h\"\n #include \"builtin.h\"\n+#include \"refs.h\"\n \n /*\n  *  --check turns on checking that the working tree matches the\n@@ -1984,6 +1985,40 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n+\t\t\t\tunsigned long *size_p)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\t*buf_p = xmalloc(100);\n+\t\t*size_p = snprintf(*buf_p, 100,\n+\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t} else {\n+\t\tenum object_type type;\n+\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n+\t\tif (!*buf_p)\n+\t\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int read_gitlink_or_skip(struct patch *patch, struct cache_entry *ce,\n+\t\t\t\tchar *buf, unsigned long alloc)\n+{\n+\tif (ce)\n+\t\treturn snprintf(buf, alloc,\n+\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\n+\t/* We can't apply the submodule change without an index, so just\n+\t * skip the patch itself and only create/remove directory.\n+\t */\n+\tpatch->fragments = NULL;\n+\treturn 0;\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tchar *buf;\n@@ -1994,20 +2029,17 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \talloc = 0;\n \tbuf = NULL;\n \tif (cached) {\n-\t\tif (ce) {\n-\t\t\tenum object_type type;\n-\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n-\t\t\talloc = size;\n-\t\t}\n+\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\t\treturn error(\"read of %s failed\", patch->old_name);\n+\t\talloc = size;\n \t}\n \telse if (patch->old_name) {\n \t\tsize = xsize_t(st->st_size);\n \t\talloc = size + 8192;\n \t\tbuf = xmalloc(alloc);\n-\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n+\t\tif (S_ISGITLINK(patch->old_mode))\n+\t\t\tsize = read_gitlink_or_skip(patch, ce, buf, alloc);\n+\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t}\n \n@@ -2055,6 +2087,20 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists)\n \treturn 0;\n }\n \n+/* Check that the directory corresponding to a gitlink is either\n+ * empty or a git repo.\n+ */\n+static int verify_gitlink_clean(const char *path)\n+{\n+\tunsigned char sha1[20];\n+\n+\tif (!rmdir(path)) {\n+\t\tmkdir(path, 0777);\n+\t\treturn 0;\n+\t}\n+\treturn resolve_gitlink_ref(path, \"HEAD\", sha1);\n+}\n+\n static int check_patch(struct patch *patch, struct patch *prev_patch)\n {\n \tstruct stat st;\n@@ -2096,8 +2142,15 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t\t    lstat(old_name, &st))\n \t\t\t\t\treturn -1;\n \t\t\t}\n-\t\t\tif (!cached)\n+\t\t\tif (!cached) {\n \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n+\t\t\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\t\t\tchanged &= TYPE_CHANGED;\n+\t\t\t\t\tif (!changed &&\n+\t\t\t\t\t    verify_gitlink_clean(patch->old_name))\n+\t\t\t\t\t\tchanged |= TYPE_CHANGED;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tif (changed)\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n@@ -2354,7 +2407,11 @@ static void remove_file(struct patch *patch, int rmdir_empty)\n \t\tcache_tree_invalidate_path(active_cache_tree, patch->old_name);\n \t}\n \tif (!cached) {\n-\t\tif (!unlink(patch->old_name) && rmdir_empty) {\n+\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\tif (rmdir(patch->old_name))\n+\t\t\t\twarning(\"unable to remove submodule %s\",\n+\t\t\t\t\tpatch->old_name);\n+\t\t} else if (!unlink(patch->old_name) && rmdir_empty) {\n \t\t\tchar *name = xstrdup(patch->old_name);\n \t\t\tchar *end = strrchr(name, '/');\n \t\t\twhile (end) {\n@@ -2387,7 +2444,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \t\t\tdie(\"unable to stat newly created file %s\", path);\n \t\tfill_stat_cache_info(ce, &st);\n \t}\n-\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n+\tif (S_ISGITLINK(mode)) {\n+\t\tif (get_sha1_hex(buf + strlen(\"Subproject commit \"), ce->sha1))\n+\t\t\tdie(\"corrupt patch for subproject %s\", path);\n+\t} else if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n \t\tdie(\"unable to create backing store for newly created file %s\", path);\n \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n \t\tdie(\"unable to add cache entry for %s\", path);\n@@ -2398,6 +2458,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tint fd;\n \tchar *nbuf;\n \n+\tif (S_ISGITLINK(mode)) {\n+\t\tstruct stat st;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\treturn 0;\n+\t\treturn mkdir(path, 0777);\n+\t}\n+\n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e8ce7cd..cede2e7 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -175,4 +175,12 @@ test_expect_success 'checkout superproject with subproject already present' '\n \tgit-checkout master\n '\n \n+test_expect_success 'rebase with subproject changes' '\n+\tgit-checkout initial &&\n+\techo t > t &&\n+\tgit add t &&\n+\tgit-commit -m \"change t\" &&\n+\tgit-rebase HEAD master\n+'\n+\n test_done\n-- \n1.5.3.rc4.68.g4411e-dirty\n"},{"id":"50652","messageId":"7vmywvfag3.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070813171349.GL999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v4] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-13T20:26:04Z","receivedAt":"2007-08-13T20:26:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> +static int read_gitlink_or_skip(struct patch *patch, struct cache_entry *ce,\n> +\t\t\t\tchar *buf, unsigned long alloc)\n> +{\n> +\tif (ce)\n> +\t\treturn snprintf(buf, alloc,\n> +\t\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n> +\n> +\t/* We can't apply the submodule change without an index, so just\n> +\t * skip the patch itself and only create/remove directory.\n> +\t */\n> +\tpatch->fragments = NULL;\n> +\treturn 0;\n> +}\n> @@ -1994,20 +2029,17 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n>  \talloc = 0;\n>  \tbuf = NULL;\n>  \tif (cached) {\n> +\t\tif (read_file_or_gitlink(ce, &buf, &size))\n> +\t\t\treturn error(\"read of %s failed\", patch->old_name);\n> +\t\talloc = size;\n> ...\n>  \telse if (patch->old_name) {\n>  \t\tsize = xsize_t(st->st_size);\n>  \t\talloc = size + 8192;\n>  \t\tbuf = xmalloc(alloc);\n> -\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n> +\t\tif (S_ISGITLINK(patch->old_mode))\n> +\t\t\tsize = read_gitlink_or_skip(patch, ce, buf, alloc);\n> +\t\telse if (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n>  \t\t\treturn error(\"read of %s failed\", patch->old_name);\n>  \t}\n\nI think the logic here sounds sane.  The read_file_or_gitlink()\nabstraction in the first if() part was so nice that I was hoping\nwe could do something similar in this \"else if\" part, without\nhiding the assignment to patch->fragments as an obscure side\neffect of calling read_gitlink_or_skip().  That assignment is a\ngitlink specific hack so it might be easier to read if this part\nis written like:\n\n\t} else if (patch->old_name && S_ISGITLINK(patch->old_mode)) {\n\t\tif (ce)\n                \tsize = snprintf(....)\n\t\telse {\n                \t/* we cannot apply gitlink mods without index */\n\t\t\tsize = 0;\n\t\t\tpatch->fragments = NULL;\n\t\t}\n        } else if (patch->old_name) {\n\t\t... original code here ...\n\n> @@ -2055,6 +2087,20 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists)\n>  \treturn 0;\n>  }\n>  \n> +/* Check that the directory corresponding to a gitlink is either\n> + * empty or a git repo.\n> + */\n> +static int verify_gitlink_clean(const char *path)\n> +{\n> +\tunsigned char sha1[20];\n> +\n> +\tif (!rmdir(path)) {\n> +\t\tmkdir(path, 0777);\n> +\t\treturn 0;\n> +\t}\n> +\treturn resolve_gitlink_ref(path, \"HEAD\", sha1);\n> +}\n\nIs it the responsibility of the caller of this function to make\nsure that path is either absent or is a directory?  Can there be\na regular file at path, which would cause rmdir to fail?  I do\nnot think it is sensible to call resolve_gitlink_ref() on such a\npath, so let's say the caller will make sure that path is\ngitlink in the current (i.e. in index) tree before calling this\nfunction.\n\n>  static int check_patch(struct patch *patch, struct patch *prev_patch)\n>  {\n>  \tstruct stat st;\n> @@ -2096,8 +2142,15 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n>  \t\t\t\t    lstat(old_name, &st))\n>  \t\t\t\t\treturn -1;\n>  \t\t\t}\n> -\t\t\tif (!cached)\n> +\t\t\tif (!cached) {\n>  \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n\nHere, ce_match_stat() sees if the index and work tree match.\nYou could have a regular file there but you haven't checked\n(which is not a crime, yet).\n\n> +\t\t\t\tif (S_ISGITLINK(patch->old_mode)) {\n\nI think the rmdir/mkdir sequence should be done only when ce is\na gitlink.  Perhaps it is just the matter of:\n\n                                if (S_ISGITLINK(patch->old_mode) &&\n                                    S_ISGITLINK(ntohl(ce->ce_mode))) {\n                                        ...\n                                }\n\n> +\t\t\t\t\tchanged &= TYPE_CHANGED;\n> +\t\t\t\t\tif (!changed &&\n> +\t\t\t\t\t    verify_gitlink_clean(patch->old_name))\n> +\t\t\t\t\t\tchanged |= TYPE_CHANGED;\n> +\t\t\t\t}\n> +\t\t\t}\n\nThis part is very confusing.  You discard all changes other than\nTYPE_CHANGED, and give TYPE_CHANGED and nothing else if gitlink\nis not clean.  I suspect \"changed &= ~TYPE_CHANGED\" might be\nwhat you meant, but I do not know what you are trying to do\nhere.\n"},{"id":"50686","messageId":"20070814083940.GN999MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"7vd4xqeilh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v4] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-14T08:39:40Z","receivedAt":"2007-08-14T08:39:40Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Mon, Aug 13, 2007 at 11:27:38PM -0700, Junio C Hamano wrote:\n>  * write_out_one_result() calls remove_file() and create_file()\n>    to match the work tree to the result you prepared with\n>    apply_data().\n> \n>    - remove_file() is changed not to do any for gitlink.  We\n>      _might_ want to try rmdir() if there is an otherwise empty\n>      directory there, but currently we cannot do much to the\n>      failure on that, so I did not bother with it.\n\nWe could at least warn about it, which is what my patch did.\n\n>    - create_file() does three things:\n>      - create a file in the work tree to match the result;\n>      - update the index with the patch result;\n>      - invalidate cache-tree entry for the path.\n> \n>      For the first task, create_one_file() is usually used to\n>      create a blob (either regular file or a symlink).  For\n>      gitlinks, we do not affect the work tree for now, just like\n>      checkout_entry().\n\nIt creates the subdirectory, though, and git-apply should do so\ntoo since it expects the subdirectory to be there for subsequent\npatches (at least in the --index case).\n\n> diff --git a/builtin-apply.c b/builtin-apply.c\n\nDid you remove the documentation on purpose ?\n\n> +static int verify_index_match(struct cache_entry *ce, struct stat *st)\n> +{\n> +\tif (!ce_match_stat(ce, st, 1))\n> +\t\treturn 0;\n> +\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n> +\t\tif (S_ISDIR(st->st_mode))\n> +\t\t\treturn 0;\n> +\t}\n\nNot a big deal, but ce_match_stat already checks for that.\nThat's why I was checking for TYPE_CHANGED in its return\nvalue.\n\n> @@ -2096,16 +2142,22 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n>  \t\t\t\t    lstat(old_name, &st))\n>  \t\t\t\t\treturn -1;\n>  \t\t\t}\n> -\t\t\tif (!cached)\n> -\t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n> -\t\t\tif (changed)\n> +\t\t\tif (!cached && verify_index_match(ce, &st))\n>  \t\t\t\treturn error(\"%s: does not match index\",\n>  \t\t\t\t\t     old_name);\n>  \t\t\tif (cached)\n>  \t\t\t\tst_mode = ntohl(ce->ce_mode);\n> +\t\t} else if (stat_ret < 0) {\n> +\t\t\tif (errno == ENOENT && S_ISGITLINK(patch->old_mode))\n> +\t\t\t\t/*\n> +\t\t\t\t * It is Ok not to have the submodule\n> +\t\t\t\t * checked out at all.\n> +\t\t\t\t */\n> +\t\t\t\t;\n> +\t\t\telse\n> +\t\t\t\treturn error(\"%s: %s\", old_name,\n> +\t\t\t\t\t     strerror(errno));\n>  \t\t}\n\nShouldn't you be consistent with the --index case and require the\nsubdirectory to exist?\n\nskimo\n"},{"id":"50689","messageId":"7vps1qcwj4.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070814083940.GN999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v4] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-14T09:09:35Z","receivedAt":"2007-08-14T09:09:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n>> diff --git a/builtin-apply.c b/builtin-apply.c\n>\n> Did you remove the documentation on purpose ?\n\nNo, I just wanted to get a feedback on a (possibly partial)\ncleanup, as I couldn't make heads or tails of your patch\nespecially around that TYPE_CHANGED part, and also the part to\nwrite out the results.\n"},{"id":"50725","messageId":"7vmywtc2e3.fsf@assigned-by-dhcp.cox.net","threadId":"9470","inReplyTo":"20070813171349.GL999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v4] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-14T20:00:36Z","receivedAt":"2007-08-14T20:00:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> @@ -2096,8 +2142,15 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n>  \t\t\t\t    lstat(old_name, &st))\n>  \t\t\t\t\treturn -1;\n>  \t\t\t}\n> -\t\t\tif (!cached)\n> +\t\t\tif (!cached) {\n>  \t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n> +\t\t\t\tif (S_ISGITLINK(patch->old_mode)) {\n> +\t\t\t\t\tchanged &= TYPE_CHANGED;\n> +\t\t\t\t\tif (!changed &&\n> +\t\t\t\t\t    verify_gitlink_clean(patch->old_name))\n> +\t\t\t\t\t\tchanged |= TYPE_CHANGED;\n> +\t\t\t\t}\n> +\t\t\t}\n\nIn this codepath, we know the patch wants to either modify the\npath at old_name or remove old_name.  If we are going to affect\nthe work tree, we have run lstat on it, and ran checkout_entry() \nif we did not have anything there and did lstat() again.\n\nI think the check \"S_ISGITLINK(patch->old_mode)\" is wrong\n(that's where my confusion while reading your patch came from).\nIt has to check ce's mode, not patch->old_mode, because we are\nverifying if the index matches with the work tree in this\ncodepath.  If you fix it to S_ISGITLINK(ntohl(ce->ce_mode)),\nI think I can see what you are trying to do.\n\nWhen ce is not a gitlink, you keep the original behaviour, which\nis assuring that you did not break things for people who do not\nuse gitlink.\n\nI am still having trouble with the TYPE_CHANGED bits.  You\ndiscard everything other than TYPE_CHANGED, and \n\n - if ce_match_stat() returned TYPE_CHANGED, then that is given\n   to later processing to cause us to fail \"oops, path is not up\n   to date\";\n\n - if ce_match_stat() did not return TYPE_CHANGED, that means we\n   found a directory at the path (ce_match_stat_basic() says\n   so).  In such a case you call verify_gitlink_clean(), but it\n   essentially says \"make sure there is either an empty\n   directory or some repository\".  Maybe we do not even have to\n   have this extra check?\n\nWhen ce is a gitlink, ce_match_stat() says DATA_CHANGED if the\ncommit in the work tree of the subproject is different.  From\nthe earlier discussions, we do want to discard DATA_CHANGED for\nthis codepath.\n\nSo it looks almost Ok after spending a few days looking at this\ncode.  Finally.\n\nHowever, if it takes _me_ three days to understand this hunk,\n(admittably, the parameter to S_ISGITLINK() completely confused\nme originally, and I also had other things to do, so it was not\n\"72 hours\"), I do not think the code with your patch is\nmaintainable by anybody.  At least we would need to have a few\nwords of comment to describe what is going on there.\n\n\tif (!cached) {\n        \tchanged = ce_match_stat(ce, &st, 1);\n                if (S_ISGITLINK(ntohl(ce->ce_mode)))\n                \t/*\n\t\t\t * ce_match_stat() reports the\n\t\t\t * difference between the commit object\n                         * name in the index and what is checked\n\t\t\t * out in the work tree of subproject;\n                         * because we do not recurse, we do not\n\t\t\t * want to insist on them matching with\n                         * each other.\n                         */\n                \tchanged &= ~DATA_CHANGED;\n\t}\n        if (changed)\n        \treturn error(\"%s: does not match index\", old_name);\n"},{"id":"50798","messageId":"20070815172209.GD1070MdfPADPa@greensroom.kotnet.org","threadId":"9470","inReplyTo":"7vps1qcwj4.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH v6] git-apply: apply submodule changes","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-08-15T17:22:09Z","receivedAt":"2007-08-15T17:22:09Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"Apply \"Subproject commit HEX\" changes produced by git-diff.\nAs usual in the current git, only the superproject itself is actually\nmodified (possibly creating empty directories for new submodules).\nAny checked-out submodule is left untouched and is not required to\nbe up-to-date.\n\nWith clean-ups from Junio C Hamano.\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n---\nOn Tue, Aug 14, 2007 at 02:09:35AM -0700, Junio C Hamano wrote:\n> Sven Verdoolaege <skimo@kotnet.org> writes:\n> >> diff --git a/builtin-apply.c b/builtin-apply.c\n> >\n> > Did you remove the documentation on purpose ?\n> \n> No, I just wanted to get a feedback on a (possibly partial)\n> cleanup, as I couldn't make heads or tails of your patch\n> especially around that TYPE_CHANGED part, and also the part to\n> write out the results.\n\nI agree that the TYPE_CHANGED thing may have been confusing,\nso I kept your version (although I switched the tests around,\nsince there is no point in checking if the stat info matches\nif you're going to ignore the result anyway).\n\nOther than that, the only change wrt to your version is that\nI added back the creation and (attempt at) removal of the\ncorresponding subdirectory.\n\nI'm not sure if you intented to remove the check for\neither an empty dir or a git repo.  You asked me to add\nit before, but then you removed it in your version.\nI didn't add it back again.\n\nskimo\n\n Documentation/git-apply.txt |   14 +++++\n builtin-apply.c             |  112 +++++++++++++++++++++++++++++++++----------\n t/t7400-submodule-basic.sh  |   17 +++++++\n 3 files changed, 118 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex f03f661..4c7e3a2 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -171,6 +171,20 @@ apply.whitespace::\n \tWhen no `--whitespace` flag is given from the command\n \tline, this configuration item is used as the default.\n \n+Submodules\n+----------\n+If the patch contains any changes to submodules then gitlink:git-apply[1]\n+treats these changes as follows.\n+\n+If --index is specified (explicitly or implicitly), then the submodule\n+commits must match the index exactly for the patch to apply.  If any\n+of the submodules are checked-out, then these check-outs are completely\n+ignored, i.e., they are not required to be up-to-date or clean and they\n+are not updated.\n+\n+If --index is not specified, then the submodule commits in the patch\n+are ignored and only the absence of presence of the corresponding\n+subdirectory is checked and (if possible) updated.\n \n Author\n ------\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex da27075..8055c7d 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1984,6 +1984,25 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n+\t\t\t\tunsigned long *size_p)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\t*buf_p = xmalloc(100);\n+\t\t*size_p = snprintf(*buf_p, 100,\n+\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t} else {\n+\t\tenum object_type type;\n+\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n+\t\tif (!*buf_p)\n+\t\t\treturn -1;\n+\t}\n+\treturn 0;\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tchar *buf;\n@@ -1994,22 +2013,32 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \talloc = 0;\n \tbuf = NULL;\n \tif (cached) {\n-\t\tif (ce) {\n-\t\t\tenum object_type type;\n-\t\t\tbuf = read_sha1_file(ce->sha1, &type, &size);\n-\t\t\tif (!buf)\n+\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\t\treturn error(\"read of %s failed\", patch->old_name);\n+\t\talloc = size;\n+\t} else if (patch->old_name) {\n+\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\tif (ce)\n+\t\t\t\tread_file_or_gitlink(ce, &buf, &size);\n+\t\t\telse {\n+\t\t\t\t/*\n+\t\t\t\t * There is no way to apply subproject\n+\t\t\t\t * patch without looking at the index.\n+\t\t\t\t */\n+\t\t\t\tpatch->fragments = NULL;\n+\t\t\t\tsize = 0;\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\tsize = xsize_t(st->st_size);\n+\t\t\talloc = size + 8192;\n+\t\t\tbuf = xmalloc(alloc);\n+\t\t\tif (read_old_data(st, patch->old_name,\n+\t\t\t\t\t  &buf, &alloc, &size))\n \t\t\t\treturn error(\"read of %s failed\",\n \t\t\t\t\t     patch->old_name);\n-\t\t\talloc = size;\n \t\t}\n \t}\n-\telse if (patch->old_name) {\n-\t\tsize = xsize_t(st->st_size);\n-\t\talloc = size + 8192;\n-\t\tbuf = xmalloc(alloc);\n-\t\tif (read_old_data(st, patch->old_name, &buf, &alloc, &size))\n-\t\t\treturn error(\"read of %s failed\", patch->old_name);\n-\t}\n \n \tdesc.size = size;\n \tdesc.alloc = alloc;\n@@ -2055,6 +2084,16 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists)\n \treturn 0;\n }\n \n+static int verify_index_match(struct cache_entry *ce, struct stat *st)\n+{\n+\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\tif (!S_ISDIR(st->st_mode))\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t}\n+\treturn ce_match_stat(ce, st, 1);\n+}\n+\n static int check_patch(struct patch *patch, struct patch *prev_patch)\n {\n \tstruct stat st;\n@@ -2065,8 +2105,14 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \tint ok_if_exists;\n \n \tpatch->rejected = 1; /* we will drop this after we succeed */\n+\n+\t/*\n+\t * Make sure that we do not have local modifications from the\n+\t * index when we are looking at the index.  Also make sure\n+\t * we have the preimage file to be patched in the work tree,\n+\t * unless --cached, which tells git to apply only in the index.\n+\t */\n \tif (old_name) {\n-\t\tint changed = 0;\n \t\tint stat_ret = 0;\n \t\tunsigned st_mode = 0;\n \n@@ -2096,15 +2142,12 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t\t    lstat(old_name, &st))\n \t\t\t\t\treturn -1;\n \t\t\t}\n-\t\t\tif (!cached)\n-\t\t\t\tchanged = ce_match_stat(ce, &st, 1);\n-\t\t\tif (changed)\n+\t\t\tif (!cached && verify_index_match(ce, &st))\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n \t\t\tif (cached)\n \t\t\t\tst_mode = ntohl(ce->ce_mode);\n-\t\t}\n-\t\telse if (stat_ret < 0)\n+\t\t} else if (stat_ret < 0)\n \t\t\treturn error(\"%s: %s\", old_name, strerror(errno));\n \n \t\tif (!cached)\n@@ -2354,7 +2397,11 @@ static void remove_file(struct patch *patch, int rmdir_empty)\n \t\tcache_tree_invalidate_path(active_cache_tree, patch->old_name);\n \t}\n \tif (!cached) {\n-\t\tif (!unlink(patch->old_name) && rmdir_empty) {\n+\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\tif (rmdir(patch->old_name))\n+\t\t\t\twarning(\"unable to remove submodule %s\",\n+\t\t\t\t\tpatch->old_name);\n+\t\t} else if (!unlink(patch->old_name) && rmdir_empty) {\n \t\t\tchar *name = xstrdup(patch->old_name);\n \t\t\tchar *end = strrchr(name, '/');\n \t\t\twhile (end) {\n@@ -2382,13 +2429,21 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_mode = create_ce_mode(mode);\n \tce->ce_flags = htons(namelen);\n-\tif (!cached) {\n-\t\tif (lstat(path, &st) < 0)\n-\t\t\tdie(\"unable to stat newly created file %s\", path);\n-\t\tfill_stat_cache_info(ce, &st);\n+\tif (S_ISGITLINK(mode)) {\n+\t\tconst char *s = buf;\n+\n+\t\tif (get_sha1_hex(s + strlen(\"Subproject commit \"), ce->sha1))\n+\t\t\tdie(\"corrupt patch for subproject %s\", path);\n+\t} else {\n+\t\tif (!cached) {\n+\t\t\tif (lstat(path, &st) < 0)\n+\t\t\t\tdie(\"unable to stat newly created file %s\",\n+\t\t\t\t    path);\n+\t\t\tfill_stat_cache_info(ce, &st);\n+\t\t}\n+\t\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n+\t\t\tdie(\"unable to create backing store for newly created file %s\", path);\n \t}\n-\tif (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)\n-\t\tdie(\"unable to create backing store for newly created file %s\", path);\n \tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n \t\tdie(\"unable to add cache entry for %s\", path);\n }\n@@ -2398,6 +2453,13 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tint fd;\n \tchar *nbuf;\n \n+\tif (S_ISGITLINK(mode)) {\n+\t\tstruct stat st;\n+\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode))\n+\t\t\treturn 0;\n+\t\treturn mkdir(path, 0777);\n+\t}\n+\n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n \t\t * terminated.\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e8ce7cd..9d142ed 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -175,4 +175,21 @@ test_expect_success 'checkout superproject with subproject already present' '\n \tgit-checkout master\n '\n \n+test_expect_success 'apply submodule diff' '\n+\tgit branch second &&\n+\t(\n+\t\tcd lib &&\n+\t\techo s >s &&\n+\t\tgit add s &&\n+\t\tgit commit -m \"change subproject\"\n+\t) &&\n+\tgit update-index --add lib &&\n+\tgit-commit -m \"change lib\" &&\n+\tgit-format-patch -1 --stdout >P.diff &&\n+\tgit checkout second &&\n+\tgit apply --index P.diff &&\n+\tD=$(git diff --cached master) &&\n+\ttest -z \"$D\"\n+'\n+\n test_done\n-- \n1.5.3.rc5.1.g7de89\n"},{"id":"50842","messageId":"7vmywsmjm5.fsf@gitster.siamese.dyndns.org","threadId":"9470","inReplyTo":"20070815172209.GD1070MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH v6] git-apply: apply submodule changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-16T00:02:58Z","receivedAt":"2007-08-16T00:02:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> I agree that the TYPE_CHANGED thing may have been confusing,\n> so I kept your version (although I switched the tests around,\n> since there is no point in checking if the stat info matches\n> if you're going to ignore the result anyway).\n>\n> Other than that, the only change wrt to your version is that\n> I added back the creation and (attempt at) removal of the\n> corresponding subdirectory.\n\nMakes much more sense than what I wrote.  Thanks.\n"}]}