fix: build when target_arch==build_arch #146

sarfata · 2023-10-03T20:20:46Z

In #82 we fixed the build when the target arch is the same as the build arch. A side effect of the fix is that the rust build-scripts (which are intended to run on the build host) are now built with the target compiler and cflags.

This is fine in most cases but it breaks when the host system is older than the system we are building. For example, we might build for a newer glibc that will be available on the target but when we run the build script on the host, this version does not exist which leads to the error described in #83.

Another version of the same problem is that we may try to use some build flags that are available on the compiler for the target (aarch64-poky-linux-gcc) but not on the host compiler (gcc) because the host compiler is an older version of gcc than the one we are using for the target.

This patch fixes the problem by using two unstable features of cargo:

UNSTABLE_TARGET_APPLIES_TO_HOST allows us to pass the CARGO_TARGET_APPLIES_TO_HOST=false setting so that the target build settings are not used when building for the host.
UNSTABLE_HOST_CONFIG allows us to set build flags for the host in a [host] section of the Cargo.toml file.

The __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS environment variable is required to use these unstable features on a stable rust. This patch works with stable rust and does not require unstable.

The inspiration for this solution comes from buildroot.

fixes #83

martijnthe · 2023-10-04T08:31:49Z

@nastevens FYI, this fixes the same problem that #133 and #137 are also trying to fix.

Thank you in advance for taking a look.

posborne · 2023-10-06T05:01:00Z

Thank you @sarfata for the patch and @martijnthe for the reference to the other tickets. I believe this is sound but would love @nastevens to take a look as well but will probably move forward with it.

@banditopazzo @samliddicott @jameshilliard if you have other thoughts or are on board with this form of the patch. Seems clear we need to land one of these changesets in tree for the case described.

jameshilliard · 2023-10-06T05:09:18Z

classes/cargo.bbclass

+    if [ "${RUST_BUILD}" = "${RUST_TARGET}" ]; then
+        export __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS="nightly"
+        export CARGO_UNSTABLE_TARGET_APPLIES_TO_HOST="true"
+        export CARGO_UNSTABLE_HOST_CONFIG="true"


Why is this being set? I don't see any CARGO_HOST_ variables being set here. In buildroot we enable this so we can set the host config option CARGO_HOST_RUSTFLAGS.

If I understood correctly, it's because of the setting is in ${CARGO_HOME}/config

meta-rust-bin/classes/cargo.bbclass

Lines 68 to 70 in ebaab20

# (Rust unstable) - See do_compile below.

echo "[host]" >> ${CARGO_HOME}/config

echo "linker = '${WRAPPER_DIR}/linker-native-wrapper.sh'" >> ${CARGO_HOME}/config

Why not just set CARGO_HOST_LINKER="${WRAPPER_DIR}/linker-native-wrapper.sh" instead of using the cargo config file?

You should also be able to get rid of the cargo config entirely by configuring the target linker and any other config file variables something like this as well:

CARGO_TARGET_${RUST_TARGET}_LINKER="${WRAPPER_DIR}/linker-wrapper.sh"

In general I think sticking to configuring cargo using env variables only seems to be less likely to break than mixing env based configs with config file based configs.

Why is this being set?
As pointed out by @banditopazzo it's for the host configuration.

Why not just set CARGO_HOST_LINKER="${WRAPPER_DIR}/linker-native-wrapper.sh" instead of using the cargo config file?

When HOST != TARGET we configure both compilers via the .cargo/config file. I chose to keep doing this this way to stay as close as possible to what we have today.

I understand you would prefer to completely get rid of .cargo/config and move everything to env vars. Sounds good to me too.

When HOST != TARGET we configure both compilers via the .cargo/config file. I chose to keep doing this this way to stay as close as possible to what we have today.

In general there should be no need to configure cargo differently for HOST != TARGET vs HOST == TARGET builds if the host/target config splitting env variables are set up correctly AFAIU.

In general there should be no need to configure cargo differently for HOST != TARGET vs HOST == TARGET builds if the host/target config splitting env variables are set up correctly AFAIU.

We still need a way to pass the HOST linker (linker-native-wrapper.sh). If we just set the host/target config splitting env variables, then cargo reverts to gcc/ld with no options when compiling for the host. That might work sometimes but I don't think it's correct because I believe we want to use the sysroot-native libraries when building anything for the host.

On my system, linker-native-wrapper.sh contains:

#!/bin/sh gcc -L/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/usr/lib -L/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/lib -Wl,--enable-new-dtags -Wl,-rpath-link,/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/usr/lib -Wl,-rpath-link,/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/lib -Wl,-rpath,/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/usr/lib -Wl,-rpath,/home/thomas/yocto/build/tmp/work/cortexa57-poky-linux/xxx/1.0-r0/recipe-sysroot-native/lib -Wl,-O1 -Wl,--allow-shlib-undefined -Wl,--dynamic-linker=/home/thomas/yocto/build/tmp/sysroots-uninative/x86_64-linux/lib/ld-linux-x86-64.so.2 "$@"

