Felipe Contreras wrote:

> They have been marked as UNINTERESTING for a reason, lets respect that.

This patch looks unsafe, and in the examples listed in the patch
description the changed behavior does not look like an improvement.
Worse, the description lists a few examples but gives no convincing
explanation to reassure about the lack of bad behavior for examples
not listed.

Perhaps this patch has a prerequisite and has come out of order.

Hope that helps,
Jonathan

Patch left unsnipped so we can get a copy in the list archive.

> Currently the first ref is handled properly, but not the rest, so:
> 
>  % git fast-export master ^master
> 
> Would currently throw a reset for master (2nd ref), which is not what we
> want.
> 
>  % git fast-export master ^foo ^bar ^roo
>  % git fast-export master salsa..tacos
> 
> Even if all these refs point to the same object; foo, bar, roo, salsa,
> and tacos would all get a reset.
> 
> This is most certainly not what we want. After this patch, nothing gets
> exported, because nothing was selected (everything is UNINTERESTING).
> 
> Signed-off-by: Felipe Contreras <felipe.contre...@gmail.com>
> ---
>  builtin/fast-export.c  | 7 ++++---
>  t/t9350-fast-export.sh | 6 ++++++
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 065f324..7fb6fe1 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array 
> *pending,
>                               typename(e->item->type));
>                       continue;
>               }
> -             if (commit->util)
> +             if (commit->util) {
>                       /* more than one name for the same object */
> -                     string_list_append(extra_refs, full_name)->util = 
> commit;
> -             else
> +                     if (!(commit->object.flags & UNINTERESTING))
> +                             string_list_append(extra_refs, full_name)->util 
> = commit;
> +             } else
>                       commit->util = full_name;
>       }
>  }
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 49bdb44..6ea8f6f 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
>       )
>  '
>  
> +test_expect_success 'proper extra refs handling' '
> +     git fast-export master ^master master..master > actual &&
> +     echo -n > expected &&
> +     test_cmp expected actual
> +'
> +
>  test_done
> -- 
> 1.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to