Hello, On Fri, Jul 12, 2019 at 12:50 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Ariadne Conill <ariadne@xxxxxxxxxxxxxxxx> 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
