Jeff King <peff@xxxxxxxx> writes:
> Ah, OK. Yeah, doing it progressively can only be accurate if our
> name-checks follow the same order as applying, because we are checking
> against a particular state.
>
> But could we instead pull this check to just before the write-out time?
> That is, to let any horrible thing happen in-core, as long as what we
> write out to the index and the filesystem is sane?
That would make me feel dirty.
I noticed one thing. The PATH_TO_BE_DELETED/PATH_WAS_DELETED crud
kicks in -only- during the actual application phase, and all patches
that remove paths from the end result should have been appropriately
marked in fn_table[] by the call to prepare_fn_table() at the
beginning of check_patch_list() as PATH_TO_BE_DELETED.
But it was wrong to call previous_patch() in my fix. The function
is the cause of evil I see in the "let's support concatenated patch,
making the later patch depend on the result of earlier ones" and
deliberately ignores PATH_TO_BE_DELETED patches. We would need to
do the early part of previous_patch() without the filtering.
This is a preparatory step to clean-up the mess I have in mind. It
does not mean to change the semantics (applied to the codebase with
or without the changes we have been discussing); it only makes it
always return the "previous" patch to the callers and makes them
responsible to see if the previous was to-be-deleted or was-deleted.
With that change, I think my symlink fix plus the "check the deleted
one with old_name, too" change has a better chance to do the moral
equivalent of your two-phase thing. Essentially, "First see what
will be deleted in the input as a whole" has already been done by
the prepare_fn_table() thing.
builtin/apply.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 41b7236..a064017 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3097,25 +3097,12 @@ static int checkout_target(struct cache_entry *ce, struct stat *st)
return 0;
}
-static struct patch *previous_patch(struct patch *patch, int *gone)
+static struct patch *previous_patch(struct patch *patch)
{
- struct patch *previous;
-
- *gone = 0;
if (patch->is_copy || patch->is_rename)
return NULL; /* "git" patches do not depend on the order */
- previous = in_fn_table(patch->old_name);
- if (!previous)
- return NULL;
-
- if (to_be_deleted(previous))
- return NULL; /* the deletion hasn't happened yet */
-
- if (was_deleted(previous))
- *gone = 1;
-
- return previous;
+ return in_fn_table(patch->old_name);
}
static int verify_index_match(const struct cache_entry *ce, struct stat *st)
@@ -3170,11 +3157,11 @@ static int load_preimage(struct image *image,
struct patch *previous;
int status;
- previous = previous_patch(patch, &status);
- if (status)
+ previous = previous_patch(patch);
+ if (was_deleted(previous))
return error(_("path %s has been renamed/deleted"),
patch->old_name);
- if (previous) {
+ if (previous && !to_be_deleted(previous)) {
/* We have a patched copy in memory; use that. */
strbuf_add(&buf, previous->result, previous->resultsize);
} else {
@@ -3384,18 +3371,18 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
{
const char *old_name = patch->old_name;
struct patch *previous = NULL;
- int stat_ret = 0, status;
+ int stat_ret = 0;
unsigned st_mode = 0;
if (!old_name)
return 0;
assert(patch->is_new <= 0);
- previous = previous_patch(patch, &status);
+ previous = previous_patch(patch);
- if (status)
+ if (was_deleted(previous))
return error(_("path %s has been renamed/deleted"), old_name);
- if (previous) {
+ if (previous && !to_be_deleted(previous)) {
st_mode = previous->new_mode;
} else if (!cached) {
stat_ret = lstat(old_name, st);
@@ -3403,6 +3390,9 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
return error(_("%s: %s"), old_name, strerror(errno));
}
+ if (to_be_deleted(previous))
+ previous = NULL;
+
if (check_index && !previous) {
int pos = cache_name_pos(old_name, strlen(old_name));
if (pos < 0) {
--
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