From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@xxxxxxx> 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@xxxxxxx> --- [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(-) 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
