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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 08 Mar 2021 14:06:00 +0100

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

> From: Elijah Newren <[email-masked]>
>
> 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 <[email-masked]>
> ---
>  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>