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

Implement conf that avoids binary skipping in the graph #14466

Merged
merged 8 commits into from
Aug 23, 2023

Conversation

AbrilRBS
Copy link
Member

@AbrilRBS AbrilRBS commented Aug 11, 2023

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.

Copy link
Member

@memsharded memsharded left a 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.

@fschoenm
Copy link

@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 tool_require, that is already built? At least those would usually not be required for my use case (OSS license handling). Is it worth it to handle different scenarios like this or is not skipping packages an all-or-nothing thing?

@memsharded
Copy link
Member

Thanks for the comments @fschoenm

I think those make some sense, and I am thinking @RubenRBS that maybe tools.graph:skip_binaries could be beneficial:

  • Easy, clear and known separation between host and build context. For licenses the difference is very clear
  • It can be applied per-package if necessary like *@:tools.graph:skip_binaries to apply to all packages from ConanCenter (without user/channel)

@AbrilRBS AbrilRBS marked this pull request as ready for review August 17, 2023 16:31
conans/model/conf.py Outdated Show resolved Hide resolved
@AbrilRBS AbrilRBS assigned czoido and unassigned czoido Aug 18, 2023
@memsharded memsharded added this to the 2.0.10 milestone Aug 18, 2023
conans/model/conf.py Outdated Show resolved Hide resolved
@AbrilRBS
Copy link
Member Author

AbrilRBS commented Aug 21, 2023

@fschoenm

What happens with requirements for a tool_require, that is already built? At least those would usually not be required for my use case (OSS license handling). Is it worth it to handle different scenarios like this or is not skipping packages an all-or-nothing thing?

The conf can be specified just for build-context packages. The idea would be to do something like:
conan install --requires fast-dds/2.10.1 --deployer=licenses --conf:host "*:tools.graph:skip_binaries=False" --conf:build "*:tools.graph:skip_binaries=True"

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)

@czoido czoido merged commit bc74147 into conan-io:release/2.0 Aug 23, 2023
@AbrilRBS AbrilRBS deleted the rr/avoid-skipping-conf branch August 24, 2023 20:29
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.

4 participants