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).
> 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.
> @@ -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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
