Re: [PATCH v4] git-apply: apply submodule changes
From: Sven Verdoolaege <hidden>
Date: 2016-06-15 22:43:29
On Mon, Aug 13, 2007 at 11:27:38PM -0700, Junio C Hamano wrote:
* write_out_one_result() calls remove_file() and create_file()
to match the work tree to the result you prepared with
apply_data().
- remove_file() is changed not to do any for gitlink. We
_might_ want to try rmdir() if there is an otherwise empty
directory there, but currently we cannot do much to the
failure on that, so I did not bother with it.We could at least warn about it, which is what my patch did.
- create_file() does three things:
- create a file in the work tree to match the result;
- update the index with the patch result;
- invalidate cache-tree entry for the path.
For the first task, create_one_file() is usually used to
create a blob (either regular file or a symlink). For
gitlinks, we do not affect the work tree for now, just like
checkout_entry().It creates the subdirectory, though, and git-apply should do so too since it expects the subdirectory to be there for subsequent patches (at least in the --index case).
quoted hunk
diff --git a/builtin-apply.c b/builtin-apply.c
Did you remove the documentation on purpose ?
+static int verify_index_match(struct cache_entry *ce, struct stat *st)
+{
+ if (!ce_match_stat(ce, st, 1))
+ return 0;
+ if (S_ISGITLINK(ntohl(ce->ce_mode))) {
+ if (S_ISDIR(st->st_mode))
+ return 0;
+ }Not a big deal, but ce_match_stat already checks for that. That's why I was checking for TYPE_CHANGED in its return value.
quoted hunk
@@ -2096,16 +2142,22 @@ static int check_patch(struct patch *patch, struct patch *prev_patch) lstat(old_name, &st)) return -1; } - if (!cached) - changed = ce_match_stat(ce, &st, 1); - if (changed) + if (!cached && verify_index_match(ce, &st)) return error("%s: does not match index", old_name); if (cached) st_mode = ntohl(ce->ce_mode); + } else if (stat_ret < 0) { + if (errno == ENOENT && S_ISGITLINK(patch->old_mode)) + /* + * It is Ok not to have the submodule + * checked out at all. + */ + ; + else + return error("%s: %s", old_name, + strerror(errno)); }
Shouldn't you be consistent with the --index case and require the subdirectory to exist? skimo