Skip to content

Commit

Permalink
fixing issues #10 and #12
Browse files Browse the repository at this point in the history
  • Loading branch information
@jtrmal
jtrmal committed Jun 18, 2018
1 parent a754372 commit 0ef24cf
Show file tree
Hide file tree
Showing 13 changed files with 23 additions and 17 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ obj/
*proj.user
/cmake
/cmake64
/CMakeSettings.json
4 changes: 3 additions & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@ set(CMAKE_CXX_STANDARD 11)

if (WIN32)
add_definitions(/bigobj)
set(WHOLEFST "/WHOLEARCHIVE:fst")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${WHOLEFST}")
#set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS 1)
#this must be disabled unless the previous option (CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS) is enabled
option(BUILD_SHARED_LIBS "Build shared libraries" OFF)
else()
option(BUILD_SHARED_LIBS "Build shared libraries" ON)
endif (WIN32)


set(SOVERSION "10")
OPTION(BUILD_USE_SOLUTION_FOLDERS "Enable grouping of projects in VS" ON)
SET_PROPERTY(GLOBAL PROPERTY USE_FOLDERS ${BUILD_USE_SOLUTION_FOLDERS})

Expand Down
2 changes: 1 addition & 1 deletion src/extensions/compact/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ add_library(fstcompact

target_link_libraries(fstcompact fst)
set_target_properties(fstcompact PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER compact
)

Expand Down
4 changes: 2 additions & 2 deletions src/extensions/const/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ add_library(fstconst
const16-fst.cc
const64-fst.cc)
target_link_libraries(fstconst fst)
set_target_properties(fstconst
PROPERTIES SOVERSION "10"
set_target_properties(fstconst PROPERTIES
SOVERSION "${SOVERSION}"
FOLDER constant
)

Expand Down
6 changes: 3 additions & 3 deletions src/extensions/far/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ add_library(fstfar
)
target_link_libraries(fstfar fst)
set_target_properties(fstfar PROPERTIES
SOVERSION "10"
FOLDER far
SOVERSION "${SOVERSION}"
FOLDER far
)

install(TARGETS fstfar
Expand All @@ -28,7 +28,7 @@ if(HAVE_SCRIPT)
)
target_link_libraries(fstfarscript fstfar fstscript fst)
set_target_properties(fstfarscript PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER far
)

Expand Down
2 changes: 1 addition & 1 deletion src/extensions/linear/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if(HAVE_SCRIPT)
fst
)
set_target_properties(fstlinearscript PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER linear
)

Expand Down
2 changes: 1 addition & 1 deletion src/extensions/lookahead/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ add_library(fstlookahead
)
target_link_libraries(fstlookahead fst)
set_target_properties(fstlookahead PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER lookahead
)

Expand Down
2 changes: 1 addition & 1 deletion src/extensions/mpdt/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ if(HAVE_SCRIPT)
add_library(fstmpdtscript mpdtscript.cc ${HEADER_FILES})
target_link_libraries(fstmpdtscript fstscript fst)
set_target_properties(fstmpdtscript PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER mpdt
)
install(TARGETS fstmpdtscript
Expand Down
4 changes: 2 additions & 2 deletions src/extensions/ngram/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ target_link_libraries(fstngram
)

set_target_properties(fstngram PROPERTIES
SOVERSION "10"
FOLDER ngram
SOVERSION "${SOVERSION}"
FOLDER ngram
)

install(TARGETS fstngram
Expand Down
2 changes: 1 addition & 1 deletion src/extensions/pdt/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ if(HAVE_SCRIPT)
add_library(fstpdtscript getters.cc pdtscript.cc ${HEADER_FILES})
target_link_libraries(fstpdtscript fstscript fst)
set_target_properties(fstpdtscript PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
FOLDER pdt
)

Expand Down
4 changes: 2 additions & 2 deletions src/extensions/special/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ add_library(fstspecial
${HEADER_FILES}
)

set_target_properties(fstspecial
PROPERTIES SOVERSION "10"
set_target_properties(fstspecial PROPERTIES
SOVERSION "${SOVERSION}"
FOLDER special
)
target_link_libraries(fstspecial
Expand Down
5 changes: 4 additions & 1 deletion src/lib/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ add_library(fst
${HEADER_FILES}
)
set_target_properties(fst PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
)



install(TARGETS fst
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
Expand Down
2 changes: 1 addition & 1 deletion src/script/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ add_library(fstscript
target_link_libraries(fstscript PRIVATE fst)

set_target_properties(fstscript PROPERTIES
SOVERSION "10"
SOVERSION "${SOVERSION}"
)
install(TARGETS fstscript
LIBRARY DESTINATION lib
Expand Down

7 comments on commit 0ef24cf

@jtrmal
Copy link
Collaborator Author

@jtrmal jtrmal commented on 0ef24cf Jun 18, 2018

Choose a reason for hiding this comment

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

@kkm, can you please revert? Sorry for making troubles. The git functionality in VS2017 is brain-dead

@jtrmal
Copy link
Collaborator Author

@jtrmal jtrmal commented on 0ef24cf Jun 18, 2018

Choose a reason for hiding this comment

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

Or leave it as it is, it should be fine, I've tested the build on Win and it went fine. Sorry, once again

@kkm000
Copy link
Owner

@kkm000 kkm000 commented on 0ef24cf Jun 18, 2018

Choose a reason for hiding this comment

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

@jtrmal Thank you, that's totally fine! I cannot sensibly review CMake anyway. Thanks for fixing the issues, and use your judgment whether you want changes reviewed, or they are good to go in as is.

I am wondering, is something like /WHOLEARCHIVE required in all OSes? This is a general problem, liker just has no clue that given .o from the library makes something essential to the functioning of the library in its static initializers. Or is CMake smart enough to convert that to proper switches for other compilers? I read this answer, and my understanding is that CMake just passes through the switches.

@kkm000
Copy link
Owner

@kkm000 kkm000 commented on 0ef24cf Jun 18, 2018

Choose a reason for hiding this comment

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

I am tempted to just #include that new file from fst.cc. It's causing too much trouble. Does not look like a well thought change on the upstream's part, indeed.

@jtrmal
Copy link
Collaborator Author

@jtrmal jtrmal commented on 0ef24cf Jun 18, 2018 via email

Choose a reason for hiding this comment

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

@kkm
Copy link

@kkm kkm commented on 0ef24cf Jun 19, 2018

Choose a reason for hiding this comment

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

doesnt work again.

@jtrmal
Copy link
Collaborator Author

@jtrmal jtrmal commented on 0ef24cf Jun 19, 2018 via email

Choose a reason for hiding this comment

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

Please sign in to comment.