interpreter: handle ExternalProgram commands in run_command #14295
Conversation
run_command() can take a list of parameters that will describe the
command being run, and an ExternalProgram object can be provided here.
When setting up the ExternalProgram from a cross-file as a list, for
instance:
[binaries]
rustc = ['rustc', '--target=arm-linux-androideabi']
The ExternalProgram.get_path() method will be used to prepare the
run_command arguments, but it would return '--target=arm-androideabi'
instead of the proper argument.
This commit ensures every part of the command is actually used in
this case.
|
I'll check if I can easily do that. Note that the part for the client (full_path() and path() API from the meson user side) is still broken though... |
|
You can put a |
Without previous commit, the test fails with:
Build machine cpu family: aarch64
Build machine cpu: aarch64
Host machine cpu family: aarch64
Host machine cpu: aarch64
Target machine cpu family: aarch64
Target machine cpu: aarch64
Program foo found: YES
Running command: /bin/echo baz --version
--- stdout ---
baz --version
--- stderr ---
test cases/unit/125 command list run_command/meson.build:12:0: ERROR: Assert failed: result.stdout() == 'bar baz --version
'
alexandre-janniaux
force-pushed
the
external-program-run-command/1
branch
from
c7071e1 to
c55fde6
Compare
|
Done |
|
Are the failures linked to my MR ? It feels like something else is breaking the tests. |
| @@ -830,7 +830,7 @@ def run_command_impl(self, | |||
| elif isinstance(a, mesonlib.File): | |||
| expanded_args.append(a.absolute_path(srcdir, builddir)) | |||
| elif isinstance(a, ExternalProgram): | |||
| expanded_args.append(a.get_path()) | |||
| expanded_args += a.command | |||
There was a problem hiding this comment.
I suspect this is causing the windows regressions. Just to confirm my theory (since I don't have a windows machine handy), can you try replacing this with:
expanded_args.append(a.get_path())
expanded_args.extend(a.command[1:])and see if that makes everything happy?
There was a problem hiding this comment.
AFAIK, one of the issue is that a.get_path() would currently return the second element from the crossfile.
|
|
||
| def test_run_command_external_program(self): | ||
| testdir = os.path.join(self.unit_test_dir, '125 command list run_command') | ||
| self.init(testdir, extra_args=['--native-file=' + testdir + '/nativefile.toml']) |
There was a problem hiding this comment.
Meson does not use terrible file formats. This is an ini file and is parsed like one. Note file extensions are technically irrelevant.
There was a problem hiding this comment.
I agree it's technically irrelevant, but don't get the "terrible", and ini doesn't have types like lists or strings so I don't really understand the point. :/
There was a problem hiding this comment.
The filename is confusing and makes no sense. Meson does not, never has, and never will utilize toml files here.
Why call it nativefile.toml? A better filename would be:
| self.init(testdir, extra_args=['--native-file=' + testdir + '/nativefile.toml']) | |
| self.init(testdir, extra_args=['--native-file=' + testdir + '/nativefile.pdf']) |
OR you could use the conventional filename which is used everywhere in the meson documentation: nativefile.ini
and ini doesn't have types like lists or strings so I don't really understand the point. :/
The value entry for a meson wrap file is an ini string, read by python's configparser module, which is then parsed by the meson.build grammar parser (mesonbuild.mparser.Parser()).
All types come from the meson.build syntax. toml types are neither needed nor desirable.
ini is used for reading off key=value pairs, which is a role it excels at.
| project('compiler_object_in_run_command', 'c') | ||
|
|
||
| foo = find_program('foo', required: true) | ||
| # This test only checks that the compiler object can be passed to |
There was a problem hiding this comment.
This isn't a compiler object, why are we talking about it like that?
| @@ -1920,3 +1920,7 @@ def test_persp_options(self): | |||
| self.check_has_flag(compdb, mainsrc, '-O3') | |||
| self.check_has_flag(compdb, sub1src, '-O2') | |||
| self.check_has_flag(compdb, sub2src, '-O2') | |||
|
|
|||
| def test_run_command_external_program(self): | |||
There was a problem hiding this comment.
I think the suggestion was to add nativefile.ini to an existing test that uses run_command already.
That would allow avoiding the overhead of doing multiple project setups.
| @@ -0,0 +1,12 @@ | |||
| project('compiler_object_in_run_command', 'c') | |||
There was a problem hiding this comment.
If you're going to create an entirely new test entry for this then avoid adding a language to it, as that adds a lot of additional time to the test runtime, and we aren't actually using a compiler for anything here.
run_command() can take a list of parameters that will describe the command being run, and an ExternalProgram object can be provided here.
When setting up the ExternalProgram from a cross-file as a list, for instance:
The ExternalProgram.get_path() method will be used to prepare the run_command arguments, but it would return '--target=arm-androideabi' instead of the proper argument.
This commit ensures every part of the command is actually used in this case.