-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
prjtrellis: switch from gcc to clang #8414
Conversation
|
||
case "${CARCH}" in | ||
i686) | ||
sed -s 's|__GNUC__|__clang__|g' -i /mingw32/i686-w64-mingw32/include/intrin.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is totally wrong, you should never modify MSYS2 sysroot inside PKGBUILD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was suggested by @Biswa96. See December 25, 2020 7:03 PM and December 25, 2020 7:24 PM. Nonetheless, you are correct, he said it was for testing only. It was failing back in december, even with that sed replacement. A few days ago, I tested it again and it worked. That is, builds on MINGW64 work regardless of that patch, but on MINGW32 it fails without that.
What alternative do you suggest for fixing the 32 bit build without that patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error should be fixed by https://gitlab.com/mati865/mingw-w64-mirror/-/commit/c1804804aff14f1b0333d782c9fac1cc904fcd9f
BTW what was the reason for using Clang?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, just removing L42-L46 of this PR should work as soon as that patch arrives in MSYS2?
BTW what was the reason for using Clang?
Consistency and testing compatibility. Several projects are built using clang on Linux as the main testing environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, just removing L42-L46 of this PR should work as soon as that patch arrives in MSYS2?
Replying to myself: it seems to work #8466.
In this PR, prjtrellis is built with clang, instead of using gcc. By the way, it's updated.