feat: Adds conan support via a conan 2.0 recipe #1066

MikeRavenelle · 2024-01-19T23:27:05Z

Added a conan 2.0 recipe
Modified the CMakeLists.txt to check for presence of conan_toolchain.cmake and include libs accordingly(for building with or without conan)
Added .md file for conan usage

I went ahead and tested the conan package on macOS and linux with example programs(using conan). I also tested the existing build procedure on macOS to make sure I did not break anything. I did no testing on Windows as I do not currently have a Windows development machine ready for use.

I would personally like to see more testing done on other users machines before a package is submitted to the conan center.

A review of my CMakeLists.txt changes also is not be a bad idea since I will admit I am not a CMake expert.

How does versioning work? I used 0.1 as a placeholder until I got feedback.

Thanks!

Code change checklist

I have ensured that all methods and functions are fully documented using doxygen style comments.
My code follows the coding style guide.
I tested that my change works before raising the PR.
I have ensured that I did not break any existing API calls.
I have not built my pull request using AI, a static analysis tool or similar without any human oversight.

netlify · 2024-01-19T23:27:11Z

✅ Deploy Preview for dpp-dev ready!

Name	Link
🔨 Latest commit	`6d1dbfa`
🔍 Latest deploy log	https://app.netlify.com/sites/dpp-dev/deploys/671964a54b4de3000856130d
😎 Deploy Preview	https://deploy-preview-1066--dpp-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

CLAassistant · 2024-01-19T23:27:12Z

All committers have signed the CLA.

…into conan_support

… CMakeLists.txt, root CMakeLists now detects conan and uses its cmake if detected

…CE_SUPPORT by default

…ry-conan folder

Jaskowicz1 · 2024-01-25T11:30:46Z

Can we automate the versioning to match the ${DPP_VERSION} cmake var? This will help automate the process. If not, put it to our latest release (10.0.29) :)

Jaskowicz1 · 2024-01-25T13:00:31Z

I've also added the "Help Wanted" label on this PR for anyone who wants to test the conan recipe (more specially on Windows).

Like you said, we need people to test the conan recipe so asking for people to join in and test is ideal.

docpages/install/install-conan.md

…folder

braindigitalis · 2024-02-03T05:22:39Z

-1073741819 is error C0000005 which is like a segmentation fault.
It's likely something you have installed is not set up correctly or your system has a hardware issue.

braindigitalis · 2024-02-19T10:31:40Z

any movement on this?

Jaskowicz1 · 2024-03-28T16:57:53Z

Renamed PR to correctly align with our conventions.

Jaskowicz1 · 2024-03-28T17:02:22Z

I'm going to run some tests on this PR for windows, just to make sure it all works.

library-conan/conanfile.py

Jaskowicz1 · 2024-03-29T00:11:52Z

Even with the suggested changes, it appears Conan just can't find D++ (despite all the files being there and cmake loading all the variables).

I end up getting:

-- Selecting Windows SDK version 10.0.22000.0 to target Windows 10.0.22631.
-- Conan: Target declared 'dpp::dpp'
CMake Error at build/cmakedeps_macros.cmake:67 (message):
  Library 'dpp' not found in package.  If 'dpp' is a system library, declare
  it with 'cpp_info.system_libs' property
Call Stack (most recent call first):
  build/dpp-Target-release.cmake:23 (conan_package_library_targets)
  build/dppTargets.cmake:24 (include)
  build/dpp-config.cmake:16 (include)
  CMakeLists.txt:10 (find_package)

As far as I understand, it's a Conan bug? I could be super wrong but I really don't understand.

memsharded

I'd suggest using the test_package functionality that can test automatically the package when created with conan create .

library-conan/conanfile.py

memsharded · 2024-03-29T21:30:49Z

library-conan/conanfile.py

+        cmake.install()
+
+    def package_info(self):
+        self.cpp_info.libs = ["dpp"]


