Skip to content

Conversation

@jeremyd2019
Copy link
Contributor

Adds a separate job using setup-msys2 to test all currently available MinGW-w64 environments there.

These currently fail a few tests relating to signgam from lgamma. I'm not sure if that's due to it calling mingw-w64's variant of lgamma or what.

@jeremyd2019
Copy link
Contributor Author

lgamma is in libmingwex, one of the default libs. But, it should only be pulled in if the symbol is needed by something, and that shouldn't be if openlibm is providing it.

@jeremyd2019
Copy link
Contributor Author

It could also be that it's pulling in mingw's signgam and openlibm's lgamma. I'll have to investigate it locally to find out exactly what's going on

@jeremyd2019
Copy link
Contributor Author

It could also be that it's pulling in mingw's signgam and openlibm's lgamma. I'll have to investigate it locally to find out exactly what's going on

That seems to be the case, lgamma is exported from openlibm, but signgam is not. Will probably have a patch quickly

it needs to be exported from the DLL also
While there are function export thunks in the import library that allow the linker to do the right thing for functions even if this is not set, it is required for the linker to do the right thing with variables being imported from a DLL (namely, signgam)
@jeremyd2019
Copy link
Contributor Author

couple more missing OLM_DLLEXPORTs it looks like

seems this whole header was overlooked
include openlibm_defs.h for OLM_DLLEXPORT
@jeremyd2019
Copy link
Contributor Author

Finally all green! Sorry for the bit of a mess on this PR. I can squash some commits, and/or split the CI and fixes into separate PRs if preferred.

@jeremyd2019
Copy link
Contributor Author

oh, the i686 clang fix I made in #242 uses is for clang32/clang-i686, which is "extra" experimental in that it's not even in the config files by default. Adding the ability to test that in this CI would require extra special cases just for it. I don't know if that's something you would care to have in your CI, given that it's so experimental upstream.

@ViralBShah
Copy link
Member

Don't worry about the squashing. We can squash and merge.

@ViralBShah
Copy link
Member

ViralBShah commented Sep 9, 2021

I think we don't need to have the experimental support in #242 in CI - just need to make sure it doesn't break anything else.

These CI greens are perfect for that.

@ViralBShah ViralBShah merged commit 60dec83 into JuliaMath:master Sep 9, 2021
@DilumAluthge
Copy link
Member

Thank you @jeremyd2019!

@jeremyd2019 jeremyd2019 deleted the jeremyd2019-msys2-ci branch September 9, 2021 22:54
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