Junio C Hamano <gitster@xxxxxxxxx> wrote: > This PATH_TO_BE_DELETED logic should be Ok for the normal case, but it > seems a bit fragile. In a sequence of patches, if you have even one > patch that makes the path disappear, you initialize it as > PATH_TO_BE_DELETED, and special case the "creation should not clobber > existing path" rule to allow it to be present in the tree. > > That may make this sequence work, I presume, with your change: > > patch #1 renames frotz.c to hello.c > patch #2 renames hello.c to frotz.c > > because of patch #2, hello.c is marked as PATH_TO_BE_DELETED > initially and then when patch #1 is handled, frotz.c is allowed to > replace it. > > But if you have further patches that do the following (the "file > table" mechanism was added to handle concatenated patches that affect > the same path more than once), I thing PATH_TO_BE_DELETED logic would > break down: > > patch #3 renames alpha.c to hello.c > patch #4 renames hello.c to alpha.c > > When patch #3 is handled, the PATH_TO_BE_DELETED mark is long gone > from hello.c, and we will see the same failure you addressed in your > patch, won't we? As far as I understand the code, diffs are applied independently (for every file apply_patch() is called) and for every apply_patch() call fn_table is cleared. So situation you described in only possible in a *single* diff and I don't think it is possible to happen. Performing two criss-cross renames results in following diff: mv file1 tmp mv file2 file1 mv tmp file2 mv file1 tmp mv file3 file1 mv tmp file3 git diff -M -B diff --git a/file3 b/file1 similarity index 100% rename from file3 rename to file1 diff --git a/file1 b/file2 similarity index 100% rename from file1 rename to file2 diff --git a/file2 b/file3 similarity index 100% rename from file2 rename to file3 However, sanity checking still may be performed and error printed on situations which cannot be resolved. > > The prepare_fn_table() may be a good place to diagnose such a > situation and warn or error out if the user feeds such an input we > cannot handle sanely. > I'll add some checks to this function as you suggest. > > Style; > > if (to_be_deleted(tpatch)) > tpatch = NULL; > > Other than that, I think it is a sensible approach. Thanks for feedback. -- Michał Kiedrowicz -- 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