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 meson.override_find_program more cross compile friendly #8664

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Apr 15, 2021

Currently, Meson doesn't track the difference between a find_program override for the build machine vs the host machine. So if you run some code like:

exe = executable(..., native : true)
meson.overide_find_program('exe', exe)

then run

exe = find_program('exe', native : false)

you'll get the exe your overwrote, even though that probably isn't what you meant. With this PR that is actually correctly tracked. The best part is that you don't need to pass a native argument to override_find_program in most cases, as Meson already knows what the built binaries are for. However, when using a script (like a python script) you do need to set the machine, otherwise it's assumed to be for the HOST machine.

@dcbaker dcbaker requested a review from xclaesse April 15, 2021 21:37
@dcbaker dcbaker force-pushed the submit/fix-override-find-program-native branch 2 times, most recently from 8f452d0 to e9e62fe Compare April 15, 2021 23:22
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/interpreter/mesonmain.py Outdated Show resolved Hide resolved
if isinstance(exe, mesonlib.File) or 'native' in kwargs:
for_machine = self.interpreter.machine_from_native_kwarg(kwargs)
else:
for_machine = exe.for_machine
Copy link
Member

Choose a reason for hiding this comment

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

I think we should error out if native kwarg does not match exe.for_machine.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that too, but sometimes things for the host machine are "runs on the host machine" and sometimes they mean "runs for the host machine", compilers, cmake, pkg-config, and some other tools are all set as "host machine binaries" but they actually run on the build machine and produce output for the host machine. Not sure if that's a worthwile thing to handle or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, the more I think about it the more I think starting with the stricter behavior and relaxing it after we find a usecase might be better.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly what I was going to say.

@dcbaker dcbaker force-pushed the submit/fix-override-find-program-native branch from e9e62fe to 259b700 Compare April 16, 2021 21:31
@dcbaker dcbaker force-pushed the submit/fix-override-find-program-native branch from 259b700 to 46c5667 Compare April 29, 2021 17:12
@nirbheek nirbheek added this to the 1.2.0 milestone May 23, 2023
@dcbaker dcbaker force-pushed the submit/fix-override-find-program-native branch from 46c5667 to 2e88c45 Compare May 24, 2023 23:10
dcbaker added 11 commits May 24, 2023 16:15
We need to know whether we checked on the host or build machine, as
they're not the same when cross compiling.
Currently we're not making use of this, but we will soon.
The subproject overrode a build (native) binary, but then the super
project called find_program() without native : true, this means that the
super expects a host binary, but gets a build binary.
A program for the host machine should only override host machine
binaries, and one for the build machine should only override build
machine binaries.

We don't need a native keyword argument here, because the Targets
already know what machine they're for, and we can use that information.
This is nice from an end user point of view as they don't need to know
whether the binary is host or build, Meson abstracts that away.
…ogram`

This is necessary when using a script (mesonlib.File) to override
a program.
It's clearer, especially when considered with `dependency_overrides`,
which is used for dependencies.
@dcbaker dcbaker force-pushed the submit/fix-override-find-program-native branch from 2e88c45 to a65594a Compare May 24, 2023 23:15
@xclaesse
Copy link
Member

As far as I remember, it was intentional to not have the native kwarg on meson.override_find_program(). The reason is a program is always used on the BUILD machine, with an exe wrapper if needed.

In your example:

exe_for_host = executable(..., native : false)
exe_for_build = executable(..., native : true)
meson.override_find_program('exe', exe_for_host)
meson.override_find_program('exe', exe_for_build)

I think erroring is actually the right thing, but the message could be better. There is no point in doing meson.override_find_program('exe', exe_for_host) because overridden programs are only useful to be run on the build machine.

On a similar topic, I had that PR: #10899. The idea is it only makes sense to override a program with something we can run. Could be a python script, an exe for build machine, or an exe for host machine and an exe wrapper is available.

@dcbaker
Copy link
Member Author

dcbaker commented May 25, 2023

Here's the problem, it's not really clear if being for BUILD means "runs natively" or "outputs for build". Certainly some HOST machine binaries are built for the HOST machine and need an exe wrapper, but others are very much not. Think of things like your compilers, linkers, strip, etc. There are also cases where programs generate different code depending on the platform/architecture they're generating for. flex and bison come to mind, where the windows versions do some trickery to make code that MSVC is happy with, but the *nix versions don't.

@xclaesse
Copy link
Member

Think of things like your compilers, linkers, strip, etc

I see, that makes more sense indeed. It can get useful together with #10938 too.

@dcbaker dcbaker modified the milestones: 1.2.0, 1.3.0 Jun 28, 2023
@dcbaker
Copy link
Member Author

dcbaker commented Sep 22, 2023

I'm closing this in favor of the version in my native subproject series

@dcbaker dcbaker closed this Sep 22, 2023
@dcbaker dcbaker deleted the submit/fix-override-find-program-native branch September 22, 2023 02:23
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.

None yet

3 participants