Closed glehmann closed 5 months ago
Nice workaround! I'm fine with this approach, I would however like to see it done in all images and not just aarch64!
Sure! I was just checking that you were ok with that before patching all the images — there is quite a lot :)
I'm thinking to start by adding a test that runs env -i ${CROSS_TARGET_RUNNER} /a/command
. That way we should be sure that we have the minimum environment configured in each image.
I also have a slight concern about what happens if the env is wiped and
@DEFAULT_QEMU_LD_PREFIX@
hasn't been replaced by us.
@DEFAULT_QEMU_LD_PREFIX@
is just the default value in ${QEMU_LD_PREFIX:-@DEFAULT_QEMU_LD_PREFIX@}
, and QEMU_LD_PREFIX
is still set in the docker image. So if @DEFAULT_QEMU_LD_PREFIX@
is not replaced by its value, cross will continue to work as it does today: just fine in most case, and not able to run the executable if the env is cleaned.
We should, if this lands, ensure that https://github.com/cross-rs/cross-toolchains also has been updated
ok, just a few more images ;-)
@DEFAULT_QEMU_LD_PREFIX@
is just the default value in${QEMU_LD_PREFIX:-@DEFAULT_QEMU_LD_PREFIX@}
, andQEMU_LD_PREFIX
is still set in the docker image. So if@DEFAULT_QEMU_LD_PREFIX@
is not replaced by its value, cross will continue to work as it does today: just fine in most case, and not able to run the executable if the env is cleaned.
I understand this, my concern is simply about what qemu does when the env has been cleared, the default has not been replaced by the docker build and the only path in QEMU_LD_PREFIX
is a relative-looking non-existant path (@DEFAULT_QEMU_LD_PREFIX@
). I don't think anything bad will happen but just wanted to raise it :D.
rg -l 'CROSS_TARGET_RUNNER=' -g 'Dockerfile.*linux*'
should be all the files that need to be fixed (haven't tested this, no access to my computer rn)
I understand this, my concern is simply about what qemu does when the env has been cleared, the default has not been replaced by the docker build and the only path in
QEMU_LD_PREFIX
is a relative-looking non-existant path (@DEFAULT_QEMU_LD_PREFIX@
). I don't think anything bad will happen but just wanted to raise it :D.
so I've tested it:
QEMU_LD_PREFIX
is not set. Qemu is probably checking if some files exist in @DEFAULT_QEMU_LD_PREFIX@
, but there's nothing visible for the end user./ci try -t linux
Starting try run. Link to action
The errors are expected I think
I just realised something I shouldve thought about 😊 the tests dont ever hit this :D
I think we should a new testcase that either uses assert_cmd or simulates the env clearing.
/ci try -t aarch64-unknown-linux- -t i686-linux*
Starting try run. Link to action
I think the tests won't help until the PR made it to the main
branch: they are not using the image from this PR.
https://github.com/cross-rs/cross/actions/runs/7679215720/job/20929912781?pr=1420##step:11:29
interesting to see that some tests are passing though — QEMU_LD_PREFIX
doesn't seem to be necessary for all the targets
They are definitely using the new images, see --tag ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main
and sed ...
oh, ok my bad: I've been confused by the name ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main
. I've downloaded it locally and it doesn't contain the changes of the PR.
I'm looking at it
I tried to rebuild the image from scratch and rerun the test locally — it still runs without problem.
I've set CROSS_DEBUG
in test.sh
to get a better chance to understand what is going on in the CI
/ci try -t aarch64-unknown-linux-* -t i686-linux
my attempt at triggering a new test run deesn't seem very effective
@Emilgardis could you trigger a new run for this PR?
/ci try -t aarch64-unknown-linux- -t i686-linux*
Starting try run. Link to action
If you can't figure it out I'll take a closer look into it myself! Seems very strange that it passes for you locally :/
Sidenote: I've been wanting to make the tests a bit more reliable to run and be more structured, meaning migrating the bash script to rust. I have a local branch doing just that but need to finish it
on the CI:
qemu-user
runner https://github.com/cross-rs/cross/actions/runs/7687457572/job/20947526693?pr=1420##step:12:4295qemu-system
runner https://github.com/cross-rs/cross/actions/runs/7687457572/job/20947526693?pr=1420##step:12:4624it fails on the line 3 of launch.rs
: it can't retreive the CARGO_TARGET_AARCH64_UNKNOWN_LINUX_RUNNER
env var.
I don't know what is qemu-system
is it possible that it clears the environment variables?
BTW I am launching the test locally with TARGET=aarch64-unknown-linux-gnu CROSS_TARGET_AARCH64_UNKNOWN_LINUX_GNU_IMAGE=cross-aarch64-unknown-linux-gnu ./ci/test.sh
, without any RUNNERS
env var, so I am never running with qemu-system
.
ofc! That's a tricky problem, I'd say lets just skip this test if the runner is qemu-system
ofc! That's a tricky problem, I'd say lets just skip this test if the runner is
qemu-system
done! Could you trigger the CI again?
/ci try -t aarch64-unknown-linux- -t i686-linux*
Starting try run. Link to action
@Emilgardis could you run the CI again? There was a remaining runner
config in Cross.toml
from the previous test that was forcing the test to running with qemu-system
/ci try -t aarch64-unknown-linux- -t i686-linux*
Starting try run. Link to action
going to do one last try with all targets to see if we missed anything obvious, I expect this try to fail for mips and riscv due to other problems not related to this changeset.
/ci try
Starting try run. Link to action
apparently we'll need a few more environment variables: https://github.com/cross-rs/cross/actions/runs/7693713886/job/20963153896#step:12:3102
the CARGO_TARGET_*_RUNNER
can't be found on windows: https://github.com/cross-rs/cross/actions/runs/7693713886/job/20963144833#step:12:847 . Same for wasm: https://github.com/cross-rs/cross/actions/runs/7693713886/job/20963151310#step:12:839 .
I'm not sure how much sense it makes to run this kind of test in wasm: are we even allowed to start a new process?
For windows I think I have to write the test to not depend on CARGO_TARGET_*_RUNNER
env var, because the launch
binary can probably launch another program without a runner. We probably have to add a .exe
in the command path…
zig test is failing, but it doesn't seem related to the changes here was it failing before?
The missing variables are CROSS_TARGETARCH
, CROSS_TARGETVARIANT
, they are only used in Dockerfile.native
and Dockerfile.native.centos
For windows the problem is similar to the issue with qemu-system
. We want to not respect the runner, just run it directly.
For wasm I have absolutely no idea :/ (but it's probably the same as windows and qemu-system
)
Hmm actually, I might've misspoke there, on windows we do want to run wine
, we don't emulate cargo...
We need to preserve WINEPATH
and WINEPREFIX
, however the error message doesn't make sense to me...
Now thinking about this even more. The issue here is that build.rs is ran on the host right?
The problem isn't "cross run --target <target>
can't execute a binary compiled for it's own <target>
via std::process::Command
" right?
If that's the case, the added test case doesn't test correctly, what it should do is compile launch
without --target
, and since that's probably hard to do, should be done in build script
The missing variables are
CROSS_TARGETARCH
,CROSS_TARGETVARIANT
, they are only used inDockerfile.native
andDockerfile.native.centos
this problem should be fixed :-)
For windows the problem is similar to the issue with
qemu-system
. We want to not respect the runner, just run it directly.
I've changed the test to only use the runner if it's defined in the env var — in fact that's exactly what I did in assert_cmd
— and I rolled back my changes to not test qemu-system
since they are now passing :-)
Now thinking about this even more. The issue here is that build.rs is ran on the host right? The problem isn't "
cross run --target <target>
can't execute a binary compiled for it's own<target>
viastd::process::Command
" right?
the problem is to run everything for the target passed to cross. That may or may not require a runner depending on the virtualisation system used. I think the test does the right thing
I think we are not that far to have everything working :-)
/ci try
Starting try run. Link to action
test should use CROSS_FLAGS
to correctly use build-std
test should use
CROSS_FLAGS
to correctly use build-std
mmm, I think they are, aren't they?
I don't see what I have changed that makes the bsd systems fail :-/
for windows, launch.exe
, which is launched with wine
is able to launch foo.exe
directly, without calling wine
.
The test fails if I make launch.exe
use wine
because it can't find the wine
executable. So I guess I have to drop the CARGO_TARGET_<target>_RUNNER
, so we don't try to use wine if we are already in wine
you removed the if RUN check, that should be there because some of these we can't run on at all
I'm working on making assert_cmd working with cross.
There is one case a bit more difficult: when using
clear_env()
on a process, orenv -i
, the binary is not able to run anymore because theQEMU_LD_PREFIX
envvar is not set anymore.A (relatively) easy fix might be to ensure in
linux-runner
— and in any other runner — that the necessary environment is configured before running the executable. Something as proposed in this PR foraarch64-unknown-linux-gnu
.Would that be an acceptable solution?
https://github.com/assert-rs/assert_cmd/pull/193