As commented in conan-io/conan#15985 (comment) this might need more info, as the folders generated in cmake.install() are not matching the defaults.

memsharded · 2024-03-29T21:31:38Z

library-conan/conanfile.py

+    default_options = {"shared": False, "fPIC": True}
+
+    def build_requirements(self):
+        self.requires("nlohmann_json/3.11.2", headers=True, libs=True)


headers=True, lib=True is the default, it is recommended to drop them and use self.requires()

memsharded · 2024-03-29T21:32:20Z

library-conan/conanfile.py

+        tc.cache_variables["CONAN_EXPORTED"] = True
+        tc.cache_variables["BUILD_VOICE_SUPPORT"] = True
+        tc.cache_variables["DPP_BUILD_TEST"] = False
+        tc.cache_variables["BUILD_SHARED_LIBS"] = False


If it is always going to be a static library, then better drop the shared option and declare package_type = "static-library"

Yeah this one is an odd one, we normally don't allow static but for some reason, trying to build shared results in ZLib linking twice.

I do believe we would like to build as a shared library (as this is what we do normally) but, as seen, are kinda forced into static due to ZLib shenanigans.

It might be in Conan 1.X, that it was overlinking (propagating too much linkage requirements). In theory, Conan 2 with the right traits/package_type, Conan should avoid this overlinking, and allow shared builds to encapsulate zlib static libs as implementation detail if desired. Feel free to submit new tickets to Conan repo for further details or discussion.

It might be in Conan 1.X, that it was overlinking (propagating too much linkage requirements). In theory, Conan 2 with the right traits/package_type, Conan should avoid this overlinking, and allow shared builds to encapsulate zlib static libs as implementation detail if desired. Feel free to submit new tickets to Conan repo for further details or discussion.

Sorry for the delayed reply, the zlib issue has been a thing as long as Mike has been trying to make this PR (I mentioned it in the issue that I posted on the conan page too!). When I then tested this PR a few days ago, it was still persisting on Conan 2.2.2.

I think it would be great to isolate this issue, create a ticket for it in the Conan repos (hopefully with something that we can reproduce on our end) and investigate this properly.

memsharded · 2024-03-29T21:34:34Z

library-conan/CMakeLists.txt

+message("-- INFO: Conan detected... finding packages")
+find_package(OpenSSL REQUIRED COMPONENTS SSL Crypto)
+find_package(ZLIB REQUIRED)
+find_package(libsodium REQUIRED)
+find_package(Opus)
+find_package(Threads REQUIRED)
+find_package(Git QUIET)
+


I understand that using Conan might need some find_package() that maybe are not used when not using Conan because some other strategy like FetchContent is done, but why the big CMakeLists.txt and all the code below? In theory Conan can integrate quite cleanly, without requiring modifying CMakeLists code.

We wanted the CMakeLists.txt to be separate (see library-vcpkg), this is just something we do with VCPKG so we wanted the same format to follow.

It should also be noted, our main CMakeLists.txt doesn't do everything hence why we have library/CMakeLists.txt, library-vcpkg/CMakeLists.txt, and now library-conan/CMakeLists.txt.

What is a bit unexpected to me is that such a file wouldn't be way shorter, at least for Conan. Besides some potential minor differences in the dependencies (like the above commented possible find_package() vs FetchContent). So it shouldn't be necessary much different logic to build the project. There are full blocks of CMake code in library-conan/CMakeLists.txt that are completely unrelated to Conan, and also seem completely unrelated to dependencies too, basic definition of the library like globbing files, handling compiler flags, etc. So I would expect that this would be common CMakeLists.txt, and the specializations for vcpkg or Conan would be way more minimal.

Am I missing something here?

memsharded · 2024-06-03T07:58:31Z

Hi @MikeRavenelle @Jaskowicz1

If you need some other advice or help to move this forward, please let me know, I can keep reviewing and advising. Thanks!

