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

Fix broken cmake support for MinGW32 #230

Merged
merged 4 commits into from
Jun 8, 2022
Merged

Conversation

daschuer
Copy link
Contributor

@daschuer daschuer commented May 6, 2021

This fixes building with mingw32 8.1

Unfortunately wx fails with g++ 8.1 due to a wrong cast.
This was fixed here:
wxWidgets/wxWidgets@424f64f27
I can build after using the patch.

@vslavik
Copy link
Owner

vslavik commented May 9, 2021

Sorry, I don't understand why does the commit titled "Update CMake support for MinGW32 8.1" do many things, none of which seem to be related to a particular MinGW version?

It's unclear to me why you add a generated config header to the repository when expat doesn't need it to be built on Windows (see: MSVS build projects).

@daschuer
Copy link
Contributor Author

daschuer commented May 9, 2021

The issue is that the cmake build in master are completely broken.

I have fist started ein MinGW 6.3, but the shipped header files do not contain all definitions required. The update to the recent MinGW 8.1 fixes the issue.

I did not notice that expat is not required. I just make it work, because it was part of the original cmake files.

When is expat required?

What needs to be done here, to put that int a mergable state?

@vslavik
Copy link
Owner

vslavik commented May 29, 2021

The issue is that the cmake build in master are completely broken.

Then that's what the commit message should say...

I did not notice that expat is not required.

It is required. Building it in this crazy-complicated way is not. Again: see the VC++ build, which does not require a generated header you added. Ergo, evidently you don't need it either. Don't add it.

What needs to be done here, to put that int a mergable state?

See above.

@daschuer daschuer changed the title Fix cmake Fix broken cmake support for MinGW32 May 31, 2021
@daschuer
Copy link
Contributor Author

Done.

Are the commit messages OK now?

Did you consider how to proceed with the pending wx build error?

@daschuer
Copy link
Contributor Author

AppVeyor succeeds now

@vslavik vslavik merged commit db98099 into vslavik:master Jun 8, 2022
@vslavik
Copy link
Owner

vslavik commented Jun 8, 2022

Thanks!

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