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 2022/04/04 18:00:28 UTC
[GitHub] [celix] pnoltes opened a new pull request, #409: Feature/rename zmq and jansson targets to align with conan
pnoltes opened a new pull request, #409:
URL: https://github.com/apache/celix/pull/409
Updates the ZMQ, CZMQ and Jansson find package/target naming so that they align with the Find<pkg>.cmake files generated when using conan.
--
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
[GitHub] [celix] PengZheng commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #409:
URL: https://github.com/apache/celix/pull/409#discussion_r842348371
##########
CMakeLists.txt:
##########
@@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework
find_package(UUID REQUIRED) #framework
Review Comment:
Following the same logic, we should also update find modules for uuid, libzip, and ffi?
```
##########
CMakeLists.txt:
##########
@@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework
find_package(UUID REQUIRED) #framework
find_package(CURL REQUIRED) #framework, etcdlib
find_package(LIBZIP REQUIRED) #framework, etcdlib
-find_package(Jansson REQUIRED) #etcdlib, dfi
+find_package(jansson REQUIRED) #etcdlib, dfi
Review Comment:
nitpick: message("Note jansson::jansson target not created by find_package(Jansson). Creating jansson::jansson Target.") should also be updated.
##########
cmake/Modules/Findczmq.cmake:
##########
@@ -37,14 +37,14 @@ set(CZMQ_INCLUDE_DIRS ${CZMQ_INCLUDE_DIR} )
include(FindPackageHandleStandardArgs)
# handle the QUIETLY and REQUIRED arguments and set CZMQ_FOUND to TRUE
# if all listed variables are TRUE
-find_package_handle_standard_args(CZMQ DEFAULT_MSG
+find_package_handle_standard_args(czmq DEFAULT_MSG
CZMQ_LIBRARY CZMQ_INCLUDE_DIR)
mark_as_advanced(CZMQ_INCLUDE_DIR CZMQ_LIBRARY)
-if (CZMQ_FOUND AND NOT TARGET CZMQ::lib)
- add_library(CZMQ::lib SHARED IMPORTED)
- set_target_properties(CZMQ::lib PROPERTIES
+if (CZMQ_FOUND AND NOT TARGET czmq::czmq)
+ add_library(czmq::czmq SHARED IMPORTED)
Review Comment:
I noticed that test_cxx_remote_services_integration is still using old targets.
##########
cmake/Modules/FindZeroMQ.cmake:
##########
@@ -18,34 +18,34 @@
# - Try to find ZMQ
# Once done this will define
-# ZMQ_FOUND - System has Zmq
-# ZMQ_INCLUDE_DIRS - The Zmq include directories
-# ZMQ_LIBRARIES - The libraries needed to use Zmq
-# ZMQ_DEFINITIONS - Compiler switches required for using Zmq
-# ZMQ::lib - Imported target for UUID
+# ZEROMQ_FOUND - System has Zmq
+# ZEROMQ_INCLUDE_DIRS - The Zmq include directories
+# ZEROMQ_LIBRARIES - The libraries needed to use Zmq
+# ZEROMQ_DEFINITIONS - Compiler switches required for using Zmq
+# ZeroMQ::ZeroMQ - Imported target for UUID
-find_path(ZMQ_INCLUDE_DIR zmq.h zmq_utils.h
+find_path(ZEROMQ_INCLUDE_DIR zmq.h zmq_utils.h
/usr/include
/usr/local/include )
-find_library(ZMQ_LIBRARY NAMES zmq
+find_library(ZEROMQ_LIBRARY NAMES zmq
PATHS /usr/lib /usr/local/lib /usr/lib64 /usr/local/lib64 )
-set(ZMQ_LIBRARIES ${ZMQ_LIBRARY} )
-set(ZMQ_INCLUDE_DIRS ${ZMQ_INCLUDE_DIR} )
+set(ZEROMQ_LIBRARIES ${ZEROMQ_LIBRARY} )
+set(ZEROMQ_INCLUDE_DIRS ${ZEROMQ_INCLUDE_DIR} )
include(FindPackageHandleStandardArgs)
# handle the QUIETLY and REQUIRED arguments and set ZMQ_FOUND to TRUE
Review Comment:
nitpick: ZEROMQ_FOUND
--
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
[GitHub] [celix] pnoltes merged pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
pnoltes merged PR #409:
URL: https://github.com/apache/celix/pull/409
--
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
[GitHub] [celix] PengZheng commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #409:
URL: https://github.com/apache/celix/pull/409#discussion_r842348371
##########
CMakeLists.txt:
##########
@@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework
find_package(UUID REQUIRED) #framework
Review Comment:
Following the same logic, shall we also update find modules for uuid, libzip, and ffi? Of course it belongs to another PR. If you are busying preparing the new release, I will pick it up when this PR merged.
--
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
[GitHub] [celix] PengZheng commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #409:
URL: https://github.com/apache/celix/pull/409#discussion_r842348371
##########
CMakeLists.txt:
##########
@@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework
find_package(UUID REQUIRED) #framework
Review Comment:
Following the same logic, shall we also update find modules for uuid, libzip, and ffi? Of course it belongs to another 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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-commenter commented on pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #409:
URL: https://github.com/apache/celix/pull/409#issuecomment-1089248899
# [Codecov](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#409](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2f35d2) into [master](https://codecov.io/gh/apache/celix/commit/4e70917e7d8c35f9970e14401d7736d242152d7a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e70917) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.
> :exclamation: Current head f2f35d2 differs from pull request most recent head fba37f8. Consider uploading reports for the commit fba37f8 to get more accurate results
```diff
@@ Coverage Diff @@
## master #409 +/- ##
==========================================
- Coverage 71.78% 71.77% -0.01%
==========================================
Files 180 180
Lines 29281 29281
==========================================
- Hits 21018 21017 -1
- Misses 8263 8264 +1
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ibs/pushstreams/api/celix/impl/PushEventConsumer.h](https://codecov.io/gh/apache/celix/pull/409/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9wdXNoc3RyZWFtcy9hcGkvY2VsaXgvaW1wbC9QdXNoRXZlbnRDb25zdW1lci5o) | `90.00% <0.00%> (-10.00%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4e70917...fba37f8](https://codecov.io/gh/apache/celix/pull/409?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [celix] pnoltes commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
pnoltes commented on code in PR #409:
URL: https://github.com/apache/celix/pull/409#discussion_r843174692
##########
cmake/Modules/Findczmq.cmake:
##########
@@ -37,14 +37,14 @@ set(CZMQ_INCLUDE_DIRS ${CZMQ_INCLUDE_DIR} )
include(FindPackageHandleStandardArgs)
# handle the QUIETLY and REQUIRED arguments and set CZMQ_FOUND to TRUE
# if all listed variables are TRUE
-find_package_handle_standard_args(CZMQ DEFAULT_MSG
+find_package_handle_standard_args(czmq DEFAULT_MSG
CZMQ_LIBRARY CZMQ_INCLUDE_DIR)
mark_as_advanced(CZMQ_INCLUDE_DIR CZMQ_LIBRARY)
-if (CZMQ_FOUND AND NOT TARGET CZMQ::lib)
- add_library(CZMQ::lib SHARED IMPORTED)
- set_target_properties(CZMQ::lib PROPERTIES
+if (CZMQ_FOUND AND NOT TARGET czmq::czmq)
+ add_library(czmq::czmq SHARED IMPORTED)
Review Comment:
Yes, I forgot this one. I will also re-enable the C++ RSA for the build server.
--
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
[GitHub] [celix] pnoltes commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan
Posted by GitBox <gi...@apache.org>.
pnoltes commented on code in PR #409:
URL: https://github.com/apache/celix/pull/409#discussion_r843176784
##########
CMakeLists.txt:
##########
@@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework
find_package(UUID REQUIRED) #framework
Review Comment:
Yes I think that it a good move, just to be sure to align everything a bit. I you want to pick this up, great :).
--
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