Jaskowicz1 · 2024-06-09T13:13:33Z

Hi @MikeRavenelle @Jaskowicz1

If you need some other advice or help to move this forward, please let me know, I can keep reviewing and advising. Thanks!

Hi there, sorry for the late reply, cheers for the shout! I'll be looking to pick this up again very shortly so I'll give you a shout if needs be, thanks for the help so far!

github-actions · 2024-08-09T02:23:01Z

This pull request has had no activity and is being marked as stale. If you still wish to continue with this pull request please comment to reopen it.

braindigitalis · 2024-08-09T08:38:44Z

is there any progress with this?

library-conan/conanfile.py

github-actions · 2024-10-14T02:37:34Z

This pull request has had no activity and is being marked as stale. If you still wish to continue with this pull request please comment to reopen it.

braindigitalis

all the cmakelists in here are now completely invalidated by the changes for DAVE and removal of libsodium. If we do proceed with this, it will need to be completely redone. I recommend closing this PR and making a new one.

Co-authored-by: James <[email protected]>

braindigitalis · 2024-10-23T22:56:32Z

This is being closed in preference of a new pr which is hopefully simpler.
The conanfile in the new pr builds, and in theory is ok, but needs feedback and probably adjustment.
See PR #1308

MikeRavenelle added 6 commits January 19, 2024 10:49

First test of SCM conan 2.0

e838136

Fixed requires

7277b21

Added package info for libs

60d31d4

renamed libdpp to dpp

5947864

Made CmakeLists work for both conan and non conan users

25943bb

Added documentation for using conan

f7ee95b

MikeRavenelle added 7 commits January 19, 2024 17:59

Merge branch 'dev' into conan_support

c2c2854

Changed how conan detected and changed how opus is checked

3abef91

Merge branch 'conan_support' of https://github.com/MikeRavenelle/DPP …

7e3a98b

…into conan_support

Added DPP_NO_CONAN to pipeline, moved conan cmake out of main library…

857c4fc

… CMakeLists.txt, root CMakeLists now detects conan and uses its cmake if detected

Reset ci.yml since github wont let me commit

0c5cbf0

Fixed 1 error and tabbing

84792c8

Added missing endif()

d5dd3f6

MikeRavenelle force-pushed the conan_support branch 3 times, most recently from a5aa313 to 0b193a1 Compare January 21, 2024 15:29

Added parent folder for commit instead of recipe folder and BUILD_VOI…

ffa9cd5

…CE_SUPPORT by default

MikeRavenelle force-pushed the conan_support branch from 0b193a1 to ffa9cd5 Compare January 21, 2024 15:30

MikeRavenelle added 2 commits January 21, 2024 09:36

Fixed CMake variables for opus and libsodium

f4ecfc3

Removed load from conan.tools.files and moved conan recipe into libra…

68560d6

…ry-conan folder

Jaskowicz1 added build Issue or Pull Request related to the build process packages Issue or Pull Request related to packaging help wanted Extra attention is needed labels Jan 25, 2024

Updated the version number

f37ce14

braindigitalis requested changes Jan 25, 2024

View reviewed changes

docpages/install/install-conan.md Outdated Show resolved Hide resolved

Remove docpage from branch so we can control docpage separately.

ea84883

Removed possibly uneeded windows flags and added exclusion for win32 …

de0c141

…folder

braindigitalis approved these changes Feb 3, 2024

View reviewed changes

Jaskowicz1 changed the title ~~Adds conan support via a conan 2.0 recipe~~ feat: Adds conan support via a conan 2.0 recipe Mar 28, 2024

Jaskowicz1 requested changes Mar 28, 2024

View reviewed changes

library-conan/conanfile.py Show resolved Hide resolved

Added flags for tests and BUILD_SHARED_LIBS

2f02ae7

memsharded reviewed Mar 29, 2024

View reviewed changes

github-actions bot added the Stale label Jun 3, 2024

braindigitalis removed the Stale label Jun 3, 2024

brainboxdotcc deleted a comment from github-actions bot Jun 3, 2024

Jaskowicz1 mentioned this pull request Jul 24, 2024

Package manager support #335

Open

11 tasks

github-actions bot added the Stale label Aug 9, 2024

github-actions bot removed the Stale label Aug 10, 2024

memsharded reviewed Aug 14, 2024

View reviewed changes

library-conan/conanfile.py Outdated Show resolved Hide resolved

github-actions bot added the Stale label Oct 14, 2024

braindigitalis requested changes Oct 20, 2024

View reviewed changes

github-actions bot removed the Stale label Oct 21, 2024

braindigitalis and others added 2 commits October 23, 2024 22:02

Update library-conan/conanfile.py

4ea4362

Co-authored-by: James <[email protected]>

Update library-conan/conanfile.py

6d1dbfa

Co-authored-by: James <[email protected]>

braindigitalis closed this Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds conan support via a conan 2.0 recipe #1066

feat: Adds conan support via a conan 2.0 recipe #1066

MikeRavenelle commented Jan 19, 2024

netlify bot commented Jan 19, 2024 •

edited

Loading

CLAassistant commented Jan 19, 2024 •

edited

Loading

Jaskowicz1 commented Jan 25, 2024

Jaskowicz1 commented Jan 25, 2024

braindigitalis commented Feb 3, 2024

braindigitalis commented Feb 19, 2024

Jaskowicz1 commented Mar 28, 2024

Jaskowicz1 commented Mar 28, 2024

Jaskowicz1 commented Mar 29, 2024

memsharded left a comment

memsharded Mar 29, 2024

memsharded Mar 29, 2024

memsharded Mar 29, 2024

Jaskowicz1 Mar 29, 2024 •

edited

Loading

memsharded Mar 31, 2024

Jaskowicz1 Apr 3, 2024

memsharded Apr 3, 2024

memsharded Mar 29, 2024

Jaskowicz1 Apr 3, 2024

Jaskowicz1 Apr 3, 2024 •

edited

Loading

memsharded Apr 3, 2024

memsharded commented Jun 3, 2024

Jaskowicz1 commented Jun 9, 2024

github-actions bot commented Aug 9, 2024

braindigitalis commented Aug 9, 2024

github-actions bot commented Oct 14, 2024

braindigitalis left a comment

braindigitalis commented Oct 23, 2024

feat: Adds conan support via a conan 2.0 recipe #1066

feat: Adds conan support via a conan 2.0 recipe #1066

Conversation

MikeRavenelle commented Jan 19, 2024

Code change checklist

netlify bot commented Jan 19, 2024 • edited Loading

✅ Deploy Preview for dpp-dev ready!

CLAassistant commented Jan 19, 2024 • edited Loading

Jaskowicz1 commented Jan 25, 2024

Jaskowicz1 commented Jan 25, 2024

braindigitalis commented Feb 3, 2024

braindigitalis commented Feb 19, 2024

Jaskowicz1 commented Mar 28, 2024

Jaskowicz1 commented Mar 28, 2024

Jaskowicz1 commented Mar 29, 2024

memsharded left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jaskowicz1 Mar 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jaskowicz1 Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memsharded commented Jun 3, 2024

Jaskowicz1 commented Jun 9, 2024

github-actions bot commented Aug 9, 2024

braindigitalis commented Aug 9, 2024

github-actions bot commented Oct 14, 2024

braindigitalis left a comment

Choose a reason for hiding this comment

braindigitalis commented Oct 23, 2024

netlify bot commented Jan 19, 2024 •

edited

Loading

CLAassistant commented Jan 19, 2024 •

edited

Loading

Jaskowicz1 Mar 29, 2024 •

edited

Loading

Jaskowicz1 Apr 3, 2024 •

edited

Loading