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

Use the Conan package manager for dependencies #4223

Closed
wants to merge 21 commits into from

Conversation

thejohnfreeman
Copy link
Collaborator

@thejohnfreeman thejohnfreeman commented Jul 7, 2022

This changeset introduces a conanfile.py (and a Conan recipe for RocksDB) to enable building the package with Conan, choosing more recent default versions of our dependencies. It removes almost all of the CMake build files related to dependencies, and the configurations for Travis CI and GitLab CI. There are very few modifications to source files. A new set of cross-platform build instructions are written in BUILD.md.

Please check out this branch and try to follow the build instructions. It should be a seamless process for everyone. If you encounter any problems, then we need to fix either the build files or the build instructions. I welcome reports from all contributors.

This changeset includes one example GitHub Action workflow for each of the Big Three platforms (Linux, OSX, Windows). We will want to eventually test a broader range of configurations, but I will leave that for a future changeset.

This changeset leaves in all of the old build instructions. I think we should remove them, in favor of the new BUILD.md, but I'm not taking that step in this changeset. I would like to hear what others think.

@cjcobb23
Copy link
Contributor

cjcobb23 commented Jul 8, 2022

@legleux can we confirm this does not break clio's build process?

- Let Windows developers use a non-debug runtime in a debug build.
- Make our build instructions work for users with Conan <1.48.
@seelabs
Copy link
Collaborator

seelabs commented Jul 11, 2022

Worked for me on linux, gcc 11.1

@greg7mdp
Copy link
Contributor

Worked for me on Windows 11, Visual Studio 2019

@gregtatcam
Copy link
Collaborator

gregtatcam commented Jul 11, 2022

Worked for me on OSX (M1) Monterey 12.4, Apple clang version 13.1.6.

@natenichols
Copy link
Contributor

Worked for me on Ubuntu 22.04, gcc (Ubuntu 11.2.0-19ubuntu1) 11.2.0

@scottschurr
Copy link
Collaborator

Worked for me on macOS Catalina 10.15.7, Apple clang version 12.0.0 (clang-1200.0.32.21).

@HowardHinnant
Copy link
Contributor

On macOS 12.4 I'm getting a linker warning out of boost concerning weak symbols. I believe the fix is to put visibility=global as an option to the b2 command. I don't know how to do that for this build. But if you can tell me what to do, I'll test that out.

@thejohnfreeman
Copy link
Collaborator Author

@HowardHinnant try -o boost:visibility=global with your conan install command.

@HowardHinnant
Copy link
Contributor

That did the trick!

@ckeshava
Copy link
Collaborator

Worked for me on Mac (Darwin Kernel Version 21.5.0), Apple clang version 13.1.6

@nbougalis nbougalis requested review from ximinez and RichardAH July 12, 2022 21:41
@nbougalis
Copy link
Contributor

Requesting reviews from @ximinez, @legleux and @RichardAH. Thank you :)

@legleux
Copy link
Collaborator

legleux commented Jul 13, 2022

I cannot build this on centos 7 with devtoolset-10 or 11 and gcc11 is what we deciced we'd be building with.

@RichardAH
Copy link
Collaborator

I cannot build this on centos 7 with devtoolset-10 or 11 and gcc11 is what we deciced we'd be building with.

We also use a similar setup for our static builds: https://github.com/phusion/holy-build-box
Centos 7 and toolset 9.
Linking fails on rocks

