subp:opt=value in main project's default_options is overridden by opt=value from command line since meson 1.8.4 #14963
Conversation
bonzini
force-pushed
the
fix14940
branch
2 times, most recently
from
b2ceea6 to
b3c6a4c
Compare
Signed-off-by: Paolo Bonzini <[email protected]>
This reverts commit 5ab871b. It is not needed anymore since the pre-1.8 semantics were the good ones.
This reverts commit 9b1eb8b. It is not needed anymore since the pre-1.8 semantics were the good ones. Only leave in place the new and clearer testcase names.
Signed-off-by: Paolo Bonzini <[email protected]>
|
Ping @jpakkane |
| - `opt=value` from machine file | ||
| - `opt=value` from command line | ||
| - `subp:opt=value` from parent project's default options | ||
| - `opt=value` from `subproject()` `default_options` |
There was a problem hiding this comment.
This seems incorrect. The command line should override anything set in project files. (given the description this list is sorted on increasing precedence)
There was a problem hiding this comment.
That was also what I thought when preparing the changes that went into 1.8.4. But it turns out that making two separate groups for global settings and augments (the machine file and command line winning in each group only) is more useful, and there are projects in the wild that rely on it.
There was a problem hiding this comment.
How is it more useful specifically?
There was a problem hiding this comment.
For example you can give -Dwerror=true but meson.build can override it to false for subprojects that are known to not behave; likewise for -Dwarning_level. It is also more in line with how default_library works in pre-1.8 releases (see the reverted test changes in this PR).
Besides, we got reports like #15006 where the new ordering breaks existing projects that worked before 1.8.4. I didn't have much time to debug what exactly caused the breakage, but this patch does fix it. So, while overall the new option code is now tamed, I think this part has to be rolled back.
There was a problem hiding this comment.
Even the documentation says so:
This is useful, for example, when building shared libraries in the main project and statically linking a subproject, or when the main project must build with no warnings but some subprojects cannot.
But the trick doesn't work anymore in 1.8.4
There was a problem hiding this comment.
The problem here seems to be that the table collates two different things making it hard to comprehend. Maybe it should be split in two, with a structure like:
- Per-subproject settings always override the default value, no matter how or where they are defined
- When configuring for the first time, values for options (and overrides) are used in the following order
And maybe have the array in decreasing order of priority, as that seems to be the more logical order.
There was a problem hiding this comment.
Even that is not unambiguously true, though I agree it's as close as you can get, because -Dopt does override "opt=value from subproject's default_options" which is subproject-specific. So I don't think it's much better than listing the possibilities one by one.
That said do you agree now that the ordering as implemented by the patch makes sense?
Move "opt=value from machine file/command line" before subproject-specific values from
project()orsubproject().Fixes: #14940