pnoltes commented on a change in pull request #353:
URL: https://github.com/apache/celix/pull/353#discussion_r681900979
##########
File path: bundles/pubsub/examples/CMakeLists.txt
##########
@@ -221,10 +221,10 @@ if (BUILD_PUBSUB_PSA_ZMQ)
BUNDLES
Celix::shell
Celix::shell_tui
- Celix::pubsub_serializer_json
- Celix::pubsub_discovery_etcd
- Celix::pubsub_topology_manager
- Celix::pubsub_admin_zmq
+ Celix::celix_pubsub_serializer_json
+ Celix::celix_pubsub_discovery_etcd
+ Celix::celix_pubsub_topology_manager
+ Celix::celix_pubsub_admin_zmq
Review comment:
Because this reflect how the bundle are actually named when using Celix
(e.g. the imported cmake target names).
Ideally the celix_ should not be part of this, but if we change this now
this will be a backwards incompatible update.
For example:
```
add_library(foo SHARED ...)
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libfoo.so` and a imported target with the name
`MyProject::foo`
And
```
add_library(myproject_foo SHARED ...)
install(TARGETS myproject_foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libmyproject_foo.so` and a imported target with
the name `MyProject::myproject_foo`
To really get what you want you need:
```
add_library(foo SHARED ...)
set_target_properties(foo PROPERTIES OUTPUT_NAME "myproject_foo")
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
which will lead to a library file `libmyproject_foo.so` and a imported
target with the name `MyProject::foo`
What I prefer to do for the pubsub bundles was to rename the bundle target
and a FILENAME attribute to the `add_bundle` so that the bundle filename
contains a celix_ prefix to ensure uniqueness, but the imported target did not
have a Celix::celix_ usage. But again would be a backwards incompatible
update.
e.g.
```
add_celix_bundle(pubsub_admin_tcp_v2
...
FILENAME celix_pubsub_admin_tcp_v2
)
...
```
##########
File path: bundles/pubsub/examples/CMakeLists.txt
##########
@@ -221,10 +221,10 @@ if (BUILD_PUBSUB_PSA_ZMQ)
BUNDLES
Celix::shell
Celix::shell_tui
- Celix::pubsub_serializer_json
- Celix::pubsub_discovery_etcd
- Celix::pubsub_topology_manager
- Celix::pubsub_admin_zmq
+ Celix::celix_pubsub_serializer_json
+ Celix::celix_pubsub_discovery_etcd
+ Celix::celix_pubsub_topology_manager
+ Celix::celix_pubsub_admin_zmq
Review comment:
Because this reflects how the bundle are actually named when using Celix
(e.g. the imported cmake target names).
Ideally the celix_ should not be part of this, but if we change this now
this will be a backwards incompatible update.
For example:
```
add_library(foo SHARED ...)
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libfoo.so` and a imported target with the name
`MyProject::foo`
And
```
add_library(myproject_foo SHARED ...)
install(TARGETS myproject_foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libmyproject_foo.so` and a imported target with
the name `MyProject::myproject_foo`
To really get what you want you need:
```
add_library(foo SHARED ...)
set_target_properties(foo PROPERTIES OUTPUT_NAME "myproject_foo")
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
which will lead to a library file `libmyproject_foo.so` and a imported
target with the name `MyProject::foo`
What I prefer to do for the pubsub bundles was to rename the bundle target
and a FILENAME attribute to the `add_bundle` so that the bundle filename
contains a celix_ prefix to ensure uniqueness, but the imported target did not
have a Celix::celix_ usage. But again would be a backwards incompatible
update.
e.g.
```
add_celix_bundle(pubsub_admin_tcp_v2
...
FILENAME celix_pubsub_admin_tcp_v2
)
...
```
##########
File path: bundles/pubsub/examples/CMakeLists.txt
##########
@@ -221,10 +221,10 @@ if (BUILD_PUBSUB_PSA_ZMQ)
BUNDLES
Celix::shell
Celix::shell_tui
- Celix::pubsub_serializer_json
- Celix::pubsub_discovery_etcd
- Celix::pubsub_topology_manager
- Celix::pubsub_admin_zmq
+ Celix::celix_pubsub_serializer_json
+ Celix::celix_pubsub_discovery_etcd
+ Celix::celix_pubsub_topology_manager
+ Celix::celix_pubsub_admin_zmq
Review comment:
Because this reflects how the bundles are actually named when using
Celix (e.g. the imported cmake target names).
Ideally the celix_ should not be part of this, but if we change this now
this will be a backwards incompatible update.
For example:
```
add_library(foo SHARED ...)
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libfoo.so` and a imported target with the name
`MyProject::foo`
And
```
add_library(myproject_foo SHARED ...)
install(TARGETS myproject_foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
will lead to a library file `libmyproject_foo.so` and a imported target with
the name `MyProject::myproject_foo`
To really get what you want you need:
```
add_library(foo SHARED ...)
set_target_properties(foo PROPERTIES OUTPUT_NAME "myproject_foo")
install(TARGETS foo EXPORT ProjectTargets)
install(EXPORT ProjectTargets NAMESPACE MyProject ...)
```
which will lead to a library file `libmyproject_foo.so` and a imported
target with the name `MyProject::foo`
What I prefer to do for the pubsub bundles was to rename the bundle target
and a FILENAME attribute to the `add_bundle` so that the bundle filename
contains a celix_ prefix to ensure uniqueness, but the imported target did not
have a Celix::celix_ usage. But again would be a backwards incompatible
update.
e.g.
```
add_celix_bundle(pubsub_admin_tcp_v2
...
FILENAME celix_pubsub_admin_tcp_v2
)
...
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
