-
-
Notifications
You must be signed in to change notification settings - Fork 610
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 Issue 23256 - must supply -mscrtlib manually when compiling for W… #14312
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#14312" |
| } | ||
| else | ||
| error(Loc.initial, "must supply `-mscrtlib` manually when cross compiling to windows"); | ||
| } |
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.
I think removing the assignment here is the wrong approach, as driverParams.mscrtlib is also used during compilation from getTargetInfo. I'd rather just use the default libcmt when cross compiling, too.
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.
When cross-compiling, it should IMO clearly default to the bundled MinGW-based libs - libcmt is hardly available on non-Windows systems.
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.
It's added back in further down.
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.
Indeed, native builds are not affected by the change. Cross compilation now silently accepts no runtime being passed, though. Some functionality requires it during compilation, e.g. C++ interop in core.stdcpp.xutility, so using a reasonable default would be better IMO (msvcrt120 for the mingw based libs is fine, too).
BTW: I don't expect cross-linking to work from within dmd as a wine wrapper needs to be used to run the linker (maybe lld could cross link).
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.
lld can definitely cross-link (using the lld-link driver/symlink). So people then only need the MinGW-based libs from the Windows package - well, once the remaining part of https://issues.dlang.org/show_bug.cgi?id=23092 (-fPIC) is fixed as well.
…indows
Only check if doing a link.