这是indexloc提供的服务，不要输入任何密码
Skip to content

Windows native support using MSVC - Visual Studio 2017 #867

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jul 14, 2017
Merged

Windows native support using MSVC - Visual Studio 2017 #867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
53e6bb3
gitignore: Windows build files
tusharpm Jul 9, 2017
102a823
buildsystem: Fix paths and usage
tusharpm Jul 9, 2017
fdc0066
guisys: Use #582 with modifications
tusharpm Jun 30, 2017
faae03b
libopenage: Fixes for MSVC
tusharpm Jul 9, 2017
2146930
openage: HACK: path usage
tusharpm Jul 9, 2017
6b85e34
doc: instructions for Windows MSVC
tusharpm Jul 7, 2017
d7c4c40
Add name to copying.md
tusharpm Jul 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
*.pyc
*.pyo
__pycache__
*.pyd

# MSVC files
*.pdb
*.dll
*.exe

# workflow
*.orig
Expand All @@ -22,6 +28,8 @@ __pycache__
# build system
/bin
/.bin
/build
/deps

# root dir run script
/run
Expand Down
7 changes: 6 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,12 @@ add_subdirectory(libopenage/)

# Python content (uses the C++ library)
set(PYEXT_LINK_LIBRARY libopenage)
set(PYEXT_CXXFLAGS "${PYEXT_CXXFLAGS} -include \"${CMAKE_SOURCE_DIR}/libopenage/pyinterface/hacks.h\"")
if(MSVC)
set(FORCE_INCLUDE_CXXFLAG "/FI")
else()
set(FORCE_INCLUDE_CXXFLAG "-include ")
endif()
set(PYEXT_CXXFLAGS "${PYEXT_CXXFLAGS} ${FORCE_INCLUDE_CXXFLAG}\"${CMAKE_SOURCE_DIR}/libopenage/pyinterface/hacks.h\"")
add_cython_modules(EMBED NOINSTALL run.py)
add_py_modules(BININSTALL run.py AS openage)
add_subdirectory(openage/)
Expand Down
4 changes: 3 additions & 1 deletion buildsystem/HandleCXXOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ endmacro()
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(EXTRA_FLAGS "${EXTRA_FLAGS} -Wall -Wextra -pedantic")
if(NOT MSVC)
set(EXTRA_FLAGS "${EXTRA_FLAGS} -Wall -Wextra -pedantic")
endif()

# check for compiler versions
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
Expand Down
4 changes: 2 additions & 2 deletions buildsystem/HandlePythonOptions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
set(PYEXT_CXXFLAGS "${PYEXT_CXXFLAGS} -Wno-#warnings")
endif()

set(PYEXT_LIBRARY "${PYTHON_LIBRARY}")
set(PYEXT_INCLUDE_DIRS "${PYTHON_INCLUDE_DIR};${NUMPY_INCLUDE_DIR}")
set(PYEXT_LIBRARY "${PYTHON_LIBRARIES}")
set(PYEXT_INCLUDE_DIRS "${PYTHON_INCLUDE_DIRS};${NUMPY_INCLUDE_DIR}")

if(NOT CMAKE_PY_INSTALL_PREFIX)
py_exec("import site, os; print(os.path.normpath(site.getsitepackages()[0]))" PREFIX)
Expand Down
2 changes: 1 addition & 1 deletion buildsystem/check_py_file_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def main():
args = cli.parse_args()

with open(args.py_file_list) as fileobj:
listed = set(fileobj.read().strip().split(';'))
listed = set(os.path.normpath(filepath) for filepath in fileobj.read().strip().split(';'))
if listed == {''}:
listed = set()

