Skip to content

Fix various rename corner cases #1943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 40 additions & 15 deletions merge-ort.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,14 @@ struct merge_options_internal {
* (e.g. "drivers/firmware/raspberrypi.c").
* * store all relevant paths in the repo, both directories and
* files (e.g. drivers, drivers/firmware would also be included)
* * these keys serve to intern all the path strings, which allows
* us to do pointer comparison on directory names instead of
* strcmp; we just have to be careful to use the interned strings.
* * these keys serve to intern *all* path strings, which allows us
* to do pointer comparisons on file & directory names instead of
* using strcmp; however, for this pointer-comparison optimization
* to work, any code path that independently computes a path needs
* to check for it existing in this strmap, and if so, point to
* the path in this strmap instead of their computed copy. See
* the "reuse known pointer" comment in
* apply_directory_rename_modifications() for an example.
*
* The values of paths:
* * either a pointer to a merged_info, or a conflict_info struct
Expand Down Expand Up @@ -2163,7 +2168,7 @@ static int handle_content_merge(struct merge_options *opt,
/*
* FIXME: If opt->priv->call_depth && !clean, then we really
* should not make result->mode match either a->mode or
* b->mode; that causes t6036 "check conflicting mode for
* b->mode; that causes t6416 "check conflicting mode for
* regular file" to fail. It would be best to use some other
* mode, but we'll confuse all kinds of stuff if we use one
* where S_ISREG(result->mode) isn't true, and if we use
Expand Down Expand Up @@ -2313,14 +2318,20 @@ static char *apply_dir_rename(struct strmap_entry *rename_info,
return strbuf_detach(&new_path, NULL);
}

static int path_in_way(struct strmap *paths, const char *path, unsigned side_mask)
static int path_in_way(struct strmap *paths,
const char *path,
unsigned side_mask,
struct diff_filepair *p)
{
struct merged_info *mi = strmap_get(paths, path);
struct conflict_info *ci;
if (!mi)
return 0;
INITIALIZE_CI(ci, mi);
return mi->clean || (side_mask & (ci->filemask | ci->dirmask));
return mi->clean || (side_mask & (ci->filemask | ci->dirmask))
/* See testcases 12[npq] of t6423 for this next condition */
|| ((ci->filemask & 0x01) &&
strcmp(p->one->path, path));
}

/*
Expand All @@ -2332,6 +2343,7 @@ static int path_in_way(struct strmap *paths, const char *path, unsigned side_mas
static char *handle_path_level_conflicts(struct merge_options *opt,
const char *path,
unsigned side_index,
struct diff_filepair *p,
struct strmap_entry *rename_info,
struct strmap *collisions)
{
Expand Down Expand Up @@ -2366,7 +2378,7 @@ static char *handle_path_level_conflicts(struct merge_options *opt,
*/
if (c_info->reported_already) {
clean = 0;
} else if (path_in_way(&opt->priv->paths, new_path, 1 << side_index)) {
} else if (path_in_way(&opt->priv->paths, new_path, 1 << side_index, p)) {
c_info->reported_already = 1;
strbuf_add_separated_string_list(&collision_paths, ", ",
&c_info->source_files);
Expand Down Expand Up @@ -2520,7 +2532,7 @@ static void compute_collisions(struct strmap *collisions,
* happening, and fall back to no-directory-rename detection
* behavior for those paths.
*
* See testcases 9e and all of section 5 from t6043 for examples.
* See testcases 9e and all of section 5 from t6423 for examples.
*/
for (i = 0; i < pairs->nr; ++i) {
struct strmap_entry *rename_info;
Expand Down Expand Up @@ -2573,14 +2585,14 @@ static void free_collisions(struct strmap *collisions)
static char *check_for_directory_rename(struct merge_options *opt,
const char *path,
unsigned side_index,
struct diff_filepair *p,
struct strmap *dir_renames,
struct strmap *dir_rename_exclusions,
struct strmap *collisions,
int *clean_merge)
{
char *new_path;
struct strmap_entry *rename_info;
struct strmap_entry *otherinfo;
const char *new_dir;
int other_side = 3 - side_index;

Expand Down Expand Up @@ -2615,14 +2627,13 @@ static char *check_for_directory_rename(struct merge_options *opt,
* to not let Side1 do the rename to dumbdir, since we know that is
* the source of one of our directory renames.
*
* That's why otherinfo and dir_rename_exclusions is here.
* That's why dir_rename_exclusions is here.
*
* As it turns out, this also prevents N-way transient rename
* confusion; See testcases 9c and 9d of t6043.
* confusion; See testcases 9c and 9d of t6423.
*/
new_dir = rename_info->value; /* old_dir = rename_info->key; */
otherinfo = strmap_get_entry(dir_rename_exclusions, new_dir);
if (otherinfo) {
if (strmap_contains(dir_rename_exclusions, new_dir)) {
path_msg(opt, INFO_DIR_RENAME_SKIPPED_DUE_TO_RERENAME, 1,
rename_info->key, path, new_dir, NULL,
_("WARNING: Avoiding applying %s -> %s rename "
Expand All @@ -2631,7 +2642,7 @@ static char *check_for_directory_rename(struct merge_options *opt,
return NULL;
}

new_path = handle_path_level_conflicts(opt, path, side_index,
new_path = handle_path_level_conflicts(opt, path, side_index, p,
rename_info,
&collisions[side_index]);
*clean_merge &= (new_path != NULL);
Expand Down Expand Up @@ -2875,6 +2886,20 @@ static int process_renames(struct merge_options *opt,
newinfo = new_ent->value;
}

/*
* Directory renames can result in rename-to-self; the code
* below assumes we have A->B with different A & B, and tries
* to move all entries to path B. If A & B are the same path,
* the logic can get confused, so skip further processing when
* A & B are already the same path.
*
* As a reminder, we can avoid strcmp here because all paths
* are interned in opt->priv->paths; see the comment above
* "paths" in struct merge_options_internal.
*/
if (oldpath == newpath)
continue;

/*
* If pair->one->path isn't in opt->priv->paths, that means
* that either directory rename detection removed that
Expand Down Expand Up @@ -3419,7 +3444,7 @@ static int collect_renames(struct merge_options *opt,
}

new_path = check_for_directory_rename(opt, p->two->path,
side_index,
side_index, p,
dir_renames_for_side,
rename_exclusions,
collisions,
Expand Down
Loading
Loading