Skip to content

Allow native Windows build using MinGW#30

Merged
malb merged 8 commits intomalb:masterfrom
ptrrsn:mingw_build
Jun 25, 2025
Merged

Allow native Windows build using MinGW#30
malb merged 8 commits intomalb:masterfrom
ptrrsn:mingw_build

Conversation

@ptrrsn
Copy link
Copy Markdown
Contributor

@ptrrsn ptrrsn commented Jun 21, 2025

This PR contains commits that allow both make and make check to work on MinGW. The commits were tested on both MinGW (through MSYS UCRT64) and Linux gcc (through WSL). However, this does not allow us to build bench on MinGW, which seems to be significantly more difficult.

As a context, this native Windows build will help us to create a native Windows build for Sagemath: sagemath/sage#38872

@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 22, 2025

cc @dimpase

@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 22, 2025

@tobiasdiez Just FYI. It's a little early because the PR has not been merged yet, but it gives a working build that passes all tests (make check).

@dimpase
Copy link
Copy Markdown
Contributor

dimpase commented Jun 22, 2025

this looks good to me, @malb

can you approve the CI runs?

@ptrrsn - can one get CI running on mingw ?

@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 23, 2025

@dimpase I added CI for Windows and updated the README. I tested the CI workflows and also fixed a few things that caused errors:

  1. Corrected the CPPFLAGS in "Disallow include<> without m4ri prefix"
  2. Added new commits: "Change > to > in m4 script"

DEFINES =
# include TOPBUILDIR for m4ri_config.h
AM_CFLAGS = -I$(TOPSRCDIR) -I$(TOPBUILDDIR) -D_XOPEN_SOURCE=600 $(DEFINES) @OPENMP_CFLAGS@ @PAPI_CFLAGS@ @LIBPNG_CFLAGS@
AM_CFLAGS = -I$(TOPSRCDIR) -I$(TOPBUILDDIR) -I$(TOPBUILDDIR)/m4ri -D_XOPEN_SOURCE=600 $(DEFINES) @OPENMP_CFLAGS@ @PAPI_CFLAGS@ @LIBPNG_CFLAGS@
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What bombs out without this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before this PR, configure automatically added -I to every subdirectories, which was causing the standard library io.h (only on MinGW Windows) got hidden by m4ri\io.h.

Then, with the addition of nostdinc in configure.ac, we stop this automatic -I additions, manually added them in Makefile.am and tests/Makefile.am strategically.

Specifically, without this, for example test_elimination will break during make distcheck because it indirectly includes misc.h (through testing.h) that has:
#include "config.h", which won't resolve (because it is under m4ri subdirectory).

In retrospect, this probably should also be -iquote similar to the other Makfile.am change to prevent potential future problem. Currently -I works here I think just because there is no indirect call to "#include <io.h>" from within the standard library during tests, so I am changing this to iquote in my new amended commit.

@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 23, 2025

Thank you for your review. I have replied to each and made the two changes I mentioned: one for the -iquote and the other for README, as you suggested.

@malb
Copy link
Copy Markdown
Owner

malb commented Jun 25, 2025

Thanks for doing this, btw

ptrrsn added 8 commits June 26, 2025 04:46
The unsigned long size in gcc on 64-bit Linux is 64-bit while on MinGW
on 64-bit Windows is 32-bit (compiled with MinGW). This not only cause
compilation error but also failing tests on MinGW.
The use of > unintentionally creates a file (interpreted as bash)
instead of comparison. When they are hits, they created files like
x80000004, etc. Instead, \> gives an actual comparison.
MinGW does not support random and srandom, so a fallback to rand and
srand is implemented for those cases.
Without this change, #include<io.h> will include m4ri's io.h, which
hides the io.h from MinGW's standard library.

After this change, there are three ways to include a header (let's name
it `x.h`) from m4ri directory: `#include "x.h"`, `#include <m4ri/x.h>`,
or `#include "m4ri/x.h"`.

This means, `#include<io.h>` will correctly resolve to the standard
library io.h instead of `m4ri/io.h`
This change allow the linker to figure out the dll created for Windows.
This will suppress the warning because MinGW does not support
-no-install.
@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 25, 2025

Thank you for the comments! I have updated the code accordingly.

@malb malb merged commit 238a608 into malb:master Jun 25, 2025
2 checks passed
@ptrrsn
Copy link
Copy Markdown
Contributor Author

ptrrsn commented Jun 25, 2025

Thank you for the reviews!

@ptrrsn ptrrsn deleted the mingw_build branch June 25, 2025 20:08
@ptrrsn ptrrsn restored the mingw_build branch June 25, 2025 20:09
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.

3 participants