Matthias Aßhauer via GitGitGadgetFrom: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
Since Commit aa1462c (introduce "format" date-mode, 2015-06-25) git log can pass user specified format strings directly to strftime(). One special format string we explicitly mention in our documentation is %c, which depends on the system locale. To accommodate for %c we added a call to setlocale() in git_setup_gettext().
In Commit cc5e1bf (gettext: avoid initialization if the locale dir is not present, 2018-04-21) we added an early exit to git_setup_gettext() in case no textdomain directory is present. This early exit is so early, that we don't even set the locale for %c in that case, despite strftime() not needing the textdomain directory at all.
This leads to a subtle bug where `git log --date=format:%c` will use C locale instead of the system locale on systems without a valid textdomain directory.
This fixes https://github.com/git-for-windows/git/issues/2959
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
---
[RFC] set LC_TIME even if locale dir is not present
This is a small bug fix with a large and unwieldy regression test. The
whole prepare_time_locale() bit and and the Makefile change is obviously
based on prepare_utf8_locale(), but I'm not really happy with it. I'm
not even sure how to fully put the issues I have with the test in words.
* I feel like it's not really testing anything on builds without
gettext, but adding a GETTEXT prerequisite to a test that something
works without gettext is very counter intuitive.
* I'm also not exactly happy about how I choose a locale, but can't
think of a better way. It's a reasonable assumption that C locale
uses a US date format on most, if not all supported systems, but I
have no good way to make sure that the selected locale actually
formats dates differently. Defining a custom locale would solve this,
but seems like a convoluted way to go about things.
* I'm not entirely happy with testing the output of git log
-format=date:%c against the output of the exact same command. I've
tried a version of the test based on date(1) and got it working with
the GNU version, but looking at the BSD version for our OS X based CI
builds and the POSIX spec for that command, they share barely more
than their name.
So, looking at the points above, I expect this to take a few re-rolls to
get into a reasonable shape.Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1189%2Frimrul%2Fdate-format-without-gettext-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1189/rimrul/date-format-without-gettext-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1189
Makefile | 7 +++++++ gettext.c | 3 ++- t/t4205-log-pretty-formats.sh | 31 +++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-)
Show changes to 3 files +40 −1
Makefile, gettext.c, t/t4205-log-pretty-formats.sh
diff --git a/Makefile b/Makefile index e8aba291d7f..ddca29b550b 100644 --- a/Makefile +++ b/Makefile @@ -410,6 +410,10 @@ include shared.mak # If it isn't set, fallback to $LC_ALL, $LANG or use the first utf-8 # locale returned by "locale -a". # +# Define GIT_TEST_TIME_LOCALE to preferred non-us locale for testing. +# If it isn't set, fallback to $LC_ALL, $LANG or use the first non-us +# locale returned by "locale -a". +# # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime. # # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC. @@ -2862,6 +2866,9 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT endif ifdef GIT_TEST_UTF8_LOCALE @echo GIT_TEST_UTF8_LOCALE=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_UTF8_LOCALE)))'\' >>$@+ +endif +ifdef GIT_TEST_TIME_LOCALE + @echo GIT_TEST_TIME_LOCALE=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_TIME_LOCALE)))'\' >>$@+ endif @echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' >>$@+ ifdef GIT_PERF_REPEAT_COUNT diff --git a/gettext.c b/gettext.c index bb5ba1fe7cc..2b614c2b8c6 100644 --- a/gettext.c +++ b/gettext.c @@ -107,6 +107,8 @@ void git_setup_gettext(void) const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT); char *p = NULL; + setlocale(LC_TIME, ""); + if (!podir) podir = p = system_path(GIT_LOCALE_PATH); @@ -117,7 +119,6 @@ void git_setup_gettext(void) bindtextdomain("git", podir); setlocale(LC_MESSAGES, ""); - setlocale(LC_TIME, ""); init_gettext_charset("git"); textdomain("git"); diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index e448ef2928a..01a1e61ecea 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -25,6 +25,29 @@ commit_msg () { fi } +prepare_time_locale () { + if test -z "$GIT_TEST_TIME_LOCALE" + then + case "${LC_ALL:-$LANG}" in + C | C.* | POSIX | POSIX.* | en_US | en_US.* ) + GIT_TEST_TIME_LOCALE=$(locale -a | sed -n '/^\(C\|POSIX\|en_US\)/I !{ + p + q + }') + ;; + *) + GIT_TEST_TIME_LOCALE="${LC_ALL:-$LANG}" + ;; + esac + fi + if test -n "$GIT_TEST_TIME_LOCALE" + then + test_set_prereq TIME_LOCALE + else + say "# No non-us locale available, some tests are skipped" + fi +} + test_expect_success 'set up basic repos' ' >foo && >bar && @@ -544,6 +567,14 @@ test_expect_success '--date=human %ad%cd is the same as %ah%ch' ' test_cmp expected actual ' +prepare_time_locale +test_expect_success TIME_LOCALE '--date=format:%c does not need gettext' ' + rm -fr no-such-dir && + LC_ALL=$GIT_TEST_TIME_LOCALE git log --date=format:%c HEAD^1..HEAD >expected && + GIT_TEXTDOMAINDIR=no-such-dir LC_ALL=$GIT_TEST_TIME_LOCALE git log --date=format:%c HEAD^1..HEAD >actual && + test_cmp expected actual +' + # get new digests (with no abbreviations) test_expect_success 'set up log decoration tests' ' head1=$(git rev-parse --verify HEAD~0) && base-commit: abf474a5dd901f28013c52155411a48fd4c09922
-- gitgitgadget
