Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases

[Stefan Beller <sbeller@google.com>]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@gmail.com> wrote:
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-recursive.c                   | 26 +++++++++++++++++++++++---
>  t/t6043-merge-rename-directories.sh |  8 ++++----
>  2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index fba1a0d207..62e4266d21 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1320,11 +1320,23 @@ static int handle_file(struct merge_options *o,
>
>         add = filespec_from_entry(&other, dst_entry, stage ^ 1);
>         if (add) {
> +               int ren_src_was_dirty = was_dirty(o, rename->path);
>                 char *add_name = unique_path(o, rename->path, other_branch);
>                 if (update_file(o, 0, &add->oid, add->mode, add_name))
>                         return -1;
>
> -               remove_file(o, 0, rename->path, 0);
> +               if (ren_src_was_dirty) {
> +                       output(o, 1, _("Refusing to lose dirty file at %s"),
> +                              rename->path);
> +               }
> +               /*
> +                * Stupid double negatives in remove_file; it somehow manages
> +                * to repeatedly mess me up.  So, just for myself:
> +                *    1) update_wd iff !ren_src_was_dirty.
> +                *    2) no_wd iff !update_wd
> +                *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
> +                */

Not sure iff this comment is at the right place and is a good addition to
the code base.

However it hints at the underlying issue of a bad API that is provided
by remove_file ?