Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a more canonical method to fix the CPU reporting on Windows #125

Closed

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 13, 2019

Eric Sunshine pointed out that I had completely forgotten about the HOST_CPU thing, and that is indeed the much better method of fixing the issue.

Cc: Eric Sunshine sunshine@sunshineco.com

In `git version --build-options`, we report also the CPU, but in Git for
Windows we actually cross-compile the 32-bit version in a 64-bit Git for
Windows, so we cannot rely on the auto-detected value.

In 3815f64 (mingw: fix CPU reporting in `git version
--build-options`, 2019-02-07), we fixed this by a Windows-only
workaround, making use of magic pre-processor constants, which works in
GCC, but most likely not all C compilers.

As pointed out by Eric Sunshine, there is a better way, anyway: to set
the Makefile variable HOST_CPU explicitly for cross-compiled Git. So
let's do that!

This reverts commit 3815f64 partially.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented Feb 13, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 13, 2019

Submitted as pull.125.git.gitgitgadget@gmail.com

@@ -6,25 +6,6 @@ typedef _sigset_t sigset_t;
#include <winsock2.h>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In `git version --build-options`, we report also the CPU, but in Git for
> Windows we actually cross-compile the 32-bit version in a 64-bit Git for
> Windows, so we cannot rely on the auto-detected value.
>
> In 3815f64b0dd9 (mingw: fix CPU reporting in `git version
> --build-options`, 2019-02-07), we fixed this by a Windows-only
> workaround, making use of magic pre-processor constants, which works in
> GCC, but most likely not all C compilers.
>
> As pointed out by Eric Sunshine, there is a better way, anyway: to set
> the Makefile variable HOST_CPU explicitly for cross-compiled Git. So
> let's do that!
>
> This reverts commit 3815f64b0dd983bdbf9242a0547706d5d81cb3e6 partially.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.h   | 19 -------------------
>  config.mak.uname |  2 ++
>  2 files changed, 2 insertions(+), 19 deletions(-)

Will apply; thanks.

>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 363f047df0..8c24ddaa3e 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -6,25 +6,6 @@ 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
> diff --git a/config.mak.uname b/config.mak.uname
> index 3ee7da0e23..194f20a8b3 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -553,9 +553,11 @@ else
>  		prefix = /usr/
>  		ifeq (MINGW32,$(MSYSTEM))
>  			prefix = /mingw32
> +			HOST_CPU = i686
>  		endif
>  		ifeq (MINGW64,$(MSYSTEM))
>  			prefix = /mingw64
> +			HOST_CPU = x86_64
>  		else
>  			COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
>  			BASIC_LDFLAGS += -Wl,--large-address-aware

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 13, 2019

This branch is now known as js/mingw-host-cpu.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 13, 2019

This patch series was integrated into pu via git@5247ce8.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 13, 2019

This patch series was integrated into next via git@f168a80.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 14, 2019

This patch series was integrated into pu via git@8593e8a.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 14, 2019

This patch series was integrated into next via git@8593e8a.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 14, 2019

This patch series was integrated into master via git@8593e8a.

@gitgitgadget gitgitgadget bot added the master label Feb 14, 2019
@gitgitgadget gitgitgadget bot closed this Feb 14, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 14, 2019

Closed via 8593e8a.

@dscho dscho deleted the mingw-host-cpu-take2 branch February 14, 2019 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant