Re: [PATCH 08/11] merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries

On Mon, Mar 8, 2021 at 5:06 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:

quoted

From: Elijah Newren <redacted>

When merge conflicts occur in paths removed by a sparse-checkout, we
need to unsparsify those paths (clear the SKIP_WORKTREE bit), and write
out the conflicted file to the working copy.  In the very unlikely case
that someone manually put a file into the working copy at the location
of the SKIP_WORKTREE file, we need to avoid overwriting whatever edits
they have made and move that file to a different location first.

Signed-off-by: Elijah Newren <redacted>
---
 merge-ort.c                       | 43 +++++++++++++++++++++----------
 t/t6428-merge-conflicts-sparse.sh |  4 +--
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/merge-ort.c b/merge-ort.c
index a998f843a1da..37b69cbe0f9a 100644
--- a/merge-ort.c
+++ b/merge-ort.c

@@ -3235,23 +3235,27 @@ static int checkout(struct merge_options *opt,
      return ret;
 }

-static int record_conflicted_index_entries(struct merge_options *opt,
-                                        struct index_state *index,
-                                        struct strmap *paths,
-                                        struct strmap *conflicted)
+static int record_conflicted_index_entries(struct merge_options *opt)
 {
      struct hashmap_iter iter;
      struct strmap_entry *e;
+     struct index_state *index = opt->repo->index;
+     struct checkout state = CHECKOUT_INIT;
      int errs = 0;
      int original_cache_nr;

-     if (strmap_empty(conflicted))
+     if (strmap_empty(&opt->priv->conflicted))
              return 0;

+     /* If any entries have skip_worktree set, we'll have to check 'em out */
+     state.force = 1;
+     state.quiet = 1;
+     state.refresh_cache = 1;
+     state.istate = index;
      original_cache_nr = index->cache_nr;

      /* Put every entry from paths into plist, then sort */
-     strmap_for_each_entry(conflicted, &iter, e) {
+     strmap_for_each_entry(&opt->priv->conflicted, &iter, e) {
              const char *path = e->key;
              struct conflict_info *ci = e->value;
              int pos;

@@ -3292,9 +3296,23 @@ static int record_conflicted_index_entries(struct merge_options *opt,
                       * the higher order stages.  Thus, we need override
                       * the CE_SKIP_WORKTREE bit and manually write those
                       * files to the working disk here.
-                      *
-                      * TODO: Implement this CE_SKIP_WORKTREE fixup.
                       */
+                     if (ce_skip_worktree(ce)) {
+                             struct stat st;
+
+                             if (!lstat(path, &st)) {
+                                     char *new_name = unique_path(&opt->priv->paths,
+                                                                  path,
+                                                                  "cruft");
+
+                                     path_msg(opt, path, 1,
+                                              _("Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s"),
+                                              path, new_name);

I see this follows existing uses in merge-ort.c, but I wonder if this
won't be quite unreadable on long paths, i.e.:

    <long x> renamed to <long x.new>

As opposed to:

    We had to rename your thing:
        from: <long x>
          to: <long x.new>

Makes sense, but it seems like something we'd want to do to a lot of
messages rather than just this one.  For now, especially given that I
expect this particular message to be *very* rare, I think I'll leave
this one as-is for now but we can address this idea in a subsequent
series or as #leftoverbits.