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

Add CI jobs for MinGW32/64 #322

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Add CI jobs for MinGW32/64 #322

merged 4 commits into from
Aug 26, 2024

Conversation

Flamefire
Copy link
Contributor

Add jobs using MSYS2 MinGW to GHA

Config copied from boost-ci

This is the configuration where I see #321 although it seems to be only triggered by -Wextra not -Wall: https://godbolt.org/z/3fqjG7zM3

@Lastique
Copy link
Member

  • C++03 is not supported
  • no actions/checkout, please. It causes too much trouble.
  • no boost-ci, please. I would like to know what and how I'm testing.
  • There already is a MinGW-w64 job, why do we need another one?
  • I'm not sure what MINGW32 is, but if this is the legacy MinGW32 (as opposed to MinGW-w64) then it is no longer supported. I think, it's been dropped by Boost.System as well.

@Flamefire
Copy link
Contributor Author

  • C++03 is not supported

So the build should be skipped by

filesystem/build/Jamfile.v2

Lines 126 to 137 in 5dc58be

local cxx_requirements = [ requires
cxx11_rvalue_references
cxx11_scoped_enums
cxx11_noexcept
cxx11_nullptr
cxx11_defaulted_functions
cxx11_defaulted_moves
cxx11_deleted_functions
cxx11_function_template_default_args
cxx11_final
cxx11_override
] ;

If you consider it not worth ensuring this skipping works in this specific configuration I can remove that.

* no `actions/checkout`, please. It causes too much trouble.

* no `boost-ci`, please. I would like to know what and how I'm testing.

I can C&P the steps from the posix jobs if you prefer that. I used boost-ci to reduce repetition

* There already is a [MinGW-w64 job](https://github.com/boostorg/filesystem/blob/5dc58be3cddaa00497f9fd95deb8766bb91e2f3a/.github/workflows/ci.yml#L563-L565), why do we need another one?

* I'm not sure what MINGW32 is, but if this is the legacy MinGW32 (as opposed to MinGW-w64) then it is no longer supported. I think, it's been dropped by Boost.System as well.

The ones added here use the MSYS2 MinGW-w64 distros for 32 & 64 bit. On a Windows machine those are called "MSYS2 MinGW x86" and "MSYS2 MinGW x64" respectively. The reason for using MSYS2 is (to quote from their website):

MSYS2 provides up-to-date native builds for GCC, mingw-w64 [...]

--> The currently tested GCC is GCC-8, MSYS2 uses GCC-13. So this adds testing for recent GCCs on Windows

@Lastique
Copy link
Member

If you consider it not worth ensuring this skipping works in this specific configuration I can remove that.

I see no point in running a C++03 CI build when it is not expected to work (whether with an error or silently skip the build is not important).

I can C&P the steps from the posix jobs if you prefer that.

Yes, that would be ok, if those steps work on MSYS2.

The ones added here use the MSYS2 MinGW-w64 distros for 32 & 64 bit.

Ok, but the 32-bit build fails as if its Windows SDK headers are incomplete, which makes me suspect it's MinGW32 rather than a 32-bit target of MinGW-w64.

32 bit version of MinGW-64 __MINGW64__ is not defined
@Flamefire
Copy link
Contributor Author

I see no point in running a C++03 CI build when it is not expected to work (whether with an error or silently skip the build is not important).

The intention was to verify the skipping works, i.e. the linked code does what it is supposed to. But I removed the jobs.

Ok, but the 32-bit build fails as if its Windows SDK headers are incomplete, which makes me suspect it's MinGW32 rather than a 32-bit target of MinGW-w64.

The 32-bit MinGW-w64 does not define __MINGW64__ hence the check was to restrictive. Googling around I don't see a comparable check used anywhere but I did find a comment in CMake that I copied here: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Source/kwsys/SystemTools.cxx#L121-122

I.e. the struct is only accessible to drivers and there wasn't a configuration tested yet that relied on it to be defined by a Windows SDK header

@Flamefire
Copy link
Contributor Author

CI is passing now in all configs after unconditionally declaring the struct only available to drivers. Anything else?

@Lastique Lastique merged commit 35f348a into boostorg:develop Aug 26, 2024
46 checks passed
@Lastique
Copy link
Member

Thank you.

@Flamefire Flamefire deleted the mingw branch August 26, 2024 11:14
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.

2 participants