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

Teach custom_target() to depend on run_target() #6984

Closed
wants to merge 1 commit into from

Conversation

elmarco
Copy link
Contributor

@elmarco elmarco commented Apr 16, 2020

No description provided.

@elmarco elmarco requested a review from jpakkane as a code owner April 16, 2020 18:07
@xclaesse
Copy link
Member

IIRC it won't work with ninja backend because ninja base dependencies on produced file mtime, but run_target() has no output file.

One approach I tried but never really finished is to allow custom_target() to have no output in which case let Meson creates a dummy output for it, can be done in meson_exe.py script.

@jpakkane
Copy link
Member

What specific problem would this approach solve?

@elmarco
Copy link
Contributor Author

elmarco commented Apr 17, 2020

@jpakkane

I reached that issue while trying to have some basic cargo build integration.

If I use custom_target(), I have to specify binary outputs. Not only the output location may vary, but it would also not allow me to install the various binaries on different locations.

So I added a run_target() rule for cargo build, and made various custom_target() depend on it.

fwiw, the embarrassing result: https://gitlab.freedesktop.org/elmarco/vnet/-/blob/master/meson.build

@xclaesse
Copy link
Member

For cargo integration you might want to look at gst-plugins-rs and #2617

@elmarco
Copy link
Contributor Author

elmarco commented May 8, 2020

IIRC it won't work with ninja backend because ninja base dependencies on produced file mtime, but run_target() has no output file.

With ninja 1.10 it works as I expected it to, it runs the run_target() before.

@eli-schwartz
Copy link
Member

I think it's evident that this PR is going nowhere.

Furthermore, I would like to say that I strongly disagree with the entire concept, and you end up with effectively build_always_stale: true as a fundamental side effect of this entire technique for handling "targets with multiple outputs". However...

... you don't need this. The proposal was based on the assumption that "Not only the output location may vary, but it would also not allow me to install the various binaries on different locations." But you can do both of these things.

Firstly, a complicated wrapper over an external build system such as cargo really needs to do all work in a private directory, then finish off after cargo is done by copying the build artifacts out of cargo and into Meson.

Secondly, installing to different locations is a Meson feature for exactly cases like this. From the custom_target documentation:

If only one install_dir is provided, all outputs are installed there. Since 0.40.0 Allows you to specify the installation directory for each corresponding output. For example:

So I think the problem this PR tries to solve can be better solved by using existing Meson features, therefore we do not need to change anything. :)

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

Successfully merging this pull request may close these issues.

4 participants