GabeRundlett / gvox_engine

MIT License
325 stars 19 forks source link

changed formatting of linker options given in CMake for MSVC #4

Closed a-day-old-bagel closed 1 year ago

a-day-old-bagel commented 1 year ago

Previously, in Windows, some linker options in CMakeLists.txt were provided like this:

target_link_options(${PROJECT_NAME} PRIVATE "-Wl,/ENTRY:mainCRTStartup,/SUBSYSTEM:WINDOWS")

I believe this sends the entire string inside those quotes to the linker to be interpreted as a single argument, and I was getting the error back from MSVC that it was an unrecognized option (because it was treating the whole clump as a single option.)

Removing the quotes and the commas fixed the issue.

GabeRundlett commented 1 year ago

This is actually how the option syntax should be, when using clang on windows (which is what I mainly do). However, you're correct that if you want to compile with MSVC, you need to change these options. For MSVC, you would not only remove the commas, but also remove the -wl so they are separate args. For me to merge this, you can either add an if clause based on the compiler, or I can do it. I just previously wasn't really supporting MSVC

a-day-old-bagel commented 1 year ago

Yeah I wasn't quite sure if these options also got interpreted by clang or not, which I wanted to ask about. I suspected that an immediate merge wouldn't be what you wanted to do, but did a PR anyway to bring it up.

I'll make the edit and test with both clang and msvc if you haven't already done it.

GabeRundlett commented 1 year ago

(should be) fixed in 9b338989f1ee7ea0587af2c7a16cae5ea6099a96, closing