LocalProgram refactoring

[xclaesse]

Reapply patchset from #15107.

[xclaesse]

@jpakkane @eli-schwartz can you reviews this please?

[eli-schwartz]

As I said previously:

Adding a new toplevel function which is an existing function with very minor behavior differences does not seem warranted to me. The main justification for a new function appears to be simply that it won't searchh PATH, but this is redundant with passing a files() to find_program(). I think this is a bad reason to add a new toplevel function.

Design wise, depends: and interpreter could just as easily be added to find_program directly, and there are potential use cases for the former even for programs found via PATH.

But I also think that this is unnecessary to allow interpreter as a kwarg at all. Existing users always pass an array around to things like

my_gen = [python, find_program(files('my_gen.py'))]
custom_target(command: my_gen + [....])

We should work with that and allow array overrides directly to meson.override_find_program(). I believe that will be a lot more intuitive than having "split-brain" interfaces and new toplevel functions depending on whether the result is intended to be passed to override_find_program.

(As a bonus, it also means for example that you can do override_find_program in an if meson.version().version_compare, the same way override_dependency was "simple to support" in a backwards compatible manner. That doesn't necessarily mean that I think new design should be based on what's easier to write backports using -- I just think it's very convenient that the thing I believe is a better design anyway, is also easier to use with backports.)

Passing an array to override_find_program means we can easily support interpreter options, which your original design didn't allow at all. I see that this new version does allow it, but it looks forced in and you specifically have to document that it works despite the obvious assumption (and the kwarg name implies it doesn't work, which is awkward UX). You still don't support arguments after the program, which prevents some use cases that would "automatically work" when just allowing to export an array, and we wouldn't have to take an opinionated stance about whether users should be allowed to do it. I can think of reasons why it would be perfectly fine to do, but it seems like you want a top-level function with an interpreter kwarg in part just because you want to ban something that is a "niche power tool" that you are afraid people will use in ways you find inelegant, and they could already do except more difficult. I don't see the point of this limitation.

...

By the way I would still like to see this support CustomTarget first, which I think we all agree should be made to work, and which you said this refactoring could be in support of instead. That would allow us to merge the groundwork independently and make it easier to reason about the scope of this proposal.

[bonzini]

That would allow us to merge the groundwork independently and make it easier to reason about the scope of this proposal.

+1 on this, even though I find using find_program for local scripts confusing and I would rather avoid adding extra functionality to find_program, or relying on little known tricks like always passing files().

[xclaesse]

TBH at this point I don't want to work on this anymore. I completely disagree and don't want to waste my time on bikeshedding again. Merge whatever you want.

[xclaesse]

I removed public API from this PR. I submitted new API on top in #15303.

[bonzini]

A couple things that are missing in this PR:

• creating the base class and then keeping the Union in most of the annotations is confusing
• LocalProgram.get_command() cannot work (and therefore BaseProgramHolder.cmd_array_method() is broken)
• BaseProgram is missing the for_machine field

[xclaesse]

• creating the base class and then keeping the Union in most of the annotations is confusing

Yeah, I still had a commit for that locally that mypy wasn't happy about, but I found the reason now. Fixed.

• LocalProgram.get_command() cannot work (and therefore BaseProgramHolder.cmd_array_method() is broken)

Fixed and made a unit test for it, because prog.cmd_array() with an exe override is broken on master too.

• BaseProgram is missing the for_machine field

done