hooks icon indicating copy to clipboard operation
hooks copied to clipboard

Please include package name "cern-root" in allow list for #KB-H050: "DEFAULT SHARED OPTION VALUE"

Open davehadley opened this issue 4 years ago • 7 comments
trafficstars

I plan to submit a pull request to conan-center-index for a recipe for the ROOT data analysis framework (https://root.cern/). My draft recipe is at https://github.com/davehadley/conan-center-index/tree/feat-add-root-recipe.

ROOT does not currently support static builds (see: https://sft.its.cern.ch/jira/browse/ROOT-6446).

Can the package name root be added to the allow list to be compiled in shared only mode?

davehadley avatar Nov 24 '20 09:11 davehadley

Is this really needed? I think it enough to raise a ConanInvalidConfiguration exception for the configurations that are not supported by the library. Is that right @uilianries?

danimtb avatar Nov 25 '20 12:11 danimtb

you don't want to throw a ConanInvalidConfiguration with the default option values, or it makes it non-trivial to be consumed from other recipes, and no dependants can be built on CCI. The simplest option in this case is simply to not provide a shared option.

ericLemanissier avatar Nov 25 '20 12:11 ericLemanissier

I would rather see a shared attribute with the default and only possible value True instead 🤔

Croydon avatar Nov 25 '20 12:11 Croydon

It's more explicit, yes. But it means the package needs to be added to the hooks exception. Or do you mean make the hook not throw for recipes having shared option with only one possible value ?

ericLemanissier avatar Nov 25 '20 12:11 ericLemanissier

If this is possible it might be a good hook improvement, until then I would rather have it more explicit in the recipe + I don't think it is a huge deal to add it in the allow list

Croydon avatar Nov 25 '20 12:11 Croydon

Thank you for your help. In summary the suggestions are:

  1. Allow the option shared=False to be set but then raise ConanInvalidConfiguration.
  2. Remove the shared option entirely and hard-code shared=True in the recipe.
  3. Keep the default option with shared=True being the only valid value.

From the discussion (3) is the preferred option but requires modification to the conan-io/hooks repository (either add root recipe to the allow list or make the hook more intelligent).

For now, I have implemented solution (2) so that I can put together a draft pull request to test my recipe with the official CI. See https://github.com/conan-io/conan-center-index/pull/3732.

It is trivial to switch to solution (3). I can easily make this change to the recipe once the required changes to hooks are implemented.

davehadley avatar Nov 29 '20 12:11 davehadley

Updating this issue as the draft recipe in conan-io/conan-center-index#3732 was renamed from root to cern-root. so that is now the pattern that will need to go into the allow list to resolve this issue.

davehadley avatar Jan 12 '21 15:01 davehadley