another round of option refactoring #14644
Closed
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
dnicolodi
reviewed
Jun 4, 2025
dnicolodi
reviewed
Jun 4, 2025
dnicolodi
reviewed
Jun 4, 2025
dcbaker
reviewed
Jun 5, 2025
bonzini
force-pushed
the
more-options
branch
2 times, most recently
from
c2bb610 to
895948b
Compare
bonzini
added
refactoring
|
Good stuff, LGTM. |
bonzini
force-pushed
the
more-options
branch
6 times, most recently
from
a2b894e to
814eec1
Compare
|
Marking as draft until #14690 is sorted out. |
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]>
Suggested-by: Dylan Baker <[email protected]> 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]>
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).