You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2024/03/21 10:23:40 UTC
[PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
swamirishi opened a new pull request, #6414:
URL: https://github.com/apache/ozone/pull/6414
## What changes were proposed in this pull request?
Currently the native lib load fails since we hardcode the rocksdbjni lib path while linking with the native lib. We need to link it with a relative path.
The patch looks into setting the RPATH for native lib which will link the rocksdblib corresponding to the location of the native lib.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10560
## How was this patch tested?
Existing Unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #6414:
URL: https://github.com/apache/ozone/pull/6414#discussion_r1539863906
##########
hadoop-hdds/rocks-native/src/CMakeLists.txt:
##########
@@ -30,25 +30,33 @@ find_package(JNI REQUIRED)
include_directories(${JNI_INCLUDE_DIRS})
set(CMAKE_CXX_STANDARD ${CMAKE_STANDARDS})
-set(linked_libraries "")
+set(CMAKE_SKIP_BUILD_RPATH FALSE)
+
+set(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
+
+set(CMAKE_INSTALL_RPATH "")
+
+set(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE)
+
if(NOT GENERATED_JAVAH)
message(FATAL_ERROR "You must set the CMake variable GENERATED_JAVAH")
endif()
include_directories(${GENERATED_JAVAH})
if(${SST_DUMP_INCLUDE})
include_directories(${ROCKSDB_HEADERS})
set(SOURCE_FILES ${NATIVE_DIR}/ManagedRawSSTFileReader.cpp ${NATIVE_DIR}/ManagedRawSSTFileIterator.cpp ${NATIVE_DIR}/cplusplus_to_java_convert.h)
- ADD_LIBRARY(rocksdb SHARED IMPORTED)
- set_target_properties(
- rocksdb
- PROPERTIES
- IMPORTED_LOCATION ${ROCKSDB_LIB})
ADD_LIBRARY(rocks_tools STATIC IMPORTED)
set_target_properties(
rocks_tools
PROPERTIES
IMPORTED_LOCATION ${ROCKSDB_TOOLS_LIB}/librocksdb_tools.a)
- set(linked_libraries ${linked_libraries} rocks_tools rocksdb)
endif()
+
add_library(ozone_rocksdb_tools SHARED ${SOURCE_FILES})
-target_link_libraries(ozone_rocksdb_tools ${linked_libraries})
+
+
+target_link_libraries(ozone_rocksdb_tools PRIVATE ${ROCKSDB_LIB})
+target_link_libraries(ozone_rocksdb_tools PRIVATE rocks_tools)
+set_target_properties(ozone_rocksdb_tools PROPERTIES
+ BUILD_WITH_INSTALL_RPATH FALSE
+ LINK_FLAGS "-Wl,-rpath -Wl,'$ORIGIN'")
Review Comment:
done
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/JniLibNamePropertyWriter.java:
##########
@@ -39,8 +39,8 @@ public static void main(String[] args) {
String filePath = args[0];
try (Writer writer = new OutputStreamWriter(
Files.newOutputStream(Paths.get(filePath)), StandardCharsets.UTF_8)) {
- writer.write("rocksdbLibName=" +
- Environment.getJniLibraryFileName("rocksdb"));
+ String libName = Environment.getJniLibraryFileName("rocksdb");
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #6414:
URL: https://github.com/apache/ozone/pull/6414#discussion_r1534484730
##########
hadoop-hdds/rocks-native/src/CMakeLists.txt:
##########
@@ -30,25 +30,33 @@ find_package(JNI REQUIRED)
include_directories(${JNI_INCLUDE_DIRS})
set(CMAKE_CXX_STANDARD ${CMAKE_STANDARDS})
-set(linked_libraries "")
+set(CMAKE_SKIP_BUILD_RPATH FALSE)
+
+set(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
+
+set(CMAKE_INSTALL_RPATH "")
+
+set(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE)
+
if(NOT GENERATED_JAVAH)
message(FATAL_ERROR "You must set the CMake variable GENERATED_JAVAH")
endif()
include_directories(${GENERATED_JAVAH})
if(${SST_DUMP_INCLUDE})
include_directories(${ROCKSDB_HEADERS})
set(SOURCE_FILES ${NATIVE_DIR}/ManagedRawSSTFileReader.cpp ${NATIVE_DIR}/ManagedRawSSTFileIterator.cpp ${NATIVE_DIR}/cplusplus_to_java_convert.h)
- ADD_LIBRARY(rocksdb SHARED IMPORTED)
- set_target_properties(
- rocksdb
- PROPERTIES
- IMPORTED_LOCATION ${ROCKSDB_LIB})
ADD_LIBRARY(rocks_tools STATIC IMPORTED)
set_target_properties(
rocks_tools
PROPERTIES
IMPORTED_LOCATION ${ROCKSDB_TOOLS_LIB}/librocksdb_tools.a)
- set(linked_libraries ${linked_libraries} rocks_tools rocksdb)
endif()
+
add_library(ozone_rocksdb_tools SHARED ${SOURCE_FILES})
-target_link_libraries(ozone_rocksdb_tools ${linked_libraries})
+
+
+target_link_libraries(ozone_rocksdb_tools PRIVATE ${ROCKSDB_LIB})
+target_link_libraries(ozone_rocksdb_tools PRIVATE rocks_tools)
+set_target_properties(ozone_rocksdb_tools PROPERTIES
+ BUILD_WITH_INSTALL_RPATH FALSE
+ LINK_FLAGS "-Wl,-rpath -Wl,'$ORIGIN'")
Review Comment:
nit: put a new line at the end of the file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #6414:
URL: https://github.com/apache/ozone/pull/6414
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #6414:
URL: https://github.com/apache/ozone/pull/6414#issuecomment-2012790585
<img width="1294" alt="Screen Shot 2024-03-21 at 03 27 22" src="https://github.com/apache/ozone/assets/47532440/4d04e815-9332-4fd9-b08c-e3915f6a8908">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #6414:
URL: https://github.com/apache/ozone/pull/6414#discussion_r1534913885
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/JniLibNamePropertyWriter.java:
##########
@@ -39,8 +39,8 @@ public static void main(String[] args) {
String filePath = args[0];
try (Writer writer = new OutputStreamWriter(
Files.newOutputStream(Paths.get(filePath)), StandardCharsets.UTF_8)) {
- writer.write("rocksdbLibName=" +
- Environment.getJniLibraryFileName("rocksdb"));
+ String libName = Environment.getJniLibraryFileName("rocksdb");
Review Comment:
Why not use `public static String getRocksDBLibFileName() {`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #6414:
URL: https://github.com/apache/ozone/pull/6414#issuecomment-2043374007
thank you for the review @kerneltime @smengcl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #6414:
URL: https://github.com/apache/ozone/pull/6414#issuecomment-2015959760
The cmake changes look fine. I will try to find some time to give this PR a spin locally and complete the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #6414:
URL: https://github.com/apache/ozone/pull/6414#issuecomment-2012357905
Got a clean CI on my fork. Opening the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10560. Link rocksdb lib to Ozone rocksdb tools lib relative path instead of absolute path [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6414:
URL: https://github.com/apache/ozone/pull/6414#issuecomment-2011863781
Please wait for clean CI run in fork before opening PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org