-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Purge much is_cross
and <things>_cross
without changing user interfaces---includes on #5263
#4010
Purge much is_cross
and <things>_cross
without changing user interfaces---includes on #5263
#4010
Conversation
Flake8 detected 7 issues on 78e71f6 |
mesonbuild/compilers/c.py
Outdated
@@ -228,9 +228,10 @@ def get_default_include_dirs(self): | |||
return [] | |||
|
|||
def gen_export_dynamic_link_args(self, env): | |||
if for_windows(env.is_cross_build(), env) or for_cygwin(env.is_cross_build(), env): | |||
m = env.machines[self.for_machine] | |||
if m.is_windows(env) or m.is_cygwin(): |
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.
Code like for_windows(env.is_cross_build(), env)
is very suspicious because it breaks on / precludes native: true
dependencies. env.machines[self.for_machine].is_windows()
tries to force the programmer to pull the machine from the target at hand, and works the same way whether in native or cross builds.
Overall I really like how this simplifies the various |
I think let's just wait on this until #3993 lands. I want to change some things here anyways, besides fixing the merge conflicts. If you like the |
78e71f6
to
bd60045
Compare
Flake8 detected 12 issues on bd60045 |
bd60045
to
6b715a7
Compare
Flake8 detected 6 issues on 6b715a7 |
Wow the Sider permissions list is crazy big |
6b715a7
to
3aab3b3
Compare
is_cross
and friends without changing user interfacesis_cross
and friends without changing user interfaces---blocked on #4445
3aab3b3
to
7ad54f9
Compare
Flake8 detected 14 issues on 7ad54f9 posted by Sider |
7ad54f9
to
2b1173c
Compare
2b1173c
to
58e92e1
Compare
515dd91
to
c1220fa
Compare
c1220fa
to
0be8aad
Compare
93e05b8
to
61fafb0
Compare
⚔️ conflicts. |
@jpakkane will fix in a moment. Reading over the very long and noisy thread, I think the most important comment so far is @dcbaker's #4010 (comment). |
adbcc39
to
431a41a
Compare
431a41a
to
f85fbb0
Compare
This is a small example of the `is_cross` removal the that abstraction enables.
In most cases instead pass `for_machine`, the name of the relevant machines (what compilers target, what targets run on, etc). This allows us to use the cross code path in the native case, deduplicating the code. As one can see, environment got bigger as more information is kept structured there, while ninjabackend got a smaller. Overall a few amount of lines were added, but the hope is what's added is a lot simpler than what's removed.
Some things, like `method[...](...)` or `x: ... = ...` python 3.5 doesn't support, so I made a comment instead with the intention that it can someday be made into a real annotation.
All uses now use `env.machines.YYY.is_XXX` instead.
`native` kwarg is already handled
(cherry picked from commit ae6426c)
f85fbb0
to
6d6af46
Compare
Wooo!!! Thanks! |
This is less hacky, and also prepares the way for mesonbuild#4010.
gobject-introspection is infamous for being difficult to get working with cross compilation. But the situation has improved recently with https://gitlab.gnome.org/GNOME/gobject-introspection/-/merge_requests/64, thanks to @kanavin from the Yocto project. The upshot is now only *some*, not all, of the executables need to be run on the host platform with the exe wrapper. On the meson side, there have been two attempts to fix things for cross: - https://github.com/mesonbuild/meson/pull/2965from @kanavin and the Yocto projejct, which because it predates mesonbuild#4010 had to be somewhat hacky - mesonbuild#7072 recently merged which allows specifying some binaries with a cross file. But, I think we can make a more seamless user interface that won't require extra config, like for native builds. gobject-introspection provides the binaries in its pkg-config file, and Meson now cleanly supports separate `native: true` and `native: false` pkg-config paths and lookup. We should just need to add separate `native: true` and `native: false` deps, and I have done that, but I am not sure exactly which should be used when. I am coming at this as a distro maintainer not even particularly involved with gnome things and so I'll need some advice on what to do next.
This is a draft of how I think cross should work. Compilers and targets track the machine they run on / target, rather than
is_cross
/want_cross
. This is usually done with afor_machine
field andMachineChoice
enum.MachineChoice
is abstract, representing the "build/host/target"' name, rather than the definition of the machines themselves. This allows the same code paths to be shared in the native and cross cases, vastly reducing the space of code paths.An example probably makes this clearer. Consider the situation of a single installed C target. As it is installed, it must be for the host machine (or target, but it's not currently possible to make things for the target platform). In the native case we have a single compiler for the build and host machines. In the cross case we have different ones for each.
As the code stands today, the host compiler goes in
compilers
in the native case orcross_compilers
in the cross case, and the target fishes out the compiler it needs from eithercompilers
in the native case, orcross_compilers
in the target case. So we have two conditional bits of code: where to put the compiler, and where to fetch the compiler, that should cancel out, but nothing really ensures that they in fact do. With the code in this PR, it's always unconditionally stored in the same places (core_data.compiler
,build.for_host.compilers
). This ensures the same behavior by construction.It's true with enough work Meson will work either way, so this sort of refactor might seem superfluous. But is also helps users of Meson catch cross-related bugs in their
meson.build
files. Currently, we error in the cross case when someone tries to install a native (build platform) target, or link together cross and native targets. But in the native case we have no idea when those things would happen. But with this change since we track build machine vs host machine vs target machine in all cases, we can always when a build platform target is installed, or when a host machine and build machine target are linked together. These errors become warnings in the native case (for backwards comparability), helping users that rarely cross-compile their code detect this bugs sooner.I hope in opening this now, I can make myself clearer in the issues, and better motivate incremental #3993. If this looks good, it can hopefully be broken up into additional manageable issues. It also lays a foundation upon which #3972 #3969 would be natural to implement.
Fixes #5102