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 link flag behaviour on Windows MSVC #11424

Merged
merged 1 commit into from
Nov 15, 2021

Conversation

HertzDevil
Copy link
Contributor

Currently, link flags passed through --link-flags on MSVC are not considered as such, because CL treats them as compiler options:

> crystal build test.cr --verbose --link-flags "/PDB:test.pdb"
cl.exe /nologo _main.obj E-xception5858C-allS-tack.obj ... /FeC:\Users\Quinton\crystal-win\test  pcre.lib gc.lib advapi32.lib WS2_32.lib legacy_stdio_definitions.lib libcmt.lib /PDB:test.pdb
cl : Command line warning D9002 : ignoring unknown option '/PDB:test.pdb'

This PR adds a missing /link command-line argument, and ensures all the arguments are on the same line to fix this (when the arguments are placed into linker_args.txt, for some reason /link stops behaving after a newline).

It also adds support for the CRYSTAL_LIBRARY_PATH environment variable, via the /LIBPATH linker flag.

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API topic:compiler:codegen labels Nov 9, 2021
@straight-shoota straight-shoota added this to the 1.3.0 milestone Nov 11, 2021
@straight-shoota straight-shoota merged commit fa9e3d9 into crystal-lang:master Nov 15, 2021
@HertzDevil HertzDevil deleted the bug/msvc-link-flags branch November 29, 2021 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API topic:compiler:codegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants