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/03 12:07:00 UTC

[GitHub] [celix] pnoltes opened a new pull request #408: Updates conanfile.py and jansson linking for macos setup

pnoltes opened a new pull request #408:
URL: https://github.com/apache/celix/pull/408


   I started testing the Celix Conan setup on a macOS system and something did not work:
   
   - conan install failed, because both cmake 3.17.5 and libzip 1.7.3 needed an different openssl version (1.1.1m and 1.1.1n). Currently I solved this by explicitly adding a openssl dep, but maybe this can be solved cleaner
   - `-Wl,--unresolved-symbols=ignore-in-shared-libs` does not work on macOS, so conanfile now does this behind an `if`
   - For me `self.version` was not so, so made this optional
   - The conan generated Findjansson creates a `jansson::jansson` imported target and not a `Jansson` imported target which was generated by Celix's FIndJansson.cmake. This has now been updated to `jansson::jansson`
   
   
   I still have some troubles with running the Celix tests and I will look into this a bit later. 
   


-- 
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 pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086863639


   Changes to self.version and jansson are OK. If the above does not work on MacOS, please let me know. 
   To run Celix test, we may need activate_run.sh under build directory.


-- 
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 change in pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on a change in pull request #408:
URL: https://github.com/apache/celix/pull/408#discussion_r841219725



##########
File path: conanfile.py
##########
@@ -193,6 +193,7 @@ def requirements(self):
         self.options['zlib'].shared = True
         self.requires("libuuid/1.0.3")
         self.options['libuuid'].shared = True
+        self.requires("openssl/[>=1.1.1n <2.0.0]")

Review comment:
       We can fixed openssl version to 1.1.1n due to the endless loop vulnerability in OpenSSL.




-- 
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 pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086862230


   It's weired cmake and libzip conflict with each other for openssl requirement, since they are in completely different contexts: 
   https://docs.conan.io/en/latest/devtools/build_requires.html?highlight=tool_requires
   
   What if you try `-pr:b default pr:h default` instead of `-pr default` when specifying build profile? As far as I know, most problems of tool requirements are due to lack of `-pr:b -pr:h`.
   
   Also `cmake` requirement can really be removed with no harm. I add this intentionally to show off the power of Conan, setup complete develop environment (including cross tool chain, build system, and build utilities) in a single command. 


-- 
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 edited a comment on pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng edited a comment on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086863639


   Change to jansson is OK. 
   For others to use celix, we have to invoke `conan create`, in which version is a must.
   If the above proposed solutions do not work on MacOS, please let me know. 
   To run Celix test, we may need activate_run.sh under build directory.


-- 
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 change in pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #408:
URL: https://github.com/apache/celix/pull/408#discussion_r841255156



##########
File path: conanfile.py
##########
@@ -226,10 +227,14 @@ def _configure_cmake(self):
             self._cmake.definitions[opt.upper()] = self.options.get_safe(opt, False)
         self._cmake.definitions["CMAKE_PROJECT_Celix_INCLUDE"] = os.path.join(self.build_folder, "conan_paths.cmake")
         # the following is workaround for https://github.com/conan-io/conan/issues/7192
-        self._cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,--unresolved-symbols=ignore-in-shared-libs"
+        if self.settings.os == "Linux":
+            self._cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,--unresolved-symbols=ignore-in-shared-libs"
         self.output.info(self._cmake.definitions)
-        v = tools.Version(self.version)
-        self._cmake.configure(defs={'CELIX_MAJOR': v.major, 'CELIX_MINOR': v.minor, 'CELIX_MICRO': v.patch})
+        if self.version is None:

Review comment:
       Thanks. Yes, this was an issue on my side. 




-- 
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 pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086859878


   It seems that `-Wl,--unresolved-symbols=ignore-in-shared-libs` equivalence on MacOS is [--allow-shlib-undefined](https://man.openbsd.org/ld#allow-shlib-undefined)
   
   We need to turn it on, because:
   [--no-allow-shlib-undefined](https://man.openbsd.org/ld#no-allow-shlib-undefined)
       Do not allow unresolved references in shared libraries. This option is enabled by default when linking an executable.


-- 
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 pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086856853


   I shall investigate `-Wl,--unresolved-symbols=ignore-in-shared-libs` for MacOS.


-- 
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 change in pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on a change in pull request #408:
URL: https://github.com/apache/celix/pull/408#discussion_r841221545



##########
File path: conanfile.py
##########
@@ -226,10 +227,14 @@ def _configure_cmake(self):
             self._cmake.definitions[opt.upper()] = self.options.get_safe(opt, False)
         self._cmake.definitions["CMAKE_PROJECT_Celix_INCLUDE"] = os.path.join(self.build_folder, "conan_paths.cmake")
         # the following is workaround for https://github.com/conan-io/conan/issues/7192
-        self._cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,--unresolved-symbols=ignore-in-shared-libs"
+        if self.settings.os == "Linux":
+            self._cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,--unresolved-symbols=ignore-in-shared-libs"
         self.output.info(self._cmake.definitions)
-        v = tools.Version(self.version)
-        self._cmake.configure(defs={'CELIX_MAJOR': v.major, 'CELIX_MINOR': v.minor, 'CELIX_MICRO': v.patch})
+        if self.version is None:

Review comment:
       version is a must when invoking `conan create`




-- 
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 pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #408:
URL: https://github.com/apache/celix/pull/408#issuecomment-1086916852


   I will continue the discussion on #401 (to prevent additional confusion 😉 )


-- 
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 change in pull request #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
PengZheng commented on a change in pull request #408:
URL: https://github.com/apache/celix/pull/408#discussion_r841214321



##########
File path: conanfile.py
##########
@@ -193,6 +193,7 @@ def requirements(self):
         self.options['zlib'].shared = True
         self.requires("libuuid/1.0.3")
         self.options['libuuid'].shared = True
+        self.requires("openssl/[>=1.1.1n <2.0.0]")

Review comment:
       Conan upstream requirement can be overridden downstream. I intentional did so for openssl when integrating Celix with legacy application using Conan. May I have a look at the complete console output? I only have Ubuntu PC at hand.
   
   IIRC, openssl does not use semantic versioning, thus version range does not work:
   
   > There are some packages that do not follow semver. A popular one would be the OpenSSL package with versions as 1.0.2n. They cannot be used with version-ranges. 
   
   https://docs.conan.io/en/latest/versioning/version_ranges.html?highlight=version%20range




-- 
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 #408: Updates conanfile.py and jansson linking for macos setup

Posted by GitBox <gi...@apache.org>.
pnoltes merged pull request #408:
URL: https://github.com/apache/celix/pull/408


   


-- 
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