-
Notifications
You must be signed in to change notification settings - Fork 991
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
Implement conf that avoids binary skipping in the graph #14466
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I think this would deserve some tests or some extra checks, like adding this to some of the already existing functional tests, to validate things are not breaking. The idea is validate that even with not skipping binaries, linkage and usage of packages is exactly the same with generators like CMakeDeps, CMakeToolchain and VirtualBuild/RunEnv.
@RubenRBS Thanks for implementing something so quickly, that seems very helpful :) What effect does not skipping packages have on the build process? Does it just add e.g. CMake configs for packages that would usually be skipped or are there any other side effects? What happens with requirements for a |
Thanks for the comments @fschoenm I think those make some sense, and I am thinking @RubenRBS that maybe
|
The conf can be specified just for build-context packages. The idea would be to do something like: This would force host-context packages not to be skipped, and build-context ones to be allowed to skip (But won't get skipped if they are needed in the install, you are the one that has to ensure that first!) (Or write the conf with the correct value to each of the corresponding profiles) |
Changelog: Feature: Add
tools.graph:skip_binaries
to control binary skipping in the graph.Docs: conan-io/docs#3342
Docs: conan-io/docs#3341
Tests missing, will asign someone once I write them up :)
Motivation comes from conan-io/conan-extensions#65 (Pinging @fschoenm) even though this was a known issue before that. Another alternative for that specific issue is listed in its comments, but this change stands on its own either way.