Skip to content

Fix mingw support#1107

Closed
letmaik wants to merge 6 commits intoUnidata:masterfrom
WRF-CMake:letmaik/mingw
Closed

Fix mingw support#1107
letmaik wants to merge 6 commits intoUnidata:masterfrom
WRF-CMake:letmaik/mingw

Conversation

@letmaik
Copy link
Copy Markdown

@letmaik letmaik commented Aug 11, 2018

This addresses #1105.

It also cleans up some redundant checks. E.g. if defined(_WIN32) || defined(_WIN64) is equivalent to if defined(_WIN32) because even in 64 bit mode, _WIN32 is defined as well.

In memio.c I had to make one exception and use #if defined (_WIN32) && !defined(__MINGW32__) as in this case mingw doesn't expose this particular Windows API function.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Aug 11, 2018

CLA assistant check
All committers have signed the CLA.

@WardF
Copy link
Copy Markdown
Member

WardF commented Aug 13, 2018

Thanks! It looks like the CI is failing, I'll dig into this further in a bit and for now I will restart them just to make sure it wasn't a glitch of some sort.

@WardF
Copy link
Copy Markdown
Member

WardF commented Aug 13, 2018

It does appear to have been something external to the PR, so no need to make any changes. We'll evaluate this, thanks!

@letmaik
Copy link
Copy Markdown
Author

letmaik commented Sep 19, 2018

Your merges from master a month ago broke the MinGW build in this branch (I haven't tested with latest master merged in):

C:/msys64/tmp/netcdf-c/libdispatch/dwinpath.c: In function 'NCremove':
C:/msys64/tmp/netcdf-c/libdispatch/dwinpath.c:232:32: error: 'ENOENT' undeclared (first use in this function)
     if(cvtname == NULL) return ENOENT;
                                ^~~~~~

It would be good if there was continuous integration setup for MinGW, e.g. with Appveyor, to detect such issues immediately.

Regarding the PR itself, are there any issues with it or can it be merged?

@DennisHeimbigner
Copy link
Copy Markdown
Collaborator

This is odd. According to this:
https://msdn.microsoft.com/en-us/library/5814770t.aspx
ENOENT is defined under visual studio. Is is really not defined
under MINGW? Did you look at the errno.h for MINGW?

Copy link
Copy Markdown
Collaborator

@DennisHeimbigner DennisHeimbigner left a comment

Choose a reason for hiding this comment

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

Since we decided to experiment with
_MSC_VER -> _WIN32
this is probably a good start, but only after the
next release.

@letmaik
Copy link
Copy Markdown
Author

letmaik commented Sep 20, 2018

ENOENT is part of errno.h, but I can't see that this file is included in dwinpath.c. It could be that it is transitively included in Linux from other standard library headers (or similarly with MSVC headers) but this doesn't seem to be the case in MinGW and to be honest is not something one should rely one in general.

@jschueller
Copy link
Copy Markdown
Contributor

Looks great, can you rebase so that it can be merged ?

@jschueller jschueller mentioned this pull request Mar 17, 2019
@letmaik
Copy link
Copy Markdown
Author

letmaik commented Mar 18, 2019

@jschueller This PR has been stale for a while, so I'm only willing to invest time if there's agreement it will be merged. Ideally, the rebasing can also be done by @DennisHeimbigner or @WardF, similar to how they've done it before in this PR.

@edhartnett
Copy link
Copy Markdown
Contributor

Is it possible to do the ming build on a standard Ubuntu linux box?

If so, I can set up a CI run to ensure that this continues to work.

@jschueller
Copy link
Copy Markdown
Contributor

jschueller commented Mar 18, 2019

totally, see https://github.com/stevengj/nlopt/blob/master/.travis.yml
you just need binutils-mingw-w64-x86-64, g++-mingw-w64-x86-64 packages, a toolchain file (https://github.com/stevengj/nlopt/blob/master/cmake/toolchain-x86_64-w64-mingw32.cmake) and pass CMAKE_TOOLCHAIN_FILE cmake variable

@WardF WardF added this to the 4.7.0 milestone Mar 20, 2019
@WardF WardF self-assigned this Mar 20, 2019
@WardF
Copy link
Copy Markdown
Member

WardF commented Mar 20, 2019

I've rebased and am prepping it for merge, but I see a failure I will follow up on when the other CI finishes.

@jschueller
Copy link
Copy Markdown
Contributor

it seems struct MagicFile has moved to dinfermodel.c, you need to rebase the changes to dfile.c against the current master

@WardF WardF modified the milestones: 4.7.0, 4.7.1 Apr 30, 2019
@WardF WardF modified the milestones: 4.7.1, 4.7.2 Aug 6, 2019
@WardF WardF removed this from the 4.7.2 milestone Oct 28, 2019
@WardF WardF added this to the 4.7.3 milestone Oct 28, 2019
@WardF WardF modified the milestones: 4.7.3, 4.8.0 Mar 27, 2020
@WardF WardF closed this Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants