Config option cleanup#493
Open
arichardson wants to merge 42 commits into
Open
Conversation
Needed for dlmalloc
We can't pass Optional[] as the parameter since it can't be used as a callable, we have to pass the actual type instead.
Needed to replace the calls to add_config_option.
This adds OptionalEnumConfigOption to the declarative options framework, permitting Enum option declarations that do not require an initial default value and default to None. This mirrors the existing OptionalIntConfigOption, OptionalStringConfigOption, and OptionalPathConfigOption classes.
…iptors This implements generic option mixin registration by dynamically injecting option dictionaries on parent mixin classes. The metaclass is updated to merge base class options from all parents to correctly support multiple inheritance hierarchies, and it now prunes any descriptors that are overridden as static non-descriptor values in subclasses. This also adds a comprehensive unit test in tests/test_project_helpers.py verifying correct option resolution and inheritance across mixins and subclasses.
This introduces a declarative extra_condition callback parameter to the PerProjectConfigOption constructor. Subclasses option registry (MRO walk in the ProjectSubclassDefinitionHook.__init__ metaclass) evaluates this condition at class creation time and prunes the option if the condition evaluates to False on the class. Instead we just populate the class member with a descriptor that returns the default value.
… defaults
When option defaults are migrated to class attributes, they can form a tree of nested ComputedDefaultValue or callable objects. For example, in disk_image.py:
1. StringConfigOption 'hostname' default evaluates to 'project.default_hostname' (a ComputedDefaultValue).
2. 'project.default_hostname' evaluates to '_default_disk_image_hostname("freebsd")' (another ComputedDefaultValue).
3. The final ComputedDefaultValue evaluates to the string 'freebsd-amd64'.
This change resolves callables recursively using a while loop to ensure the concrete resolved option value is returned instead of the intermediate ComputedDefaultValue objects.
Checks if an option is inherited as a ConfigOptionHandle from a concrete parent class, and registers it target-specifically on the subclass if it has not been registered directly yet. This fixes a bug where subclass target classes of concrete parent target classes did not register any of their parent classes' options. For example, BuildCheriAllianceQEMU (cheri-std093-qemu) inherits 'targets' option from BuildQEMU (qemu), and we must register it specifically so it resolves to the custom default_targets list. Also adds a test in test_project_helpers.py to verify option registration.
Converts the set-subtraction in supported_architectures() to a stable list comprehension. This prevents Python's unstable set iteration order from shuffling get_cross_target_index() results and randomly swapping default SSH port allocations on different runs.
Migrates 'configure-options' in AutotoolsProject to a class attribute ListConfigOption descriptor, and removes its setup_config_options method.
Migrates 'cmake-options' in CMakeProject to a class attribute ListConfigOption descriptor, and removes its setup_config_options method.
Enables passing a Callable[[type], bool] for both show_help and use_default_fallback_config_names, resolving them dynamically in add_config_option against the target class.
A follow-up commit will cause this to trigger an assertion that we are instantiating a project recursively, but since we already have access to the project we can avoid calling _get_or_create_project_no_setup.
Migrates all remaining config options in Project class to class attribute descriptors (including build-directory, use-asan, use-msan, use-ccache, auto-var-init, skip-update, reconfigure, install-directory, git-revision, repository, use-lto, use-cfi, linkage, build-type, and build-tests), and completely removes Project.setup_config_options. Also adds fallback return statement to _build_type_basic_compiler_flags to prevent type checking errors on project.py.
This ensures that tests using it don't pollute the rest of the test run.
arichardson
force-pushed
the
config-option-cleanup
branch
from
a0b0f44 to
3869a38
Compare
Member
Author
|
These are the trivial ones, I'll create a new PR with CheriBSD/Project options since they have conditional logic |
Member
This is going to need very painstaking manual review... |
Member
|
(No, I do not for one minute trust an LLM to vomit up something correct without appropriate human review) |
Member
Author
I agree, fortunately it's basically move out of function, unindent, replace .add_foo with FooConfigOption, and all arguments are basically the same. To complicated to write a sensible regex replace, but letting the LLM do it and manually review isn't too bad. |
Member
Author
|
I'll split this into a NFC and functional changes PR. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Use class variables instead of overriding setup_config_options.
This will eventually allow turning on the bad-attribute type checks.
The repetitive class-level conversion of setup_config_options->class attributes and adding tests was done using Gemini, but most of the logic changes were manual.
Confirmed the difference in default config options against HEAD using:
This actually includes a few minor fixes where config options were not inherited properly previously, but otherwise this has no functional changes.