Skip to contentSkip to conversation

mingw: avoid fallback for {local,gm}time_r()

PATCH Superseded Carlo Marcelo Arenas Belón via GitGitGadget 3 participants 5 messages Nov 25, 2021 – Nov 29, 2021

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mingw: avoid fallback for {local,gm}time_r()

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>

mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is no support for the *lockfile() functions required[1]) defined _POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).

the bug was fixed in winphtreads, but as a sideeffect, leaves the reentrant functions from time.h not longer visible and therefore breaks the build.

since the intention all along was to avoid using the fallback functions, formalize the use of POSIX by setting the corresponding feature flag and to make the intention clearer compile out the fallback functions.

[1] https://unix.org/whitepapers/reentrant.html

---
mingw: avoid fallback for {local,gm}time_r()

This is a cherry-pick from git-for-windows/git@adf0cd8, that is needed to be able to build recent master with the Git for Windows SDK or recent MinGW64.

It was discussed recently on list[1], and might need further adjustements if the 32-bit Git for Windows SDK also updates their winpthread headers, requiring a similar fix, but since without it a plain build from master wouldn't work in Windows I think it is worth reviewing on its own merits, and had withstood the test of time in the git for windows fork since it was originally merged there late August.

[1] https://lore.kernel.org/git/[email protected]/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1142%2Fcarenas%2Fwinbuild-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1142/carenas/winbuild-v1
Pull-Request: https://github.com/git/git/pull/1142

 compat/mingw.c    | 2 ++
 git-compat-util.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)
compat/mingw.c +2 -0
index 9e0cd1e097f..e14f2d5f77c 100644
@@ -1083,6 +1083,7 @@ int pipe(int filedes[2])
 	return 0;
 }
 
+#ifndef __MINGW64__
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
 	if (gmtime_s(result, timep) == 0)
@@ -1096,6 +1097,7 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
 		return result;
 	return NULL;
 }
