From: Derrick Stolee <stolee@gmail.com>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH 07/11] merge-ort: add implementation of both sides renaming differently
Date: Thu, 10 Dec 2020 22:39:55 -0500 [thread overview]
Message-ID: <5b9f5db0-b4d5-f60d-b5c5-2f8376d4bf12@gmail.com> (raw)
In-Reply-To: <d4595397052568ea6ea0cf46190374c74140fed7.1607542887.git.gitgitgadget@gmail.com>
On 12/9/2020 2:41 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> Implement rename/rename(1to2) handling, i.e. both sides of history
> renaming a file and rename it differently. This code replaces the
> following from merge-recurisve.c:
>
> * all the 1to2 code in process_renames()
> * the RENAME_ONE_FILE_TO_TWO case of process_entry()
> * handle_rename_rename_1to2()
>
> Also, there is some shared code from merge-recursive.c for multiple
> different rename cases which we will no longer need for this case (or
> other rename cases):
>
> * handle_file_collision()
> * setup_rename_conflict_info()
>
> The consolidation of five separate codepaths into one is made possible
> by a change in design:
Excellent!
> /* This is a rename/rename(1to2) */
> - die("Not yet implemented");
> + clean_merge = handle_content_merge(opt,
> + pair->one->path,
> + &base->stages[0],
> + &side1->stages[1],
> + &side2->stages[2],
> + pathnames,
> + 1 + 2 * opt->priv->call_depth,
> + &merged);
(this method currently die()s. ok)
> + if (!clean_merge &&
> + merged.mode == side1->stages[1].mode &&
> + oideq(&merged.oid, &side1->stages[1].oid)) {
> + was_binary_blob = 1;
> + }
nit: Extraneous braces?
> + memcpy(&side1->stages[1], &merged, sizeof(merged));
> + if (was_binary_blob) {
> + /*
> + * Getting here means we were attempting to
> + * merge a binary blob.
> + *
> + * Since we can't merge binaries,
> + * handle_content_merge() just takes one
> + * side. But we don't want to copy the
> + * contents of one side to both paths. We
> + * used the contents of side1 above for
> + * side1->stages, let's use the contents of
> + * side2 for side2->stages below.
> + */
> + oidcpy(&merged.oid, &side2->stages[2].oid);
> + merged.mode = side2->stages[2].mode;
> + }
> + memcpy(&side2->stages[2], &merged, sizeof(merged));
> +
> + side1->path_conflict = 1;
> + side2->path_conflict = 1;
> + /*
> + * TODO: For renames we normally remove the path at the
> + * old name. It would thus seem consistent to do the
> + * same for rename/rename(1to2) cases, but we haven't
> + * done so traditionally and a number of the regression
> + * tests now encode an expectation that the file is
> + * left there at stage 1. If we ever decide to change
> + * this, add the following two lines here:
> + * base->merged.is_null = 1;
> + * base->merged.clean = 1;
> + * and remove the setting of base->path_conflict to 1.
> + */
> + base->path_conflict = 1;
I'm getting the point of the review/evening where I'm starting to gloss
over these important details. Time to take a break (after this patch).
> + path_msg(opt, oldpath, 0,
> + _("CONFLICT (rename/rename): %s renamed to "
> + "%s in %s and to %s in %s."),
> + pathnames[0],
> + pathnames[1], opt->branch1,
> + pathnames[2], opt->branch2);
This output differs a bit from handle_rename_rename_1to2() in
merge-recursive.c:
output(opt, 1, _("CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
"rename \"%s\"->\"%s\" in \"%s\"%s"),
o->path, a->path, ci->ren1->branch,
o->path, b->path, ci->ren2->branch,
opt->priv->call_depth ? _(" (left unresolved)") : "");
How much do we want to have _exact_ output matches between the
two strategies, at least in the short term?
> @@ -1257,13 +1309,13 @@ static void process_entry(struct merge_options *opt,
> int side = (ci->filemask == 4) ? 2 : 1;
> ci->merged.result.mode = ci->stages[side].mode;
> oidcpy(&ci->merged.result.oid, &ci->stages[side].oid);
> - ci->merged.clean = !ci->df_conflict;
> + ci->merged.clean = !ci->df_conflict && !ci->path_conflict;
> } else if (ci->filemask == 1) {
> /* Deleted on both sides */
> ci->merged.is_null = 1;
> ci->merged.result.mode = 0;
> oidcpy(&ci->merged.result.oid, &null_oid);
> - ci->merged.clean = 1;
> + ci->merged.clean = !ci->path_conflict;
These exist because this is the first time we assign path_conflict.
Sure.
Thanks,
-Stolee
next prev parent reply other threads:[~2020-12-11 3:45 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-09 19:41 [PATCH 00/11] merge-ort: add basic rename detection Elijah Newren via GitGitGadget
2020-12-09 19:41 ` [PATCH 01/11] merge-ort: add basic data structures for handling renames Elijah Newren via GitGitGadget
2020-12-11 2:03 ` Derrick Stolee
2020-12-11 9:41 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 02/11] merge-ort: add initial outline for basic rename detection Elijah Newren via GitGitGadget
2020-12-11 2:39 ` Derrick Stolee
2020-12-11 9:40 ` Elijah Newren
2020-12-13 7:47 ` Elijah Newren
2020-12-14 14:33 ` Derrick Stolee
2020-12-14 15:42 ` Johannes Schindelin
2020-12-14 16:11 ` Elijah Newren
2020-12-14 16:50 ` Johannes Schindelin
2020-12-14 17:35 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 03/11] merge-ort: implement detect_regular_renames() Elijah Newren via GitGitGadget
2020-12-11 2:54 ` Derrick Stolee
2020-12-11 17:38 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 04/11] merge-ort: implement compare_pairs() and collect_renames() Elijah Newren via GitGitGadget
2020-12-11 3:00 ` Derrick Stolee
2020-12-11 18:43 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 05/11] merge-ort: add basic outline for process_renames() Elijah Newren via GitGitGadget
2020-12-11 3:24 ` Derrick Stolee
2020-12-11 20:03 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 06/11] merge-ort: add implementation of both sides renaming identically Elijah Newren via GitGitGadget
2020-12-11 3:32 ` Derrick Stolee
2020-12-09 19:41 ` [PATCH 07/11] merge-ort: add implementation of both sides renaming differently Elijah Newren via GitGitGadget
2020-12-11 3:39 ` Derrick Stolee [this message]
2020-12-11 21:56 ` Elijah Newren
2020-12-09 19:41 ` [PATCH 08/11] merge-ort: add implementation of rename collisions Elijah Newren via GitGitGadget
2020-12-09 19:41 ` [PATCH 09/11] merge-ort: add implementation of rename/delete conflicts Elijah Newren via GitGitGadget
2020-12-09 19:41 ` [PATCH 10/11] merge-ort: add implementation of normal rename handling Elijah Newren via GitGitGadget
2020-12-09 19:41 ` [PATCH 11/11] merge-ort: add implementation of type-changed " Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 00/11] merge-ort: add basic rename detection Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 01/11] merge-ort: add basic data structures for handling renames Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 02/11] merge-ort: add initial outline for basic rename detection Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 03/11] merge-ort: implement detect_regular_renames() Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 04/11] merge-ort: implement compare_pairs() and collect_renames() Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 05/11] merge-ort: add basic outline for process_renames() Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 06/11] merge-ort: add implementation of both sides renaming identically Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 07/11] merge-ort: add implementation of both sides renaming differently Elijah Newren via GitGitGadget
2020-12-14 16:21 ` [PATCH v2 08/11] merge-ort: add implementation of rename collisions Elijah Newren via GitGitGadget
2020-12-15 14:09 ` Derrick Stolee
2020-12-15 16:56 ` Elijah Newren
2020-12-14 16:21 ` [PATCH v2 09/11] merge-ort: add implementation of rename/delete conflicts Elijah Newren via GitGitGadget
2020-12-15 14:23 ` Derrick Stolee
2020-12-15 17:07 ` Elijah Newren
2020-12-15 14:27 ` Derrick Stolee
2020-12-14 16:21 ` [PATCH v2 10/11] merge-ort: add implementation of normal rename handling Elijah Newren via GitGitGadget
2020-12-15 14:27 ` Derrick Stolee
2020-12-14 16:21 ` [PATCH v2 11/11] merge-ort: add implementation of type-changed " Elijah Newren via GitGitGadget
2020-12-15 14:31 ` Derrick Stolee
2020-12-15 17:11 ` Elijah Newren
2020-12-15 14:34 ` [PATCH v2 00/11] merge-ort: add basic rename detection Derrick Stolee
2020-12-15 22:09 ` Junio C Hamano
2020-12-15 18:27 ` [PATCH v3 " Elijah Newren via GitGitGadget
2020-12-15 18:27 ` [PATCH v3 01/11] merge-ort: add basic data structures for handling renames Elijah Newren via GitGitGadget
2020-12-15 18:27 ` [PATCH v3 02/11] merge-ort: add initial outline for basic rename detection Elijah Newren via GitGitGadget
2020-12-15 18:27 ` [PATCH v3 03/11] merge-ort: implement detect_regular_renames() Elijah Newren via GitGitGadget
2020-12-15 18:27 ` [PATCH v3 04/11] merge-ort: implement compare_pairs() and collect_renames() Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 05/11] merge-ort: add basic outline for process_renames() Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 06/11] merge-ort: add implementation of both sides renaming identically Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 07/11] merge-ort: add implementation of both sides renaming differently Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 08/11] merge-ort: add implementation of rename/delete conflicts Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 09/11] merge-ort: add implementation of rename collisions Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 10/11] merge-ort: add implementation of normal rename handling Elijah Newren via GitGitGadget
2020-12-15 18:28 ` [PATCH v3 11/11] merge-ort: add implementation of type-changed " Elijah Newren via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5b9f5db0-b4d5-f60d-b5c5-2f8376d4bf12@gmail.com \
--to=stolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).