linux-kernel - Re: [ANNOUNCE] Git v2.20.0-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPig+cSzyT5N5=YeX+VgRq1t0VbWqXLHSB=g=V=O-nLdCWrE9g@mail.gmail.com>
Date:   Thu, 22 Nov 2018 14:27:04 -0500
From:   Eric Sunshine <sunshine@...shineco.com>
To:     Ævar Arnfjörð Bjarmason <avarab@...il.com>
Cc:     Junio C Hamano <gitster@...ox.com>, Git List <git@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        git-packagers@...glegroups.com
Subject: Re: [ANNOUNCE] Git v2.20.0-rc1

On Thu, Nov 22, 2018 at 10:58 AM Ævar Arnfjörð Bjarmason
<avarab@...il.com> wrote:
> There's a regression related to this that I wanted to send a headsup
> for, but don't have time to fix today. Now range-diff in format-patch
> includes --stat output. See e.g. my
> https://public-inbox.org/git/20181122132823.9883-1-avarab@gmail.com/

Umf. Unfortunate fallout from [1].

> diff --git a/builtin/log.c b/builtin/log.c
> @@ -1094,9 +1094,12 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>         if (rev->rdiff1) {
> +               const int oldfmt = rev->diffopt.output_format;
>                 fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
> +               rev->diffopt.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY);
>                 show_range_diff(rev->rdiff1, rev->rdiff2,
>                                 rev->creation_factor, 1, &rev->diffopt);
> +               rev->diffopt.output_format = oldfmt;
>         }
>  }

A few questions/observations:

Does this same "fix" need to be applied to the --interdiff case just
above this --range-diff block?

Does the same "fix" need to be applied to the --interdiff and
--range-diff cases in log-tree.c:show_log()?

Aside from fixing the broken --no-patches option[2], a goal of the
series was to some day make --stat do something useful. Doesn't this
"fix" go against that goal?

The way this change needs to be spread around at various locations is
making it feel like a BandAid "fix" rather than a proper solution. It
seems like it should be fixed at a different level, though I'm not
sure yet if that level is higher (where the options get set) or lower
(where they actually affect the operation).

Until we figure out the answers to these questions, I wonder if a more
sensible short-term solution would be to back out [1] and just keep
[2], which fixed the --no-patches regression.

> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> @@ -248,8 +248,10 @@ test_expect_success 'dual-coloring' '
>  for prev in topic master..topic
>  do
>         test_expect_success "format-patch --range-diff=$prev" '
> +               cat actual &&

Debugging session gunk?

>                 git format-patch --stdout --cover-letter --range-diff=$prev \
>                         master..unmodified >actual &&
> +               ! grep "a => b" actual &&
>                 grep "= 1: .* s/5/A" actual &&

[1]: https://public-inbox.org/git/20181113185558.23438-4-avarab@gmail.com/
[2]: https://public-inbox.org/git/20181113185558.23438-3-avarab@gmail.com/