/opt/rh/devtoolset-9/root/usr/libexec/gcc/x86_64-redhat-linux/9/ld: /hbb_exe/lib/libstdc++.a(atexit_thread.o): relocation R_X86_64_32 against symbol `__pthread_key_create@@GLIBC_2.2.5' can not be used when making a shared object; recompile with -fPIC

I'd also request / recommend @thejohnfreeman WasmEdge https://github.com/WasmEdge/ be added to the new build system, as it is needed for building Hooks. WasmEdge requires LLVM12, and might be slightly challenging to add.

@thejohnfreeman
Copy link
Collaborator Author

I was able to build and pass all tests in Docker container centos:7 for devtoolset versions 8, 9, and 11 using the below script. I assume 10 will work just the same, but I didn't take the time to test. The only thing I had to change from the build instructions was to use --build instead of --build missing for the conan install command, because CentOS uses an ancient version of libstdc++ that is incompatible with the binaries cached in Conan Center.

Please try again. If you still hit errors, please copy the commands you ran and their output to a Gist and link it here.

yum install -y centos-release-scl https://packages.endpointdev.com/rhel/7/os/x86_64/endpoint-repo.x86_64.rpm

version=9 # Change to 8, 10, or 11 if you want.
yum install -y devtoolset-${version} git python3 vim
scl enable devtoolset-${version} bash

gcc --version # 8.3.1, 11.2.1
git --version # 2.36.0

pip3 install cmake conan

cmake --version # 3.22.5
conan --version # 1.50.0

conan profile new --detect default
conan profile update settings.compiler.libcxx=libstdc++11 default

mkdir /home/root
cd /home/root

git clone https://github.com/thejohnfreeman/rippled -b conan
cd rippled

conan export external/*
mkdir .build
cd .build

conan install .. --output-folder . --build --settings build_type=Release

cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=Release ..

cmake --build .

./rippled --unittest 2>&1 | tee test.log

WasmEdge is out of scope for this PR. The purpose of this PR is just to migrate the existing build to Conan.

@legleux
Copy link
Collaborator

legleux commented Jul 14, 2022

I'm not having any issues other than a fairly reproducible failure on this test in docker containers.
I've seen it occasionally before and don't believe it's related to the PR in any way.
@undertome mentioned there's a maxWait timeout value that could be adjusted if need be.

ripple.rpc.NodeToShardRPC

0> #7 failed: NodeToShardRPC_test.cpp(252)
0> FTL:ShardStore shard 10. failed storing to SQLite databases. Ledger hash B63DC3FA5532F23110F7CF0F4AEA29BA2C121ADE9C864D04D4CE00D659ADAB39. Ledger sequence 2658
0> ripple.rpc.NodeToShardRPC Stop
0> FTL:ShardStore shard 2. failed storing to SQLite databases. Ledger hash F8CCCE4CEBC3DD90AE8FDAA9FDC71A9170D26A1895DFB8678028E58C5337031A. Ledger sequence 762

My previous issue with not being able to build was what John said and mitigated by building all dependencies locally with --build

conanfile.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

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

Looks like all my build issues have been resolved.

@intelliot intelliot requested a review from cjcobb23 November 28, 2022 21:13
@intelliot intelliot requested review from legleux and seelabs November 29, 2022 05:39
@intelliot intelliot added Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc) Needs Review labels Nov 29, 2022
Copy link
Collaborator

@seelabs seelabs left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this. Looking forward to seeing this merged. Nice job on this!

@intelliot intelliot added Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. and removed Needs Review labels Dec 2, 2022
@intelliot
Copy link
Collaborator

Just some notes that might help other newbies like me. I am on an M1 Mac.

  1. build instructions: https://github.com/XRPLF/rippled/blob/20190cf5931ed51cba5ed3aee38a67893aa50721/BUILD.md
  2. pip install conan - to install Conan
  3. If you get this error:
    ERROR: Profile not found: default
    
    Then do: conan profile new --detect default
  4. If you get this error:
    
    ERROR: Invalid setting 'libstdc++11' is not a valid 'settings.compiler.libcxx' value.
    Possible values are ['libstdc++', 'libc++']
    Read "http://docs.conan.io/en/latest/faq/troubleshooting.html#error-invalid-setting"
    
    Then do: conan profile update settings.compiler.libcxx=libc++ default

@legleux legleux mentioned this pull request Dec 9, 2022
@intelliot
Copy link
Collaborator

@thejohnfreeman can you rebase or squash this to signed commits?

We are now requiring commits to be signed before/when merging. Let us know if you have any problems getting this signed.

thejohnfreeman added a commit to thejohnfreeman/rippled that referenced this pull request Dec 14, 2022
@intelliot intelliot closed this in c3a9f3d Dec 16, 2022
@thejohnfreeman thejohnfreeman deleted the conan branch December 20, 2022 18:50
dangell7 pushed a commit to Transia-RnD/rippled that referenced this pull request Dec 27, 2022
Introduces a conanfile.py (and a Conan recipe for RocksDB) to enable building the package with Conan, choosing more recent default versions of dependencies. It removes almost all of the CMake build files related to dependencies, and the configurations for Travis CI and GitLab CI. A new set of cross-platform build instructions are written in BUILD.md.

Includes example GitHub Actions workflow for each of Linux, macOS, Windows.

* Test on macos-12

We use the <concepts> library which was not added to Apple Clang until
version 13.1.6. The default Clang on macos-11 (the sometimes current
version of macos-latest) is 13.0.0, and the default Clang on macos-12 is
14.0.0.

Closes XRPLF#4223.
dangell7 pushed a commit to Transia-RnD/rippled that referenced this pull request Mar 5, 2023
Introduces a conanfile.py (and a Conan recipe for RocksDB) to enable building the package with Conan, choosing more recent default versions of dependencies. It removes almost all of the CMake build files related to dependencies, and the configurations for Travis CI and GitLab CI. A new set of cross-platform build instructions are written in BUILD.md.

Includes example GitHub Actions workflow for each of Linux, macOS, Windows.

* Test on macos-12

We use the <concepts> library which was not added to Apple Clang until
version 13.1.6. The default Clang on macos-11 (the sometimes current
version of macos-latest) is 13.0.0, and the default Clang on macos-12 is
14.0.0.

Closes XRPLF#4223.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build System Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc) Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.