Re: [PATCH 2/2] blame: allow blame --reverse --first-parent when it makes sense

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 21, 2015 at 12:08 AM, Max Kirillov <max@xxxxxxxxxx> wrote:
> Do not die immediately when the two flags are specified. Instead
> check that the specified range is along first-parent chain.  Explioit

s/Explioit/Exploit/

> how prepare_revision_walk() handles first_parent_only flag: the commits
> outside of first-parent chain are either unknown (and do not have any
> children recorded) or appear as non-first parent of a commit along the
> first-parent chain.
>
> Since the check seems fragile, add test which makes sure that blame dies
> in both cases.
>
> Signed-off-by: Max Kirillov <max@xxxxxxxxxx>
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 295ce92..27de544 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2692,8 +2692,6 @@ parse_done:
>         }
>         else if (contents_from)
>                 die("--contents and --children do not blend well.");
> -       else if (revs.first_parent_only)
> -               die("combining --first-parent and --reverse is not supported");
>         else {
>                 final_commit_name = prepare_initial(&sb);
>                 sb.commits.compare = compare_commits_by_reverse_commit_date;
> @@ -2721,6 +2719,15 @@ parse_done:
>         if (prepare_revision_walk(&revs))
>                 die(_("revision walk setup failed"));
>
> +       if (reverse && revs.first_parent_only) {
> +               struct commit_list *final_children = lookup_decoration(&revs.children,
> +                                                                      &sb.final->object);
> +               if (!final_children ||
> +                   hashcmp(final_children->item->parents->item->object.sha1,
> +                           sb.final->object.sha1))
> +                   die("--reverse --first-parent together require range along first-parent chain");
> +       }
> +
>         if (is_null_sha1(sb.final->object.sha1)) {
>                 o = sb.final->util;
>                 sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
> diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
> index 9f40613..042863b 100755
> --- a/t/t8009-blame-reverse.sh
> +++ b/t/t8009-blame-reverse.sh
> @@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
>         test_cmp expect actual
>         '
>
> -test_expect_failure 'blame --reverse --first-parent finds A1' '
> +test_expect_success 'blame --reverse --first-parent finds A1' '
>         git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full &&
>         head -1 <actual_full | sed -e "sX .*XX" >actual &&
>         git rev-parse A1 >expect &&
>         test_cmp expect actual
>         '
>
> +test_expect_success 'blame --reverse --first-parse dies if no first parent chain' '
> +       test_must_fail git blame --porcelain --reverse --first-parent B1..A3 -- file.t &&
> +       test_must_fail git blame --porcelain --reverse --first-parent B2..A3 -- file.t
> +       '
> +
>  test_done
> --
> 2.3.4.2801.g3d0809b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]