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