The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32595.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

Opened in draft for feedback on approach.

There is also a bypass for the boost package (in cmake/module/AddBoostIfNeeded.cmake) which I'd like to get working with the dependency provider ideally.

Note that this also bumps the minimum cmake version to 3.24.

cc @purpleKarrot

Provides stronger guarantees about where dependencies come from during a (depends) build; i.e. only from depends.

Can you elaborate on this? Currently, CMake configured with a depends toolchain, should never look outside depends (doing so would be a bug). What are the stronger guarantees?

Provides stronger guarantees about where dependencies come from during a (depends) build; i.e. only from depends.

Can you elaborate on this? Currently, CMake configured with a depends toolchain, should never look outside depends (doing so would be a bug). What are the stronger guarantees?

Right, perhaps I didn't use the best wording here.

When using the toolchain I have not found us pulling in extraneous deps from outside of depends, but have encountered us missing dependencies from inside of depends, e.g. in my original issue.

By using a dependency provider and intercepting calls to find_package(), as I understand it, we are getting even more control over which packages can be found than when using the toolchain.

This is what I termed "stronger guarantees", perhaps misleadingly!

The windows job fails as currently we activate the dependency provider automatically based on whether a(ny) toolchain is in use.

This works fine for depends, but doesn't make sense on windows where the vcpkg toolchain is used, without depends.

The idea for doing it this way was to try and avoid users having to pass two flags in order to build from depends -- a toolchain and a dependency-provider -- but perhaps this is necessary after all...

I do recall reading somewhere in cmake docs that choice of dependency provider should always be left to the user too, so I suppose doing it this way would fit with that.

Pushed an update which now only activates the dependency provider when using the depends toolchain.

This will hopefully appease the windows CI job, as well as making more sense in general and being more expected behaviour.

Thank you for working on a dependency provider!

I think core should provide two dependency providers, which will give users three different ways to build:

1. Not using a dependency provider: CMake will produce an error when dependencies are not found on the system. This will be the preferred way for distro packagers.
2. A fallback provider: This will use system packages when they are found and fall back to compiling vendored depends. Using this provider, find_package will always succeed, but it will not compile any "unnecessary" code. This approach may be preferred by developers or for quick experiments.
3. A depends provider: This will ignore system packages and always use vendored depends. This approach will be used for reproducible builds.

At the moment, the dependency provider points to prebuilt depends. Alternatively, the dependency provider could take the responsibility of building depends at the moment they are requested. This would make the current, Makefile based depends system an implementation detail that can be ported piece by piece to CMake.

Thanks for the feedback @purpleKarrot.

I like the sound of your approach here myself and will be happy to adapt to it. I think i'll start off by targetting 3.:

A depends provider: This will ignore system packages and always use vendored depends. This approach will be used for reproducible builds.

...without the optionl "auto-build depends" functionality. This is close to what I have here already (and solves my immediate personal itch regarding building using depends on NixOS).

I would appreciate your input though on one more aspect of this; namely the (dev) UX involved with the 3 options. Option 1 would not see any UX changes.

Option 2 would require dev to specify the "fallback provider" file with a -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES flag.

Option 3 would require the dev to specify both a --toolchain and a -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES.

Option 3 in particular feels a little clunky to me (as a cmake novice), as I'd expect the toolchain to "contain everything needed to build". But I am guessing this can be abstracted away with a CMakePreset handling passing of the toolchain and dep. provider. This would become even more seamless in a version which auto-built depends for you too.

Does my understanding above sound roughly logical to you? If so I can make the changes later this week.

A fallback provider: This will use system packages when they are found and fall back to compiling vendored depends. Using this provider, find_package will always succeed, but it will not compile any "unnecessary" code. This approach may be preferred by developers or for quick experiments.

I can't tell from this if you are saying to use depends to "fill in" any missing system packages, or, if all the required system packages aren't available, we would switch to using depends (unclear if that is actually desirable). Certainly a NACK on mixing and matching the two.

I can't tell from this if you are saying to use depends to "fill in" any missing system packages,

Yes. The idea is to provide an approach to build core with minimal effort. If you NACK mixing system packages with vendored dependencies, are you saying that nobody will ever want that, or that nobody should ever want that?

Yes. The idea is to provide an approach to build core with minimal effort.

I think make -C depends && cmake -B --toolchain and apt install your_dependencies && cmake -B are both already pretty minimal effort. It's not really clear why a developer would apt install x, but not apt install x y, and then would need a behaviour to fallback to depends for y?

I'm also not currently aware of a platform that we support, where the dependencies needed to compile Core, aren't available via the system package manager; one used to be our specific version of BDB 4.x, but that dependency has now been removed. So this fallback usecase mostly seems like additional build complexity, which creates a larger matrix of "supported" builds to test / debug, for not much gain. We used to have a similar kind of option in our depends system (ALLOW_HOST_PACKAGES), but allowing the use of system packages, with depends, defeats the point of a self-contained dependency builder, so it was removed.

There's also some nuance around how this would function, are you imaging it all integrated into into CMake, so depends is no-longer a stand alone system? Otherwise, we need logic in CMake, to detect if the depends requirements (curl, compilers, patch, binutils etc) are actually installed / usable (i.e otherwise the fallback would fail because curl isn't installed, probably leaving the user wondering why it's trying to download stuff), as well as making sure that build flags / compiler selection are all passed through to depends (i.e if the user is trying to build with Clang & libc++, then any tools / flags need to be forwarded to the depends compilation).

I'm also just not sure that if a dependency is missing, downloading and compiling something is even the best behaviour. Maybe the user just forgot to apt install it, and will do so when CMake fails to configure?

OK I reworked this a little so that it works as described in top comment.

The overview is that the provider must be manually specified with -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES, thus keeping full control for the user.

The toolchain remains usable (as today) without the provider being specified.

https://cirrus-ci.com/task/5352790488252416?logs=ci#L1675:

[09:31:48.288] Extracting freetype...
[09:31:48.316] /ci_container_base/depends/sources/freetype-2.11.0.tar.xz: OK
[09:31:48.562] Preprocessing freetype...
[09:31:48.575] Configuring freetype...
[09:31:48.656] CMake Error at CMakeLists.txt:100 (cmake_minimum_required):
[09:31:48.656]   Compatibility with CMake < 3.5 has been removed from CMake.
[09:31:48.656] 
[09:31:48.656]   Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
[09:31:48.656]   to tell CMake that the project requires at least <min> but has been updated
[09:31:48.656]   to work with policies introduced by <max> or earlier.
[09:31:48.656] 
[09:31:48.656]   Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.
[09:31:48.656] 
[09:31:48.656] 
[09:31:48.656] -- Configuring incomplete, errors occurred!