-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixes for issues introduced in 5e627c #14865
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1194,7 +1194,7 @@ def generate_gir(self, state: 'ModuleState', args: T.Tuple[T.List[T.Union[Execut | |
| scan_command: T.List[T.Union[str, Executable, 'ExternalProgram', 'OverrideProgram']] = [giscanner] | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| if not state.environment.has_exe_wrapper(): | ||
| mlog.error('generate_gir requires exe_wrapper') | ||
|
|
||
|
|
@@ -1269,6 +1269,18 @@ def generate_gir(self, state: 'ModuleState', args: T.Tuple[T.List[T.Union[Execut | |
| # We have to cast here because mypy can't figure this out | ||
| T.cast('T.Dict[str, T.Any]', kwargs)) | ||
|
|
||
| # The g-ir-compiler must match the host architecture. If we have | ||
| # found a g-ir-compiler for the build machine (due to it being | ||
| # specified in the cross file or meson just falling back to the | ||
| # system g-ir-compiler), check if its command begins with | ||
| # the command for exe_wrapper, in which case assume that it is | ||
| # actually compiled for the host architecture. | ||
| if state.environment.is_cross_build() and state.environment.need_exe_wrapper() and gicompiler.for_machine is MachineChoice.BUILD: | ||
| binary_wrapper = state.environment.get_exe_wrapper().get_command() | ||
| if gicompiler.get_command()[:len(binary_wrapper)] != binary_wrapper: | ||
| msg = 'Architecture of g-ir-compiler must match the one of the host machine' | ||
| raise MesonException(msg) | ||
|
|
||
| typelib_output = f'{ns}-{nsversion}.typelib' | ||
| typelib_cmd = [gicompiler, scan_target, '--output', '@OUTPUT@'] | ||
| typelib_cmd += state.get_include_args(gir_inc_dirs, prefix='--includedir=') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 donative=support_cross or for_machine is MachineChoice.bool. But ifsupport_crossis always true, one might as well writenative=True.(*) True if the output is machine-independent or can be tailored to an arbitrary machine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bonzini: Isn't that functionally the same as my proposed
T.Set[MachineChoice]?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave it as is, no problem for me.