Skip to content

Fix more option regressions#14549

Merged
dcbaker merged 7 commits intomesonbuild:masterfrom
bonzini:fixmoreoptions
May 6, 2025
Merged

Fix more option regressions#14549
dcbaker merged 7 commits intomesonbuild:masterfrom
bonzini:fixmoreoptions

Conversation

@bonzini
Copy link
Copy Markdown
Contributor

@bonzini bonzini commented May 2, 2025

Fixes: #14528

Fixes: #14532

And also fixes an assertion failure that I triggered while writing tests.

@bonzini bonzini added regression options Meson configuration options labels May 2, 2025
@bonzini bonzini added this to the 1.9 milestone May 2, 2025
@bonzini bonzini marked this pull request as ready for review May 3, 2025 17:06
@bonzini bonzini requested a review from jpakkane as a code owner May 3, 2025 17:06
@jpakkane jpakkane modified the milestones: 1.9, 1.8.1 May 4, 2025
@jpakkane
Copy link
Copy Markdown
Member

jpakkane commented May 4, 2025

Change the milestone to 1.8.1, otherwise they script does not take these commits to that release (which it clearly should).

@eli-schwartz
Copy link
Copy Markdown
Member

From a quick look this all seems right to me, and regression fixes are top priority for backporting indeed.

@eli-schwartz
Copy link
Copy Markdown
Member

Rebasing should fix the CI.

Copy link
Copy Markdown
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few suggestions but they can be addressed later since this is correct as is, and fixes regressions.

bonzini added 3 commits May 6, 2025 18:49
Something like

   subproject('sub', default_options: ['sub2:from_subp=true'])

will cause an assertion failure due to "key.subproject is None"
obviously being false.  Just support this, since it's easy to do so.
No code changes, just making mypy annotations truthful.

Signed-off-by: Paolo Bonzini <[email protected]>
bonzini added 4 commits May 6, 2025 18:49
Always use a dictionary (even though later OptionStore will convert it back to list
for hackish historical reasons) to make it easy to apply overrides.  Long term
we probably want OptionStore to not know about T.List[str] at all, anyway.

Signed-off-by: Paolo Bonzini <[email protected]>
Apply the default_library=... default after the default options have been
converted to a dictionary, to avoid having to deal with all the possible types
of the default_options keyword argument.

Fixes: mesonbuild#14532
Signed-off-by: Paolo Bonzini <[email protected]>
@bonzini
Copy link
Copy Markdown
Contributor Author

bonzini commented May 6, 2025

The only change from the approved version is

diff --git a/mesonbuild/interpreter/interpreter.py b/mesonbuild/interpreter/interpreter.py
index 77199a757..8fb660d1f 100644
--- a/mesonbuild/interpreter/interpreter.py
+++ b/mesonbuild/interpreter/interpreter.py
@@ -883,9 +883,7 @@ class Interpreter(InterpreterBase, HoldableObject):
         if isinstance(default_options, list):
             default_options = dict((x.split('=', 1) for x in default_options))
         if extra_default_options:
-            default_options = copy.copy(default_options)
-            for k, v in extra_default_options.items():
-                default_options.setdefault(k, v)
+            default_options = {**extra_default_options, **default_options}
 
         if subp_name == '':
             raise InterpreterException('Subproject name must not be empty.')

So the failure should be transient?? It does not show up in #14559.

@dcbaker
Copy link
Copy Markdown
Member

dcbaker commented May 6, 2025

still LGTM.

I kicked the one CI job, we'll see if that clears up

@dcbaker dcbaker merged commit a46371f into mesonbuild:master May 6, 2025
46 of 47 checks passed
@bonzini bonzini deleted the fixmoreoptions branch May 23, 2025 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

options Meson configuration options regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with default options for dependencies with 1.8.0 [1.8] Cannot set per-project warning_level&werror from the parent project's default_options

5 participants