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: [Actions] msys2/MinGW CI #8698

Merged
merged 1 commit into from Jun 10, 2021
Merged

Add: [Actions] msys2/MinGW CI #8698

merged 1 commit into from Jun 10, 2021

Conversation

glx22
Copy link
Contributor

@glx22 glx22 commented Feb 19, 2021

Motivation / Problem

Sometime we inadvertently break compilation with MinGW, and it can be unnoticed for a long time (until me or @LordAro happen to try to build with MinGW).

Description

I added MinGW to the CI build (for 32bit and 64bit).

Limitations

32bit currently fail, but it's not caused by this PR ;)

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@michicc
Copy link
Member

michicc commented Feb 19, 2021

Will compiling both 32-bit and 64-bit give us much extra info, or would one of them be enough for 99% of the good test? Doing just one could make sure CI run time doesn't get too long (applies to MSVC 2017 as well).

@glx22
Copy link
Contributor Author

glx22 commented Feb 19, 2021

Sometimes 32-bit fails while 64-bit passes. It was the case here, I needed 3 extra commits to build 32-bit.

@TrueBrain
Copy link
Member

Wow, takes 13 minutes to run a MingW build .. what is taking so long for this target?

Anyway, still more warnings to fix before we can merge this: https://github.com/OpenTTD/OpenTTD/actions/runs/582953728 :)

@LordAro
Copy link
Member

LordAro commented Feb 20, 2021

I've never been able to work out how to fix the format & function-cast warnings. I suspect the best solution is just to disable them

@glx22
Copy link
Contributor Author

glx22 commented Feb 20, 2021

Yeah I tried many things for these warnings, but nothing worked.

@michicc
Copy link
Member

michicc commented Feb 20, 2021

The warning on src/3rdparty/squirrel/squirrel/sqfuncstate.cpp#L88 seems to imply that OTTD_PRINTF64 is not defined properly, which would be an actual problem.

@glx22
Copy link
Contributor Author

glx22 commented Feb 20, 2021

But it's defined properly and it's the only place we get a warning.

@TrueBrain
Copy link
Member

Otherwise maybe you can PR the fixes for mingw from this PR, so we can at least accept that?

Adding CI targets that generate warnings isn't that nice, especially as we are currently completely clear of any :) So I guess that would take some more effort to fix/change .. but that doesn't mean we shouldn't already accept the fixes that do work :D

@glx22 glx22 merged commit acb6348 into OpenTTD:master Jun 10, 2021
@glx22 glx22 deleted the mingw branch June 10, 2021 21:55
@TrueBrain
Copy link
Member

Is it just me, or is the caching not really working? Every run it takes 90s to setup MSYS2, despite it saying it loaded it from cache. Bit odd, I think?

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.

None yet

4 participants