PengZheng commented on code in PR #472:
URL: https://github.com/apache/celix/pull/472#discussion_r1070251475
##########
conanfile.py:
##########
@@ -229,6 +231,14 @@ def requirements(self):
self.requires("czmq/4.2.0")
self.options['czmq'].shared = True
+ #If czmq is needed, disable crypto for libzip to prevent openssl
conflict:
+ # 'czmq/4.2.0' requires 'openssl/1.1.1m' while 'libcurl/7.86.0'
requires 'openssl/1.1.1s'
+ self.options['libzip'].crypto = False
+
+ #If czmq is needed, disable ssl for libcurl to prevent openssl
conflict:
+ # 'czmq/4.2.0' requires 'openssl/1.1.1m' while 'libcurl/7.87.0'
requires 'openssl/1.1.1s'
+ self.options['libcurl'].with_ssl = False
Review Comment:
Not necessary as explained above.
##########
conanfile.py:
##########
@@ -229,6 +231,14 @@ def requirements(self):
self.requires("czmq/4.2.0")
self.options['czmq'].shared = True
+ #If czmq is needed, disable crypto for libzip to prevent openssl
conflict:
+ # 'czmq/4.2.0' requires 'openssl/1.1.1m' while 'libcurl/7.86.0'
requires 'openssl/1.1.1s'
+ self.options['libzip'].crypto = False
Review Comment:
It's not necessary to disable libzip. We can override openssl requirement in
this recipe. Downstream can always override upstream requirement.
##########
conanfile.py:
##########
@@ -229,6 +231,14 @@ def requirements(self):
self.requires("czmq/4.2.0")
self.options['czmq'].shared = True
+ #If czmq is needed, disable crypto for libzip to prevent openssl
conflict:
+ # 'czmq/4.2.0' requires 'openssl/1.1.1m' while 'libcurl/7.86.0'
requires 'openssl/1.1.1s'
+ self.options['libzip'].crypto = False
Review Comment:
Actually you can do this on the conan command line using
`--require-override` to specify openssl version.
If the version you specify differs from czmq or libcurl's requirement, they
will be rebuilt with a different package id.
So there's nothing to worry about.
##########
bundles/remote_services/discovery_shm/CMakeLists.txt:
##########
@@ -37,7 +37,9 @@ target_include_directories(rsa_discovery_shm PRIVATE
$<TARGET_PROPERTY:Celix::rsa_discovery_common,INCLUDE_DIRECTORIES>
$<TARGET_PROPERTY:Celix::civetweb,INCLUDE_DIRECTORIES>
)
-target_link_libraries(rsa_discovery_shm PRIVATE Celix::framework CURL::libcurl
${LIBXML2_LIBRARIES})
+target_link_libraries(rsa_discovery_shm PRIVATE
+ Celix::framework CURL::libcurl ${LIBXML2_LIBRARIES}
Celix::log_helper
Review Comment:
I suggest adding find module for libxml2.
##########
bundles/cxx_remote_services/discovery_configured/CMakeLists.txt:
##########
@@ -39,6 +39,12 @@ target_link_libraries(RsaConfiguredDiscovery PRIVATE
Celix::rsa_spi
Celix::log_helper
)
+
+#Note import target RapidJSON::RapidJSO only created when using conan, else
/usr/include is used.
Review Comment:
AddingFindRapidJSON.cmake may be a better way?
##########
conanfile.py:
##########
@@ -124,8 +125,9 @@ class CelixConan(ConanFile):
"build_launcher": False,
"build_promises": False,
"build_pushstreams": False,
- "celix_cxx14": False,
- "celix_cxx17": False,
+ "build_experimental": False,
Review Comment:
IIRC, device_access has been removed. Assuming no user exists, we should
also remove the corresponding option.
--
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: dev-unsubscr...@celix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
