Fix incorrectly reported CPU in 32-bit Windows
cover Fix incorrectly reported CPU in 32-bit Windows
While in review, Git for Windows' original design was changed, to use the output of uname -m instead of (necessarily incomplete) pre-processor magic to determine which CPU to report.
Both 32-bit and 64-bit versions of Git for Windows are built in the 64-bit Git for Windows SDK, however, whose uname -m always reports x86_64. Even for 32-bit Git for Windows.
Let's fix that by going back to the pre-processor magic, making it specific to Git for Windows' SDK (where it actually is complete).
This is yet another patch I forgot to upstream, and I hope that it will make it into v2.21.0-rc1.
Johannes Schindelin (1):
mingw: fix CPU reporting in git version --build-options
compat/mingw.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
base-commit: d62dad7a7dca3f6a65162bf0e52cdf6927958e78
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-121%2Fdscho%2Fmingw-build-options-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-121/dscho/mingw-build-options-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/121
Signature · 1 line
gitgitgadget
PATCH 1/1 mingw: fix CPU reporting in `git version --build-options`
From: Johannes Schindelin <[email protected]>
We cannot rely on uname -m in Git for Windows' SDK to tell us what architecture we are compiling for, as we can compile both 32-bit and 64-bit git.exe from a 64-bit SDK, but the uname -m in that SDK will always report x86_64.
So let's go back to our original design. And make it explicitly Windows-specific.
--- compat/mingw.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
compat/mingw.h +19 -0
@@ -6,6 +6,25 @@ typedef _sigset_t sigset_t;#include <winsock2.h> #include <ws2tcpip.h> +#ifdef __MINGW64_VERSION_MAJOR +/* +* In Git for Windows, we cannot rely on `uname -m` to report the correct +* architecture: /usr/bin/uname.exe will report the architecture with which the +* current MSYS2 runtime was built, not the architecture for which we are +* currently compiling (both 32-bit and 64-bit `git.exe` is built in the 64-bit +* Git for Windows SDK). +*/ +#undef GIT_HOST_CPU +/* This was figured out by looking at `cpp -dM </dev/null`'s output */ +#if defined(__x86_64__) +#define GIT_HOST_CPU "x86_64" +#elif defined(__i686__) +#define GIT_HOST_CPU "i686" +#else +#error "Unknown architecture" +#endif +#endif +/* MinGW-w64 reports to have flockfile, but it does not actually have it. */ #ifdef __MINGW64_VERSION_MAJOR#undef _POSIX_THREAD_SAFE_FUNCTIONS
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
This is yet another patch I forgot to upstream, and I hope that it will make it into v2.21.0-rc1.
I guess this affects nobody other than you and perhaps J6t, the point not being "there are only just two" but being "all of them know how to deal with possible breakages if any in this change".
I am tempted to bypass the usual "from pu down to next down to master" dance on this one, because of that.
;-)
Hi Junio,
Quoted 9 lines · Junio C Hamano
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
This is yet another patch I forgot to upstream, and I hope that it will make it into v2.21.0-rc1.
I guess this affects nobody other than you and perhaps J6t, the point not being "there are only just two" but being "all of them know how to deal with possible breakages if any in this change".
Correct. This patch is already in Git for Windows, so contributors building from that fork will have had the problem already.
Quoted 4 lines
I am tempted to bypass the usual "from pu down to next down to master" dance on this one, because of that.
;-)
I would be very grateful for that :-)
Ciao,
Dscho
Re: [PATCH 1/1] mingw: fix CPU reporting in `git version --build-options`
In reply to Johannes Schindelin via GitGitGadgetQuoted 7 lines · Johannes Schindelin via GitGitGadget
We cannot rely on
uname -min Git for Windows' SDK to tell us what architecture we are compiling for, as we can compile both 32-bit and 64-bitgit.exefrom a 64-bit SDK, but theuname -min that SDK will always reportx86_64.So let's go back to our original design. And make it explicitly Windows-specific.
b22894049f (version --build-options: also report host CPU, 2017-12-15) took this sort of case into consideration by introducing Makefile variable HOST_CPU (which takes precedence over uname -m), with the intention that, when cross-compiling, your build environment should (somehow) set HOST_CPU to the canonical name of the CPU on which the built Git will run (for instance, "x86_64" or "i686"). It would be nice to employ this mechanism to solve this issue rather than (re-)introducing a manually-maintained list of CPU names.
Can you say a few words (here in the email thread) about how the Git for Windows SDK is instructed to build for one architecture or the other? With such information, perhaps we can figure out how to get the build environment itself to set HOST_CPU automatically so we don't have to resort to and worry about maintenance costs of a hard-coded CPU name list.
Quoted 22 lines
Signed-off-by Johannes Schindelin <[email protected]>---
compat/mingw.h +18
-0@@ -6,6 +6,25 @@ typedef _sigset_t sigset_t; +#ifdef __MINGW64_VERSION_MAJOR +/* +* In Git for Windows, we cannot rely on `uname -m` to report the correct +* architecture: /usr/bin/uname.exe will report the architecture with which the +* current MSYS2 runtime was built, not the architecture for which we are +* currently compiling (both 32-bit and 64-bit `git.exe` is built in the 64-bit +* Git for Windows SDK). +*/ +#undef GIT_HOST_CPU +/* This was figured out by looking at `cpp -dM </dev/null`'s output */ +#if defined(__x86_64__) +#define GIT_HOST_CPU "x86_64" +#elif defined(__i686__) +#define GIT_HOST_CPU "i686" +#else +#error "Unknown architecture" +#endif +#endif
Re: [PATCH 1/1] mingw: fix CPU reporting in `git version --build-options`
Hi Eric,
Quoted 10 lines · Eric Sunshine
Quoted 7 lines · Johannes Schindelin via GitGitGadget
We cannot rely on
uname -min Git for Windows' SDK to tell us what architecture we are compiling for, as we can compile both 32-bit and 64-bitgit.exefrom a 64-bit SDK, but theuname -min that SDK will always reportx86_64.So let's go back to our original design. And make it explicitly Windows-specific.
Quoted 8 lines
b22894049f(version --build-options: also report host CPU, 2017-12-15) took this sort of case into consideration by introducingMakefilevariableHOST_CPU(which takes precedence overuname -m), with the intention that, when cross-compiling, your build environment should (somehow) setHOST_CPUto the canonical name of the CPU on which the built Git will run (for instance, "x86_64" or "i686"). It would be nice to employ this mechanism to solve this issue rather than (re-)introducing a manually-maintained list of CPU names.
Heh, this is also manually-maintained, but I agree that it is cleaner.
Quoted 3 lines
Can you say a few words (here in the email thread) about how the Git for Windows SDK is instructed to build for one architecture or the other?
To cross-compile a 32-bit Git in a 64-bit Git for Windows SDK, use this incantation:
MSYSTEM=MINGW32 PATH=/mingw32/bin:$PATH make
Quoted 4 lines
With such information, perhaps we can figure out how to get the build environment itself to set
HOST_CPUautomatically so we don't have to resort to and worry about maintenance costs of a hard-coded CPU name list.
Indeed, we can set HOST_CPU in the same conditionals as prefix (which is /mingw32 for 32-bit and /mingw64 for 64-bit) in config.mak.uname.
Patch incoming,
Dscho
Quoted 23 lines
Quoted 22 lines
Signed-off-by Johannes Schindelin <[email protected]>---
compat/mingw.h +18
-0@@ -6,6 +6,25 @@ typedef _sigset_t sigset_t; +#ifdef __MINGW64_VERSION_MAJOR +/* +* In Git for Windows, we cannot rely on `uname -m` to report the correct +* architecture: /usr/bin/uname.exe will report the architecture with which the +* current MSYS2 runtime was built, not the architecture for which we are +* currently compiling (both 32-bit and 64-bit `git.exe` is built in the 64-bit +* Git for Windows SDK). +*/ +#undef GIT_HOST_CPU +/* This was figured out by looking at `cpp -dM </dev/null`'s output */ +#if defined(__x86_64__) +#define GIT_HOST_CPU "x86_64" +#elif defined(__i686__) +#define GIT_HOST_CPU "i686" +#else +#error "Unknown architecture" +#endif +#endif