If we do not set any host option, we will revert to just calling gcc and we do not get the -Wl,-rpath-link,... setting when building the build-script. Without this setting, when we run the build-script later, it cannot find the right version of glibc. This was causing #83.

There are two ways to set the Host Linker (via cargo_home/config like I have done here) or via CARGO_HOST_LINKER (like in the other PR). I chose the former because this way, the config file looks the same whether TARGET==HOST or not.

Without this setting, when we run the build-script later, it cannot find the right version of glibc. This was causing #83.

I think that was a different issue where cargo is picking up target linker flags for host builds rather than not picking up additional native flags for the host build, although I'm more familiar with buildroot which does things somewhat differently.

I chose the former because this way, the config file looks the same whether TARGET==HOST or not.

I mean, why not just get rid of the cargo config file entirely and pass all config options as env variables? In both cases they should look the same for TARGET==HOST or not AFAIU.

In buildroot we use only the env varibles since we'd otherwise have to regenerate the config file for each package build due to our per-package build isolation changing things like linker paths for each package build, by using env variables we don't have to worry about that as the env is updated automatically with the correct paths for each package.

Additionally this makes it easier for us to handle say python package builds with cargo modules(ie python-cryptography) as we can simply append the cargo environment to our python environment without having to also trigger a config regeneration operation.

jameshilliard · 2023-10-06T05:10:32Z

classes/cargo.bbclass

+    # When RUST_BUILD == RUST_TARGET, we need to use an unstable Rust feature
+    # to specify different build options for the target and the Host.
+    # The Host configuration is set in do_configure() above.
+    if [ "${RUST_BUILD}" = "${RUST_TARGET}" ]; then


Probably best to remove this check and just unconditionally apply these env variables like we do in buildroot.

banditopazzo · 2023-10-06T11:16:49Z

I'm not a big fan of using ${CARGO_HOME}/config to pass settings.

What's the reason behind this instead of using only env variables?

sarfata · 2023-10-06T17:43:10Z

I'm not a big fan of using ${CARGO_HOME}/config to pass settings.

What's the reason behind this instead of using only env variables?

I was just following what existed before the PR. I can change to env vars everywhere.

banditopazzo · 2023-10-06T18:27:10Z

I'm not a big fan of using ${CARGO_HOME}/config to pass settings.
What's the reason behind this instead of using only env variables?

I was just following what existed before the PR. I can change to env vars everywhere.

ok, @posborne what do you think?

posborne · 2023-10-06T20:09:27Z

@sarfata I'm good with using env vars over the config file and agree that when possible (which I think is most or all cases now) that is preferred. I believe when we were first developing this layer, that mechanism was either unavailable or not widely publicized which I suspect is that reason we used the config file approach but I don't believe there is any reason to continue with that approach now.

jameshilliard · 2023-10-06T20:39:03Z

when possible (which I think is most or all cases now) that is preferred

AFAIU all cargo config file options can be set via the env variables. Are there any that aren't working via the env?

posborne · 2023-10-07T02:14:58Z

AFAIU all cargo config file options can be set via the env variables. Are there any that aren't working via the env?

I believe this is the case now. @nastevens wrote the earliest versions of this layer back in 2016 originally, so I think the support may not have been there in cargo back then. Doesn't matter these days.

sarfata · 2023-10-09T06:56:17Z

Thanks everyone for opinions on this PR. Sorry for the delay getting the changes up. I am testing on a few different configuration and hoping to have this ready tomorrow.

sarfata · 2023-10-09T17:06:48Z

I have removed the cargo_home/config file to only use environment variables.
And instead of distinguishing "TARGET==HOST vs TARGET!=HOST", I now always use the CARGO_HOST_LINKER variable.

I think this addresses all the comments. I have tested this when TARGET==HOST and when not.

I had to use a little bit of inline python to rewrite the dashes into underscores for the environment variable name. I am not sure if there is a more Yocto-typical way to do this.

export CARGO_TARGET_${@rust_target(d, 'BUILD').replace('-','_')}_LINKER="${WRAPPER_DIR}/linker-wrapper.sh"

@posborne @jameshilliard @banditopazzo please let me know if you would like me to include anything else here.

posborne · 2023-10-09T19:45:00Z

classes/cargo_bin.bbclass

    export PKG_CONFIG_ALLOW_CROSS="1"
    export LDFLAGS=""
    export RUSTFLAGS="${RUSTFLAGS}"
-    export SSH_AUTH_SOCK="${SSH_AUTH_SOCK}"


@sarfata Is the drop of this intentional?

Nope - Good catch thanks. Just fixed.

posborne

LGTM and will approve and merge with SSH_AUTH_SOCK added back (open to more discussion). That flag was added in #143

