[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. [cmake_modules] shared libs for yaml and gumbo on macOS I noticed that even in DEBUG build yaml and gumbo libraries were linked in statically when building on macOS. It turned out the system could not find shared libraries since their pattern contained non-macOS '.so' suffix. This patch address the issue. I verified that non-debug binaries are linked dynamically with the libraries mentioned above. I also added some extra code to protect against using static libraries in place of the shared ones. It's possible to apply this approach to other thirdparty libraries used by Kudu, but I think it's better to do so in a separate changelist. Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Reviewed-on: http://gerrit.cloudera.org:8080/15182 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M cmake_modules/FindGumboParser.cmake M cmake_modules/FindGumboQuery.cmake M cmake_modules/FindYaml.cmake 3 files changed, 36 insertions(+), 6 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 11 Feb 2020 18:00:02 + Gerrit-HasComments: No
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Hello Kudu Jenkins, Adar Dembo, Volodymyr Verovkin, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15182 to look at the new patch set (#3). Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. [cmake_modules] shared libs for yaml and gumbo on macOS I noticed that even in DEBUG build yaml and gumbo libraries were linked in statically when building on macOS. It turned out the system could not find shared libraries since their pattern contained non-macOS '.so' suffix. This patch address the issue. I verified that non-debug binaries are linked dynamically with the libraries mentioned above. I also added some extra code to protect against using static libraries in place of the shared ones. It's possible to apply this approach to other thirdparty libraries used by Kudu, but I think it's better to do so in a separate changelist. Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 --- M cmake_modules/FindGumboParser.cmake M cmake_modules/FindGumboQuery.cmake M cmake_modules/FindYaml.cmake 3 files changed, 36 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15182/3 -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15182/2/cmake_modules/FindGumboParser.cmake File cmake_modules/FindGumboParser.cmake: http://gerrit.cloudera.org:8080/#/c/15182/2/cmake_modules/FindGumboParser.cmake@33 PS2, Line 33: find_library(GUMBO_PARSER_SHARED_LIB gumbo > What about replacing 'gumbo' with 'libgumbo${CMAKE_SHARED_LIBRARY_SUFFIX}'? CMAKE_SHARED_LIBRARY_SUFFIX on macOS is ".tbd;.dylib;.so;.a", so nope, it would not work. I also tried to set CMAKE_SHARED_LIBRARY_SUFFIX just for shared libs (i.e. '.so' on linux and '.dylib' on macOS) in the top-level CMakeLists.txt in $KUDU_ROOT, but it didn't work: if when using lib.a for static library part, cmake could not find the static library (apparently, it needs '.a' to be in the suffix list even if lib.a is given in find_library() function). Yep, I can do this in every other file in this patch. And next patch we could introduce a function/macro around find_library(), something find_static_library() and find_shared_library(). -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 11 Feb 2020 06:00:15 + Gerrit-HasComments: Yes
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15182/2/cmake_modules/FindGumboParser.cmake File cmake_modules/FindGumboParser.cmake: http://gerrit.cloudera.org:8080/#/c/15182/2/cmake_modules/FindGumboParser.cmake@33 PS2, Line 33: find_library(GUMBO_PARSER_SHARED_LIB gumbo What about replacing 'gumbo' with 'libgumbo${CMAKE_SHARED_LIBRARY_SUFFIX}'? Would that work without the set/restore stuff? https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LIBRARY_SUFFIX.html BTW, I'd be fine with doing what every Find*.cmake does in this patch, then maybe doing this sort of robustness fix in a separate patch across the board. -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 11 Feb 2020 05:53:41 + Gerrit-HasComments: Yes
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 2: Verified+1 unrelated test failures -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 11 Feb 2020 05:24:31 + Gerrit-HasComments: No
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has removed a vote on this change. Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Hello Kudu Jenkins, Adar Dembo, Volodymyr Verovkin, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15182 to look at the new patch set (#2). Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. [cmake_modules] shared libs for yaml and gumbo on macOS I noticed that even in DEBUG build yaml and gumbo libraries were linked in statically when building on macOS. It turned out the system could not find shared libraries since their pattern contained non-macOS '.so' suffix. This patch address the issue: I verified that non-debug binaries are linked dynamically with the libraries mentioned above. I also added some extra work to protect against using static gumbo library in place of the shared one. The approach could be applied for other libraries as well, but I'm not sure it's worth it. Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 --- M cmake_modules/FindGumboParser.cmake M cmake_modules/FindGumboQuery.cmake M cmake_modules/FindYaml.cmake 3 files changed, 16 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15182/2 -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15182/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15182/1//COMMIT_MSG@14 PS1, Line 14: with > Nit: too many with Done http://gerrit.cloudera.org:8080/#/c/15182/1/cmake_modules/FindGumboParser.cmake File cmake_modules/FindGumboParser.cmake: http://gerrit.cloudera.org:8080/#/c/15182/1/cmake_modules/FindGumboParser.cmake@25 PS1, Line 25: gumbo > If libgumbo.so (or libgumbo.dylib on macOS) is missing, does this match aga Yup, it happily consumes the static library as a dynamic one here if the file of the actual shared library is missing: Added shared library dependency gumbo-parser: /Users/aserbin/Projects/kudu/thirdparty/installed/uninstrumented/lib/libgumbo.a I sketched a way to handle that in a more reliable way, but it would be necessary to make it more generic and apply to other Find*.cmake files as well. -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 11 Feb 2020 01:51:15 + Gerrit-HasComments: Yes
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15182 ) Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15182/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15182/1//COMMIT_MSG@14 PS1, Line 14: with Nit: too many with http://gerrit.cloudera.org:8080/#/c/15182/1/cmake_modules/FindGumboParser.cmake File cmake_modules/FindGumboParser.cmake: http://gerrit.cloudera.org:8080/#/c/15182/1/cmake_modules/FindGumboParser.cmake@25 PS1, Line 25: gumbo If libgumbo.so (or libgumbo.dylib on macOS) is missing, does this match against libgumbo.a? -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Fri, 07 Feb 2020 20:20:50 + Gerrit-HasComments: Yes
[kudu-CR] [cmake modules] shared libs for yaml and gumbo on macOS
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15182 Change subject: [cmake_modules] shared libs for yaml and gumbo on macOS .. [cmake_modules] shared libs for yaml and gumbo on macOS I noticed that even in DEBUG build yaml and gumbo libraries were linked in statically when building on macOS. It turned out the system could not find shared libraries since their pattern contained non-macOS '.so' suffix. This patch address the issue: I verified that with with patch non-debug binaries are linked dynamically with the libraries mentioned above. Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 --- M cmake_modules/FindGumboParser.cmake M cmake_modules/FindGumboQuery.cmake M cmake_modules/FindYaml.cmake 3 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15182/1 -- To view, visit http://gerrit.cloudera.org:8080/15182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie1e56b9dd988501af010bbd1a256671b0460f526 Gerrit-Change-Number: 15182 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin