search

dannyedel / dspdfviewer

Dual-Screen PDF Viewer for latex-beamer
http://dspdfviewer.danny-edel.de
GNU General Public License v2.0
218 stars 27 forks source link

MSVC compiler switch and /MD-/MT replacement options #122

Closed projekter closed 8 years ago

dannyedel commented 8 years ago

Sorry about the ambiguous comment, but:

Is this okay to merge as-is (see comments above)? I can't test anything MSVC-related, obviously : )

projekter commented 8 years ago

This PR works, yes. I removed the conditional. But I don't know what you mean with ${LIST_INCLUDE_DIRS}. I put all the includes in external_libraries.cmake (but I can move those to the MSVC specific file), as they are related to external libraries, but not to MSVC particularly - you would need those for any static build (that's also why I hesitated putting this MT replacement in the repository - it is only necessary with this specific choice of compiling the external libraries.

dannyedel commented 8 years ago

I put all the includes in external_libraries.cmake

Ah okay, then it's fine by me. It's already hidden behind an if (MSVC) inside there, so it can stay as-is.

I was just a bit surprised that it worked because you set (in external_libraries) the directories to the LIST_INCLUDE_DIRS variable, but you never take its value (to pass it to the compiler). I guess MSVC can automatically find it if it knows about the .lib file.

I will merge this as-is - we can always change it later.

Right now, "your way" is the only way on Windows, so until we find a nice way to handle the build-deps, I'd rather have it all in the upstream repo, keeping the diff to yours small (or zero) and allowing us to actually exchange code instead of build-system-foo.