Expand Down
3 changes: 2 additions & 1 deletion buildsystem/cpp.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ function(add_sources binary_name)
if(NOT IS_ABSOLUTE "${source}")
set(source "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
endif()
file(TO_CMAKE_PATH "${source}" source)

set_property(
GLOBAL APPEND PROPERTY
Expand Down Expand Up @@ -88,7 +89,7 @@ function(finalize_binary target_name output_name type)
message(FATAL_ERROR "finalize_binary flag 'allow_no_undefined' is only valid for libraries!")
endif()

if(NOT "${CMAKE_CXX_FLAGS}" MATCHES "-fsanitize")
if(NOT "${CMAKE_CXX_FLAGS}" MATCHES "-fsanitize" AND NOT MSVC)
Copy link
Member

@TheJJ TheJJ Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so on windows undefined symbols in the libopenage would not throw errors, right? can we somehow display those errors with msvc?

Copy link
Member Author

@tusharpm tusharpm Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. Undefined symbols would result in link-time errors.
Can you please elaborate what errors we're looking for?

Copy link
Member

@TheJJ TheJJ Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undefined symbols are only discovered at link-time for executables, not for libraries like libopenage. Those cflags ensure that errors are shown nevertheless. I guess msvc has something that does the same.

Copy link
Member Author

@tusharpm tusharpm Jul 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm or deny if libopenage is actually a shared library (possibly named libopenage.so or libopenage.dylib on other platforms)? If it is a shared library then the linker should complain about any missing symbols. AFAIK VS doesn't have an equivalent of -undefined dynamic_lookup, so no worries there. Otherwise, if it is a static library (like libopenage.a), I'll have to search in the MSVC docs for a flag which can do that, if one exists.

Copy link
Member

@TheJJ TheJJ Jul 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openage is and has to be a dynamic library so python can load it (import libopenage). This can never be done for a static library.

On Linux it did not complain, which is why we added those options. Hmm.

if(APPLE)
set(link_flag "-undefined,error")
else()
Expand Down
2 changes: 1 addition & 1 deletion buildsystem/cythonize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def read_list_from_file(filename):
with open(filename) as fileobj:
data = fileobj.read().strip()

data = data.split(';')
data = [os.path.normpath(filename) for filename in data.split(';')]
if data == ['']:
return []

Expand Down
8 changes: 7 additions & 1 deletion buildsystem/inplacemodules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2015-2016 the openage authors. See copying.md for legal info.
# Copyright 2015-2017 the openage authors. See copying.md for legal info.

"""
Installs the Python extension modules that were created in the build directory
Expand All @@ -21,6 +21,9 @@ def main():
cli.add_argument("binary_dir", help=(
"the build directory where those files will be found."
))
cli.add_argument("configuration", help=(
"the build configuration like Debug or Release"
))
cli.add_argument("--clean", action="store_true", help=(
"remove instead of creating"
))
Expand All @@ -39,6 +42,9 @@ def main():
sourcefile = module
targetfile = os.path.relpath(module, args.binary_dir)

# If `targetfile` has a configuration component, remove it.
targetfile = os.path.normpath(targetfile.replace(args.configuration, '.'))

if os.path.exists(targetfile):
if args.clean:
print(targetfile)
Expand Down
57 changes: 26 additions & 31 deletions buildsystem/modules/FindPython.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#
# PYTHON_FOUND - True when python was found.
# PYTHON - The full path to the Python interpreter.
# PYTHON_INCLUDE_DIR - Include path for Python extensions.
# PYTHON_LIBRARY - Library and Linker options for Python extensions.
# PYTHON_INCLUDE_DIRS - Include path for Python extensions.
# PYTHON_LIBRARIES - Library and Linker options for Python extensions.
#
# Also defines py_exec and py_get_config_var.
#
Expand Down Expand Up @@ -54,21 +54,6 @@ function(py_get_config_var VAR RESULTVAR)
set("${RESULTVAR}" "${RESULT}" PARENT_SCOPE)
endfunction()

function(py_get_lib_name RESULTVAR)
# uses py_exec to compute Python's C/C++ library name, just like python-config does.
py_get_config_var(VERSION PYTHON_VERSION)
if(NOT "${PYTHON_VERSION}" VERSION_LESS "3.2")
py_exec(
"import sys; print(sys.abiflags)"
ABIFLAGS
)
else()
set(ABIFLAGS, "")
endif()

set("${RESULTVAR}" "python${PYTHON_VERSION}${ABIFLAGS}" PARENT_SCOPE)
endfunction()

function(find_python_interpreter_builtin)
find_package(PythonInterp "${PYTHON_MIN_VERSION}" QUIET)
if(PYTHONINTERP_FOUND)
Expand Down Expand Up @@ -170,19 +155,30 @@ endforeach()
# test all the found interpreters; break on success.
foreach(PYTHON ${PYTHON_INTERPRETERS})

# ask the interpreter for the essential extension-building flags
py_get_config_var(INCLUDEPY PYTHON_INCLUDE_DIR)
py_get_config_var(LIBDIR PYTHON_LIBRARY_DIR)
py_get_lib_name(PYTHON_LIBRARY_NAME)
# If the current python interpreter equals the one found at the very beginning with PythonInterp,
# we can use the PythonLibs find-module to find the matching libraries.
# Otherwise we ask that interpreter where its matching libraries are.
if (PYTHON STREQUAL PYTHON_EXECUTABLE)
find_package(PythonLibs REQUIRED)
else()
# ask the interpreter for the essential extension-building flags
py_get_config_var(INCLUDEPY PYTHON_INCLUDE_DIRS)
py_get_config_var(LIBDIR PYTHON_LIBRARY_DIR)
py_get_config_var(LIBPL PYTHON_LIBPL)
py_get_config_var(LDLIBRARY PYTHON_LIBRARY_NAME)
find_library(PYTHON_LIBRARIES ${PYTHON_LIBRARY_NAME}
PATHS "${PYTHON_LIBRARY_DIR}" "${PYTHON_LIBPL}"
)
endif()

# there's a static_assert that tests the Python version.
# that way, we verify the interpreter and the library version.
# (the interpreter provided us the library location)
try_compile(PYTHON_TEST_RESULT
"${CMAKE_BINARY_DIR}"
SOURCES "${CMAKE_CURRENT_LIST_DIR}/FindPython_test.cpp"
LINK_LIBRARIES "${PYTHON_LIBRARY_NAME}"
CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${PYTHON_INCLUDE_DIR}" "-DLINK_DIRECTORIES=${PYTHON_LIBRARY_DIR}" "-DCMAKE_CXX_STANDARD=14"
LINK_LIBRARIES ${PYTHON_LIBRARIES}
CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${PYTHON_INCLUDE_DIRS}" "-DCMAKE_CXX_STANDARD=14"
Copy link
Member

@TheJJ TheJJ Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need the LINK_DIRECTORIES? If the library is in a non-standard path then the lib won't be found. I think that was a problem on mac then.

Copy link
Member Author

@tusharpm tusharpm Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable PYTHON_LIBRARIES has the full path of the libraries (actually just one library per configuration) that need to be linked. I made (rather, tried to make) the variable consistent for other ways to locate the Python interpreter and libraries. I must've missed something that made kevin fail.

COMPILE_DEFINITIONS "-DTARGET_VERSION=${PYTHON_MIN_VERSION_HEX}"
OUTPUT_VARIABLE PYTHON_TEST_OUTPUT
)
Expand All @@ -191,7 +187,6 @@ foreach(PYTHON ${PYTHON_INTERPRETERS})
message("-- Using python interpreter: ${PYTHON}")

set(PYTHON_INTERP "${PYTHON}")
set(PYTHON_LIBRARY "-l${PYTHON_LIBRARY_NAME} -L${PYTHON_LIBRARY_DIR}")
break()
else()
set(PYTHON_TEST_ERRORS "${PYTHON_TEST_ERRORS}python candidate ${PYTHON}:\n${PYTHON_TEST_OUTPUT}\n\n")
Expand All @@ -207,12 +202,12 @@ if(NOT PYTHON_INTERP)
message("We need an interpreter that is shipped with libpython and header files.")
message("Specify your own with -DPYTHON=/path/to/executable\n\n\n")
set(PYTHON_INTERP "")
set(PYTHON_INCLUDE_DIR "")
set(PYTHON_LIBRARY "")
set(PYTHON_INCLUDE_DIRS "")
set(PYTHON_LIBRARIES "")

unset(PYTHON CACHE)
unset(PYTHON_LIBRARY CACHE)
unset(PYTHON_INCLUDE_DIR CACHE)
unset(PYTHON_LIBRARIES CACHE)
unset(PYTHON_INCLUDE_DIRS CACHE)
endif()

if(NOT PYTHON)
Expand All @@ -221,8 +216,8 @@ if(NOT PYTHON)
endif()

set(PYTHON "${PYTHON_INTERP}" CACHE FILEPATH "Location of the Python interpreter" ${LOL_FORCE})
set(PYTHON_LIBRARY "${PYTHON_LIBRARY}" CACHE STRING "Linker invocation for the Python library" ${LOL_FORCE})
set(PYTHON_INCLUDE_DIR "${PYTHON_INCLUDE_DIR}" CACHE PATH "Location of the Python include dir" ${LOL_FORCE})
set(PYTHON_LIBRARIES "${PYTHON_LIBRARIES}" CACHE STRING "Linker invocation for the Python library" ${LOL_FORCE})
set(PYTHON_INCLUDE_DIRS "${PYTHON_INCLUDE_DIRS}" CACHE PATH "Location of the Python include dir" ${LOL_FORCE})


unset(LOL_FORCE)
Expand All @@ -233,4 +228,4 @@ mark_as_advanced(PYTHON_TEST_ERRORS)
mark_as_advanced(PYTHON_INTERPRETERS)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Python REQUIRED_VARS PYTHON PYTHON_INCLUDE_DIR PYTHON_LIBRARY)
find_package_handle_standard_args(Python REQUIRED_VARS PYTHON PYTHON_INCLUDE_DIRS PYTHON_LIBRARIES)
4 changes: 2 additions & 2 deletions buildsystem/modules/FindSDL2Image.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2014-2015 the openage authors. See copying.md for legal info.
# Copyright 2014-2017 the openage authors. See copying.md for legal info.

find_package(PackageHandleStandardArgs)

Expand All @@ -24,6 +24,6 @@ else()
find_path(SDL2IMAGE_INCLUDE_DIRS SDL2/SDL_image.h DOC "Include directory for SDL2_image")
endif()

# handle the QUIETLY and REQUIRED arguments and set OPENGL_FOUND to TRUE if
# handle the QUIETLY and REQUIRED arguments and set SDL2Image_FOUND to TRUE if
# all listed variables are TRUE
find_package_handle_standard_args(SDL2Image DEFAULT_MSG SDL2IMAGE_LIBRARIES)
4 changes: 2 additions & 2 deletions buildsystem/pxdgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def get_pxd_lines(self):
if namespace != previous_namespace:
yield (
"cdef extern "
"from \"" + self.filename + "\" "
"from r\"" + self.filename + "\" "
"namespace \"" + namespace + "\" "
"nogil"
":"
Expand Down Expand Up @@ -403,7 +403,7 @@ def parse_args():
def main():
""" CLI entry point """
args = parse_args()
cppdir = CWD + "/libopenage"
cppdir = os.path.join(CWD, "libopenage")

if args.verbose:
hdr_count = len(args.all_files)
Expand Down
17 changes: 10 additions & 7 deletions buildsystem/python.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function(add_cython_modules)
add_executable("${TARGETNAME}" "${CPPNAME}")

# TODO: use full ldflags and cflags provided by python${VERSION}-config
target_link_libraries("${TARGETNAME}" "${PYEXT_LIBRARY}")
target_link_libraries("${TARGETNAME}" ${PYEXT_LIBRARY})
else()
set_property(GLOBAL APPEND PROPERTY SFT_CYTHON_MODULES "${source}")
add_library("${TARGETNAME}" MODULE "${CPPNAME}")
Expand All @@ -113,6 +113,9 @@ function(add_cython_modules)
PREFIX ""
SUFFIX "${PYEXT_SUFFIX}"
)
if(MSVC)
target_link_libraries("${TARGETNAME}" ${PYEXT_LIBRARY})
endif()
endif()

if(NOINSTALL_NEXT)
Expand Down Expand Up @@ -410,16 +413,16 @@ function(python_finalize)
list(APPEND cython_module_files_expr "$<TARGET_FILE:${cython_module_target}>")
endforeach()
file(GENERATE
OUTPUT "${CMAKE_BINARY_DIR}/py/inplace_module_list"
OUTPUT "${CMAKE_BINARY_DIR}/py/inplace_module_list$<CONFIG>"
CONTENT "${cython_module_files_expr}"
)
set(INPLACEMODULES_TIMEFILE "${CMAKE_BINARY_DIR}/py/inplacemodules_timefile")
add_custom_command(OUTPUT "${INPLACEMODULES_TIMEFILE}"
COMMAND "${PYTHON}" -m buildsystem.inplacemodules
"${CMAKE_BINARY_DIR}/py/inplace_module_list"
"${CMAKE_BINARY_DIR}"
"${CMAKE_BINARY_DIR}/py/inplace_module_list$<CONFIG>"
"${CMAKE_BINARY_DIR}" "$<CONFIG>"
DEPENDS
"${CMAKE_BINARY_DIR}/py/inplace_module_list"
"${CMAKE_BINARY_DIR}/py/inplace_module_list$<CONFIG>"
${cython_module_targets}
COMMAND "${CMAKE_COMMAND}" -E touch "${INPLACEMODULES_TIMEFILE}"
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
Expand All @@ -432,8 +435,8 @@ function(python_finalize)

add_custom_target(cleancython
COMMAND "${PYTHON}" -m buildsystem.inplacemodules --clean
"${CMAKE_BINARY_DIR}/py/inplace_module_list"
"${CMAKE_BINARY_DIR}"
"${CMAKE_BINARY_DIR}/py/inplace_module_list$<CONFIG>"
"${CMAKE_BINARY_DIR}" "$<CONFIG>"
COMMAND "${PYTHON}" -m buildsystem.cythonize --clean
"${CMAKE_BINARY_DIR}/py/cython_modules"
"${CMAKE_BINARY_DIR}/py/cython_modules_embed"
Expand Down
1 change: 1 addition & 0 deletions copying.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ _the openage authors_ are:
| Folkert van Verseveld | methos, medicijnman | folkert dawt van dawt verseveld à gmail dawt com |
| Neel Patel | ohn0 | silverskinx à gmail dawt com |
| David Carlier | devnexen | devnexen à gmail dawt com |
| Tushar Maheshwari | tusharpm | tushar27192 à gmail dawt com |

If you're a first-time committer, add yourself to the above list. This is not
just for legal reasons, but also to keep an overview of all those nicknames.
Expand Down
Loading