interpreter: add depend_files kwarg to run_command()

[dvdhrm]

run_command() already treats all file paths in argv as build-def dependencies. Unfortunately, it does not allow the user to specify such dependencies manually. Add depend_files to allow manually adding build-def dependencies, rather than requiring users to stash them into the argument array, even if unused, just to get the same behavior.

[eli-schwartz]

configure_file() provides a depfile kwarg instead, and expects the command being run to output the files it ends up using.

[eli-schwartz]

Given the kwarg is part of func_run_command I don't know why it's being read+used in the totally different run_command_impl, but it is the cause of the CI failures.

[dvdhrm]

func_run_command() just forwards to run_command_impl(), and the latter shares the type-definition of the kwargs argument.

I think the CI failure is because this should read:

for f in kwargs.get('depend_files', []):
            self.add_build_def_file(f)

I wonder, though, why this didn't trigger errors in my local tests. I used:

./run_project_tests.py --only "common/33 run program"

And this worked fine. Hmm. But maybe I am missing something here. How exactly would you adjust func_run_command() to support the option?

[dvdhrm]

If I got this right, then func_run_command() properly fills in the default value in kwargs, and as such I get an empty depend_files when called via run_command() in meson build files. However, there are 2 other users of the actual implementation of func_run_command(), which is run_command_impl(), and those do not fill in defaults but pass kwargs manually (and explicitly specify all kwargs).

Do you prefer to update explicit users to specify all options, or should I use the kwargs.get(..., []) style?

[eli-schwartz]

I already told you what I want, but I can explain the distinction between the two and why your analysis doesn't apply in this case. Sorry for not elucidating earlier.

We have two functions:

• a DSL entry-point with functionality relevant to the DSL
• a backend refactoring function so that we can share the logic for "spawn a command" in various places where this is relevant

Most of the logic for the former (technically before this PR, all of it) needs to be forwarded regardless, because it tunes the details of the command spawning itself. It isn't work that occurs independently beforehand. Even though other uses don't necessarily need to control these details, run_command_impl must somehow accept knobs for e.g. env, and it could be done via a dict kwarg or via separate arguments but we happened to use this.

depend_files is unnecessary to expose to run_command_impl, it can live wholly inside of func_run_command which means avoiding unnecessary complexity inside other internal callers of run_command_impl.

[dvdhrm]

configure_file() provides a depfile kwarg instead, and expects the command being run to output the files it ends up using.

Does that mean you prefer the depfile style? I copied depend_files from custom_target(). I did not investigate which one was added later, or which one is considered the better option.

depend_files is certainly very easy to use, but splits authority between the script and the build system, which can be confusing. But the design of depfile kinda means you create an intermediate script as part of the build system, anyway (unless your target script exactly supports the semantics of depfile, and all the semantics of run_command()).

I don't care too much. Would you prefer if it was depfile?

[eli-schwartz]

There are advantages and disadvantages to each. But don't you already need a script if you're doing globbing?

[dvdhrm]

I already told you what I want

No, I feel like you didn't. None of your comments explain how you would like to see this implemented. You mention depfile on configure_file(), without stating how that information is to be applied. Then you state that the code reads an argument in a "totally different" function. Great, how would you envision this to be done instead?

Why do I deserve this rude response?

To me, this is in no way a motivation to contribute to Meson, sorry. Feel free to pick this up yourself. I can use the described workaround by passing files as dummy-arguments.

[bonzini]

@dvdhrm what is your usecase? There are different reasons for depfile and depend_files. Depfile is for indirect inputs you don't know about but the script does; and the other is for direct inputs that you know about (and the script probably doesn't care, e.g. included files). Both have a reason to exist.

Would you be okay with fixing the PR to move the logic entirely in func_run_command, and reopening it.

[bonzini]

Both have a reason to exist.

Meaning, both configure_file and run_command could have both.

[dvdhrm]

@dvdhrm what is your usecase?

I want to use Rust as first-class build system for Rust libraries. It is a nice alternative to Cargo and has much better linux distro integration. For now, I love how it worked out. However, I still want other Rust projects to easily use the library, even if they do not switch to Meson. Hence, I provide Cargo.toml as well.

Since I want to avoid duplication of information, I currently fetch the version information of the individual cargo crates that make up the workspace via run_command() and a script that basically just dumps the Cargo.toml metadata as JSON. I then use this version information in meson.build. With this PR I wanted to ensure that Meson triggers a reconfiguration if the Cargo.toml files changed. As workaround, I now pass all relevant files as argument to my script, but it feels kinda unclean.

(btw., I do something similar for other no-dep crates where I provide meson.build via subprojects/packagefiles/ so I can pull them in as wrap)

Overall, I found workarounds for all issues I faced and am pretty happy about using Meson first-class for Rust libs. The only issues left is that meson-test does not print detailed information about rust unit-tests #14719 and that Meson has no support to deserialize JSON, which would make all data passing so much cleaner.

There are different reasons for depfile and depend_files. Depfile is for indirect inputs you don't know about but the script does; and the other is for direct inputs that you know about (and the script probably doesn't care, e.g. included files). Both have a reason to exist.

I would love if both were supported. depfile would work nicely in this situation, since cargo metadata already dumps the paths of all Cargo.toml files in the workspace. But depend_files is certainly easier to use when just reading in configuration from files.

Would you be okay with fixing the PR to move the logic entirely in func_run_command, and reopening it.

No, thank you. The way some maintainers respond to contributions of volunteers kinda demotivates me. Maybe next time. Feel free to pick this up.

[bonzini]

I then use this version information in meson.build

You're reinventing #14639 and #15069, but yes that's the only way to do it for now. It looks like your plan will be compatible with that API once it's ready; in the meanwhile you can experiment and provide bug reports for Meson's Rust support, so who am I to complain! :)

QEMU is doing something similar, extracting for example compile arguments from the [lints] section.

Feel free to pick this up.

Thanks, I'll try to.