git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions
@ 2024-02-05 16:25 Achu Luma
  2024-02-05 16:25 ` [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c Achu Luma
  2024-02-05 17:34 ` [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions rsbecker
  0 siblings, 2 replies; 10+ messages in thread
From: Achu Luma @ 2024-02-05 16:25 UTC (permalink / raw)
  To: git; +Cc: christian.couder, Achu Luma, Christian Couder

In a following commit we are going to port code from
"t/helper/test-date.c" and "t/t0006-date.sh" to a new
"t/unit-tests/t-date.c" file using the recently added unit test
framework.

We cannot fully port all the code from "t/helper/test-date.c" though, as
the test-tool date helper is still used by a number of "t/*.sh" tests.
The TIME_IS_64BIT and TIME_T_IS_64BIT prereqs are especially used by
"t5000-tar-tree.sh", "t5318-commit-graph.sh" and
"t5328-commit-graph-64bit-time.sh" while checking those prereqs will be
required in the new "t/unit-tests/t-date.c" file too.

To avoid duplicating in both "t/helper/test-date.c" and
"t/unit-tests/t-date.c" the small amount of code checking these prereqs,
let's move it into inline functions in "date.h".

The names of these new inline functions contain "TIME_IS_64BIT" or
"TIME_T_IS_64BIT" as it will simplify the macros we will use when
we will port code to "t/unit-tests/t-date.c" in a following commit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
---
 date.h               | 6 ++++++
 t/helper/test-date.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/date.h b/date.h
index 6136212a19..fb70490a51 100644
--- a/date.h
+++ b/date.h
@@ -70,4 +70,10 @@ void datestamp(struct strbuf *out);
 timestamp_t approxidate_careful(const char *, int *);
 int date_overflows(timestamp_t date);
 time_t tm_to_time_t(const struct tm *tm);
+static inline int check_prereq_TIME_IS_64BIT(void) {
+	return sizeof(timestamp_t) == 8;
+}
+static inline int check_prereq_TIME_T_IS_64BIT(void) {
+	return sizeof(time_t) == 8;
+}
 #endif
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index 0683d46574..be0b8679c3 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -126,9 +126,9 @@ int cmd__date(int argc UNUSED, const char **argv)
 	else if (!strcmp(*argv, "getnanos"))
 		getnanos(argv+1);
 	else if (!strcmp(*argv, "is64bit"))
-		return sizeof(timestamp_t) == 8 ? 0 : 1;
+		return !check_prereq_TIME_IS_64BIT();
 	else if (!strcmp(*argv, "time_t-is64bit"))
-		return sizeof(time_t) == 8 ? 0 : 1;
+		return !check_prereq_TIME_T_IS_64BIT();
 	else
 		usage(usage_msg);
 	return 0;
--
2.43.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-02-05 16:25 [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions Achu Luma
@ 2024-02-05 16:25 ` Achu Luma
  2024-03-28 12:35   ` Ghanshyam Thakkar
  2024-02-05 17:34 ` [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions rsbecker
  1 sibling, 1 reply; 10+ messages in thread
From: Achu Luma @ 2024-02-05 16:25 UTC (permalink / raw)
  To: git; +Cc: christian.couder, Achu Luma, Christian Couder

In the recent codebase update (8bf6fbd (Merge branch
'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
merged, providing a standardized approach for testing C code. Prior to
this update, some unit tests relied on the test helper mechanism,
lacking a dedicated unit testing framework. It's more natural to perform
these unit tests using the new unit test framework.

This commit migrates the unit tests for C date functions
(show_date_relative(), show_date(), etc) from the legacy approach
using the test-tool command `test-tool date` in t/helper/test-date.c
to the new unit testing framework (t/unit-tests/test-lib.h).

The migration involves refactoring the tests to utilize the testing
macros provided by the framework (TEST() and check_*()).

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
---
 I am currently facing challenges in handling the time zone (TZ)
 variable, in t-date.c. The tests, which were initially passing
 in the t0006-date.sh  on Windows, fail after porting to the new
 framework. I have tried to set the timezone using the setenv,
 but unfortunately, the issue persists. I suspect that the problem
 might be related to how Windows C compilers process the setenv
 function compared to POSIX environments. Please If anyone has
 insights into managing environment variables in a cross-platform
 manner or has encountered similar issues, your help would be
 highly appreciated.

 t/helper/test-date.c  |  91 +---------------
 t/t0006-date.sh       | 169 ------------------------------
 t/unit-tests/t-date.c | 237 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+), 259 deletions(-)
 delete mode 100755 t/t0006-date.sh
 create mode 100644 t/unit-tests/t-date.c

diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index be0b8679c3..b9cb2c5455 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -3,89 +3,11 @@
 #include "trace.h"

 static const char *usage_msg = "\n"
-"  test-tool date relative [time_t]...\n"
-"  test-tool date human [time_t]...\n"
-"  test-tool date show:<format> [time_t]...\n"
-"  test-tool date parse [date]...\n"
-"  test-tool date approxidate [date]...\n"
 "  test-tool date timestamp [date]...\n"
 "  test-tool date getnanos [start-nanos]\n"
 "  test-tool date is64bit\n"
 "  test-tool date time_t-is64bit\n";

-static void show_relative_dates(const char **argv)
-{
-	struct strbuf buf = STRBUF_INIT;
-
-	for (; *argv; argv++) {
-		time_t t = atoi(*argv);
-		show_date_relative(t, &buf);
-		printf("%s -> %s\n", *argv, buf.buf);
-	}
-	strbuf_release(&buf);
-}
-
-static void show_human_dates(const char **argv)
-{
-	for (; *argv; argv++) {
-		time_t t = atoi(*argv);
-		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_MODE(HUMAN)));
-	}
-}
-
-static void show_dates(const char **argv, const char *format)
-{
-	struct date_mode mode = DATE_MODE_INIT;
-
-	parse_date_format(format, &mode);
-	for (; *argv; argv++) {
-		char *arg;
-		timestamp_t t;
-		int tz;
-
-		/*
-		 * Do not use our normal timestamp parsing here, as the point
-		 * is to test the formatting code in isolation.
-		 */
-		t = parse_timestamp(*argv, &arg, 10);
-		while (*arg == ' ')
-			arg++;
-		tz = atoi(arg);
-
-		printf("%s -> %s\n", *argv, show_date(t, tz, &mode));
-	}
-
-	date_mode_release(&mode);
-}
-
-static void parse_dates(const char **argv)
-{
-	struct strbuf result = STRBUF_INIT;
-
-	for (; *argv; argv++) {
-		timestamp_t t;
-		int tz;
-
-		strbuf_reset(&result);
-		parse_date(*argv, &result);
-		if (sscanf(result.buf, "%"PRItime" %d", &t, &tz) == 2)
-			printf("%s -> %s\n",
-			       *argv, show_date(t, tz, DATE_MODE(ISO8601)));
-		else
-			printf("%s -> bad\n", *argv);
-	}
-	strbuf_release(&result);
-}
-
-static void parse_approxidate(const char **argv)
-{
-	for (; *argv; argv++) {
-		timestamp_t t;
-		t = approxidate(*argv);
-		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_MODE(ISO8601)));
-	}
-}
-
 static void parse_approx_timestamp(const char **argv)
 {
 	for (; *argv; argv++) {
@@ -106,22 +28,11 @@ static void getnanos(const char **argv)

 int cmd__date(int argc UNUSED, const char **argv)
 {
-	const char *x;

 	argv++;
 	if (!*argv)
 		usage(usage_msg);
-	if (!strcmp(*argv, "relative"))
-		show_relative_dates(argv+1);
-	else if (!strcmp(*argv, "human"))
-		show_human_dates(argv+1);
-	else if (skip_prefix(*argv, "show:", &x))
-		show_dates(argv+1, x);
-	else if (!strcmp(*argv, "parse"))
-		parse_dates(argv+1);
-	else if (!strcmp(*argv, "approxidate"))
-		parse_approxidate(argv+1);
-	else if (!strcmp(*argv, "timestamp"))
+	if (!strcmp(*argv, "timestamp"))
 		parse_approx_timestamp(argv+1);
 	else if (!strcmp(*argv, "getnanos"))
 		getnanos(argv+1);
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
deleted file mode 100755
index e18b160286..0000000000
--- a/t/t0006-date.sh
+++ /dev/null
@@ -1,169 +0,0 @@
-#!/bin/sh
-
-test_description='test date parsing and printing'
-
-TEST_PASSES_SANITIZE_LEAK=true
-. ./test-lib.sh
-
-# arbitrary reference time: 2009-08-30 19:20:00
-GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
-
-check_relative() {
-	t=$(($GIT_TEST_DATE_NOW - $1))
-	echo "$t -> $2" >expect
-	test_expect_${3:-success} "relative date ($2)" "
-	test-tool date relative $t >actual &&
-	test_cmp expect actual
-	"
-}
-
-check_relative 5 '5 seconds ago'
-check_relative 300 '5 minutes ago'
-check_relative 18000 '5 hours ago'
-check_relative 432000 '5 days ago'
-check_relative 1728000 '3 weeks ago'
-check_relative 13000000 '5 months ago'
-check_relative 37500000 '1 year, 2 months ago'
-check_relative 55188000 '1 year, 9 months ago'
-check_relative 630000000 '20 years ago'
-check_relative 31449600 '12 months ago'
-check_relative 62985600 '2 years ago'
-
-check_show () {
-	format=$1
-	time=$2
-	expect=$3
-	prereqs=$4
-	zone=$5
-	test_expect_success $prereqs "show date ($format:$time)" '
-		echo "$time -> $expect" >expect &&
-		TZ=${zone:-$TZ} test-tool date show:"$format" "$time" >actual &&
-		test_cmp expect actual
-	'
-}
-
-# arbitrary but sensible time for examples
-TIME='1466000000 +0200'
-check_show iso8601 "$TIME" '2016-06-15 16:13:20 +0200'
-check_show iso8601-strict "$TIME" '2016-06-15T16:13:20+02:00'
-check_show rfc2822 "$TIME" 'Wed, 15 Jun 2016 16:13:20 +0200'
-check_show short "$TIME" '2016-06-15'
-check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
-check_show raw "$TIME" '1466000000 +0200'
-check_show unix "$TIME" '1466000000'
-check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
-check_show raw-local "$TIME" '1466000000 +0000'
-check_show unix-local "$TIME" '1466000000'
-
-check_show 'format:%z' "$TIME" '+0200'
-check_show 'format-local:%z' "$TIME" '+0000'
-check_show 'format:%Z' "$TIME" ''
-check_show 'format-local:%Z' "$TIME" 'UTC'
-check_show 'format:%%z' "$TIME" '%z'
-check_show 'format-local:%%z' "$TIME" '%z'
-
-check_show 'format:%Y-%m-%d %H:%M:%S' "$TIME" '2016-06-15 16:13:20'
-check_show 'format-local:%Y-%m-%d %H:%M:%S' "$TIME" '2016-06-15 09:13:20' '' EST5
-
-check_show 'format:%s' '123456789 +1234' 123456789
-check_show 'format:%s' '123456789 -1234' 123456789
-check_show 'format-local:%s' '123456789 -1234' 123456789
-
-# arbitrary time absurdly far in the future
-FUTURE="5758122296 -0400"
-check_show iso       "$FUTURE" "2152-06-19 18:24:56 -0400" TIME_IS_64BIT,TIME_T_IS_64BIT
-check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000" TIME_IS_64BIT,TIME_T_IS_64BIT
-
-check_parse() {
-	echo "$1 -> $2" >expect
-	test_expect_${4:-success} "parse date ($1${3:+ TZ=$3})" "
-	TZ=${3:-$TZ} test-tool date parse '$1' >actual &&
-	test_cmp expect actual
-	"
-}
-
-check_parse 2008 bad
-check_parse 2008-02 bad
-check_parse 2008-02-14 bad
-check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
-check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
-check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
-check_parse '20080214T20:30:45' '2008-02-14 20:30:45 +0000'
-check_parse '20080214T20:30' '2008-02-14 20:30:00 +0000'
-check_parse '20080214T20' '2008-02-14 20:00:00 +0000'
-check_parse '20080214T203045' '2008-02-14 20:30:45 +0000'
-check_parse '20080214T2030' '2008-02-14 20:30:00 +0000'
-check_parse '20080214T000000.20' '2008-02-14 00:00:00 +0000'
-check_parse '20080214T00:00:00.20' '2008-02-14 00:00:00 +0000'
-check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'
-check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'
-check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'
-check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'
-check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'
-check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'
-check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'
-check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500'
-check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000'
-check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'
-check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
-check_parse 'Thu, 7 Apr 2005 15:14:13 -0700' '2005-04-07 15:14:13 -0700'
-
-check_approxidate() {
-	echo "$1 -> $2 +0000" >expect
-	test_expect_${3:-success} "parse approxidate ($1)" "
-	test-tool date approxidate '$1' >actual &&
-	test_cmp expect actual
-	"
-}
-
-check_approxidate now '2009-08-30 19:20:00'
-check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
-check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
-check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
-check_approxidate yesterday '2009-08-29 19:20:00'
-check_approxidate 3.days.ago '2009-08-27 19:20:00'
-check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'
-check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
-check_approxidate 3.months.ago '2009-05-30 19:20:00'
-check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
-
-check_approxidate '6am yesterday' '2009-08-29 06:00:00'
-check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
-check_approxidate '3:00' '2009-08-30 03:00:00'
-check_approxidate '15:00' '2009-08-30 15:00:00'
-check_approxidate 'noon today' '2009-08-30 12:00:00'
-check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
-check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
-check_approxidate '10am noon' '2009-08-29 12:00:00'
-
-check_approxidate 'last tuesday' '2009-08-25 19:20:00'
-check_approxidate 'July 5th' '2009-07-05 19:20:00'
-check_approxidate '06/05/2009' '2009-06-05 19:20:00'
-check_approxidate '06.05.2009' '2009-05-06 19:20:00'
-
-check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
-check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
-check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
-
-check_approxidate '2008-12-01' '2008-12-01 19:20:00'
-check_approxidate '2009-12-01' '2009-12-01 19:20:00'
-
-check_date_format_human() {
-	t=$(($GIT_TEST_DATE_NOW - $1))
-	echo "$t -> $2" >expect
-	test_expect_success "human date $t" '
-		test-tool date human $t >actual &&
-		test_cmp expect actual
-'
-}
-
-check_date_format_human 18000 "5 hours ago" # 5 hours ago
-check_date_format_human 432000 "Tue Aug 25 19:20" # 5 days ago
-check_date_format_human 1728000 "Mon Aug 10 19:20" # 3 weeks ago
-check_date_format_human 13000000 "Thu Apr 2 08:13" # 5 months ago
-check_date_format_human 31449600 "Aug 31 2008" # 12 months ago
-check_date_format_human 37500000 "Jun 22 2008" # 1 year, 2 months ago
-check_date_format_human 55188000 "Dec 1 2007" # 1 year, 9 months ago
-check_date_format_human 630000000 "Sep 13 1989" # 20 years ago
-
-test_done
diff --git a/t/unit-tests/t-date.c b/t/unit-tests/t-date.c
new file mode 100644
index 0000000000..dd5dbbb2e0
--- /dev/null
+++ b/t/unit-tests/t-date.c
@@ -0,0 +1,237 @@
+#include "test-lib.h"
+#include "date.h"
+#include "strbuf.h"
+
+/* Reference time: 2009-08-30 19:20:00. */
+#define GIT_TEST_DATE_NOW 1251660000
+
+/* The time corresponds to Wed, 15 Jun 2016 16:13:20 +0200. */
+static const char test_time[] = "1466000000 +0200";
+
+enum prerequisites {
+    	TIME_IS_64BIT = 1 << 0,
+    	TIME_T_IS_64BIT = 1 << 1,
+};
+
+/* Macro to check prerequisites */
+#define CHECK_PREREQ(var, prereq) \
+    	do { \
+		if ((var) & prereq && !check_prereq_##prereq()) { \
+			test_skip("missing prerequisite " #prereq); \
+			return 0; \
+		} \
+	} while (0)
+
+/* Return 1 if all prereqs are satisfied, 0 otherwise */
+static int check_prereqs(unsigned int prereqs) {
+    	CHECK_PREREQ(prereqs, TIME_IS_64BIT);
+    	CHECK_PREREQ(prereqs, TIME_T_IS_64BIT);
+
+    	return 1;
+}
+
+static void set_TZ_env(const char *zone) {
+	setenv("TZ", zone, 1);
+	tzset();
+}
+
+static void check_relative_dates(int time_val, const char *expected_date) {
+	struct strbuf buf = STRBUF_INIT;
+	timestamp_t diff = GIT_TEST_DATE_NOW - time_val;
+
+	show_date_relative(diff, &buf);
+	check_str(buf.buf, expected_date);
+	strbuf_release(&buf);
+}
+
+#define TEST_RELATIVE_DATE(value, expected_output) \
+    	TEST(check_relative_dates(value, expected_output), \
+        	"relative date (%s) works", #expected_output )
+
+static void check_show_date(const char *format, const char *TIME, const char *expected, unsigned int prereqs, const char *zone) {
+	struct date_mode mode = DATE_MODE_INIT;
+	char *arg;
+	timestamp_t t;
+	int tz;
+
+	if (!check_prereqs(prereqs))
+		return;
+	if (strcmp(zone, ""))
+		set_TZ_env(zone);
+
+	parse_date_format(format, &mode);
+	t = parse_timestamp(TIME, &arg, 10);
+	tz = atoi(arg);
+
+	check_str(show_date(t, tz, &mode), expected);
+
+	if (strcmp(zone, ""))
+		set_TZ_env("UTC");
+	date_mode_release(&mode);
+}
+
+#define TEST_SHOW_DATE(format, time, expected, prereqs, zone) \
+	TEST(check_show_date(format, time, expected, prereqs, zone), \
+	     "show date (%s) works", #format)
+
+static void check_parse_date(const char *given, const char *expected, const char *zone) {
+	struct strbuf result = STRBUF_INIT;
+	timestamp_t t;
+	int tz;
+
+	if (strcmp(zone, ""))
+		set_TZ_env(zone);
+
+	parse_date(given, &result);
+	if (sscanf(result.buf, "%"PRItime" %d", &t, &tz) == 2)
+		check_str(show_date(t, tz, DATE_MODE(ISO8601)), expected);
+	else
+		check_str("bad", expected);
+
+	if (strcmp(zone, ""))
+		set_TZ_env("UTC");
+	strbuf_release(&result);
+}
+
+#define TEST_PARSE_DATE(given, expected_output, zone) \
+    	TEST(check_parse_date(given, expected_output, zone), \
+        	"parse date (%s) works", #expected_output)
+
+static void check_approxidate(const char *given, const char *expected) {
+	timestamp_t t = approxidate(given);
+	char *expected_with_offset = xstrfmt("%s +0000", expected);
+
+	check_str(show_date(t, 0, DATE_MODE(ISO8601)), expected_with_offset);
+	free(expected_with_offset);
+}
+
+#define TEST_APPROXIDATE(given, expected_output) \
+    	TEST(check_approxidate(given, expected_output), \
+        	"parse approxidate (%s) works", #given)
+
+static void check_date_format_human(int given, const char *expected) {
+	timestamp_t diff = GIT_TEST_DATE_NOW - given;
+	check_str(show_date(diff, 0, DATE_MODE(HUMAN)), expected);
+}
+
+#define TEST_DATE_FORMAT_HUMAN(given, expected_output) \
+    	TEST(check_date_format_human(given, expected_output), \
+        	"human date (%s) works", #given)
+
+int cmd_main(int argc, const char **argv) {
+	set_TZ_env("UTC");
+	setenv("GIT_TEST_DATE_NOW", "1251660000", 1);
+	setenv("LANG", "C", 1);
+
+	TEST_RELATIVE_DATE(5, "5 seconds ago");
+	TEST_RELATIVE_DATE(300, "5 minutes ago");
+	TEST_RELATIVE_DATE(18000, "5 hours ago");
+	TEST_RELATIVE_DATE(432000, "5 days ago");
+	TEST_RELATIVE_DATE(1728000, "3 weeks ago");
+	TEST_RELATIVE_DATE(13000000, "5 months ago");
+	TEST_RELATIVE_DATE(37500000, "1 year, 2 months ago");
+	TEST_RELATIVE_DATE(55188000, "1 year, 9 months ago");
+	TEST_RELATIVE_DATE(630000000, "20 years ago");
+	TEST_RELATIVE_DATE(31449600, "12 months ago");
+	TEST_RELATIVE_DATE(62985600, "2 years ago");
+
+	TEST_SHOW_DATE("iso8601", test_time, "2016-06-15 16:13:20 +0200", 0, "");
+	TEST_SHOW_DATE("iso8601-strict", test_time, "2016-06-15T16:13:20+02:00", 0, "");
+	TEST_SHOW_DATE("rfc2822", test_time, "Wed, 15 Jun 2016 16:13:20 +0200", 0, "");
+	TEST_SHOW_DATE("short", test_time, "2016-06-15", 0, "");
+	TEST_SHOW_DATE("default", test_time, "Wed Jun 15 16:13:20 2016 +0200", 0, "");
+	TEST_SHOW_DATE("raw", test_time, test_time, 0, "");
+	TEST_SHOW_DATE("unix", test_time, "1466000000", 0, "");
+	TEST_SHOW_DATE("iso-local", test_time, "2016-06-15 14:13:20 +0000", 0, "");
+	TEST_SHOW_DATE("raw-local", test_time, "1466000000 +0000", 0, "");
+	TEST_SHOW_DATE("unix-local", test_time, "1466000000", 0, "");
+
+	TEST_SHOW_DATE("format:%z", test_time, "+0200", 0, "");
+	TEST_SHOW_DATE("format-local:%z", test_time, "+0000", 0, "");
+	TEST_SHOW_DATE("format:%Z", test_time, "", 0, "");
+	TEST_SHOW_DATE("format-local:%Z", test_time, "UTC", 0, "");
+	TEST_SHOW_DATE("format:%%z", test_time, "%z", 0, "");
+	TEST_SHOW_DATE("format-local:%%z", test_time, "%z", 0, "");
+
+	TEST_SHOW_DATE("format:%Y-%m-%d %H:%M:%S", test_time, "2016-06-15 16:13:20", 0, "");
+
+	TEST_SHOW_DATE("format-local:%Y-%m-%d %H:%M:%S", test_time, "2016-06-15 09:13:20", 0, "EST5");
+
+	TEST_SHOW_DATE("format:%s", "123456789 +1234", "123456789", 0, "");
+	TEST_SHOW_DATE("format:%s", "123456789 -1234", "123456789", 0, "");
+	TEST_SHOW_DATE("format-local:%s", "123456789 -1234", "123456789", 0, "");
+
+	/* Arbitrary time absurdly far in the future */
+	TEST_SHOW_DATE("iso", "5758122296 -0400", "2152-06-19 18:24:56 -0400", TIME_IS_64BIT | TIME_T_IS_64BIT, "");
+	TEST_SHOW_DATE("iso-local", "5758122296 -0400", "2152-06-19 22:24:56 +0000", TIME_IS_64BIT | TIME_T_IS_64BIT, "");
+
+	TEST_PARSE_DATE("2000", "bad", "");
+	TEST_PARSE_DATE("2008-02", "bad", "");
+	TEST_PARSE_DATE("2008-02-14", "bad", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -0500", "2008-02-14 20:30:45 -0500", "");
+	TEST_PARSE_DATE("2008.02.14 20:30:45 -0500", "2008-02-14 20:30:45 -0500", "");
+	TEST_PARSE_DATE("20080214T20:30:45", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("20080214T20:30", "2008-02-14 20:30:00 +0000", "");
+	TEST_PARSE_DATE("20080214T20", "2008-02-14 20:00:00 +0000", "");
+	TEST_PARSE_DATE("20080214T203045", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("20080214T2030", "2008-02-14 20:30:00 +0000", "");
+	TEST_PARSE_DATE("20080214T000000.20", "2008-02-14 00:00:00 +0000", "");
+	TEST_PARSE_DATE("20080214T00:00:00.20", "2008-02-14 00:00:00 +0000", "");
+	TEST_PARSE_DATE("20080214T203045-04:00", "2008-02-14 20:30:45 -0400", "");
+
+	TEST_PARSE_DATE("20080214T203045 -04:00", "2008-02-14 20:30:45 -0400", "");
+	TEST_PARSE_DATE("20080214T203045.019-04:00", "2008-02-14 20:30:45 -0400", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45.019-04:00", "2008-02-14 20:30:45 -0400", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -0015", "2008-02-14 20:30:45 -0015", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -5", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -5:", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -05", "2008-02-14 20:30:45 -0500", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -:30", "2008-02-14 20:30:45 +0000", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45 -05:00", "2008-02-14 20:30:45 -0500", "");
+	TEST_PARSE_DATE("2008-02-14 20:30:45", "2008-02-14 20:30:45 -0500", "EST5");
+
+	TEST_PARSE_DATE("Thu, 7 Apr 2005 15:14:13 -0700", "2005-04-07 15:14:13 -0700", "");
+
+	TEST_APPROXIDATE("now", "2009-08-30 19:20:00");
+	TEST_APPROXIDATE("5 seconds ago", "2009-08-30 19:19:55");
+	TEST_APPROXIDATE("10 minutes ago", "2009-08-30 19:10:00");
+	TEST_APPROXIDATE("yesterday", "2009-08-29 19:20:00");
+	TEST_APPROXIDATE("3 days ago", "2009-08-27 19:20:00");
+	TEST_APPROXIDATE("12:34:56.3 days ago", "2009-08-27 12:34:56");
+	TEST_APPROXIDATE("3 weeks ago", "2009-08-09 19:20:00");
+	TEST_APPROXIDATE("3 months ago", "2009-05-30 19:20:00");
+	TEST_APPROXIDATE("2 years 3 months ago", "2007-05-30 19:20:00");
+
+	TEST_APPROXIDATE("6am yesterday", "2009-08-29 06:00:00");
+	TEST_APPROXIDATE("6pm yesterday", "2009-08-29 18:00:00");
+	TEST_APPROXIDATE("3:00", "2009-08-30 03:00:00");
+	TEST_APPROXIDATE("15:00", "2009-08-30 15:00:00");
+	TEST_APPROXIDATE("noon today", "2009-08-30 12:00:00");
+	TEST_APPROXIDATE("noon yesterday", "2009-08-29 12:00:00");
+	TEST_APPROXIDATE("January 5th noon pm", "2009-01-05 12:00:00");
+	TEST_APPROXIDATE("10am noon", "2009-08-29 12:00:00");
+
+	TEST_APPROXIDATE("last tuesday", "2009-08-25 19:20:00");
+	TEST_APPROXIDATE("July 5th", "2009-07-05 19:20:00");
+	TEST_APPROXIDATE("06/05/2009", "2009-06-05 19:20:00");
+	TEST_APPROXIDATE("06.05.2009", "2009-05-06 19:20:00");
+
+	TEST_APPROXIDATE("Jun 6, 5AM", "2009-06-06 05:00:00");
+	TEST_APPROXIDATE("5AM Jun 6", "2009-06-06 05:00:00");
+	TEST_APPROXIDATE("6AM, June 7, 2009", "2009-06-07 06:00:00");
+
+	TEST_APPROXIDATE("2008-12-01", "2008-12-01 19:20:00");
+	TEST_APPROXIDATE("2009-12-01", "2009-12-01 19:20:00");
+
+	TEST_DATE_FORMAT_HUMAN(18000, "5 hours ago");
+	TEST_DATE_FORMAT_HUMAN(432000, "Tue Aug 25 19:20");
+	TEST_DATE_FORMAT_HUMAN(1728000, "Mon Aug 10 19:20");
+	TEST_DATE_FORMAT_HUMAN(13000000, "Thu Apr 2 08:13");
+	TEST_DATE_FORMAT_HUMAN(31449600, "Aug 31 2008");
+	TEST_DATE_FORMAT_HUMAN(37500000, "Jun 22 2008");
+	TEST_DATE_FORMAT_HUMAN(55188000, "Dec 1 2007");
+	TEST_DATE_FORMAT_HUMAN(630000000, "Sep 13 1989");
+
+	return test_done();
+}
--
2.43.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions
  2024-02-05 16:25 [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions Achu Luma
  2024-02-05 16:25 ` [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c Achu Luma
@ 2024-02-05 17:34 ` rsbecker
  2024-02-06  8:39   ` Christian Couder
  1 sibling, 1 reply; 10+ messages in thread
From: rsbecker @ 2024-02-05 17:34 UTC (permalink / raw)
  To: 'Achu Luma', git; +Cc: christian.couder, 'Christian Couder'

On Monday, February 5, 2024 11:25 AM, Achu Luma wrote:
>In a following commit we are going to port code from "t/helper/test-date.c"
and
>"t/t0006-date.sh" to a new "t/unit-tests/t-date.c" file using the recently
added unit
>test framework.
>
>We cannot fully port all the code from "t/helper/test-date.c" though, as
the test-
>tool date helper is still used by a number of "t/*.sh" tests.
>The TIME_IS_64BIT and TIME_T_IS_64BIT prereqs are especially used by
"t5000-
>tar-tree.sh", "t5318-commit-graph.sh" and
"t5328-commit-graph-64bit-time.sh"
>while checking those prereqs will be required in the new
"t/unit-tests/t-date.c" file
>too.
>
>To avoid duplicating in both "t/helper/test-date.c" and
"t/unit-tests/t-date.c" the
>small amount of code checking these prereqs, let's move it into inline
functions in
>"date.h".
>
>The names of these new inline functions contain "TIME_IS_64BIT" or
>"TIME_T_IS_64BIT" as it will simplify the macros we will use when we will
port code
>to "t/unit-tests/t-date.c" in a following commit.
>
>Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>Signed-off-by: Achu Luma <ach.lumap@gmail.com>
>---
> date.h               | 6 ++++++
> t/helper/test-date.c | 4 ++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/date.h b/date.h
>index 6136212a19..fb70490a51 100644
>--- a/date.h
>+++ b/date.h
>@@ -70,4 +70,10 @@ void datestamp(struct strbuf *out);  timestamp_t
>approxidate_careful(const char *, int *);  int date_overflows(timestamp_t
date);
>time_t tm_to_time_t(const struct tm *tm);
>+static inline int check_prereq_TIME_IS_64BIT(void) {
>+	return sizeof(timestamp_t) == 8;
>+}
>+static inline int check_prereq_TIME_T_IS_64BIT(void) {
>+	return sizeof(time_t) == 8;
>+}
> #endif
>diff --git a/t/helper/test-date.c b/t/helper/test-date.c index
>0683d46574..be0b8679c3 100644
>--- a/t/helper/test-date.c
>+++ b/t/helper/test-date.c
>@@ -126,9 +126,9 @@ int cmd__date(int argc UNUSED, const char **argv)
> 	else if (!strcmp(*argv, "getnanos"))
> 		getnanos(argv+1);
> 	else if (!strcmp(*argv, "is64bit"))
>-		return sizeof(timestamp_t) == 8 ? 0 : 1;
>+		return !check_prereq_TIME_IS_64BIT();
> 	else if (!strcmp(*argv, "time_t-is64bit"))
>-		return sizeof(time_t) == 8 ? 0 : 1;
>+		return !check_prereq_TIME_T_IS_64BIT();
> 	else
> 		usage(usage_msg);
> 	return 0;
>--
>2.43.0.windows.1

I would suggest that you also take into account whether time_t is signed or
not (more difficult perhaps). Some platforms use signed time_t to allow
representation of dates prior to 1970-01-01, while others make this signed.
Some other platforms (S/390 for example) have retained time_t as 32-bits but
have a time64_t for 64 bits. It might be useful to account for this.
--Randall


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions
  2024-02-05 17:34 ` [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions rsbecker
@ 2024-02-06  8:39   ` Christian Couder
  0 siblings, 0 replies; 10+ messages in thread
From: Christian Couder @ 2024-02-06  8:39 UTC (permalink / raw)
  To: rsbecker; +Cc: Achu Luma, git, Christian Couder

On Mon, Feb 5, 2024 at 6:34 PM <rsbecker@nexbridge.com> wrote:
> On Monday, February 5, 2024 11:25 AM, Achu Luma wrote:

> I would suggest that you also take into account whether time_t is signed or
> not (more difficult perhaps). Some platforms use signed time_t to allow
> representation of dates prior to 1970-01-01, while others make this signed.
> Some other platforms (S/390 for example) have retained time_t as 32-bits but
> have a time64_t for 64 bits. It might be useful to account for this.

The goal of this small series is just to port some existing tests to
the new unit test framework. I think it's a different topic to improve
the existing tests to take into account whether time_t is signed or
not. But thanks for the info.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-02-05 16:25 ` [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c Achu Luma
@ 2024-03-28 12:35   ` Ghanshyam Thakkar
  2024-03-28 16:35     ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Ghanshyam Thakkar @ 2024-03-28 12:35 UTC (permalink / raw)
  To: Achu Luma; +Cc: git, christian.couder, Christian Couder, Johannes Schindelin

On 24/02/05 05:25PM, Achu Luma wrote:
> In the recent codebase update (8bf6fbd (Merge branch
> 'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
> merged, providing a standardized approach for testing C code. Prior to
> this update, some unit tests relied on the test helper mechanism,
> lacking a dedicated unit testing framework. It's more natural to perform
> these unit tests using the new unit test framework.
> 
> This commit migrates the unit tests for C date functions
> (show_date_relative(), show_date(), etc) from the legacy approach
> using the test-tool command `test-tool date` in t/helper/test-date.c
> to the new unit testing framework (t/unit-tests/test-lib.h).
> 
> The migration involves refactoring the tests to utilize the testing
> macros provided by the framework (TEST() and check_*()).
> 
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Achu Luma <ach.lumap@gmail.com>
> ---
>  I am currently facing challenges in handling the time zone (TZ)
>  variable, in t-date.c. The tests, which were initially passing
>  in the t0006-date.sh  on Windows, fail after porting to the new
>  framework. I have tried to set the timezone using the setenv,
>  but unfortunately, the issue persists. I suspect that the problem
>  might be related to how Windows C compilers process the setenv
>  function compared to POSIX environments. Please If anyone has
>  insights into managing environment variables in a cross-platform
>  manner or has encountered similar issues, your help would be
>  highly appreciated.

I believe the issue might not be related to the setenv function, but rather
with tzset(). As you can see here[1], when we set TZ before we call the
unit-testing binaries, the tests which were failing (EST5 ones that I
separated with t-datetest) pass on 'win test (0)', and the ones which
were passing (UTC ones, t-date) fail. (Although some tests on linux are also
failing, but that can be explained by the fact that t-date runs first
and sets the TZ to UTC, afterwhich t-datetest runs and fails, although
this is not conclusive). Therefore, I am almost certain that the issue
is with changing the timezone during runtime on windows and not with setting
TZ variable with setenv(). CC'ing Johannes to see if he has any insights
on this.

Other that that, there are many places in this patch where indentation
is done with spaces instead of tabs, so it'd be good to change that.

Thanks.

[1]:
https://github.com/spectre10/git/actions/runs/8466649087/job/23196586161
> 
>  t/helper/test-date.c  |  91 +---------------
>  t/t0006-date.sh       | 169 ------------------------------
>  t/unit-tests/t-date.c | 237 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 238 insertions(+), 259 deletions(-)
>  delete mode 100755 t/t0006-date.sh
>  create mode 100644 t/unit-tests/t-date.c
> 
> diff --git a/t/helper/test-date.c b/t/helper/test-date.c
> index be0b8679c3..b9cb2c5455 100644
> --- a/t/helper/test-date.c
> +++ b/t/helper/test-date.c
> @@ -3,89 +3,11 @@
>  #include "trace.h"
> 
>  static const char *usage_msg = "\n"
> -"  test-tool date relative [time_t]...\n"
> -"  test-tool date human [time_t]...\n"
> -"  test-tool date show:<format> [time_t]...\n"
> -"  test-tool date parse [date]...\n"
> -"  test-tool date approxidate [date]...\n"
>  "  test-tool date timestamp [date]...\n"
>  "  test-tool date getnanos [start-nanos]\n"
>  "  test-tool date is64bit\n"
>  "  test-tool date time_t-is64bit\n";
> 
> -static void show_relative_dates(const char **argv)
> -{
> -	struct strbuf buf = STRBUF_INIT;
> -
> -	for (; *argv; argv++) {
> -		time_t t = atoi(*argv);
> -		show_date_relative(t, &buf);
> -		printf("%s -> %s\n", *argv, buf.buf);
> -	}
> -	strbuf_release(&buf);
> -}
> -
> -static void show_human_dates(const char **argv)
> -{
> -	for (; *argv; argv++) {
> -		time_t t = atoi(*argv);
> -		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_MODE(HUMAN)));
> -	}
> -}
> -
> -static void show_dates(const char **argv, const char *format)
> -{
> -	struct date_mode mode = DATE_MODE_INIT;
> -
> -	parse_date_format(format, &mode);
> -	for (; *argv; argv++) {
> -		char *arg;
> -		timestamp_t t;
> -		int tz;
> -
> -		/*
> -		 * Do not use our normal timestamp parsing here, as the point
> -		 * is to test the formatting code in isolation.
> -		 */
> -		t = parse_timestamp(*argv, &arg, 10);
> -		while (*arg == ' ')
> -			arg++;
> -		tz = atoi(arg);
> -
> -		printf("%s -> %s\n", *argv, show_date(t, tz, &mode));
> -	}
> -
> -	date_mode_release(&mode);
> -}
> -
> -static void parse_dates(const char **argv)
> -{
> -	struct strbuf result = STRBUF_INIT;
> -
> -	for (; *argv; argv++) {
> -		timestamp_t t;
> -		int tz;
> -
> -		strbuf_reset(&result);
> -		parse_date(*argv, &result);
> -		if (sscanf(result.buf, "%"PRItime" %d", &t, &tz) == 2)
> -			printf("%s -> %s\n",
> -			       *argv, show_date(t, tz, DATE_MODE(ISO8601)));
> -		else
> -			printf("%s -> bad\n", *argv);
> -	}
> -	strbuf_release(&result);
> -}
> -
> -static void parse_approxidate(const char **argv)
> -{
> -	for (; *argv; argv++) {
> -		timestamp_t t;
> -		t = approxidate(*argv);
> -		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_MODE(ISO8601)));
> -	}
> -}
> -
>  static void parse_approx_timestamp(const char **argv)
>  {
>  	for (; *argv; argv++) {
> @@ -106,22 +28,11 @@ static void getnanos(const char **argv)
> 
>  int cmd__date(int argc UNUSED, const char **argv)
>  {
> -	const char *x;
> 
>  	argv++;
>  	if (!*argv)
>  		usage(usage_msg);
> -	if (!strcmp(*argv, "relative"))
> -		show_relative_dates(argv+1);
> -	else if (!strcmp(*argv, "human"))
> -		show_human_dates(argv+1);
> -	else if (skip_prefix(*argv, "show:", &x))
> -		show_dates(argv+1, x);
> -	else if (!strcmp(*argv, "parse"))
> -		parse_dates(argv+1);
> -	else if (!strcmp(*argv, "approxidate"))
> -		parse_approxidate(argv+1);
> -	else if (!strcmp(*argv, "timestamp"))
> +	if (!strcmp(*argv, "timestamp"))
>  		parse_approx_timestamp(argv+1);
>  	else if (!strcmp(*argv, "getnanos"))
>  		getnanos(argv+1);
> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> deleted file mode 100755
> index e18b160286..0000000000
> --- a/t/t0006-date.sh
> +++ /dev/null
> @@ -1,169 +0,0 @@
> -#!/bin/sh
> -
> -test_description='test date parsing and printing'
> -
> -TEST_PASSES_SANITIZE_LEAK=true
> -. ./test-lib.sh
> -
> -# arbitrary reference time: 2009-08-30 19:20:00
> -GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
> -
> -check_relative() {
> -	t=$(($GIT_TEST_DATE_NOW - $1))
> -	echo "$t -> $2" >expect
> -	test_expect_${3:-success} "relative date ($2)" "
> -	test-tool date relative $t >actual &&
> -	test_cmp expect actual
> -	"
> -}
> -
> -check_relative 5 '5 seconds ago'
> -check_relative 300 '5 minutes ago'
> -check_relative 18000 '5 hours ago'
> -check_relative 432000 '5 days ago'
> -check_relative 1728000 '3 weeks ago'
> -check_relative 13000000 '5 months ago'
> -check_relative 37500000 '1 year, 2 months ago'
> -check_relative 55188000 '1 year, 9 months ago'
> -check_relative 630000000 '20 years ago'
> -check_relative 31449600 '12 months ago'
> -check_relative 62985600 '2 years ago'
> -
> -check_show () {
> -	format=$1
> -	time=$2
> -	expect=$3
> -	prereqs=$4
> -	zone=$5
> -	test_expect_success $prereqs "show date ($format:$time)" '
> -		echo "$time -> $expect" >expect &&
> -		TZ=${zone:-$TZ} test-tool date show:"$format" "$time" >actual &&
> -		test_cmp expect actual
> -	'
> -}
> -
> -# arbitrary but sensible time for examples
> -TIME='1466000000 +0200'
> -check_show iso8601 "$TIME" '2016-06-15 16:13:20 +0200'
> -check_show iso8601-strict "$TIME" '2016-06-15T16:13:20+02:00'
> -check_show rfc2822 "$TIME" 'Wed, 15 Jun 2016 16:13:20 +0200'
> -check_show short "$TIME" '2016-06-15'
> -check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
> -check_show raw "$TIME" '1466000000 +0200'
> -check_show unix "$TIME" '1466000000'
> -check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
> -check_show raw-local "$TIME" '1466000000 +0000'
> -check_show unix-local "$TIME" '1466000000'
> -
> -check_show 'format:%z' "$TIME" '+0200'
> -check_show 'format-local:%z' "$TIME" '+0000'
> -check_show 'format:%Z' "$TIME" ''
> -check_show 'format-local:%Z' "$TIME" 'UTC'
> -check_show 'format:%%z' "$TIME" '%z'
> -check_show 'format-local:%%z' "$TIME" '%z'
> -
> -check_show 'format:%Y-%m-%d %H:%M:%S' "$TIME" '2016-06-15 16:13:20'
> -check_show 'format-local:%Y-%m-%d %H:%M:%S' "$TIME" '2016-06-15 09:13:20' '' EST5
> -
> -check_show 'format:%s' '123456789 +1234' 123456789
> -check_show 'format:%s' '123456789 -1234' 123456789
> -check_show 'format-local:%s' '123456789 -1234' 123456789
> -
> -# arbitrary time absurdly far in the future
> -FUTURE="5758122296 -0400"
> -check_show iso       "$FUTURE" "2152-06-19 18:24:56 -0400" TIME_IS_64BIT,TIME_T_IS_64BIT
> -check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000" TIME_IS_64BIT,TIME_T_IS_64BIT
> -
> -check_parse() {
> -	echo "$1 -> $2" >expect
> -	test_expect_${4:-success} "parse date ($1${3:+ TZ=$3})" "
> -	TZ=${3:-$TZ} test-tool date parse '$1' >actual &&
> -	test_cmp expect actual
> -	"
> -}
> -
> -check_parse 2008 bad
> -check_parse 2008-02 bad
> -check_parse 2008-02-14 bad
> -check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
> -check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
> -check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
> -check_parse '20080214T20:30:45' '2008-02-14 20:30:45 +0000'
> -check_parse '20080214T20:30' '2008-02-14 20:30:00 +0000'
> -check_parse '20080214T20' '2008-02-14 20:00:00 +0000'
> -check_parse '20080214T203045' '2008-02-14 20:30:45 +0000'
> -check_parse '20080214T2030' '2008-02-14 20:30:00 +0000'
> -check_parse '20080214T000000.20' '2008-02-14 00:00:00 +0000'
> -check_parse '20080214T00:00:00.20' '2008-02-14 00:00:00 +0000'
> -check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'
> -check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'
> -check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'
> -check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'
> -check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'
> -check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'
> -check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'
> -check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500'
> -check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000'
> -check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'
> -check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
> -check_parse 'Thu, 7 Apr 2005 15:14:13 -0700' '2005-04-07 15:14:13 -0700'
> -
> -check_approxidate() {
> -	echo "$1 -> $2 +0000" >expect
> -	test_expect_${3:-success} "parse approxidate ($1)" "
> -	test-tool date approxidate '$1' >actual &&
> -	test_cmp expect actual
> -	"
> -}
> -
> -check_approxidate now '2009-08-30 19:20:00'
> -check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
> -check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
> -check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
> -check_approxidate yesterday '2009-08-29 19:20:00'
> -check_approxidate 3.days.ago '2009-08-27 19:20:00'
> -check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'
> -check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
> -check_approxidate 3.months.ago '2009-05-30 19:20:00'
> -check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
> -
> -check_approxidate '6am yesterday' '2009-08-29 06:00:00'
> -check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
> -check_approxidate '3:00' '2009-08-30 03:00:00'
> -check_approxidate '15:00' '2009-08-30 15:00:00'
> -check_approxidate 'noon today' '2009-08-30 12:00:00'
> -check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
> -check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
> -check_approxidate '10am noon' '2009-08-29 12:00:00'
> -
> -check_approxidate 'last tuesday' '2009-08-25 19:20:00'
> -check_approxidate 'July 5th' '2009-07-05 19:20:00'
> -check_approxidate '06/05/2009' '2009-06-05 19:20:00'
> -check_approxidate '06.05.2009' '2009-05-06 19:20:00'
> -
> -check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
> -check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
> -check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
> -
> -check_approxidate '2008-12-01' '2008-12-01 19:20:00'
> -check_approxidate '2009-12-01' '2009-12-01 19:20:00'
> -
> -check_date_format_human() {
> -	t=$(($GIT_TEST_DATE_NOW - $1))
> -	echo "$t -> $2" >expect
> -	test_expect_success "human date $t" '
> -		test-tool date human $t >actual &&
> -		test_cmp expect actual
> -'
> -}
> -
> -check_date_format_human 18000 "5 hours ago" # 5 hours ago
> -check_date_format_human 432000 "Tue Aug 25 19:20" # 5 days ago
> -check_date_format_human 1728000 "Mon Aug 10 19:20" # 3 weeks ago
> -check_date_format_human 13000000 "Thu Apr 2 08:13" # 5 months ago
> -check_date_format_human 31449600 "Aug 31 2008" # 12 months ago
> -check_date_format_human 37500000 "Jun 22 2008" # 1 year, 2 months ago
> -check_date_format_human 55188000 "Dec 1 2007" # 1 year, 9 months ago
> -check_date_format_human 630000000 "Sep 13 1989" # 20 years ago
> -
> -test_done
> diff --git a/t/unit-tests/t-date.c b/t/unit-tests/t-date.c
> new file mode 100644
> index 0000000000..dd5dbbb2e0
> --- /dev/null
> +++ b/t/unit-tests/t-date.c
> @@ -0,0 +1,237 @@
> +#include "test-lib.h"
> +#include "date.h"
> +#include "strbuf.h"
> +
> +/* Reference time: 2009-08-30 19:20:00. */
> +#define GIT_TEST_DATE_NOW 1251660000
> +
> +/* The time corresponds to Wed, 15 Jun 2016 16:13:20 +0200. */
> +static const char test_time[] = "1466000000 +0200";
> +
> +enum prerequisites {
> +    	TIME_IS_64BIT = 1 << 0,
> +    	TIME_T_IS_64BIT = 1 << 1,
> +};
> +
> +/* Macro to check prerequisites */
> +#define CHECK_PREREQ(var, prereq) \
> +    	do { \
> +		if ((var) & prereq && !check_prereq_##prereq()) { \
> +			test_skip("missing prerequisite " #prereq); \
> +			return 0; \
> +		} \
> +	} while (0)
> +
> +/* Return 1 if all prereqs are satisfied, 0 otherwise */
> +static int check_prereqs(unsigned int prereqs) {
> +    	CHECK_PREREQ(prereqs, TIME_IS_64BIT);
> +    	CHECK_PREREQ(prereqs, TIME_T_IS_64BIT);
> +
> +    	return 1;
> +}
> +
> +static void set_TZ_env(const char *zone) {
> +	setenv("TZ", zone, 1);
> +	tzset();
> +}
> +
> +static void check_relative_dates(int time_val, const char *expected_date) {
> +	struct strbuf buf = STRBUF_INIT;
> +	timestamp_t diff = GIT_TEST_DATE_NOW - time_val;
> +
> +	show_date_relative(diff, &buf);
> +	check_str(buf.buf, expected_date);
> +	strbuf_release(&buf);
> +}
> +
> +#define TEST_RELATIVE_DATE(value, expected_output) \
> +    	TEST(check_relative_dates(value, expected_output), \
> +        	"relative date (%s) works", #expected_output )
> +
> +static void check_show_date(const char *format, const char *TIME, const char *expected, unsigned int prereqs, const char *zone) {
> +	struct date_mode mode = DATE_MODE_INIT;
> +	char *arg;
> +	timestamp_t t;
> +	int tz;
> +
> +	if (!check_prereqs(prereqs))
> +		return;
> +	if (strcmp(zone, ""))
> +		set_TZ_env(zone);
> +
> +	parse_date_format(format, &mode);
> +	t = parse_timestamp(TIME, &arg, 10);
> +	tz = atoi(arg);
> +
> +	check_str(show_date(t, tz, &mode), expected);
> +
> +	if (strcmp(zone, ""))
> +		set_TZ_env("UTC");
> +	date_mode_release(&mode);
> +}
> +
> +#define TEST_SHOW_DATE(format, time, expected, prereqs, zone) \
> +	TEST(check_show_date(format, time, expected, prereqs, zone), \
> +	     "show date (%s) works", #format)
> +
> +static void check_parse_date(const char *given, const char *expected, const char *zone) {
> +	struct strbuf result = STRBUF_INIT;
> +	timestamp_t t;
> +	int tz;
> +
> +	if (strcmp(zone, ""))
> +		set_TZ_env(zone);
> +
> +	parse_date(given, &result);
> +	if (sscanf(result.buf, "%"PRItime" %d", &t, &tz) == 2)
> +		check_str(show_date(t, tz, DATE_MODE(ISO8601)), expected);
> +	else
> +		check_str("bad", expected);
> +
> +	if (strcmp(zone, ""))
> +		set_TZ_env("UTC");
> +	strbuf_release(&result);
> +}
> +
> +#define TEST_PARSE_DATE(given, expected_output, zone) \
> +    	TEST(check_parse_date(given, expected_output, zone), \
> +        	"parse date (%s) works", #expected_output)
> +
> +static void check_approxidate(const char *given, const char *expected) {
> +	timestamp_t t = approxidate(given);
> +	char *expected_with_offset = xstrfmt("%s +0000", expected);
> +
> +	check_str(show_date(t, 0, DATE_MODE(ISO8601)), expected_with_offset);
> +	free(expected_with_offset);
> +}
> +
> +#define TEST_APPROXIDATE(given, expected_output) \
> +    	TEST(check_approxidate(given, expected_output), \
> +        	"parse approxidate (%s) works", #given)
> +
> +static void check_date_format_human(int given, const char *expected) {
> +	timestamp_t diff = GIT_TEST_DATE_NOW - given;
> +	check_str(show_date(diff, 0, DATE_MODE(HUMAN)), expected);
> +}
> +
> +#define TEST_DATE_FORMAT_HUMAN(given, expected_output) \
> +    	TEST(check_date_format_human(given, expected_output), \
> +        	"human date (%s) works", #given)
> +
> +int cmd_main(int argc, const char **argv) {
> +	set_TZ_env("UTC");
> +	setenv("GIT_TEST_DATE_NOW", "1251660000", 1);
> +	setenv("LANG", "C", 1);
> +
> +	TEST_RELATIVE_DATE(5, "5 seconds ago");
> +	TEST_RELATIVE_DATE(300, "5 minutes ago");
> +	TEST_RELATIVE_DATE(18000, "5 hours ago");
> +	TEST_RELATIVE_DATE(432000, "5 days ago");
> +	TEST_RELATIVE_DATE(1728000, "3 weeks ago");
> +	TEST_RELATIVE_DATE(13000000, "5 months ago");
> +	TEST_RELATIVE_DATE(37500000, "1 year, 2 months ago");
> +	TEST_RELATIVE_DATE(55188000, "1 year, 9 months ago");
> +	TEST_RELATIVE_DATE(630000000, "20 years ago");
> +	TEST_RELATIVE_DATE(31449600, "12 months ago");
> +	TEST_RELATIVE_DATE(62985600, "2 years ago");
> +
> +	TEST_SHOW_DATE("iso8601", test_time, "2016-06-15 16:13:20 +0200", 0, "");
> +	TEST_SHOW_DATE("iso8601-strict", test_time, "2016-06-15T16:13:20+02:00", 0, "");
> +	TEST_SHOW_DATE("rfc2822", test_time, "Wed, 15 Jun 2016 16:13:20 +0200", 0, "");
> +	TEST_SHOW_DATE("short", test_time, "2016-06-15", 0, "");
> +	TEST_SHOW_DATE("default", test_time, "Wed Jun 15 16:13:20 2016 +0200", 0, "");
> +	TEST_SHOW_DATE("raw", test_time, test_time, 0, "");
> +	TEST_SHOW_DATE("unix", test_time, "1466000000", 0, "");
> +	TEST_SHOW_DATE("iso-local", test_time, "2016-06-15 14:13:20 +0000", 0, "");
> +	TEST_SHOW_DATE("raw-local", test_time, "1466000000 +0000", 0, "");
> +	TEST_SHOW_DATE("unix-local", test_time, "1466000000", 0, "");
> +
> +	TEST_SHOW_DATE("format:%z", test_time, "+0200", 0, "");
> +	TEST_SHOW_DATE("format-local:%z", test_time, "+0000", 0, "");
> +	TEST_SHOW_DATE("format:%Z", test_time, "", 0, "");
> +	TEST_SHOW_DATE("format-local:%Z", test_time, "UTC", 0, "");
> +	TEST_SHOW_DATE("format:%%z", test_time, "%z", 0, "");
> +	TEST_SHOW_DATE("format-local:%%z", test_time, "%z", 0, "");
> +
> +	TEST_SHOW_DATE("format:%Y-%m-%d %H:%M:%S", test_time, "2016-06-15 16:13:20", 0, "");
> +
> +	TEST_SHOW_DATE("format-local:%Y-%m-%d %H:%M:%S", test_time, "2016-06-15 09:13:20", 0, "EST5");
> +
> +	TEST_SHOW_DATE("format:%s", "123456789 +1234", "123456789", 0, "");
> +	TEST_SHOW_DATE("format:%s", "123456789 -1234", "123456789", 0, "");
> +	TEST_SHOW_DATE("format-local:%s", "123456789 -1234", "123456789", 0, "");
> +
> +	/* Arbitrary time absurdly far in the future */
> +	TEST_SHOW_DATE("iso", "5758122296 -0400", "2152-06-19 18:24:56 -0400", TIME_IS_64BIT | TIME_T_IS_64BIT, "");
> +	TEST_SHOW_DATE("iso-local", "5758122296 -0400", "2152-06-19 22:24:56 +0000", TIME_IS_64BIT | TIME_T_IS_64BIT, "");
> +
> +	TEST_PARSE_DATE("2000", "bad", "");
> +	TEST_PARSE_DATE("2008-02", "bad", "");
> +	TEST_PARSE_DATE("2008-02-14", "bad", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -0500", "2008-02-14 20:30:45 -0500", "");
> +	TEST_PARSE_DATE("2008.02.14 20:30:45 -0500", "2008-02-14 20:30:45 -0500", "");
> +	TEST_PARSE_DATE("20080214T20:30:45", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("20080214T20:30", "2008-02-14 20:30:00 +0000", "");
> +	TEST_PARSE_DATE("20080214T20", "2008-02-14 20:00:00 +0000", "");
> +	TEST_PARSE_DATE("20080214T203045", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("20080214T2030", "2008-02-14 20:30:00 +0000", "");
> +	TEST_PARSE_DATE("20080214T000000.20", "2008-02-14 00:00:00 +0000", "");
> +	TEST_PARSE_DATE("20080214T00:00:00.20", "2008-02-14 00:00:00 +0000", "");
> +	TEST_PARSE_DATE("20080214T203045-04:00", "2008-02-14 20:30:45 -0400", "");
> +
> +	TEST_PARSE_DATE("20080214T203045 -04:00", "2008-02-14 20:30:45 -0400", "");
> +	TEST_PARSE_DATE("20080214T203045.019-04:00", "2008-02-14 20:30:45 -0400", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45.019-04:00", "2008-02-14 20:30:45 -0400", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -0015", "2008-02-14 20:30:45 -0015", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -5", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -5:", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -05", "2008-02-14 20:30:45 -0500", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -:30", "2008-02-14 20:30:45 +0000", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45 -05:00", "2008-02-14 20:30:45 -0500", "");
> +	TEST_PARSE_DATE("2008-02-14 20:30:45", "2008-02-14 20:30:45 -0500", "EST5");
> +
> +	TEST_PARSE_DATE("Thu, 7 Apr 2005 15:14:13 -0700", "2005-04-07 15:14:13 -0700", "");
> +
> +	TEST_APPROXIDATE("now", "2009-08-30 19:20:00");
> +	TEST_APPROXIDATE("5 seconds ago", "2009-08-30 19:19:55");
> +	TEST_APPROXIDATE("10 minutes ago", "2009-08-30 19:10:00");
> +	TEST_APPROXIDATE("yesterday", "2009-08-29 19:20:00");
> +	TEST_APPROXIDATE("3 days ago", "2009-08-27 19:20:00");
> +	TEST_APPROXIDATE("12:34:56.3 days ago", "2009-08-27 12:34:56");
> +	TEST_APPROXIDATE("3 weeks ago", "2009-08-09 19:20:00");
> +	TEST_APPROXIDATE("3 months ago", "2009-05-30 19:20:00");
> +	TEST_APPROXIDATE("2 years 3 months ago", "2007-05-30 19:20:00");
> +
> +	TEST_APPROXIDATE("6am yesterday", "2009-08-29 06:00:00");
> +	TEST_APPROXIDATE("6pm yesterday", "2009-08-29 18:00:00");
> +	TEST_APPROXIDATE("3:00", "2009-08-30 03:00:00");
> +	TEST_APPROXIDATE("15:00", "2009-08-30 15:00:00");
> +	TEST_APPROXIDATE("noon today", "2009-08-30 12:00:00");
> +	TEST_APPROXIDATE("noon yesterday", "2009-08-29 12:00:00");
> +	TEST_APPROXIDATE("January 5th noon pm", "2009-01-05 12:00:00");
> +	TEST_APPROXIDATE("10am noon", "2009-08-29 12:00:00");
> +
> +	TEST_APPROXIDATE("last tuesday", "2009-08-25 19:20:00");
> +	TEST_APPROXIDATE("July 5th", "2009-07-05 19:20:00");
> +	TEST_APPROXIDATE("06/05/2009", "2009-06-05 19:20:00");
> +	TEST_APPROXIDATE("06.05.2009", "2009-05-06 19:20:00");
> +
> +	TEST_APPROXIDATE("Jun 6, 5AM", "2009-06-06 05:00:00");
> +	TEST_APPROXIDATE("5AM Jun 6", "2009-06-06 05:00:00");
> +	TEST_APPROXIDATE("6AM, June 7, 2009", "2009-06-07 06:00:00");
> +
> +	TEST_APPROXIDATE("2008-12-01", "2008-12-01 19:20:00");
> +	TEST_APPROXIDATE("2009-12-01", "2009-12-01 19:20:00");
> +
> +	TEST_DATE_FORMAT_HUMAN(18000, "5 hours ago");
> +	TEST_DATE_FORMAT_HUMAN(432000, "Tue Aug 25 19:20");
> +	TEST_DATE_FORMAT_HUMAN(1728000, "Mon Aug 10 19:20");
> +	TEST_DATE_FORMAT_HUMAN(13000000, "Thu Apr 2 08:13");
> +	TEST_DATE_FORMAT_HUMAN(31449600, "Aug 31 2008");
> +	TEST_DATE_FORMAT_HUMAN(37500000, "Jun 22 2008");
> +	TEST_DATE_FORMAT_HUMAN(55188000, "Dec 1 2007");
> +	TEST_DATE_FORMAT_HUMAN(630000000, "Sep 13 1989");
> +
> +	return test_done();
> +}
> --
> 2.43.0.windows.1
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-03-28 12:35   ` Ghanshyam Thakkar
@ 2024-03-28 16:35     ` Junio C Hamano
  2024-05-28 13:20       ` Patrick Steinhardt
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2024-03-28 16:35 UTC (permalink / raw)
  To: Ghanshyam Thakkar
  Cc: Achu Luma, git, christian.couder, Christian Couder, Johannes Schindelin

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> I believe the issue might not be related to the setenv function, but rather
> with tzset(). As you can see here[1], when we set TZ before we call the
> unit-testing binaries, the tests which were failing (EST5 ones that I
> separated with t-datetest) pass on 'win test (0)', and the ones which
> were passing (UTC ones, t-date) fail. (Although some tests on linux are also
> failing, but that can be explained by the fact that t-date runs first
> and sets the TZ to UTC, afterwhich t-datetest runs and fails, although
> this is not conclusive). Therefore, I am almost certain that the issue
> is with changing the timezone during runtime on windows and not with setting
> TZ variable with setenv(). CC'ing Johannes to see if he has any insights
> on this.

Interesting.  Sometime before I started working on Git, I learned
that no program did tzset() after it started running to switch
multiple timezones and worked correctly on many different variants
of UNIXes (there were many of them back then), and because I never
got interested in writing a world-clock program, I didn't know, and
kind of surprised to learn that it works on some platforms (like
Linux and macOS) to switch zones with tzset() these days ;-).

So, if Windows runtime is unhappy with the program calling tzset()
more than once, I wouldn't be too surprised.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-03-28 16:35     ` Junio C Hamano
@ 2024-05-28 13:20       ` Patrick Steinhardt
  2024-05-28 16:41         ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Patrick Steinhardt @ 2024-05-28 13:20 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ghanshyam Thakkar, Achu Luma, git, christian.couder,
	Christian Couder, Johannes Schindelin

[-- Attachment #1: Type: text/plain, Size: 2539 bytes --]

On Thu, Mar 28, 2024 at 09:35:39AM -0700, Junio C Hamano wrote:
> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> 
> > I believe the issue might not be related to the setenv function, but rather
> > with tzset(). As you can see here[1], when we set TZ before we call the
> > unit-testing binaries, the tests which were failing (EST5 ones that I
> > separated with t-datetest) pass on 'win test (0)', and the ones which
> > were passing (UTC ones, t-date) fail. (Although some tests on linux are also
> > failing, but that can be explained by the fact that t-date runs first
> > and sets the TZ to UTC, afterwhich t-datetest runs and fails, although
> > this is not conclusive). Therefore, I am almost certain that the issue
> > is with changing the timezone during runtime on windows and not with setting
> > TZ variable with setenv(). CC'ing Johannes to see if he has any insights
> > on this.
> 
> Interesting.  Sometime before I started working on Git, I learned
> that no program did tzset() after it started running to switch
> multiple timezones and worked correctly on many different variants
> of UNIXes (there were many of them back then), and because I never
> got interested in writing a world-clock program, I didn't know, and
> kind of surprised to learn that it works on some platforms (like
> Linux and macOS) to switch zones with tzset() these days ;-).
> 
> So, if Windows runtime is unhappy with the program calling tzset()
> more than once, I wouldn't be too surprised.
> 
> Thanks.

As I was debugging other Windows-specific issues in a VM already, Chris
asked me to also have a look at this issue. And indeed, most of the
tests fail deterministically. I also found a fix:

    diff --git a/t/unit-tests/t-date.c b/t/unit-tests/t-date.c
    index dd5dbbb2e0..2d7b1f085a 100644
    --- a/t/unit-tests/t-date.c
    +++ b/t/unit-tests/t-date.c
    @@ -31,7 +31,7 @@ static int check_prereqs(unsigned int prereqs) {
     }
     
     static void set_TZ_env(const char *zone) {
    -	setenv("TZ", zone, 1);
    +	_putenv_s("TZ", zone);
        tzset();
     }

I have no idea why that works though, and the fix is of course not
portable. But with this change, the timezones do get picked up by
`tzset()` and related date functions as expected.

I'm quite dumb when it comes to the Windows API, so I don't have much of
a clue why this works. The documentation also didn't point out anything
obvious. Dscho, do you happen to have an explanation for this?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-05-28 13:20       ` Patrick Steinhardt
@ 2024-05-28 16:41         ` Junio C Hamano
  2024-05-29  5:49           ` Patrick Steinhardt
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2024-05-28 16:41 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Ghanshyam Thakkar, Achu Luma, git, christian.couder,
	Christian Couder, Johannes Schindelin

Patrick Steinhardt <ps@pks.im> writes:

> As I was debugging other Windows-specific issues in a VM already, Chris
> asked me to also have a look at this issue. And indeed, most of the
> tests fail deterministically. I also found a fix:
> ...
>     -	setenv("TZ", zone, 1);
>     +	_putenv_s("TZ", zone);
>         tzset();
>      }
>
> I have no idea why that works though, and the fix is of course not
> portable. But with this change, the timezones do get picked up by
> `tzset()` and related date functions as expected.

The header compat/mingw.h already talks about implementing its own
replacement by making gitsetenv() call mingw_putenv().

gitsetenv() emulates setenv() in terms of putenv(), and on Windows
mingw_putenv() is what implements putenv(), so the difference you
are observing is coming from the difference between mingw_putenv()
and _putenv_s(), I would guess.  As the former is isolated within
compat/mingw.c, it would not involve any additional portability
issues to redo the former in terms of the latter, I would imagine.

> I'm quite dumb when it comes to the Windows API, so I don't have much of
> a clue why this works. The documentation also didn't point out anything
> obvious. Dscho, do you happen to have an explanation for this?

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-05-28 16:41         ` Junio C Hamano
@ 2024-05-29  5:49           ` Patrick Steinhardt
  2024-06-27  6:49             ` Johannes Schindelin
  0 siblings, 1 reply; 10+ messages in thread
From: Patrick Steinhardt @ 2024-05-29  5:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ghanshyam Thakkar, Achu Luma, git, christian.couder,
	Christian Couder, Johannes Schindelin

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

On Tue, May 28, 2024 at 09:41:47AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > As I was debugging other Windows-specific issues in a VM already, Chris
> > asked me to also have a look at this issue. And indeed, most of the
> > tests fail deterministically. I also found a fix:
> > ...
> >     -	setenv("TZ", zone, 1);
> >     +	_putenv_s("TZ", zone);
> >         tzset();
> >      }
> >
> > I have no idea why that works though, and the fix is of course not
> > portable. But with this change, the timezones do get picked up by
> > `tzset()` and related date functions as expected.
> 
> The header compat/mingw.h already talks about implementing its own
> replacement by making gitsetenv() call mingw_putenv().
> 
> gitsetenv() emulates setenv() in terms of putenv(), and on Windows
> mingw_putenv() is what implements putenv(), so the difference you
> are observing is coming from the difference between mingw_putenv()
> and _putenv_s(), I would guess.  As the former is isolated within
> compat/mingw.c, it would not involve any additional portability
> issues to redo the former in terms of the latter, I would imagine.

Ah, thanks for the pointer. And indeed, `mingw_putenv()` uses
`SetEnvironmentVariableW()` to provide the functionality. This is what
MSDN has to say [1]:

    getenv and _putenv use the copy of the environment pointed to by the
    global variable _environ to access the environment. getenv operates
    only on the data structures accessible to the run-time library and
    not on the environment "segment" created for the process by the
    operating system. Therefore, programs that use the envp argument to
    main or wmain may retrieve invalid information.

So calling `SetEnvironmentVariableW()` will not affect calls to
`getenv()`. See also issues like for example [2].

This works just fine for us because we also stub out `getenv()` to use
`GetEnvironmentVariableW()`, which is its counterpart. But everything
else in the C runtime that uses `getenv()` will not see the new values,
including our date-related functions. We don't ever set any of those
environment variables though, except for now in this new unit test.

Now the question is why we use `SetEnvironmentVariableW()` over
`_putenv_s`, and whether changing it would be safe. If the answer is a
strict "yes" then we could do that, but if it's a "maybe" then I'd
rather not want to change it just to make these unit tests work. In that
case, we might aim for a localized fix like the one I have posted.

Patrick

[1]: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/getenv-wgetenv?view=msvc-170
[2]: https://github.com/curl/curl/issues/4774

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c
  2024-05-29  5:49           ` Patrick Steinhardt
@ 2024-06-27  6:49             ` Johannes Schindelin
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Schindelin @ 2024-06-27  6:49 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Junio C Hamano, Ghanshyam Thakkar, Achu Luma, git,
	christian.couder, Christian Couder

Hi Patrick,

On Wed, 29 May 2024, Patrick Steinhardt wrote:

> Now the question is why we use `SetEnvironmentVariableW()` over
> `_putenv_s`, and whether changing it would be safe.

The reason is that Git for Windows internally uses UTF-8 _always_. But
`_putenv_s()` knows nothing of that choice, it uses the "active code
page", which -- you guessed it -- is not at all controlled by `LC_CTYPE`
but requires its own call to a Win32 Console API function.

Now, there is theoretically that thing that you _could_ switch the active
Win32 Console to CP_UTF8, i.e. the code page that corresponds to UTF-8.
However, for that to work as well as Git for Windows' users deserve it, it
would require a recent Windows 10 version, and Git for Windows still tries
to support Windows 7 and Windows 8 [*1*].

For that reason, Git for Windows performs the conversion from UTF-8 to
UTF-16 and then uses the `*W()` Win32 API function that accepts Unicode
(no matter what the current code page is).

With that in mind, I would love to find a solution that still uses that
`*W()` Win32 API function.

Ciao,
Johannes

Footnote *1*: That support was unfortunately already partially broken when
Git LFS dropped support for Windows 7 and Windows 8, where it now fails
with a segmentation fault (or "Access Violation" in Windows speak) and
only prints a cryptic error message instead. For full details, see
https://github.com/git-for-windows/git/issues/4996. You may note that this
breakage was accepted and not reverted by the Git LFS team, citing
security concerns ;-)

So you could argue that Git for Windows is already somewhat broken for
Windows versions prior to Windows 10, but that's not because of a
carefully planned roadmap but instead due to forces that are outside my
control.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-06-27  6:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-05 16:25 [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions Achu Luma
2024-02-05 16:25 ` [Outreachy][PATCH 2/2] Port helper/test-date.c to unit-tests/t-date.c Achu Luma
2024-03-28 12:35   ` Ghanshyam Thakkar
2024-03-28 16:35     ` Junio C Hamano
2024-05-28 13:20       ` Patrick Steinhardt
2024-05-28 16:41         ` Junio C Hamano
2024-05-29  5:49           ` Patrick Steinhardt
2024-06-27  6:49             ` Johannes Schindelin
2024-02-05 17:34 ` [Outreachy][PATCH 1/2] date: refactor 64 bit prereq code into reusable functions rsbecker
2024-02-06  8:39   ` Christian Couder

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 NNTP newsgroup(s).