[PATCH] set LC_TIME even if locale dir is not present

From: Matthias Aßhauer via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 27, 2022, 08:58 UTC
Message-ID: <pull.1189.git.1648371489398.gitgitgadget@gmail.com>

From: =?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(-)

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