+#endif
 
 char *mingw_getcwd(char *pointer, int len)
 {
git-compat-util.h +3 -1
index d70ce142861..c8005db3fb6 100644
@@ -127,7 +127,9 @@
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#if defined(__sun__)
+#ifdef __MINGW64__
+#define _POSIX_C_SOURCE 1
+#elif defined(__sun__)
  /*
   * On Solaris, when _XOPEN_EXTENDED is set, its header file
   * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE

base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc

Signature · 1 line
gitgitgadget
#1 lore raw

"Carlo Marcelo Arenas Belón via GitGitGadget"

Quoted 8 lines · <[email protected]> writes:

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>

mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is no support for the *lockfile() functions required[1]) defined _POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).

the bug was fixed in winphtreads, but as a sideeffect, leaves the

s/the/The/; s/sideeffect/side effect/;

Quoted 1 line

reentrant functions from time.h not longer visible and therefore breaks

s/not longer/no longer/

Quoted 3 lines

the build.

since the intention all along was to avoid using the fallback functions,

s/since/Since/;

Quoted 2 lines

formalize the use of POSIX by setting the corresponding feature flag and to make the intention clearer compile out the fallback functions.

I cannot parse the last sentence. I am guessing "... make the intention clearer by compiling out..." or "... make the intention clearer to compile out...", but I cannot quite tell.

Quoted 37 lines
compat/mingw.c +2 -0
index 9e0cd1e097f..e14f2d5f77c 100644
@@ -1083,6 +1083,7 @@ int pipe(int filedes[2])
 	return 0;
 }
 
+#ifndef __MINGW64__
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
 	if (gmtime_s(result, timep) == 0)
@@ -1096,6 +1097,7 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
 		return result;
 	return NULL;
 }
+#endif
 
 char *mingw_getcwd(char *pointer, int len)
 {
git-compat-util.h +3 -1
index d70ce142861..c8005db3fb6 100644
@@ -127,7 +127,9 @@
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#if defined(__sun__)
+#ifdef __MINGW64__
+#define _POSIX_C_SOURCE 1
+#elif defined(__sun__)
  /*
   * On Solaris, when _XOPEN_EXTENDED is set, its header file
   * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE

base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc

#2 lore raw
In reply to Carlo Marcelo Arenas Belón via GitGitGadget

Hi Carlo,

Quoted 11 lines · Carlo Marcelo Arenas Belón via GitGitGadget

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>

mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is no support for the *lockfile() functions required[1]) defined _POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).

the bug was fixed in winphtreads, but as a sideeffect, leaves the reentrant functions from time.h not longer visible and therefore breaks the build.

Quoted 7 lines

since the intention all along was to avoid using the fallback functions, formalize the use of POSIX by setting the corresponding feature flag and to make the intention clearer compile out the fallback functions.

[1] https://unix.org/whitepapers/reentrant.html

Yep, this patch works well for Git for Windows already for a good while.

ACK,
Dscho

Quoted 63 lines

mingw: avoid fallback for {local,gm}time_r()

This is a cherry-pick from git-for-windows/git@adf0cd8, that is needed to be able to build recent master with the Git for Windows SDK or recent MinGW64.

It was discussed recently on list[1], and might need further adjustements if the 32-bit Git for Windows SDK also updates their winpthread headers, requiring a similar fix, but since without it a plain build from master wouldn't work in Windows I think it is worth reviewing on its own merits, and had withstood the test of time in the git for windows fork since it was originally merged there late August.

[1] https://lore.kernel.org/git/[email protected]/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1142%2Fcarenas%2Fwinbuild-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1142/carenas/winbuild-v1
Pull-Request: https://github.com/git/git/pull/1142

 compat/mingw.c    | 2 ++
 git-compat-util.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)
compat/mingw.c +2 -0
index 9e0cd1e097f..e14f2d5f77c 100644
@@ -1083,6 +1083,7 @@ int pipe(int filedes[2])
 	return 0;
 }
 
+#ifndef __MINGW64__
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
 	if (gmtime_s(result, timep) == 0)
@@ -1096,6 +1097,7 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
 		return result;
 	return NULL;
 }
+#endif
 
 char *mingw_getcwd(char *pointer, int len)
 {
git-compat-util.h +3 -1
index d70ce142861..c8005db3fb6 100644
@@ -127,7 +127,9 @@
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#if defined(__sun__)
+#ifdef __MINGW64__
+#define _POSIX_C_SOURCE 1
+#elif defined(__sun__)
  /*
   * On Solaris, when _XOPEN_EXTENDED is set, its header file
   * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE

base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc --
gitgitgadget

#3 lore raw

PATCH v2 1/1 mingw: avoid fallback for {local,gm}time_r()

In reply to Carlo Marcelo Arenas Belón via GitGitGadget

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>

mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is no support for the *lockfile() functions required[1]) defined _POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).

The bug was fixed in winphtreads, but as a side effect, leaves the reentrant functions from time.h no longer visible and therefore breaks the build.

Since the intention all along was to avoid using the fallback functions, formalize the use of POSIX by setting the corresponding feature flag and compile out the implementation for the fallback functions.

[1] https://unix.org/whitepapers/reentrant.html

---
mingw: fix 64 bit build because of missing *time_r() function definitions

This is a cherry-pick from git-for-windows/git#3398, that is needed to be able to build recent master with the Git for Windows SDK or recent MinGW64.

It was discussed recently on list[1], and might need further adjustements if the 32-bit Git for Windows SDK also updates their winpthread headers, requiring a similar fix, but since without it a plain build from master wouldn't work in Windows I think it is worth reviewing on its own merits, and had withstood the test of time in the Git for Windows fork since it was originally merged there late August.

Changes since v1:

* Fixed grammar in the commit message (suggested by Junio)
* Added ACK (proposed by dscho)

[1] https://lore.kernel.org/git/[email protected]/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1142%2Fcarenas%2Fwinbuild-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1142/carenas/winbuild-v2
Pull-Request: https://github.com/git/git/pull/1142

Range-diff vs v1:

1: 363b644f801 ! 1: c6b3fc5a16a mingw: avoid fallback for {local,gm}time_r()
@@ Commit message
_POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).

- the bug was fixed in winphtreads, but as a sideeffect, leaves the
- reentrant functions from time.h not longer visible and therefore breaks
+ The bug was fixed in winphtreads, but as a side effect, leaves the
+ reentrant functions from time.h no longer visible and therefore breaks
the build.

- since the intention all along was to avoid using the fallback functions,
+ Since the intention all along was to avoid using the fallback functions,
formalize the use of POSIX by setting the corresponding feature flag and
- to make the intention clearer compile out the fallback functions.
+ compile out the implementation for the fallback functions.

[1] https://unix.org/whitepapers/reentrant.html

+ Acked-by: Johannes Schindelin <[email protected]>

## compat/mingw.c ##
@@ compat/mingw.c: int pipe(int filedes[2])

 compat/mingw.c    | 2 ++
 git-compat-util.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)
compat/mingw.c +2 -0
index 9e0cd1e097f..e14f2d5f77c 100644
@@ -1083,6 +1083,7 @@ int pipe(int filedes[2])
 	return 0;
 }
 
+#ifndef __MINGW64__
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
 	if (gmtime_s(result, timep) == 0)
@@ -1096,6 +1097,7 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
 		return result;
 	return NULL;
 }
+#endif
 
 char *mingw_getcwd(char *pointer, int len)
 {
git-compat-util.h +3 -1
index d70ce142861..c8005db3fb6 100644
@@ -127,7 +127,9 @@
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#if defined(__sun__)
+#ifdef __MINGW64__
+#define _POSIX_C_SOURCE 1
+#elif defined(__sun__)
  /*
   * On Solaris, when _XOPEN_EXTENDED is set, its header file
   * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE

base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc

Signature · 1 line
gitgitgadget
#4 lore raw

"Carlo Marcelo Arenas Belón via GitGitGadget"

Quoted 11 lines · <[email protected]> writes:

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>

mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is no support for the *lockfile() functions required[1]) defined _POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).
...
Changes since v1:

* Fixed grammar in the commit message (suggested by Junio)
* Added ACK (proposed by dscho)

Thanks. Queued.