Skip to content

Conversation

@bonzini
Copy link
Collaborator

@bonzini bonzini commented Nov 20, 2025

This cleans up OverrideProgram and OverrideExecutable, replacing them with a single class that is a sibling of ExternalProgram. The two classes are a mess and complicate type annotations noticeably; this change removes the complication and even removes the need for a type alias, because you can almost always just use Program instead of ExternalProgram | Executable | OverrideProgram.

Extracted from #15107, removing support for depends/depend_files (not yet supported by find_program()) and for the ill-fated local_program() function.

Fixes: #15080

@bonzini bonzini requested a review from jpakkane as a code owner November 20, 2025 12:59
@bonzini bonzini force-pushed the program-hierarchy branch 3 times, most recently from 7388a93 to 6ad6658 Compare November 20, 2025 14:10
@bonzini bonzini added the refactoring No behavior changes label Nov 20, 2025
@xclaesse
Copy link
Member

It's sad that you're reimplementing what I did already just because @eli-schwartz reverts commits he doesn't like. I'm disappointed to see so much work wasted, really discouraging.

@bonzini
Copy link
Collaborator Author

bonzini commented Nov 24, 2025

A lot of this work was anyway on top of yours, so there's not much reimplementation. I don't think it's productive either to complain.

Free the name for a class.

Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
OverrideProgram is a subclass of ExternalProgram, so you do not need to
have both.

Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
@bonzini bonzini force-pushed the program-hierarchy branch 3 times, most recently from 8980ecb to 85a3e5d Compare November 25, 2025 08:01
@xclaesse
Copy link
Member

A lot of this work was anyway on top of yours, so there's not much reimplementation.

Please rebase on top of #15133 then. I removed public API so it is already reviewed and ready to go. That's easier than trying to find out what you modified in my code, or what you recommited under your name.

I don't think it's productive either to complain.

Yes I think it's important to complain, what @eli-schwartz did is extremely frustrating and a waste of everyone's time.

@eli-schwartz
Copy link
Member

It's sad that you're reimplementing what I did already just because @eli-schwartz reverts commits he doesn't like. I'm disappointed to see so much work wasted, really discouraging.

I don't think it's productive either to complain.

Yes I think it's important to complain, what @eli-schwartz did is extremely frustrating and a waste of everyone's time.

I find this discussion highly disappointing. For a couple reasons. But the biggest reason is because I heavily object to your immature libel.

I don't want to get into a fight about this. But if you're determined to force my hand, then I feel I need to point out for the record:

When you submit a PR for which there is strong technical disagreement, it is unfortunate but understandable that such things happen. Not everyone always needs to agree, and we can try to negotiate or sway the other person or if really necessary, find an arbiter such as the project lead, who exists expressly for this purpose. We all (hopefully) want to make meson better, but the corollary to this is that sometimes the other person believes he is making meson better by being a pedant about the design and UX.

Instead of even offering a fig leaf towards building consensus, you self-merged a MAJOR new feature on your call alone and over objections, apparently under the belief that having a merge bit means that you are allowed to do anything you want whatsoever in a kind of Chaotic Evil free-for-all.

I object to this and I reverted that PR for being dishonestly merged and I will do it a hundred times again if I need to, which I probably will need to because this is the tail end of a long time pattern. I have no qualms about doing it -- on the contrary it is my ethical duty to do it.

I find it hugely offensive in the extreme that your reaction to this is to make it about "Eli versus other people" simply because I'm the person who you picked a fight with, and then libelously claim that I'm simply deleting features I don't personally like (and for a feature that you self-merged because it's a feature YOU personally like).

Presumably now once again you'll go on a long rant about how Meson is a terrible project and you hate working on meson "because things take too long to merge and everything becomes some long drawn out discussion", which is still, every time you complain about it, just another way of saying "I'm mad that working on a project with other developers means I have to respect their opinions".

Please. Work within the system using the documented consensus-building system, rather than against it. You are egregiously wasting everyone's time here -- and if you don't care about anyone else's time, that still includes your own.

Add a separate object that is like OverrideExecutable but isn't duck-typing
with Program.

[Paolo: remove depends/depend_files, as they are not supported yet by
 find_program(); make get_command() fail for non-build LocalProgram so
 that other code in Meson *has* to go through Backend.get_target_dir()]

Signed-off-by: Paolo Bonzini <[email protected]>
override_find_program() until now returned an OverrideExecutable,
which is a kind of BuildTarget, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach custom_target() and run_target() that it is okay to
receive any Program.

Signed-off-by: Paolo Bonzini <[email protected]>
xclaesse and others added 11 commits November 25, 2025 17:59
override_find_program() until now returned an OverrideExecutable,
which is a kind of BuildTarget, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach configure_file() and run_command() that it is
okay to receive any Program.

If it is not runnable, Meson will test and complain.

Signed-off-by: Paolo Bonzini <[email protected]>
override_find_program() until now returned an OverrideExecutable,
which is a kind of Executable, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach test() and benchmark() that it is okay
to receive any Program.

Signed-off-by: Paolo Bonzini <[email protected]>
Move handling of Executable generators out of the backends, and generalize it
to any target (as long as it is wrapped with a Program).

Signed-off-by: Paolo Bonzini <[email protected]>
override_find_program() until now returned an OverrideExecutable,
which is a kind of Executable, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach generator() that it is okay to receive any Program.

Signed-off-by: Paolo Bonzini <[email protected]>
override_find_program() until now returned an OverrideExecutable,
which is a kind of Executable, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach find_program() that it is okay to return any
Program, and propagate the new type to all the users.

Signed-off-by: Paolo Bonzini <[email protected]>
override_find_program() until now returned an OverrideExecutable,
which is a kind of Executable, or an OverrideProgram, which is
a kind of ExternalProgram.  In preparation for making it return a
LocalProgram, teach add_*_script() that it is okay to receive any
Program.

Signed-off-by: Paolo Bonzini <[email protected]>
This is not needed anymore now that there is a proper
LocalProgram object that find_program() returns.

Signed-off-by: Paolo Bonzini <[email protected]>
This matches the Python class name, which shows up in errors, and is simpler.

Signed-off-by: Paolo Bonzini <[email protected]>
These are useful for the many cases of creating lists of commands that
are mixtures of strings and programs
@bonzini
Copy link
Collaborator Author

bonzini commented Nov 25, 2025

Please rebase on top of #15133 then

That's exactly where I started from, because the new base class could provide a simple type for all programs (like @dcbaker wanted) without needing a type alias. Since you said you weren't working on it, I didn't feel compelled to keep your commits as the base, especially when there were bugfixes involved as is the case for cmd_array() or full_path().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring No behavior changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OverrideExecutable and build.find_overrides should be in Interpreter

5 participants