From: Ariadne Conill <ariadne@dereferenced.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v1 0/2] document deprecation of log.mailmap=false default
Date: Fri, 12 Jul 2019 13:00:51 -0500 [thread overview]
Message-ID: <CAAOiGNzcNhWmmr+COXhv2p9=KF5k4hHLEsNsfA1H+P0JQOTTqg@mail.gmail.com> (raw)
In-Reply-To: <xmqqpnmfp2gs.fsf@gitster-ct.c.googlers.com>
Hello,
On Fri, Jul 12, 2019 at 12:50 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ariadne Conill <ariadne@dereferenced.org> writes:
>
> > Based on the discussion of the previous patches, we concluded that
> > changing the default will require a transitional period.
>
> OK.
>
> > As such, I have introduced a deprecation warning that is printed when
> > the log builtin commands are used.
>
> ... used when the user did not configure and did not give --[no-]use-mailmap
> option from the command line, right? Otherwise the warning would be annoying.
Yes, the default state is changed to -1, which using the command line
options or explicitly configuring the option would change to either 0
or 1.
> Have you run the test suite before posting? To me, at least t0203,
> t4212, t7006 and t9700 seem to be having trouble with this change,
> that may need adjusting.
I ran parts of the testsuite, but I can go through the whole thing and
will fix the fallout.
> 0203.1 expects that "git show" gives nothing to its standard error
> stream. Perhaps something like this should serve as a good
> workaround.
>
> diff --git a/t/t0203-gettext-setlocale-sanity.sh b/t/t0203-gettext-setlocale-sanity.sh
> index 0ce1f22eff..76f774c8b8 100755
> --- a/t/t0203-gettext-setlocale-sanity.sh
> +++ b/t/t0203-gettext-setlocale-sanity.sh
> @@ -10,7 +10,7 @@ test_description="The Git C functions aren't broken by setlocale(3)"
> test_expect_success 'git show a ISO-8859-1 commit under C locale' '
> . "$TEST_DIRECTORY"/t3901/8859-1.txt &&
> test_commit "iso-c-commit" iso-under-c &&
> - git show >out 2>err &&
> + git show --use-mailmap >out 2>err &&
> test_must_be_empty err &&
> grep -q "iso-c-commit" out
> '
>
>
> 4212.3 is the same way.
>
> 4212.4 raises an interesting question. It wants to see the output
> of this command:
>
> git log --format="%an+%ae+%ad" broken_email >actual.out 2>actual.err &&
>
> The question is, when the user explicitly asked for the "true"
> identity values (not the mapped one via %aN, %aE and their friends),
> if --use-mailmap should affect the outcome?
I don't think it should.
> A secondary question is if we should be issuing a warning against
> this command line in the first place, if the answer to the above
> question is "no" (i.e. --[no-]use-mailmap, and the future default
> switch, do not affect the outcome). There is no point issuing a
> warning if the command line is already future-proofed.
I will disable the warning if a custom format is specified.
> 7006.43 (among others) expects its "colorful" shell helper function
> to be a reliable way to detect if color escape sequence is in use in
> the output but the helper only reads the first line and expects it
> to always begin with the colored "commit 0c9f6db7183a...", which fails
> when an error or a warning message comes first. I think the best fix
> there should be to make the helper more robust, perhaps by borrowing
> ideas from test-lib-functions.sh::test_decode_color, i.e. find byte
> sequence "\033[m" in there (I think assuming the presence of RESET
> sequence alone is sufficient for the purpose of t7006 tests that
> wants to see "have we tried to color or emit non-colored output?".
The colorful shell helper function irritates me from time to time with
that sort of thing. I might look into fixing it, but I think for now
it makes more sense to just defang those tests by killing the warning.
A thought I had was to perhaps use isatty(STDERR_FILENO) to determine
if stderr is attached to a terminal session and only issue the warning
in that case. What do you think? It should fix all of these tests.
> A possible workaround for 9007 would be to set log.mailmap=true in
> the set-up step, but there may be a better alternative.
>
> diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> index 102c133112..a7a336947d 100755
> --- a/t/t9700-perl-git.sh
> +++ b/t/t9700-perl-git.sh
> @@ -36,6 +36,7 @@ test_expect_success \
> echo "changed file 1" > file1 &&
> git commit -a -m "second commit" &&
>
> + git config log.mailmap true &&
> git config --add color.test.slot1 green &&
> git config --add test.string value &&
> git config --add test.dupstring value1 &&
This seems reasonable.
> There may be issues with other tests. I didn't do any exhaustive
> vetting.
Thanks wholeheartedly for all of your help so far, I know there are
many git users who will be very happy once all of this work is done
(basically anyone who has changed their name).
Ariadne
next prev parent reply other threads:[~2019-07-12 18:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-12 15:58 [PATCH v1 0/2] document deprecation of log.mailmap=false default Ariadne Conill
2019-07-12 15:59 ` [PATCH v1 1/2] log: add warning for unspecified log.mailmap setting Ariadne Conill
2019-07-12 15:59 ` [PATCH v1 2/2] documentation: mention --no-use-mailmap and log.mailmap false setting Ariadne Conill
2019-07-12 17:49 ` [PATCH v1 0/2] document deprecation of log.mailmap=false default Junio C Hamano
2019-07-12 18:00 ` Ariadne Conill [this message]
2019-07-12 19:30 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAOiGNzcNhWmmr+COXhv2p9=KF5k4hHLEsNsfA1H+P0JQOTTqg@mail.gmail.com' \
--to=ariadne@dereferenced.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).