Preliminary command handling cleanup #12064
Conversation
xclaesse
changed the title
|
I'm working on a much bigger refactoring of handling commands, this is just a few easy commits to get started. |
xclaesse
force-pushed
the
cmd-es-fix
branch
2 times, most recently
from
5dd1971
to
45a917d
Compare
| build_by_default: true, | ||
| ) | ||
|
|
||
| custom_target( | ||
| output: 'foo2', | ||
| command: [find_program('check_args.py'), gen[0]], | ||
| build_by_default: true, |
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh god, digging that one I found yet another inconsistency: when using @INPUT@, gen[0] is passed as path relative to builddir. But when passing gen[0] directly on the command kwarg, it's passed as absolute path. That test was supposed to catch that but since it didn,t have build_by_default: true, it never actually tested it...
It was already supported for `custom_target()` by converting it to a File object, but it's better to explicitly support it in the backend so we can use it in more places in the future, such as `meson.add_install_script()`.
This has side effect to allow CustomTarget and CustomTargetIndex as first argument of add_install_script() which is more consistent with custom_target() command kwarg.
| @@ -2685,7 +2685,7 @@ def func_configure_file(self, node: mparser.BaseNode, args: T.List[TYPE_var], | |||
| mlog.log('Configuring', mlog.bold(output), 'with command') | |||
| res = self.run_command_impl(cmd, | |||
| {'capture': True, 'check': True, 'env': EnvironmentVariables()}, | |||
| True) | |||
| in_builddir=True) | |||
There was a problem hiding this comment.
Is there a particular reason that passing this (non-ambiguous) argument as a kwarg is advantageous? I don't understand what difference this is supposed to make.
There was a problem hiding this comment.
Just a small pet peeve of mine, when passing just "true" you have to check signature to know what it means. I find it more readable like that.
There was a problem hiding this comment.
Do you plan to change the entire codebase like this? My text editor shows function signatures anyway :D
There was a problem hiding this comment.
I can drop that commit if you want, not really important.
| (build.Executable, ExternalProgram, compilers.Compiler, mesonlib.File, str), | ||
| varargs=(build.Executable, ExternalProgram, compilers.Compiler, mesonlib.File, str)) | ||
| varargs=(build.Executable, ExternalProgram, compilers.Compiler, mesonlib.File, str), | ||
| min_varargs=1) |
There was a problem hiding this comment.
Getting rid of this pointless parsing complexity makes sense, yeah.
run_command: Make boolean argument explicit
run_command: Merge first argument with the rest in a single list