You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2023/01/14 11:27:03 UTC
[GitHub] [celix] PengZheng commented on a diff in pull request #472: Feature/update GitHub actions
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org