Skip to content

rewriter: allow reading dict-valued kwargs; improve key-value pairs in CLI; minor improvements#14740

Merged
dcbaker merged 7 commits intomesonbuild:masterfrom
bgilbert:rewriter
Nov 18, 2025
Merged

rewriter: allow reading dict-valued kwargs; improve key-value pairs in CLI; minor improvements#14740
dcbaker merged 7 commits intomesonbuild:masterfrom
bgilbert:rewriter

Conversation

@bgilbert
Copy link
Copy Markdown
Contributor

@bgilbert bgilbert requested a review from jpakkane as a code owner June 23, 2025 23:35
@bonzini bonzini added this to the 1.9 milestone Jul 9, 2025
@bonzini
Copy link
Copy Markdown
Contributor

bonzini commented Jul 9, 2025

Fixes: #14739

@bgilbert
Copy link
Copy Markdown
Contributor Author

bgilbert commented Jul 9, 2025

@bonzini Nope, this PR allows reading dict-valued kwargs but not writing them.

@bonzini bonzini modified the milestones: 1.9, 1.10 Aug 1, 2025
@bgilbert bgilbert force-pushed the rewriter branch 2 times, most recently from d60fd11 to 98c5d44 Compare September 3, 2025 22:43
The project ID must be "/" and cannot be an arbitrary string.
Fixes: e81acbd ("Use a single coredata dictionary for options")
Many rewriter subcommands expect key/value pairs as adjacent arguments.
If the last value is missing, show a user-friendly error instead of a
backtrace.

For: mesonbuild#13234
@dcbaker
Copy link
Copy Markdown
Member

dcbaker commented Sep 23, 2025

This all looks good to me.

I'm not sure if we can land the drop unused command-line arguments patch. That changes the API in a way that has the potential to break users who are depending on that behavior. Of course, I'm not really sure what to do about it, because the behavior we have right now is less than ideal.

@bgilbert
Copy link
Copy Markdown
Contributor Author

Well, the documentation says:

The rewriter itself is considered stable, however the user interface and the "script mode" API might change in the future. These changes may also break backwards compatibility to older releases.

We are also open to suggestions for API improvements.

No doubt users are depending on it anyway, but at least there's guidance we can point to.

@bonzini
Copy link
Copy Markdown
Contributor

bonzini commented Sep 23, 2025

There was even a bug that wasn't closed, so I think it is doable... Maybe detect an empty even-position value and ignore it with a warning, so that it's at least possible to write a script that works for both old and new? Does it make sense for any rewriter use case to use empty keys?

The `default-options delete` and `kwargs delete` subcommands required
key/value pairs, where the key was deleted and the specified value was
ignored.  This matches the JSON script mode interface but is unpleasant
as a CLI.  Have the CLI `delete` commands accept a list of keys instead.

We can make this change because the UI is documented to be unstable.
However, to allow scripts to work with both old and new Meson, ignore
even-numbered arguments if they're all equal to the empty string.

Fixes: mesonbuild#13234
We can't modify dict-valued kwargs (mesonbuild#14739) but this lets us at least read
them.
@bgilbert
Copy link
Copy Markdown
Contributor Author

Maybe detect an empty even-position value and ignore it with a warning, so that it's at least possible to write a script that works for both old and new? Does it make sense for any rewriter use case to use empty keys?

Good idea, done. Empty keys don't make sense; kwargs currently rejects them but default-options will agree to remove a nonsensical default option like '=foo'. However, I've opted to conservatively ignore-with-warning only when all even-position values are blank.

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.

LGTM me. Thanks for dealing with the backwards compat questions, I don't like asking them and I know no one likes dealing with them :D

@bonzini
Copy link
Copy Markdown
Contributor

bonzini commented Nov 18, 2025

@dcbaker this was approved months ago, anything blocking the merge?

@dcbaker dcbaker merged commit d49ab92 into mesonbuild:master Nov 18, 2025
33 checks passed
@dcbaker
Copy link
Copy Markdown
Member

dcbaker commented Nov 18, 2025

Just my memory :(

@bgilbert bgilbert deleted the rewriter branch November 18, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

meson rewrite kwargs delete project / <option> raises unhandled exception unless a dummy second value is passed

4 participants