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

Make it possible to override find_program [skip ci] #3218

Merged
merged 9 commits into from
Apr 16, 2018
Merged

Conversation

jpakkane
Copy link
Member

This is not yet finished, hence the CI skipping.

The point of this PR is that you can do meson.override_find_program('progname', [program object]) which causes all find_programs for programe to use the given program object instead.

  • trying to override the name after it has been used is a hard error
  • trying to override an overridden name is a hard error
  • works only for ExternalProgram objects but could in theory be expanded to do Executable as well but that has more corner cases
  • documentation updates are missing

The point of this is to gather comments to see whether this is a usable approach. With this it should be possible to both bootstrap glib by itself and also to make it usable as a subproject on platforms where it is not available.

@nirbheek
Copy link
Member

I'm really glad you posted this! Just yesterday I was building gstreamer on Windows to prepare for the 1.14 release and it was too difficult to do that without this feature.

trying to override the name after it has been used is a hard error
trying to override an overridden name is a hard error

These are strict requirements, but I can see why they exist. I think it's good to start with these and relax them if/when we find reliable ways to do that.

works only for ExternalProgram objects but could in theory be expanded to do Executable as well but that has more corner cases

Build-time targets do need a bit of extra work, but I think we can have configure_file() working in this PR since they will be available on-disk when meson.override_find_program() is called with the appropriate object.

Without that, it's actually really hard to build anything that uses the GNOME module using subprojects. For instance, if you want to build gstreamer on Windows, you need the glib subproject which provides glib-tools. The most common ones are glib-mkenums, glib-genmarshal and gdbus-codegen all of which are configure_file().

It's the same if you want to build anything that uses introspection on Windows, since the gobject-introspection project provides g-ir-scanner, g-ir-compiler, etc, which are configure_file().

PS: [skip ci] has to be in the commit title, not the PR title 😄

@nirbheek
Copy link
Member

Actually, supporting the File object returned by configure_file() is almost trivial: just run ExternalProgram on it inside override_find_program_method().

@sarum9in
Copy link
Contributor

For distro overrides it would be super convenient to have some flag or option to override specific programs without meson.build modifications. Probably this would be better as separate feature though.

@jpakkane
Copy link
Member Author

That would be #3001 again.

@textshell
Copy link
Contributor

Looking forward a bit i would like to have this flexible enough to be able to use this for say qt or other situation where the tools are actual compiled programs as well.

From irc, it seems that this could be extended to exectuables, but that using them at meson.build evaluation time would have to be a hard error. That seems like a strange special case for find_program, but maybe that is ok? Or maybe we would want to make this more explicit with some kwarg or an different method than find_program?

exe = exe.held_object
if isinstance(exe, mesonlib.File):
abspath = exe.absolute_path(self.interpreter.environment.source_dir,
self.interpreter.environment.build_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Flake8]

[E127] continuation line over-indented for visual indent

@jpakkane
Copy link
Member Author

Now works with configure_file, please try it out. All tests should pass, documentation still missing.

@nirbheek
Copy link
Member

nirbheek commented Apr 5, 2018

So, I found that one particular workflow didn't work: checking if a tool is provided by the system, and taking some action to 'provide' (override) it if it isn't.

This has two use-cases:

  1. Preferring system versions of tools if available (in glib, for example)
  2. Taking some expensive action (such as building a subproject) to provide a tool only when absolutely necessary

I added a WIP commit for that, but this does change the semantics of the system, since now find_program() calls can now pass later even if previous (optional) calls failed. We should think of a better way to expose this, or a better way to cover those use-cases.

Alternatively, we can go with this and say that people should override their programs as soon as possible.

@nirbheek
Copy link
Member

nirbheek commented Apr 5, 2018

FWIW, with the commit I pushed, I have gst-build building with just a compiler + meson + ninja on Windows \o/

@jpakkane
Copy link
Member Author

jpakkane commented Apr 8, 2018

Documentation updated. This should be ready for final review.

Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is my WIP commit still in the list on purpose?


```meson
prog = find_program('my_custom_script')
meson.override_find_program('mycodegen', prog)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an example for configure_file() too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably go to actual documentation, people on average don't read old release notes for stuff. That would require writing a proper page for that, though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should start linking to the release notes for each feature; the (added in v0.xx.y) text is a good candidate for that.

@tp-m tp-m added this to the meson-next milestone Apr 10, 2018
nirbheek added a commit to centricular/gst-build that referenced this pull request Apr 11, 2018
The patches from the following PR (or 0.46.0, not released yet) are
needed:

mesonbuild/meson#3218
@nirbheek
Copy link
Member

I made all the changes I mentioned, +1 from me now!

@nirbheek
Copy link
Member

Would be great to have this in 0.46!

@nirbheek
Copy link
Member

I just realized that a nicer name for this is meson.provide_program(), in case we want to rename it 😄

gnomesysadmins pushed a commit to GNOME/glib that referenced this pull request Apr 15, 2018
Several of our tools are installed and are used by other projects to
generate code. However, there is no 'install' when projects use glib
as a subproject, so we needed some way for glib to 'provide' these
tools.

Starting from Meson 0.46, this can be done with the
meson.override_find_program() function. This also means that we can
use these tools ourselves via the GNOME module and don't need to
manually create command-lines.

WIP because it requires mesonbuild/meson#3218
nirbheek and others added 4 commits April 15, 2018 13:32
Otherwise we can't do the following workflow:

if not find_program('foo', required : false).found()
  subproject('provides-foo')
endif

Where 'provides-foo' has a meson.override_find_program() on
a configure_file() or similar.
Also link to the release notes snippet from the Reference manual
gnomesysadmins pushed a commit to GNOME/glib that referenced this pull request Apr 15, 2018
Several of our tools are installed and are used by other projects to
generate code. However, there is no 'install' when projects use glib
as a subproject, so we needed some way for glib to 'provide' these
tools.

Starting from Meson 0.46, this can be done with the
meson.override_find_program() function. This also means that we can
use these tools ourselves via the GNOME module and don't need to
manually create command-lines.

WIP because it requires mesonbuild/meson#3218
The user doesn't need to know whether or not the program was found,
especially not when it's spammed for every gnome.foo() function
nirbheek added a commit to centricular/gst-build that referenced this pull request Apr 15, 2018
The patches from the following PR (or 0.46.0, not released yet) are
needed:

mesonbuild/meson#3218
@jpakkane
Copy link
Member Author

I prefer override_find_program because the name is very explicit in what it does, moreso than provides_program. Also if we get the generic overrider functionality it would be nice to consistently use "override" to mean "change the default behaviour of some feature".

@nirbheek
Copy link
Member

👍

@jpakkane jpakkane merged commit 0e00470 into master Apr 16, 2018
@jpakkane jpakkane deleted the findoverrider branch April 16, 2018 19:22
nirbheek added a commit to centricular/gst-build that referenced this pull request Apr 21, 2018
The patches from the following PR (or 0.46.0, not released yet) are
needed:

mesonbuild/meson#3218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants