-
Notifications
You must be signed in to change notification settings - Fork 5
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
cmake hacks for windows #68
base: master
Are you sure you want to change the base?
Conversation
Based on your changes, I have some updates lined up for ARPA2CM 0.6 and QD to fix this more generally. See also branch python-wrangling in the repo right now. |
moet ; niet worden geescaped? |
cmake/PythonSupport.cmake
Outdated
@@ -11,7 +11,7 @@ | |||
# SPDX-License-Identifier: BSD-2-Clause.degroot | |||
# License-Filename: LICENSES/BSD-2-Clause.degroot | |||
|
|||
if (WIN32) | |||
if("${CMAKE_HOST_SYSTEM}" MATCHES ".*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.
https://cmake.org/cmake/help/v3.13/variable/WIN32.html?highlight=win32
Maar goed, jij checkt host en niet target system. Daar is wat voor te zeggen, in dit geval. Het is meer idiomatisch om if(CMAKE_HOST_SYSTEM MATCHES "Windows")
te schrijven -- variable expansion is implicit, en de match is niet anchored en hoeft niet de hele string te matchen. Dus "ergens komt Windows voor in de host system" hoeft geen sterretjes.
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.
Er .. language settings :) If you specifically want to check the host system (and I guess that makes sense, since that's where the build is happening and you need to run python during the build, on the host) then that can be done more idiomatically as ..
Why would ; need to be escaped, and where? |
cmake/MacroASN1Module.cmake
Outdated
@@ -119,7 +119,7 @@ endmacro() | |||
|
|||
macro(add_asn1_document _docname _groupname) | |||
add_custom_command (OUTPUT doc/${_docname}.md | |||
COMMAND ${CMAKE_SOURCE_DIR}/python/scripts/asn1literate ${CMAKE_CURRENT_SOURCE_DIR}/${_docname}.asn1 ${_docname}.md | |||
COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_SOURCE_DIR}/python/scripts/asn1literate ${CMAKE_CURRENT_SOURCE_DIR}/${_docname}.asn1 ${_docname}.md |
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 a straight-up bug in the module, and doesn't need to be hidden in a hacks-branch
cmake/MacroASN1Module.cmake:
lib/CMakeLists.txt
On windows shared libraries need ARCHIVE DESTINATION: https://cmake.org/pipermail/cmake/2008-September/023876.html