In rust-embedded#82 we fixed the build when the target arch is the same as the build arch. A side effect of the fix is that the rust build-scripts (which are intended to run on the build host) are now built with the target compiler and cflags. This is fine in most cases but it breaks when the host system is older than the system we are building. For example, we might build for a newer glibc that will be available on the target but when we run the build script on the host, this version does not exist which leads to the error described in rust-embedded#83. Another version of the same problem is that we may try to use some build flags that are available on the compiler for the target (`aarch64-poky-linux-gcc`) but not on the host compiler (`gcc`) because the host compiler is older than the one we are using for the target. This patch fixes the problem by using two unstable features of cargo: - [`UNSTABLE_TARGET_APPLIES_TO_HOST`](https://doc.rust-lang.org/cargo/reference/unstable.html#target-applies-to-host) allows us to pass the `CARGO_TARGET_APPLIES_TO_HOST=false` setting so that the target build settings are not used when building for the host. - [`UNSTABLE_HOST_CONFIG`](https://doc.rust-lang.org/cargo/reference/unstable.html#host-config) allows us to set build flags for the host. The `__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS` environment variable is required to use these unstable features on a stable rust. This patch works with stable rust and does not require unstable. During review of this PR, it was decided to drop the `cargo_home/config` file completely and use only environment variables. The inspiration for this solution [comes from buildroot](https://github.com/buildroot/buildroot/blob/25d865996d1d9753fe7d4dfe39cf18c7e9f91224/package/pkg-cargo.mk#L26-L47). fixes rust-embedded#83

sarfata · 2023-10-09T21:24:42Z

@posborne SSH_AUTH_SOCK is back, I have squashed the commits and slightly updated the PR description. Ready to go I think.

posborne · 2023-10-10T06:13:58Z

Thanks @sarfata and everyone else that participated in the review and other iterations of the change.

posborne requested a review from nastevens October 6, 2023 05:01

posborne approved these changes Oct 6, 2023

View reviewed changes

posborne mentioned this pull request Oct 6, 2023

Fix incorrect LD/TARGET_LD env vars #129

Closed

jameshilliard reviewed Oct 6, 2023

View reviewed changes

posborne self-requested a review October 6, 2023 20:14

sarfata force-pushed the sarfata/fix-build-when-target_arch==build_arch branch 2 times, most recently from 223fb15 to b917586 Compare October 9, 2023 16:57

posborne reviewed Oct 9, 2023

View reviewed changes

sarfata force-pushed the sarfata/fix-build-when-target_arch==build_arch branch from b917586 to 8fb174f Compare October 9, 2023 21:23

posborne merged commit 2089ae6 into rust-embedded:master Oct 10, 2023

igiona mentioned this pull request Oct 11, 2023

Cross compilation broken since #146 #151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: build when target_arch==build_arch #146

fix: build when target_arch==build_arch #146

sarfata commented Oct 3, 2023

martijnthe commented Oct 4, 2023 •

edited

Loading

posborne commented Oct 6, 2023

jameshilliard Oct 6, 2023

banditopazzo Oct 6, 2023

jameshilliard Oct 6, 2023

sarfata Oct 6, 2023

jameshilliard Oct 6, 2023

sarfata Oct 6, 2023

jameshilliard Oct 6, 2023

jameshilliard Oct 6, 2023

sarfata Oct 6, 2023

banditopazzo commented Oct 6, 2023

sarfata commented Oct 6, 2023

banditopazzo commented Oct 6, 2023

posborne commented Oct 6, 2023

jameshilliard commented Oct 6, 2023

posborne commented Oct 7, 2023

sarfata commented Oct 9, 2023

sarfata commented Oct 9, 2023

posborne Oct 9, 2023

sarfata Oct 9, 2023

posborne left a comment

sarfata commented Oct 9, 2023

posborne commented Oct 10, 2023

	# (Rust unstable) - See do_compile below.
	echo "[host]" >> ${CARGO_HOME}/config
	echo "linker = '${WRAPPER_DIR}/linker-native-wrapper.sh'" >> ${CARGO_HOME}/config

fix: build when target_arch==build_arch #146

fix: build when target_arch==build_arch #146

Conversation

sarfata commented Oct 3, 2023

martijnthe commented Oct 4, 2023 • edited Loading

posborne commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banditopazzo commented Oct 6, 2023

sarfata commented Oct 6, 2023

banditopazzo commented Oct 6, 2023

posborne commented Oct 6, 2023

jameshilliard commented Oct 6, 2023

posborne commented Oct 7, 2023

sarfata commented Oct 9, 2023

sarfata commented Oct 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

posborne left a comment

Choose a reason for hiding this comment

sarfata commented Oct 9, 2023

posborne commented Oct 10, 2023

martijnthe commented Oct 4, 2023 •

edited

Loading