Re: [PATCH 03/10] merge-ort: implement unique_path() helper

[Derrick Stolee <stolee@gmail.com>]

On 12/18/2020 12:51 AM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> Implement unique_path(), based on the one from merge-recursive.c.  It is
> simplified, however, due to: (1) using strmaps, and (2) the fact that
> merge-ort lets the checkout codepath handle possible collisions with the
> working tree means that other code locations don't have to.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-ort.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/merge-ort.c b/merge-ort.c
> index d300a02810e..1adc27a11bc 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -343,11 +343,34 @@ static void path_msg(struct merge_options *opt,
>  	strbuf_addch(sb, '\n');
>  }
>  
> +/* add a string to a strbuf, but converting "/" to "_" */
> +static void add_flattened_path(struct strbuf *out, const char *s)
> +{
> +	size_t i = out->len;
> +	strbuf_addstr(out, s);
> +	for (; i < out->len; i++)
> +		if (out->buf[i] == '/')
> +			out->buf[i] = '_';
> +}
> +

Thank you for pointing out that you based your code on merge-recursive.c.
I see that this implementation is identical to the one there. I question
whether this causes collisions in a problematic way, when "a/b/c" and
"a_b_c" both exist in a tree.

To avoid such a problem, we'd likely need to also expand "_" to "__" or
similar. This might never actually affect any users because of the
strange filename matches _and_ we need to be in a directory/file conflict.

And maybe it's not a hole at all? If it is, we can consider patching or
at least documenting the problem.

Thanks,
-Stolee