Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfe: extend run_target() to match run_command() #13206

Open
stsp opened this issue May 11, 2024 · 15 comments
Open

rfe: extend run_target() to match run_command() #13206

stsp opened this issue May 11, 2024 · 15 comments

Comments

@stsp
Copy link
Contributor

stsp commented May 11, 2024

Currently it seems impossible to run
some command at build time, and
capture its output.
It would be good to add the capture
arg to run_target() (similar to the one
of run_command()), and add an stdout()
method to the class returned by
run_target().

@eli-schwartz
Copy link
Member

Adding a .stdout() method to a run_target doesn't make much sense since a run_target would be executed by ninja and not by meson.

run_target doesn't produce output artifacts, it is used for creating "PHONY" targets such as ninja flash to copy a built firmware image to some connected hardware. If you want to capture its output, surely it should just be a custom_target, which does have a capture arg?

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

surely it should just be a custom_target, which does have a capture arg?

But its return type doesn't seem to
have stdout() method, so something
like this isn't currently possible:

t = custom_target(..., capture: true)
t2 = custom_target(..., depends: t, command: [some_cmd, '-e', t.stdout()])

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

I.e. custom_target() does have capture:,
but it still wants to create the file. It doesn't
give you the output as a variable that you
can use later.

@eli-schwartz
Copy link
Member

surely it should just be a custom_target, which does have a capture arg?

But its return type doesn't seem to have stdout() method, so something like this isn't currently possible:

t = custom_target(..., capture: true)
t2 = custom_target(..., depends: t, command: [some_cmd, '-e', t.stdout()])

That is because you can capture the stdout in one of two ways:

  • at build time (when ninja is run) by storing it in a file
  • at setup time (when meson is run) by returning it in a meson variable

Both run_target and custom_target are compile rules, they are not run at all when meson is run and cannot return stdout in the meson.build file. run_command is different -- it is only run by meson, and it returns stdout to the meson.build file and cannot be re-evaluated by ninja.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

Yes, I understand what you say.
What I don't understand, however,
is how the problem should be solved.
You seem to imply whatever runs by
ninja, cannot carry stdout between
targets. Something I easily do with
make.
So what should be a solution?
Extend ninja?

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

I mean, why ninja can't be instructed
to put stdout into a variable?
Why it can't be instructed to substitute
that variable somewhere later?

@eli-schwartz
Copy link
Member

eli-schwartz commented May 16, 2024

I am not sure what you mean by that. Make can't do what you're saying either.

Make does mix setup and compile in the same action though:

T = $(shell run_command ...)

custom_target1:
    some_cmd -e $(T)

Then, Make converts the command block, replacing variables computed during the setup pass, into a shell script, and executes it with the replacement T.

But make can't do that with two compile rules since each one is run inside its own new shell. This doesn't work:

custom_target1:
    T=$$(some_cmd1)

custom_target2: custom_target1
    some_cmd2 -e $$(T)

You can still do what make allows, with meson too. But the setup stage isn't run every time, so the value of T will be the same every time you run ninja.

If you want it to change without having to rerun meson each time, you could have some_cmd allow reading its data from a file instead.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

You can still do what make allows, with meson too. But the setup stage isn't run every time, so the value of T
will be the same every time you run ninja.

No, the problem is, I really need to
run shell only after some other
custom_target() is already built.
So what I want, is:

t1 = custom_target()
t = run_shell(depends: t1, capture: true, check: true, command: some_cmd)
t2 = custom_target(..., depends: t1, command: [some_cmd2, '-e', t.stdout()])

This is possible with make simply because
the variable assigned with = is expanded
when target is being built, which is what I need.
If I use := for an assignment, then it is
expanded when parsed, which would be
an equivalent of run_command() in meson,
but its not what I need.
Of course when I write
t = run_shell(depends: t1, capture: true, check: true, command: some_cmd)
I realize that in make I can't precisely tell
that the assignment should be done after
t1 is being built. I have to rely on the fact
that t2 is built after t1 and the variable is
expanded when t2 is being built. Its fine
with me, although with meson I was hoping
to be able to do exactly this:

