Conan: [question] Deprecate (?) settings restriction inside recipes

[jgsogo]

Taken from https://github.com/conan-io/conan-center-index/pull/841#issuecomment-585155998

Some recipes declare valid settings like:

class Recipes(ConanFile):
    ...
    settings = {"os" : ["Windows"]}

others use the ConanInvalidConfiguration strategy:

class Recipe(ConanFile):
    ...
    settings = "os"

    def configure(self):
        if self.settings.os != "Windows":
            raise ConanInvalidConfiguration("Only Windows supported")

Do we want to deprecate the first syntax? If not, it should raise a ConanInvalidConfiguration too.

---

Probably the decision should be enforced for Conan v2 (added to CONAN_V2_MODE)

[SSE4]

adding some recipe authors (sorry for the noise)
/cc @solvingj @madebr @Croydon @theirix @uilianries @SpaceIm @ericLemanissier @Minimonium
if you have some opinion on this, pls feel free to share :)

[SSE4]

my personal opinion:
declarative is much better: shorter, cleaner, easier to read.
e.g. the following code:

"os": ["Windows"],
"arch": ["x86_64", "x86"],
"compiler": ["Visual Studio"],

will be very long after the conversion...
I personally used the former syntax in few recipes (e.g. Android NDK)
so far it was reliable, didn't introduce any problems, was easy to maintain.

from the docs:

The disadvantage of this is that possible settings are hardcoded in the recipe, and in case new values are used in the future, it will require the recipe to be modified explicitly.

I think it should be okay as long as we're sure about supported configurations (e.g. we know for sure that WTL is Windows-only, and Windows-specific on its nature)

[jgsogo]

@memsharded , any concerns regarding the graph, config, settings propagation,... that we should take into account here?

[memsharded]

This has implications related to the non-existence of binaries, and how this might be managed in the future, if we decide to expand always the build-requires (the complete graph). So in any case, this might be moved all together to its own validation method, not in the declaration of settings. It is feasible to implement helpers in that method to have declarative powerful syntax.

This can wait for final decision until we have more insights about how the future graph computation will be done in Conan 2.0.

[solvingj]

This is a very interesting issue. I will add the following points:

If they're not functionally equivalent (i.e. if Constrained Settings has different behavior when computing graph), then i think it's a strong argument to deprecate it. But... in the case they are equivalent but it's just about UX, then I have the following points.

Regarding Constrained Settings strategy...

1. shorter
2. cleaner
3. easier to read

I fully agree with all of these, and like this syntax better.

Regarding ConanInvalidConfiguration strategy...

1. All recipe-specific constraining logic for settings/options defined in one function
2. More people are aware of this convention and are used to looking here
3. Users can customize the message (might be desired in some enterprise cases)

My opinion:

I prefer declarative syntax. Sadly, I don't think it's possible to take away support for ConanInvalidConfiguration strategy. The general purpose python conditions are still needed for a lot of other things, and you would have to go to significant lengths to prevent people from using ConanInvalidConfiguration strategy moving forward. I do think it's highly valuable that "logic for constraining settings" be contained in the same place as "logic for constraining options" and overriding options. Ironically, the Constrained Settings syntax is the only thing I think that would be technically feasible to deprecate if we wanted to reduce the number of ways of doing this.

I actually think @memsharded has the best idea. Give a helper function with a similar declarative-api for constraining settings and options. This keeps the logic for constraining settings and options in one "place": configure().

Also, while I list "customizable log message" as a benefit for ConanInvalidConfiguration, it makes it much harder for users to google search to understand what's going on. Enterprise recipe authors might write bad error messages. A helper function could probably produce a uniform error message about invalid settings and options (by default) , which I would also consider a small improvement.

[memsharded]

Regarding the shorter/cleaner, that is not always true. For example try to remove just the compiler.runtime subsetting from accepted settings, while leaving the rest untouched. Current syntax not very pretty or straightforward.

[solvingj]

Right, shorter/cleaner only "mostly" true (simple cases).