Re: [PATCH 28/48] merge-recursive: Split update_stages_and_entry; only update stages at end
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 18, 2011, 23:39 UTC
- Message-ID
- <7vvcuzb1kp.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1307518278-23814-29-git-send-email-newren@gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 55 quoted lines
> Instead of having the process_renames logic update the stages in the index
> for the rename destination, have the index updated after process_entry or
> process_df_entry. This will also allow us to have process_entry determine
> whether a file was tracked and existed in the working copy before the
> merge started.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> merge-recursive.c | 35 +++++++++++++++++------------------
> 1 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index b4baa35..7878b30 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -90,6 +90,7 @@ struct stage_data {
> } stages[4];
> struct rename_df_conflict_info *rename_df_conflict_info;
> unsigned processed:1;
> + unsigned involved_in_rename:1;
> };
>
> static inline void setup_rename_df_conflict_info(enum rename_type rename_type,
> @@ -516,15 +517,11 @@ static int update_stages(const char *path, const struct diff_filespec *o,
> return 0;
> }
>
> -static int update_stages_and_entry(const char *path,
> - struct stage_data *entry,
> - struct diff_filespec *o,
> - struct diff_filespec *a,
> - struct diff_filespec *b,
> - int clear)
> +static void update_entry(struct stage_data *entry,
> + struct diff_filespec *o,
> + struct diff_filespec *a,
> + struct diff_filespec *b)
> {
> - int options;
> -
> entry->processed = 0;
> entry->stages[1].mode = o->mode;
> entry->stages[2].mode = a->mode;
> @@ -532,7 +529,6 @@ static int update_stages_and_entry(const char *path,
> hashcpy(entry->stages[1].sha, o->sha1);
> hashcpy(entry->stages[2].sha, a->sha1);
> hashcpy(entry->stages[3].sha, b->sha1);
> - return update_stages(path, o, a, b);
> }
>
> static int remove_file(struct merge_options *o, int clean,
> @@ -1084,12 +1080,11 @@ static int process_renames(struct merge_options *o,
> ren2->dst_entry);
> } else {
> remove_file(o, 1, ren1_src, 1);Hopefully this unconditional removal from the working tree will be fixed by the end of the series, at least for the virtual ancestor merges.
Show 11 quoted lines
> - update_stages_and_entry(ren1_dst, > - ren1->dst_entry, > - ren1->pair->one, > - ren1->pair->two, > - ren2->pair->two, > - 1 /* clear */); > + update_entry(ren1->dst_entry, > + ren1->pair->one, > + ren1->pair->two, > + ren2->pair->two); > + ren1->dst_entry->involved_in_rename = 1;
So the idea is to just record what the stage data should be, and delay calling update_stages() until we run the content level merge?
Show 35 quoted lines
> @@ -1291,6 +1287,7 @@ static void handle_delete_modify(struct merge_options *o,
> }
>
> static int merge_content(struct merge_options *o,
> + unsigned involved_in_rename,
> const char *path,
> unsigned char *o_sha, int o_mode,
> unsigned char *a_sha, int a_mode,
> @@ -1331,6 +1328,8 @@ static int merge_content(struct merge_options *o,
> reason = "submodule";
> output(o, 1, "CONFLICT (%s): Merge conflict in %s",
> reason, path);
> + if (involved_in_rename)
> + update_stages(path, &one, &a, &b);
> }
>
> if (df_conflict_remains) {
> @@ -1415,7 +1414,7 @@ static int process_entry(struct merge_options *o,
> } else if (a_sha && b_sha) {
> /* Case C: Added in both (check for same permissions) and */
> /* case D: Modified in both, but differently. */
> - clean_merge = merge_content(o, path,
> + clean_merge = merge_content(o, entry->involved_in_rename, path,
> o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
> NULL);
> } else if (!o_sha && !a_sha && !b_sha) {
> @@ -1459,7 +1458,7 @@ static int process_df_entry(struct merge_options *o,
> char *src;
> switch (conflict_info->rename_type) {
> case RENAME_NORMAL:
> - clean_merge = merge_content(o, path,
> + clean_merge = merge_content(o, entry->involved_in_rename, path,
> o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
> conflict_info->branch1);
> break;Feels sane from a cursory look, but are there cases where we do update_entry() above and end up not calling merge_content() at all?