another round of option refactoring #14644
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bonzini
force-pushed
the
more-options
branch
4 times, most recently
from
2854d0a to
cb92d55
Compare
bonzini
force-pushed
the
more-options
branch
2 times, most recently
from
b315ec5 to
946ca76
Compare
The converter in DEFAULT_OPTIONS makes a mapping from OptionKey to Python values, so use the correct type. Signed-off-by: Paolo Bonzini <[email protected]>
Use the type that is produced by the converter, and fix the type for methods_map which has a wrongly-placed bracket. Signed-off-by: Paolo Bonzini <[email protected]>
Starting with Meson 1.8.0, "meson configure" prints some options as ":foo" instead of "foo". Print the option as it was passed by the user. While at it, make errors more consistent and/or correct (e.g. "Unknown option" instead of "Unknown options"). Signed-off-by: Paolo Bonzini <[email protected]>
Once unknown options will go through accept_as_pending_option, only system options that really exist in Meson will be accepted. Adjust the unit tests. Signed-off-by: Paolo Bonzini <[email protected]>
They must be there when running re-configuring, because the backend cannot be changed, but they can be pending on the first invocation. Signed-off-by: Paolo Bonzini <[email protected]>
There is common logic hiding between project() and "meson configure": the complication that the comment mentions for the "default_options" case actually applies to "meson configure", to machine files, to command line options and to project options. Reuse the same function in all four cases. Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
self.project_options is set already a couple lines above. Signed-off-by: Paolo Bonzini <[email protected]>
The cmd_line_options dictionary is described as having OptionKey keys, so make sure that cmd_the keys do have the correct type. Signed-off-by: Paolo Bonzini <[email protected]>
Thanks to several fixes applied between commit d37d649 ("Make all Meson level options overridable per subproject.", 2025-02-13) and now, OptionStore never gets a string key. Tighten the type of OptionDict, and use it whenever possible. Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
The key is already correct at this point, no need to look up CFLAGS_MAPPING. Signed-off-by: Paolo Bonzini <[email protected]>
They all look the same now, yay. Signed-off-by: Paolo Bonzini <[email protected]>
dnicolodi
reviewed
Jun 4, 2025
mesonbuild/options.py
Outdated
| else: | ||
| # Setting a project option with default_options. | ||
| # Argubly this should be a hard error, the default | ||
| # arguably should be a hard error, the default |
There was a problem hiding this comment.
I think the full stop on the previous line removed, and I like the adverb after the modal verb, but that may be just me.
dnicolodi
reviewed
Jun 4, 2025
|
|
||
| with self.subTest('unknown builtin option'): | ||
| self.new_builddir() | ||
| out = self.init(testdir, extra_args=['-Db_not_an_option=1'], allow_fail=True) | ||
| self.assertIn('ERROR: Unknown options: "b_not_an_option"', out) | ||
| self.assertIn('ERROR: Unknown option: "b_not_an_option"', out) |
There was a problem hiding this comment.
I have the impression that these two hunks should have been included in the previous commit.
dnicolodi
reviewed
Jun 4, 2025
| @@ -712,9 +712,10 @@ def parse_cmd_line_options(args: SharedCMDOptions) -> None: | |||
| args.cmd_line_options = {} | |||
| for o in args.projectoptions: | |||
| try: | |||
| (key, value) = o.split('=', 1) | |||
| (keystr, value) = o.split('=', 1) | |||
There was a problem hiding this comment.
While at it, you can remove the parentheses: they should not be needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This gets to the point where everything that goes into the
OptionStorehasOptionKeyinstead ofstr(with the exception ofconfigurecommands, but that's pretty well isolated and could even be moved out of OptionStore altogether).