custom_target1:
    T=$$(some_cmd1)

custom_target2: custom_target1
    some_cmd2 -e $$(T)

which would be an advantage over make.

@eli-schwartz
Copy link
Member

The assignment to T=$$(...) happens in a sh shell, not in make. It disappears as soon as the sh shell finishes building custom_target1.

You can have t2 = custom_target(.........) use a command that looks like this:

command: ['sh, '-c', 'T=$(cat custom_target1.out); some_cmd -e "$T";']

meson will produce a build rule that runs the entire sh command for you, including reading the "captured to a file" output of the first custom target so it can be passed as an argument to the current build rule.

This isn't performed at setup time.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

Yes, something like this can work,
except that command for T is quite long,
and has sh -c and substitutions on its
own.
Googling a bit about ninja, it seems they
suggest to just write the wrapper scripts
for such cases.
Wrapper script can work, in which case I'll
end up with something like this:
command: ['sh', '-c', 'wrapper.sh', t1.full_path(), '@INPUT@', '@OUTPUT@'])
I'll do that, but I still wonder why meson
can't do that for me, allowing a simpler
syntax and no silly scripts.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

So if there is nothing to improve on
meson side, please close.
I ended up with a clumsy script that
takes 7 parameters, and that works.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

Why it takes 7 parameters, is because
I pass many find_program().full_path()
to it, which obviously could be much
better handled within meson.

@eli-schwartz
Copy link
Member

Wrapper script can work, in which case I'll end up with something like this: command: ['sh', '-c', 'wrapper.sh', t1.full_path(), '@INPUT@', '@OUTPUT@'])

You can probably just use:

command: [find_program('wrapper.sh'), t1.full_path(), '@INPUT@', '@OUTPUT@']

I'll do that, but I still wonder why meson can't do that for me, allowing a simpler syntax and no silly scripts.

Someone would have to define what that syntax would look like for meson, and how it would perform that wrapping.

I worry that it is the kind of feature request easier to ask for than to implement...

@eli-schwartz
Copy link
Member

Why it takes 7 parameters, is because I pass many find_program().full_path() to it, which obviously could be much better handled within meson.

You can just pass the find_program itself, no need for full_path(). Using full_path() is only really useful when the relative path that meson writes to the ninja file is "wrong" for the shell script because the shell script internally does a cd ... and the path breaks... but you could also use realpath in the shell script to get the absolute location of the arguments.

@stsp
Copy link
Contributor Author

stsp commented May 16, 2024

Ok, first let me point out that what
I want, is possible with make.
Proof:

all: bar

foo:
        $(eval P != echo $$@ | sed 's/oo/tst/')

bar: foo
        @echo $(P)

This example replaces oo in foo
with tst, so the output will be ftst.
Please note that I print the output in
a different recipe, which means I
"moved" the stdout of foo to bar.

As for not passing full_path() - thanks,
this indeed works when I pass the
individual arguments. If however I
concatenate strings for /bin/sh -c, I
still need to use full_path().

Someone would have to define what that syntax would look like for meson, and how it would perform that wrapping.

I think this can look like this:

t1 = custom_target()
c1 = run_shell(depends: t1, command: [find_program(p1), t1])
c2 = run_shell(depends: c1, command: [find_program(p2), c1.stduot()])
...
t2 = custom_target(depends: c5, command: [some_cmd_that_uses_c1...5.stdout()])

As was pointed out, make supports
this natively. I only showed an example
of variable passing, but the whole concept
can use the intermediate targets this way:

all: t2
.INTERMEDIATE: c1

t1:
        touch $@

c1: t1
        $(eval P != echo $$@tst)

t2: c1
        @echo $(P)
        touch $@

c1 here is exactly the intermediate target
that only runs a shell command. It needs to
be marked as .INTERMEDIATE:, in which
case make won't re-build anything based
on a missing file with that name.
So this fully matches the initial task.

If ninja doesn't support that natively, then
meson can use temporary files. It can translate
every run_shell() into a target that creates
a temporary file, and the stdout() method
will just print its content. Ordering is also
possible to keep with temporary files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants