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

chore(build): prefer system deps for fmt/nlohmann_json/range-v3 #15414

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

chenrui333
Copy link

While regression build solidity, I saw it always pull the deps rather than using the system deps, thus updating the build to prefer system deps

@chenrui333 chenrui333 changed the title chore(build): prefer system deps chore(build): prefer system deps for fmt/nlohmann_json/range-v3 Sep 8, 2024
Copy link

github-actions bot commented Sep 8, 2024

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

@cameel
Copy link
Member

cameel commented Sep 8, 2024

We have the IGNORE_VENDORED_DEPENDENCIES=ON option for this:

solidity/CMakeLists.txt

Lines 40 to 45 in 09e9aa6

option(
IGNORE_VENDORED_DEPENDENCIES
"Ignore libraries provided as submodules of the repository and allow CMake to look for \
them in the typical locations, including system-wide dirs."
OFF
)

The only difference is that it does not run find_package(), but that could be added as an else here:

solidity/CMakeLists.txt

Lines 60 to 64 in 09e9aa6

if (NOT IGNORE_VENDORED_DEPENDENCIES)
include(fmtlib)
include(nlohmann-json)
include(range-v3)
endif()

Which I'd expect to be necessary. I wonder why it worked without it for @aarlt (who added it) though?

@aarlt
Copy link
Member

aarlt commented Sep 9, 2024

Hey @chenrui333! Thanks for your contribution. Initially I created this IGNORE_VENDORED_DEPENDENCIES mechanism so that the solidity libraries can be easily used within vcpkg. The idea of that first version was, that a "root" cmake file will just search for the dependencies. However, we changed some internal libraries since then and I just did a small test here https://github.com/aarlt/cmake_vcpkg_solidity and it looks like that everything should just work, if you do what @cameel suggested. Just add an else to if (NOT IGNORE_VENDORED_DEPENDENCIES) in solidity/CMakeLists.txt, where you just ad find_package's for fmt, nlohmann_json and range-v3.

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

Successfully merging this pull request may close these issues.

3 participants