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 2021/03/01 18:31:49 UTC
[GitHub] [celix] pnoltes opened a new pull request #324: Feature/gcc 4 9 0 fixes
pnoltes opened a new pull request #324:
URL: https://github.com/apache/celix/pull/324
This PR fixes some issue with the C++ API when building with gcc 4.9.0 (c++11).
This also disable the typename infer using rtti, this needs to be tested first.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #324:
URL: https://github.com/apache/celix/pull/324#issuecomment-788189667
# [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=desc) (7d21e65) into [master](https://codecov.io/gh/apache/celix/commit/7a0cc48ba01ee6313759c3aa6e4f858a483ba437?el=desc) (7a0cc48) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/324/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
- Coverage 71.00% 70.99% -0.01%
==========================================
Files 175 175
Lines 31552 31553 +1
==========================================
- Hits 22402 22400 -2
- Misses 9150 9153 +3
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/include/celix/Utils.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9VdGlscy5o) | `93.75% <ø> (ø)` | |
| [libs/framework/include/celix/Bundle.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9CdW5kbGUuaA==) | `100.00% <100.00%> (ø)` | |
| [libs/framework/include/celix/Properties.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9Qcm9wZXJ0aWVzLmg=) | `98.71% <100.00%> (+0.03%)` | :arrow_up: |
| [libs/framework/include/celix/Trackers.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9UcmFja2Vycy5o) | `90.93% <100.00%> (-0.04%)` | :arrow_down: |
| [.../pubsub\_admin\_zmq/v2/src/pubsub\_zmq\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3ptcS92Mi9zcmMvcHVic3ViX3ptcV90b3BpY19zZW5kZXIuYw==) | `74.05% <0.00%> (-0.88%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=footer). Last update [7a0cc48...7d21e65](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #324:
URL: https://github.com/apache/celix/pull/324#discussion_r586385635
##########
File path: libs/framework/include/celix/Properties.h
##########
@@ -253,6 +259,38 @@ namespace celix {
std::size_t size() const {
return celix_properties_size(cProps.get());
}
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string map.
+ */
+ std::map<std::string, std::string> convertToMap() const {
+ std::map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string unordered map.
+ */
+ std::unordered_map<std::string, std::string> convertToUnorderedMap() const {
+ std::unordered_map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief cast the celix::Properties to a std::string, std::string map.
+ * @warning This method is added to ensure backwards compatibility with the celix::dm::Properties, but the
+ * use of this cast should be avoided.
+ * This method will eventually be removed.
+ */
+ operator std::map<std::string, std::string>() const {
Review comment:
Agree. I added a define for this.
We can enable this - for now - downstream and eventually remove this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #324:
URL: https://github.com/apache/celix/pull/324#discussion_r586378142
##########
File path: libs/framework/include/celix/Properties.h
##########
@@ -253,6 +259,38 @@ namespace celix {
std::size_t size() const {
return celix_properties_size(cProps.get());
}
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string map.
+ */
+ std::map<std::string, std::string> convertToMap() const {
+ std::map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string unordered map.
+ */
+ std::unordered_map<std::string, std::string> convertToUnorderedMap() const {
+ std::unordered_map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief cast the celix::Properties to a std::string, std::string map.
+ * @warning This method is added to ensure backwards compatibility with the celix::dm::Properties, but the
+ * use of this cast should be avoided.
+ * This method will eventually be removed.
+ */
+ operator std::map<std::string, std::string>() const {
Review comment:
Could do it in this PR, could also make a new ticket like #327
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io commented on pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #324:
URL: https://github.com/apache/celix/pull/324#issuecomment-788189667
# [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=desc) (818cb59) into [master](https://codecov.io/gh/apache/celix/commit/101db50ddfa06b7b28a0c36a9d9ed283c2f38dc8?el=desc) (101db50) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/324/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
- Coverage 71.00% 71.00% -0.01%
==========================================
Files 175 175
Lines 31545 31546 +1
==========================================
Hits 22400 22400
- Misses 9145 9146 +1
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/include/celix/Utils.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9VdGlscy5o) | `93.75% <ø> (ø)` | |
| [libs/framework/include/celix/Bundle.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9CdW5kbGUuaA==) | `100.00% <100.00%> (ø)` | |
| [libs/framework/include/celix/Properties.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9Qcm9wZXJ0aWVzLmg=) | `98.71% <100.00%> (+0.03%)` | :arrow_up: |
| [libs/framework/include/celix/Trackers.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9UcmFja2Vycy5o) | `90.93% <100.00%> (-0.04%)` | :arrow_down: |
| [...dmin\_websocket/src/pubsub\_websocket\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3dlYnNvY2tldC9zcmMvcHVic3ViX3dlYnNvY2tldF90b3BpY19zZW5kZXIuYw==) | `83.24% <0.00%> (-1.12%)` | :arrow_down: |
| [libs/utils/src/hash\_map.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvaGFzaF9tYXAuYw==) | `94.23% <0.00%> (+0.28%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=footer). Last update [101db50...818cb59](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #324:
URL: https://github.com/apache/celix/pull/324#discussion_r586377374
##########
File path: libs/framework/include/celix/Properties.h
##########
@@ -253,6 +259,38 @@ namespace celix {
std::size_t size() const {
return celix_properties_size(cProps.get());
}
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string map.
+ */
+ std::map<std::string, std::string> convertToMap() const {
+ std::map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief Converts the properties a (new) std::string, std::string unordered map.
+ */
+ std::unordered_map<std::string, std::string> convertToUnorderedMap() const {
+ std::unordered_map<std::string, std::string> result{};
+ for (const auto& pair : *this) {
+ result[pair.first] = pair.second;
+ }
+ return result;
+ }
+
+ /**
+ * @brief cast the celix::Properties to a std::string, std::string map.
+ * @warning This method is added to ensure backwards compatibility with the celix::dm::Properties, but the
+ * use of this cast should be avoided.
+ * This method will eventually be removed.
+ */
+ operator std::map<std::string, std::string>() const {
Review comment:
Without the `explicit` keyword, this results in interesting behaviour where it might not be expected. While providing a good backwards-compatible approach, we might want to think about some ifdef construction where the explicit keyword is added if enabled. This will allow easier migration in the future.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #324:
URL: https://github.com/apache/celix/pull/324#issuecomment-789696568
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes merged pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
pnoltes merged pull request #324:
URL: https://github.com/apache/celix/pull/324
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #324:
URL: https://github.com/apache/celix/pull/324#issuecomment-789072886
> My apologies, I missed the following compile error when compiling in Release mode:
>
> ```
> /home/oipo-unencrypted/Programming/celix-apache/libs/framework/benchmark/src/LookupServicesBenchmark.cc: In function ‘void findSingleService(benchmark::State&, bool, bool)’:
> /home/oipo-unencrypted/Programming/celix-apache/libs/framework/benchmark/src/LookupServicesBenchmark.cc:80:22: error: unused variable ‘svcId’ [-Werror=unused-variable]
> 80 | long svcId = celix_bundleContext_findServiceWithOptions(cCtx, &opts);
> | ^~~~~
> /home/oipo-unencrypted/Programming/celix-apache/libs/framework/benchmark/src/LookupServicesBenchmark.cc:86:22: error: unused variable ‘svcId’ [-Werror=unused-variable]
> 86 | long svcId = celix_bundleContext_findService(cCtx, IService::NAME);
> | ^~~~~
> /home/oipo-unencrypted/Programming/celix-apache/libs/framework/benchmark/src/LookupServicesBenchmark.cc:94:22: error: unused variable ‘svcId’ [-Werror=unused-variable]
> 94 | long svcId = ctx->findServiceWithName<IService>(IService::NAME, filter);
> | ^~~~~
> /home/oipo-unencrypted/Programming/celix-apache/libs/framework/benchmark/src/LookupServicesBenchmark.cc:100:22: error: unused variable ‘svcId’ [-Werror=unused-variable]
> 100 | long svcId = ctx->findServiceWithName<IService>(IService::NAME);
> ```
Replaced assert with State::SkipWithError
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #324: Feature/gcc 4 9 0 fixes
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #324:
URL: https://github.com/apache/celix/pull/324#issuecomment-788189667
# [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=h1) Report
> Merging [#324](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=desc) (5ac6c8d) into [master](https://codecov.io/gh/apache/celix/commit/7a0cc48ba01ee6313759c3aa6e4f858a483ba437?el=desc) (7a0cc48) will **increase** coverage by `0.27%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/324/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
+ Coverage 71.00% 71.27% +0.27%
==========================================
Files 175 175
Lines 31552 31554 +2
==========================================
+ Hits 22402 22491 +89
+ Misses 9150 9063 -87
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/include/celix/Utils.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9VdGlscy5o) | `93.75% <ø> (ø)` | |
| [libs/framework/include/celix/Bundle.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9CdW5kbGUuaA==) | `100.00% <100.00%> (ø)` | |
| [libs/framework/include/celix/BundleActivator.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9CdW5kbGVBY3RpdmF0b3IuaA==) | `69.69% <100.00%> (+0.94%)` | :arrow_up: |
| [libs/framework/include/celix/Properties.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9Qcm9wZXJ0aWVzLmg=) | `98.71% <100.00%> (+0.03%)` | :arrow_up: |
| [libs/framework/include/celix/Trackers.h](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9UcmFja2Vycy5o) | `90.93% <100.00%> (-0.04%)` | :arrow_down: |
| [.../pubsub\_admin\_zmq/v2/src/pubsub\_zmq\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3ptcS92Mi9zcmMvcHVic3ViX3ptcV90b3BpY19zZW5kZXIuYw==) | `74.05% <0.00%> (-0.88%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_admin\_udp\_mc/src/large\_udp.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3VkcF9tYy9zcmMvbGFyZ2VfdWRwLmM=) | `33.51% <0.00%> (+22.90%)` | :arrow_up: |
| [...ubsub\_admin\_udp\_mc/src/pubsub\_udpmc\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/324/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3VkcF9tYy9zcmMvcHVic3ViX3VkcG1jX3RvcGljX3NlbmRlci5j) | `84.39% <0.00%> (+28.32%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=footer). Last update [7a0cc48...5ac6c8d](https://codecov.io/gh/apache/celix/pull/324?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org