Allow feature options to accept boolean values #11279
Conversation
Making using of allow_unknown here cleverly allows us to pass the complete dictionary of keyword arguments in, ignoring all of those *except* the ones that we want to narrow the typing of. This moves more validation into the interpreter (a good thing), and will allow us to make use of the typed_kwarg helpers as well.
Currently, if an option is a boolean, and you want to upgrade it to a feature in a backwards compatible way, you need to first convert it to a combo option with `true` and `false` deprecated, set on that for a while, then change it to a feature. There really isn't a reason for that though, we can just accept `true` and `false` as values to features, and then convert them to `enabled` and `disabled`, respectively.
|
I'm against this because # A boolean option has been replaced by a feature, old true/false values are remapped.
option('o4', type: 'feature', deprecated: {'true': 'enabled', 'false': 'disabled'})
# A feature option has been replaced by a boolean, enabled/disabled/auto values are remapped.
option('o5', type: 'boolean', deprecated: {'enabled': 'true', 'disabled': 'false', 'auto': 'false'}) |
There was a problem hiding this comment.
I agree, this is literally the exact justification we used when we said "let's add deprecated options". We should not have two ways to do the same thing -- deprecated options are an explicit mapping that the project defines based on the project's understanding of whether e.g. false used to mean auto or disabled in the previous version of meson.build's scripting support.
can we have a bool to enabled/disabled/auto mapping to define what (just speaking as a developer for a project using meson (mesa), not as a meson expert) |
if you mean that the error output doesn't even tell you which option is wrong, then that would be: #6797 |
oh, maybe gives us that? Although I think in nearly all cases mapping true to enabled and false to disabled will be the common case, I guess it would be nice to have a less verbose way to express that? |
Per my comment that you quoted, yes. Since you're specifically talking about mesa, https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20409 bumped to 0.59 instead of 0.63 despite comments in the MR about bumping to 0.64 0.63 is needed for deprecated options. |
|
Instead of mapping true/false to features, we could allow enabled/disabled to booleans. That is unambiguous and solves the problem too. |
|
Do you mean allowing -Dfoo=enabled to set a boolean to true? Because I'm not sure how that solves the problem of wanting to preserve a deprecated "true" value when migrating to feature tristates. I also don't see how that's any less ambiguous, because it is exactly the same problem. |
|
Sorry, forgot the context, was replying to
Maybe I misunderstood, but I thought he meant as a user it is painful to know when to pass -Dfoo=true and when it is -Dfoo=enabled. Which is a fair complain IMHO. We could easily make it -Dfoo=enabled for booleans too. |
Mainly point 2. I don't think logically mapping two options to three options works in either direction, it's not just about moving to more values. The problem isn't that there's more possible values -- the problem is that each individual option doesn't have an actual analogue. |
This reduces the pain of Meson having picked inconsistent value names. See also mesonbuild/meson#11279 where Meson devs argue that allowing `foo=false` could be interpreted by users to mean either `disabled` or `auto`, which I personally don't see. Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21485>
One of the more painful things I've run into is wanted to update booleans to features (or deciding to use features when we really want booleans for fear of backwards compatibility issues). This is painful because booleans only accept
trueandfalse, and features accept onlyenabled,disabled, andauto. This is a really artificial limitation, and one that prevents a natural upgrade from booleans to features as the need arises.The changes I've made are to allow
trueandfalseto be passed to features, but to be converted toenabledanddisabledbefore assignment.