diff mbox series

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

Message ID pull.1142.git.git.1637817792914.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series mingw: avoid fallback for {local,gm}time_r() | expand

Commit Message

Carlo Marcelo Arenas Belón Nov. 25, 2021, 5:23 a.m. UTC 
From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com>

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

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
    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/20211005063936.588874-1-mh@glandium.org/

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(-)


base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc

Comments

Junio C Hamano Nov. 26, 2021, 6:28 a.m. UTC | #1 
"Carlo Marcelo Arenas Belón via GitGitGadget"
<gitgitgadget@gmail.com> writes:

> From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com>
>
> 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/;

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

s/not longer/no longer/

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

s/since/Since/;

> 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.

> diff --git a/compat/mingw.c b/compat/mingw.c
> index 9e0cd1e097f..e14f2d5f77c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -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)
>  {
> diff --git a/git-compat-util.h b/git-compat-util.h
> index d70ce142861..c8005db3fb6 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -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
Johannes Schindelin Nov. 26, 2021, 1:55 p.m. UTC | #2 
Hi Carlo,

On Thu, 25 Nov 2021, Carlo Marcelo Arenas Belón via GitGitGadget wrote:

> From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com>
>
> 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
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

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

ACK,
Dscho

>     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/20211005063936.588874-1-mh@glandium.org/
>
> 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(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 9e0cd1e097f..e14f2d5f77c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -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)
>  {
> diff --git a/git-compat-util.h b/git-compat-util.h
> index d70ce142861..c8005db3fb6 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -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
>
diff mbox series

Patch

diff --git a/compat/mingw.c b/compat/mingw.c
index 9e0cd1e097f..e14f2d5f77c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -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)
 {
diff --git a/git-compat-util.h b/git-compat-util.h
index d70ce142861..c8005db3fb6 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -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