Fixes for issues introduced in 5e627c #14865
Conversation
This was added on dcbaker request[0], but doesn't actually work the way we need it too, as meson always has too assume the tools from the pkg-config file are build machine tools. [0]:mesonbuild#14422 (comment)
|
I think I'd be a good idea in the future to replace all the internal use of |
|
This seems to fix things for GStreamer's cerbero, thank you! Verified it's using the Meson from this PR: |
| # Check if pkgconfig has a variable | ||
| dep = self.dependency(depname, native=for_machine is MachineChoice.BUILD, | ||
| required=False, wanted=wanted) | ||
| dep = self.dependency(depname, native=True, required=False, wanted=wanted) |
There was a problem hiding this comment.
This looks wrong to me, since now pkg-config will return a program targeting the build machine even when it's been explicitly requested to get one for the host machine.
There was a problem hiding this comment.
The way meson handles find_program, we can't rely on anything being the of the machine type we requested (a find_program with HOST will still end up falling to the system binaries and a one with BUILD will still pick a overridden one for the HOST). IMO this has to change, which is why I talked about replacing all use of native & for_machine with a T.Set[MachineChoice], but that clearly shouldn't be part of a rc bugfix.
The issue specifically is, that meson just passes the path pointed at by the pkg-config into ExternalProgram, which will simply assume that the executable pointed to by the path is executable by the build machine, which it may not if it comes from a HOST dependency.
There was a problem hiding this comment.
It is weird but it makes sense, a tool should preferrably be built for the build machine. You could have an extra argument support_cross: bool(*) and do native=support_cross or for_machine is MachineChoice.bool. But if support_cross is always true, one might as well write native=True.
(*) True if the output is machine-independent or can be tailored to an arbitrary machine
There was a problem hiding this comment.
@bonzini: Isn't that functionally the same as my proposed T.Set[MachineChoice]?
There was a problem hiding this comment.
I would leave it as is, no problem for me.
| scan_command += ['--quiet'] | ||
|
|
||
| if state.environment.is_cross_build() and state.environment.need_exe_wrapper(): | ||
| if state.environment.is_cross_build() and state.environment.need_exe_wrapper() and giscanner.for_machine is MachineChoice.BUILD: |
There was a problem hiding this comment.
What is going on here? There should never need to be an exe_wrapper for the build machine, the build machine is the machine that is natively running the compilation. the exe_wrapper is to allow the build machine to run binaries compiled for the host machine.
There was a problem hiding this comment.
This checks if the giscanner is from the build machine (right now its a python script with a native module, so this should always be the case [but I plan to change that]).
The exe_wrapper check is to pass the --use-binary-wrapper ... --{begin,end}-binary-wrapper-args to the build machine g-ir-scanner if that is required. So meson invokes the g-ir-scanner on the build machine directly and it makes use of the exe_wrapper whenever it needs to itself internally.
|
Given that #14908 required reverting part of the underlying PR this fixes, we should probably then also revert 5e627c entirely for now and revisit things after the release is out. |
|
I created a revert MR for this in #14929. If someone does not have a better course of action, I'll merge it tomorrow and then create rc3. |
jpakkane
force-pushed
the
master
branch
2 times, most recently
from
00d6953 to
6447832
Compare
|
Part of #14933 |
7 participants
CC: @dcbaker @xclaesse