updates and fixes for windows build #884
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| cmake_minimum_required(VERSION 3.7.2) | ||
| project(psp) | ||
|
|
||
| set(CMAKE_BUILD_TYPE "Release") | ||
|
|
||
|
|
||
| set(CMAKE_CXX_STANDARD 14) | ||
| set(CMAKE_EXPORT_COMPILE_COMMANDS ON) | ||
|
|
@@ -197,6 +196,10 @@ message(WARNING "${BUILD_MESSAGE}") | |
| ####################### | ||
| include_directories("${CMAKE_SOURCE_DIR}/src/include") | ||
|
|
||
| if(NOT WIN32) | ||
| set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG") | ||
| set(CMAKE_C_FLAGS_RELEASE "-O3 -DNDEBUG") | ||
| endif() | ||
|
|
||
| if (PSP_WASM_BUILD) | ||
| #################### | ||
|
|
@@ -264,16 +267,31 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD) | |
| ##################### | ||
| # VANILLA CPP BUILD # | ||
| ##################### | ||
| if(WIN32) | ||
| if(CMAKE_BUILD_TYPE_LOWER STREQUAL debug) | ||
| set(OPT_FLAGS " \ | ||
|
There was a problem hiding this comment. @sc1f we should set debug flags (see prev comment) instead of having this opt flags thing |
||
| /DEBUG \ | ||
| /Z7 \ | ||
| /Zi | ||
| ") | ||
| else() | ||
| set(OPT_FLAGS " \ | ||
| /NDEBUG \ | ||
| /O2 \ | ||
| ") | ||
| endif() | ||
| else() | ||
| if(CMAKE_BUILD_TYPE_LOWER STREQUAL debug) | ||
| set(OPT_FLAGS " \ | ||
| -O0 \ | ||
| -g3 \ | ||
| ") | ||
| else() | ||
| set(OPT_FLAGS " \ | ||
| -O3 \ | ||
| -g0 \ | ||
| ") | ||
| endif() | ||
| endif() | ||
| set(SYNC_MODE_FLAGS "") | ||
| set(ASYNC_MODE_FLAGS "") | ||
|
|
@@ -377,9 +395,7 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD) | |
| message(FATAL_ERROR "${Red}PyArrow could not be located${ColorReset}") | ||
| else() | ||
| message(WARNING "${Cyan}PyArrow found: ${PYTHON_PYARROW_INCLUDE_DIR}${ColorReset}") | ||
| message(WARNING "${Cyan}Using pre-built ${PYTHON_PYARROW_PYTHON_SHARED_LIBRARY} ${PYTHON_PYARROW_ARROW_SHARED_LIBRARY} from: ${PYTHON_PYARROW_LIBRARY_DIR}${ColorReset}") | ||
| endif() | ||
| include_directories(${PYTHON_PYARROW_INCLUDE_DIR}) | ||
| link_directories(${PYTHON_PYARROW_LIBRARY_DIR}) | ||
|
|
@@ -392,7 +408,7 @@ psp_build_dep("hopscotch" "${PSP_CMAKE_MODULE_PATH}/hopscotch.txt.in") | |
| psp_build_dep("ordered-map" "${PSP_CMAKE_MODULE_PATH}/ordered-map.txt.in") | ||
|
|
||
| # For all non-MacOS and non-linux builds, or if building WASM/CPP, build minimal arrow from source | ||
| if (NOT PSP_PYTHON_BUILD) | ||
| # build arrow + dependencies from source for Emscripten and C++ | ||
| message("${Cyan}Building minimal Apache Arrow${ColorReset}") | ||
|
|
||
|
|
@@ -516,7 +532,7 @@ set (PYTHON_SOURCE_FILES | |
| ) | ||
|
|
||
| if (WIN32) | ||
| set(CMAKE_CXX_FLAGS " /EHsc /MP") | ||
| else() | ||
| set(CMAKE_CXX_FLAGS " ${CMAKE_CXX_FLAGS} ${CMAKE_C_FLAGS}") | ||
| endif() | ||
|
|
@@ -548,7 +564,9 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD) | |
| set(module_install_rpath "@loader_path/") | ||
| else() | ||
| set(module_install_rpath "\$ORIGIN") | ||
| endif() | ||
| else() | ||
| set(CMAKE_SHARED_LIBRARY_PREFIX lib) | ||
|
There was a problem hiding this comment. @sc1f we should consider not prefixing with lib, and setting the prefixes to "" (this will mean we need to change python imports from |
||
| endif() | ||
|
|
||
| if(PSP_PYTHON_BUILD) | ||
|
|
@@ -562,28 +580,45 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD) | |
|
|
||
| target_compile_definitions(psp PRIVATE PSP_ENABLE_PYTHON=1) | ||
| target_compile_definitions(binding PRIVATE PSP_ENABLE_PYTHON=1) | ||
|
|
||
| if (WIN32) | ||
| target_compile_definitions(binding PRIVATE WIN32=1) | ||
| target_compile_definitions(binding PRIVATE _WIN32=1) | ||
|
|
||
| target_link_libraries(psp ${Boost_FILESYSTEM_LIBRARY}) | ||
| target_link_libraries(binding ${Boost_FILESYSTEM_LIBRARY}) | ||
|
|
||
| # .dll not importable | ||
| set_property(TARGET binding PROPERTY SUFFIX .pyd) | ||
| else() | ||
| target_compile_options(binding PRIVATE -Wdeprecated-declarations) | ||
| set_property(TARGET psp PROPERTY INSTALL_RPATH ${CMAKE_INSTALL_RPATH} ${module_install_rpath} ${PYTHON_PYARROW_LIBRARY_DIR}) | ||
| set_property(TARGET binding PROPERTY INSTALL_RPATH ${CMAKE_INSTALL_RPATH} ${module_install_rpath} ${PYTHON_PYARROW_LIBRARY_DIR}) | ||
| endif() | ||
|
|
||
| target_link_libraries(psp ${PYTHON_PYARROW_LIBRARIES}) | ||
| target_link_libraries(binding ${PYTHON_PYARROW_LIBRARIES}) | ||
|
|
||
| target_link_libraries(psp ${PYTHON_LIBRARIES}) | ||
| target_link_libraries(binding ${PYTHON_LIBRARIES}) | ||
|
|
||
| target_link_libraries(psp tbb) | ||
| target_link_libraries(binding tbb) | ||
|
|
||
| target_link_libraries(binding psp) | ||
|
|
||
| add_custom_command(TARGET psp POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:psp> ${PSP_PYTHON_SRC}/table/) | ||
| add_custom_command(TARGET binding POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:binding> ${PSP_PYTHON_SRC}/table/) | ||
|
|
||
| if(WIN32) | ||
| # inline arrow dlls | ||
| file(GLOB ARROW_DLLS "${PYTHON_PYARROW_LIBRARY_DIR}/*.dll") | ||
|
There was a problem hiding this comment. There was a problem hiding this comment. I don't understand what the requirement being solved by copying is, nor why a ton of stuff being brought is the instigator, so I'm not really in a position to offer constructive advice here. Copying is bad of course. |
||
|
|
||
| add_custom_command(TARGET binding POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy ${ARROW_DLLS} ${PSP_PYTHON_SRC}/table/) | ||
| if(NOT TBB_FOUND) | ||
| add_custom_command(TARGET binding POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:tbb> ${PSP_PYTHON_SRC}/table/) | ||
| endif() | ||
| endif() | ||
| ######################## | ||
| else() | ||
| add_library(psp SHARED ${SOURCE_FILES} ${HEADER_FILES}) | ||
|
|
@@ -600,10 +635,10 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD) | |
| elseif(WIN32) | ||
| target_compile_definitions(psp PRIVATE PERSPECTIVE_EXPORTS=1) | ||
| target_compile_definitions(psp PRIVATE WIN32=1) | ||
| target_compile_definitions(psp PRIVATE _WIN32=1) | ||
| endif() | ||
|
|
||
| target_link_libraries(psp tbb) | ||
| endif() | ||
|
|
||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,49 +207,49 @@ template <> | |
| t_tscalar t_tscalar::coerce_numeric<bool>() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::int64_t t_tscalar::get() const; | ||
|
There was a problem hiding this comment. @sc1f i assume all these specializations are necessary? There was a problem hiding this comment. Yeah, they all need to be explicitly specialized. |
||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::int32_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::int16_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::int8_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::uint64_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::uint32_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::uint16_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT std::uint8_t t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT t_date t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT t_time t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT const char* t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT t_none t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT double t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT float t_tscalar::get() const; | ||
|
|
||
| template <> | ||
| PERSPECTIVE_EXPORT bool t_tscalar::get() const; | ||
|
|
||
| template <template <typename COMPARED_T> class COMPARER_T> | ||
| bool | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ | |
|
|
||
| namespace perspective { | ||
|
|
||
| class PERSPECTIVE_EXPORT t_symtable { | ||
| typedef tsl::hopscotch_map<const char*, const char*, t_cchar_umap_hash, t_cchar_umap_cmp> | ||
| t_mapping; | ||
|
|
||
|
|
@@ -31,8 +31,8 @@ class t_symtable { | |
| t_mapping m_mapping; | ||
| }; | ||
|
|
||
| PERSPECTIVE_EXPORT const char* get_interned_cstr(const char* s); | ||
|
There was a problem hiding this comment. @sc1f this is currently being exposed as part of |
||
| PERSPECTIVE_EXPORT t_tscalar get_interned_tscalar(const char* s); | ||
| PERSPECTIVE_EXPORT t_tscalar get_interned_tscalar(const t_tscalar& s); | ||
|
|
||
| } // end namespace perspective | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,8 @@ | |
| "name": "perspective-python.node", | ||
| "version": "0.4.1", | ||
| "dependencies": { | ||
| "@finos/perspective": "^0.4.1", | ||
| "zerorpc": "^0.9.8" | ||
|
There was a problem hiding this comment. @texodus please confirm There was a problem hiding this comment. At this point I see honestly zero reason to maintain this feature - |
||
| }, | ||
| "devDependencies": { | ||
| "@finos/perspective-webpack-plugin": "^0.4.1", | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@sc1f note this