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

add -m64 CFLAGS when targeting mingw64, add -m32/-m64 to LDFLAGS #134

Merged
merged 1 commit into from Jan 9, 2015

Conversation

Projects
None yet
3 participants
@vszakats
Member

vszakats commented Jan 8, 2015

No description provided.

@gvanem

This comment has been minimized.

Member

gvanem commented Jan 8, 2015

How can you be sure that this gcc can create dual targets (both -m32 and -m64). Are you assuming MingW-builds or Ruben's gcc-MingW-w64 here? Last time I tried them, they sucked. Only TDM-gcc works for dual-targeting IMHO.

@gknauf

This comment has been minimized.

Contributor

gknauf commented Jan 8, 2015

On 08.01.2015 20:17, Gisle Vanem wrote:

How can you be sure that this |gcc| can create dual targets (both |-m32|
and |-m64|). Are you assuming MingW-builds or Ruben's gcc-MingW-w64
here? Last time I tried them, they sucked. Only TDM-gcc works for
dual-targeting IMHO.
hmm, when I recently added the -m32 flag I've tested that various MinGW
32-bit versions accept it; also I use -m32 already for a long time with
my NetWare builds, and it didnt matter up to now if it was a 32-bit or
64-bit Linux gcc;
so I assume that at least gcc >= 3.2 does either understand or ignore
the -m32 flag; I've not yet tested if the same goes for the -m64 flag,
but if so it may be safe too to add it ...
what problems did you face?

Gün.

@vszakats

This comment has been minimized.

Member

vszakats commented Jan 8, 2015

Dual mingw-builds work fine with this switch, and as far as I know they are supported by single target 64-bit ones, too, so they are either ignored (when using a mingw that targets 64-bit by default anyway), or required (in dual target mingws when building to non-default target), or rejected when in disagreement with mingw capabilities (misconfiguration).

These are the mingw distro combinations I'm aware of:

  1. 32-bit hosted, 32-bit target – -m64 rejected (ARCH setting should be corrected in this case)
  2. 64-bit hosted, 64-bit target – -m64 ignored as the default
  3. 32-bit hosted, dual target, defaulting to 32-bit – -m64 required if one wants a 64-bit target
  4. 64-bit hosted, dual target, defaulting to 64-bit – -m64 ignored as the default

Above should be true for all mingw distros, what I actually made tests with lately, is this one: https://sourceforge.net/projects/mingw-w64/ (they didn't suck in my experience, and we had a fair share of suckage with all distros in various points of time - since some years, this one works well)

@vszakats

This comment has been minimized.

Member

vszakats commented Jan 8, 2015

[ To be precise the niXman builds worked well for us from https://sourceforge.net/projects/mingwbuilds/, and haven't tried Ruben's builds (these seem to be outdated now.) Recently the builds formerly known as niXman can be found at https://sourceforge.net/projects/mingw-w64/files/ under /Toolchains targetting Win[32|64]/Personal Builds/mingw-builds/. ]

@vszakats

This comment has been minimized.

Member

vszakats commented Jan 8, 2015

As for the -m32 added to LDFLAGS in line https://github.com/bagder/curl/pull/134/files#diff-cff6f4d0b937841a93595fb097eaeb12R98, it should be just as safe as the CFLAGS usage in the existing preceding line.

@gknauf

This comment has been minimized.

Contributor

gknauf commented Jan 8, 2015

On 08.01.2015 22:11, Viktor Szakáts wrote:

As for the |-m32| added to |LDFLAGS| in line
https://github.com/bagder/curl/pull/134/files#diff-cff6f4d0b937841a93595fb097eaeb12R98,
it should be just as safe as the |CFLAGS| usage in the existing previous
line.
I've just tested your patch, and see no issues with my MinGW and MinGW64
versions.

Daniel or Steve, can you please commit? I'm not (yet) familar with the
pull request stuff, and fear to not doing it properly :-P

Gün.

@vszakats

This comment has been minimized.

Member

vszakats commented Jan 8, 2015

Thanks for the tests Günter.

@gknauf gknauf self-assigned this Jan 9, 2015

gknauf added a commit that referenced this pull request Jan 9, 2015

Merge pull request #134 from vszakats/mingw-m64
add -m64 CFLAGS when targeting mingw64, add -m32/-m64 to LDFLAGS

@gknauf gknauf merged commit d21b668 into curl:master Jan 9, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@vszakats vszakats deleted the vszakats:mingw-m64 branch Jan 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment