Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: use conan for dependencies #1818

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

n-morales
Copy link
Contributor

@n-morales n-morales commented Dec 13, 2024

This is gonna be a draft because there are still some things that need to be worked out. But this should be a decent proof of concept.

Note this is based on top of #1746 so the file diff will be against that

I would love feedback and if y'all are on linux try this PR out (note you have to rebuild the docker image locally).

Goal of this work

The goal of this work is to make the numerous dependencies of Recoil consistent across platforms. Right now Linux, Windows MinGW, and Windows MSVC all use different dependency versions. As part of this, the intent is to make it very easy to add new dependencies instead of having to wrangle them into the build system as submodules or add them to a completely different repo (e.g. spring-static-libs).

A secondary goal is to make it significantly easier for new developers to get started. The ideal workflow is:

  1. Clone repo
  2. Run conan install .
  3. Run cmake and cmake --build

Caveats

  • Right now I've only tested this on the linux docker v2 build. It probably won't work in any other way
  • You need to patch pr-downloader (see below)
  • If you are trying out this PR you need to build the docker image locally
  • Our compiler/flag configuration means that no binaries are available in the conan remote and so unless you have them cached the build process will build all dependencies from source. This should just be a one-time thing but we should look into hosting conan binary packages (gitlab and gitea support this for example. I don't think github does).
  • This only replaces dependencies found in spring-static-libs. Once this PR is complete we won't need that repo anymore, or the two windows based ones :)
  • Submodule dependencies are left untouched -- for now
  • This requires a very recent version of conan, or needs a version of conan configured on using the new conan2 center remote. In the docker v2 build I install conan so you don't need to on your host machine if you are using the docker build
  • I'm not 100% sure the build as is can generated releases properly, as I'm not familiar with the release process.
  • I haven't generated a conan lockfile yet, but plan on doing so before the PR is merged.
  • It goes without saying that CI right now will fail horribly.

Patch required for pr-downloader

diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt	(revision bdac30330eccb5ec73da299922491f3f4ee8debe)
+++ b/CMakeLists.txt	(date 1734107372971)
@@ -31,7 +31,7 @@
 find_package(Threads REQUIRED)
 
 # CURL
-if(UNIX)
+if(UNIX AND NOT PR_DOWNLOADER_FIND_CURL_NO_PKGCONFIG)
     find_package(PkgConfig REQUIRED)
     pkg_check_modules(libcurl REQUIRED IMPORTED_TARGET libcurl>=7.84)
     add_library(prd::libcurl ALIAS PkgConfig::libcurl)

More information

The docker image will generate (and mount as a volume) a new directory alongside .cache called .conan2-{linux|windows}. This is the Conan home directory and will persist so you don't have to rebuild binaries all the time. Deleting this folder will make conan to rebuild all dependencies the next time you run a build.

The conan "build folder" (i.e. that contains all the cmake scripts and so on) is in build-{linux|windows}/conan.

There is a new stage in the docker v2 build script called "deps". You can pass in the --deps flag to just install conan dependencies. This will re-use any binaries of dependencies that are cached.

I'll make a PR for the tool

Comment on lines +14 to +28
libgl-dev \
libglu-dev \
xorg-dev \
libxcb1-dev \
libxcb-util-dev \
libxcb-render-util0-dev \
libxcb-xkb-dev \
libxcb-icccm4-dev \
libxcb-image0-dev \
libxcb-keysyms1-dev \
libxcb-xinerama0-dev \
libxcb-cursor-dev \
libxcb-composite0-dev \
libxcb-ewmh-dev \
libxcb-res0-dev \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are added because conan's xorg/system package uses system packages.

@@ -0,0 +1,9 @@
#!/bin/bash

conan graph info \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prints out useful info (i.e. what all packages are being built and what packages are depending on other packages)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's non-obvious enough to require a comment here, it probably warrants a proper comment in the file. Ditto for the other one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do so, I mostly commented as I'm not sure how familiar people are with conan

self.options["glew"].with_glu = "system"
self.options["openal-soft"].shared = True
if self.settings.os == "Linux":
self.options["sdl"].wayland = False # Disable wayland build for now
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I use engine natively on Wayland

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now I had a lot of issues with the SDL recipe to actually build with wayland. I'll try and get it working before making the PR ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants