Update Docker images to Ubuntu-24.04 and Cargo dependencies [continuation]#1779
Update Docker images to Ubuntu-24.04 and Cargo dependencies [continuation]#1779MichaIng wants to merge 22 commits into
Conversation
Co-Authored-By: itpetey <3638076+itpetey@users.noreply.github.com> Co-Authored-By: JanDiederich <29060204+JanDiederich@users.noreply.github.com>
Co-authored-by: Michalng <28480705+Michalng@users.noreply.github.com>
Co-authored-by: Michalng <28480705+Michalng@users.noreply.github.com>
Previous commits of this PR implicitly raised MSRV to 1.88, which stableized `let` in `&&` combinations. This commit freezes a dependency to keep MSRV 1.85 compatibility, aligns edition 2024 across xtask, and updates the format accordingly. Signed-off-by: MichaIng <micha@dietpi.com>
Use identical github.token payload instead of legacy secrets.GITHUB_TOKEN, as CI can run into unnecessary format access isues to secrets, while the default GitHub Actions token is actually no secret. Signed-off-by: MichaIng <micha@dietpi.com>
Docker accepts lowercase characters for registry targets only. Sadly there is no native way in GitHub Actions to transform it, hence this needs to be done in shell. Signed-off-by: MichaIng <micha@dietpi.com>
Signed-off-by: MichaIng <micha@dietpi.com>
Signed-off-by: MichaIng <micha@dietpi.com>
|
/ci try |
This comment has been minimized.
This comment has been minimized.
| Starting try run. [Link to action](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}?pr=${{ github.event.issue.number }})" | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| GH_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
It took me a while to find the repo setting to grant GITHUB_TOKEN write access to PR comments, to leave /ci try related comments. The error message without it gave the impression access to secrets would be the issue, hence using the identical github.token payload was an attempt to fix this on my fork, before I found the repo setting which actually fixed it.
Hence changes to both workflows could be reverted, but github.token is used in some places already, and the modern/clearer way to access the token. Hence I suggest to leave it this way. Functionally, both are 100% identical, including their masking in logs etc.
There was a problem hiding this comment.
Agreed, dont know why we went eith the secret here, maybe just habit 😅
| echo '# Cargo.toml | ||
| [workspace] | ||
| members = ["'"$(basename "$td")"'"] | ||
| resolver = "3" |
There was a problem hiding this comment.
With edition = 2024, the implicit default is resolver = 3, while otherwise the default is resolver = 1. When using edition = 2024, if any workspace does not declare resolver explicitly, a warning is emitted stating this. Image test logs in CI hence get very bloated with this. To mute them, I added it here for all test builds.
| # ^^subst is not supported on macOS bash (bash <4) | ||
|
|
||
| # shellcheck disable=SC2155 | ||
| export TARGET_UPPER=$(echo "$TARGET" | awk '{print toupper($0)}') | ||
| export TARGET_UPPER=${TARGET_UPPER^^} |
There was a problem hiding this comment.
Since those old macOS runners are not available anymore (the oldest available x86_64 is macos-15-intel), there is no bash <4 in any case. Hence switched to ^^ and used ,, below.
|
|
||
| # OCI does not support uppercase characters from orga/owner names | ||
| IMAGE=${IMAGE,,} | ||
| CROSS_IMAGE=${CROSS_IMAGE,,} |
There was a problem hiding this comment.
Parts of CI failed on my fork since my account name has uppercase letters, and the owner/orga name is used as basis for GitHub container registry access/uploads/image names/tags etc, failing all over the place. GitHub workflow files do not support any syntax to turn github.repository_owner and such to lowercase. It can be done in shell in a step, but applies to the current job only. Furthermore, try.yml always invokes the default branch version of ci.yml, which made it impossible to run vast parts of CI on a PR on my fork. Best solution was to assure relevant variables are lowercase in CI test scripts and Rust code. This makes life easier for people who are testing on their fork.
| CT_REMOVE_DOCS=y | ||
| GLIBC_MAKEINFO_WORKAROUND=y | ||
| CT_LOG_PROGRESS_BAR=n | ||
| CT_LOG_ALL=y |
There was a problem hiding this comment.
The arm-unknown-linux-gnueabihf build was failing, and the only way to find out why was to unmute STDOUT (see removal of silence_stdout in crosstool-ng.sh below), disable the progress bar (actually a spinner) that otherwise still masks STDOUT with a flood or single character lines, and raise the log level to include actual object install steps. The latter is causing a lot of output lines, so one might want to revert this (remove CT_LOG_ALL=y), but on the other hand, this is for our CI only, hence no real reason to be frugal with informational output?
The reason for the failure was the attempt to install (build) a particular manual file from the gcc build for Android. The 1st commit (of #1580) lowered the target version from 2.31 to 2.17 (not sure what it has to do with the aim of the PR anyway), and this old gcc version manual has an incompatibility with recent texinfo. I found some flags to disable manual builds, but they are actually set by default (CT_REMOVE_DOCS=y which implies CT_BUILD_MANUALS=n), and have no effect on the gcc build, where manuals are always built and installed, without any native way to disable/skip. But turns out, crosstool-ng recognized this issue as well and added an additional flag GLIBC_MAKEINFO_WORKAROUND=y to solve exactly this: It patches the old gcc sources to skip manual builds.
| sed -i 's/lower <= value <= upper/lower <= int(value) <= upper/' ./bionic/libc/fs_config_generator.py | ||
|
|
||
| # Soong looks for a python2.7 executable explicitly | ||
| ln -s python3 /usr/bin/python2.7 |
There was a problem hiding this comment.
There is a lot of Python 2-only code in these old Android sources, but Ubuntu 24.04 has no Python 2 available anymore. We could pull it from elsewhere, but luckily Ubuntu 24.04 (Python 3.12) is the last one which still contains 2to3 to convert scripts, most importantly print 'foo' => print('foo'). 2to3 fails on a bunch of scripts, some of them are in weird binary format (most likely embedded binary code), hence they need to be skipped. .repo/repo/ is pure Python 3 already, but has a lot of scripts 2to3 cannot process, hence it is skipped entirely. All those which caused failures during the Android build can be migrated, with only a single manual sed needed (value is a numerical str, which was fine for Python 2, but not for Python 3).
An alternative solution would be to switch to newer Android sources, but I didn't want to cause a breaking change, if avoidable.
| debsource="deb http://http.debian.net/debian/ buster main" | ||
| debsource="${debsource}\ndeb http://security.debian.org/ buster/updates main" | ||
| kernel='4.*-4kc-malta' | ||
| ncurses="=6.1*" |
There was a problem hiding this comment.
Already in main there is no mips target which uses the linux-runner, and it wouldn't be possible (was the reason for those switching to qemu-runner) for the same reason we cannot use a Debian Bookworm mipsel/mips64el runner on Ubuntu 24.04: the provided glibc ABI versions do not overlap. While getting an overview I was confused about the mips entry here, hence removing it from this script for clarity. This cannot work ever again for any mips target.
| # there is no buster powerpc port, so we use jessie | ||
| # use a more recent kernel from backports | ||
| kversion='4.9.0-0.bpo.6' | ||
| kernel="${kversion}-powerpc" | ||
| debsource="deb http://archive.debian.org/debian jessie main" | ||
| debsource="${debsource}\ndeb http://archive.debian.org/debian jessie-backports main" | ||
| debsource="${debsource}\ndeb http://ftp.ports.debian.org/debian-ports unstable main" | ||
| debsource="${debsource}\ndeb http://ftp.ports.debian.org/debian-ports unreleased main" | ||
|
|
||
| # archive.debian.org Release files are expired. | ||
| echo "Acquire::Check-Valid-Until false;" | tee -a /etc/apt/apt.conf.d/10-nocheckvalid | ||
| echo "APT::Get::AllowUnauthenticated true;" | tee -a /etc/apt/apt.conf.d/10-nocheckvalid | ||
| echo "Acquire::AllowInsecureRepositories True;" | tee -a /etc/apt/apt.conf.d/10-nocheckvalid | ||
|
|
||
| dropbear="dropbear" | ||
| deps=(libcrypt1:"${arch}") | ||
| # there is no stable port | ||
| # https://deb.debian.org/debian-ports/pool-powerpc/main/l/linux/ | ||
| kernel='7.*-powerpc' | ||
| debsource="deb https://deb.debian.org/debian-ports unstable main" | ||
| debsource="${debsource}\ndeb https://deb.debian.org/debian-ports unreleased main" | ||
| # Debian Forky splits linux-image-* packages into linux-binary-* and linux-modules-* and linux-base-* | ||
| kernel_pkg_split=1 |
There was a problem hiding this comment.
This was confusing: Adding ancient Jessie sources but also Debian ports unstable, which means that all packages are pulled from unstable. Was it only for the kernel version? For which reason? Debian ports unstable ships a recent powerpc kernel, not sure why anyone would want Linux 4.9, overriding all APT security checks etc. I hence aligned this with powerpc64, sparc64, and other targets which are available from Debian ports, which implies a switch to Linux 7.x and split kernel packages.
| kernel='6.*-riscv64' | ||
| debsource="deb http://deb.debian.org/debian unstable main" | ||
| deps=(libcrypt1:"${arch}") | ||
| kernel="${kversion}-riscv64" |
There was a problem hiding this comment.
riscv64 is in stable Debian Trixie, no unstable needed anymore.
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/archive-key-{7.0,8,9,10,11,12}.asc' -O | ||
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/archive-key-{8,9,10,11,12}-security.asc' -O | ||
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/release-{7,8,9,10,11,12}.asc' -O | ||
| curl --retry 3 -sSfL 'https://www.ports.debian.org/archive_{2020,2021,2022,2023,2024,2025}.key' -O | ||
|
|
||
| for key in *.asc *.key; do | ||
| apt-key add "${key}" | ||
| rm "${key}" | ||
| done | ||
| cd /etc/apt/trusted.gpg.d | ||
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/archive-key-{10,11,12,13}.asc' -O | ||
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/archive-key-{10,11,12,13}-security.asc' -O | ||
| curl --retry 3 -sSfL 'https://ftp-master.debian.org/keys/release-{10,11,12,13}.asc' -O | ||
| curl --retry 3 -sSfL 'https://www.ports.debian.org/archive_2026.key' -o archive_2026.asc |
There was a problem hiding this comment.
apt-key is deprecated, and was removed with Debian Trixie. Ubuntu 24.04 still ships it, but it is obsolete since a decade or so: even EOL Debian/Ubuntu versions support *.asc keys in /etc/apt/trusted.gpg.d natively, no apt-key and no gpg --dearmor needed.
| if [[ "${arch}" == "riscv64" ]]; then | ||
| # Symlink dynamic loader to /lib/ld-linux-riscv64-lp64d.so.1 | ||
| mkdir -p "${root}/lib" | ||
| ln -s /usr/lib/riscv64-linux-gnu/ld-linux-riscv64-lp64d.so.1 "${root}/lib/ld-linux-riscv64-lp64d.so.1" | ||
| fi |
There was a problem hiding this comment.
As mentioned in #1580: After setting up the runner userland usrmerged above, this step for riscv64/trixie is redundant, and fails as the target file exists already.
| # https://apt.dilos.org/dilos is currently down | ||
| add-apt-repository -y 'deb https://mirrors.dotsrc.org/mirrors/pub/dilos/apt dilos2 main' |
There was a problem hiding this comment.
DilOS seems dead. Their website and APT repo are down. The mirror however is still live.
| return __xnet_socket(domain, type, protocol); | ||
| } | ||
| _EOF_ | ||
| "${target}-gcc" -c compat.c -o /compat.o |
There was a problem hiding this comment.
This was broken before. Sadly there is no other APT-based Solaris distro, at least none with an actual APT repo to pull packages from. And DilOS packages are too old to be fully compatible with recent Rust. So I went down the rabbit hole and tried all ways to map or alias modern symbols Rust is expecting with the older ones present in DilOS libc. This was the only way that worked: a dedicated C object, used as linker script, which is added in test.sh via RUSTFLAGS+=' -C link-arg=/compat.o' for Solaris targets.
However, it would better if this was done by default, passed via Dockerfile ENV? But I couldn't find a way to do that, the documented way does not work for me:
- https://github.com/cross-rs/cross/blob/main/docs/cargo_configuration.md?plain=1#L14-L24
- I tried to pass
ENV CARGO_TARGET_X86_64_PC_SOLARIS_RUSTFLAGS="-C link-arg=foo"withDockerfile.x86_64-pc-solaris, but the flags did not appear in thecargocalls of the tests. Not sure if I was doing something wrong, or ifRUSTFLAGSis overwritten in our CI, or if it is broken?
Anyway, for now, applying the needed flag to RUSTFLAGS in test.sh for Solaris targets, turns CI green, and at least provides a way for users to solve these issues via custom linker flag. I saw the alternative is to let cross pull older Rust, with cross +<version>.
| mkdir -p /etc/apt/keyrings | ||
| mv winehq.key /etc/apt/keyrings/winehq-archive.key | ||
|
|
||
| wget -nc https://dl.winehq.org/wine-builds/ubuntu/dists/focal/winehq-focal.sources |
There was a problem hiding this comment.
I found it weird that wget is installed only to download a key and a list, while curl is present and used with consistent 3 retries everywhere else. I hence aligned that here, and reordered things to make more sense, like define the WINE version below the comment which talks about the hardcoded version.
The line # workaround for wine server synchronization, see #1035 above is actually a bit misleading, as if installing the key for a 3rd party APT repo was a "workaround", rather than a totally normal step. It would be also possible to download the key to /usr/share/keyrings. Not common standard for non-packaged keys, but APT does not enforce or exclude any path, as long as it matches the signed-by in the APT list. Anyway, I left that as is.
| "ghcr.io/cross-rs" | ||
| } | ||
| }; | ||
| pub const CROSS_IMAGE: &str = env!("CROSS_IMAGE_LOWER"); |
There was a problem hiding this comment.
Since a constant naturally cannot be altered (to turn CROSS_IMAGE lowercase if the repo owner/orga is not), and there is no real way to do that before assigning the env var with the type it is returned as, the CROSS_IMAGE is now obtained in build.rs, turned lowercase there, and passed as CROSS_IMAGE_LOWER to be used here.
| owo-colors = { version = "4.0", features = ["supports-colors"] } | ||
| semver = "1.0.16" | ||
| is_ci = "1.1.1" | ||
| tempfile = "=3.24.0" |
There was a problem hiding this comment.
Hardcoding tempfile 3.24.0 is needed, otherwise cargo pulls v3.27.0 which pulls in dependencies that depend on Rust v1.88.0. The next higher v3.25.0 depends on v1.87.0.
This Cargo.toml however declares rust-version = "1.85.0", which we probably want to keep? Restoring MSRV 1.85.0 caused rustfmt to revert some formatting changes applied in #1580. I aligned edition = "2024" and rust-version = "1.85.0" across "cross" and xtask, for a consistent cargo update without warnings.
| # home v0.5.11 | ||
| { name = "windows-sys", version = "0.59.0" }, | ||
| # toml v0.9 uses winnow 0.7 directly, while toml_parser (sub-dep of toml) uses winnow 1.0 | ||
| { name = "winnow", version = "0.7" }, |
There was a problem hiding this comment.
CI checks for doubled 2nd level dependencies, if different versions are pulled in by 1st level dependencies. There is already the windows-sys exception caused by home v0.5.11, hence I added an exception for winnow the same way, as toml v0.9 pulls in two versions of it with two of its own dependency tails 😄.
| [[target]] | ||
| target = "x86_64-apple-darwin" | ||
| os = "macos-13" | ||
| os = "macos-15-intel" |
There was a problem hiding this comment.
The macos-13 runner has been removed a longer while ago, the macos-14-intel runner quite recently. macos-14 exists but is Apple Silicon. macos-15-intel is hence the oldest freely available hosted macOS x86_64 runner on GitHub.
For completeness: With an enterprise subscription, there would be macos-14-large available, which is x86_64 as well.
| [[target]] | ||
| target = "x86_64-pc-windows-msvc" | ||
| os = "windows-2019" | ||
| os = "windows-2022" |
There was a problem hiding this comment.
windows-2019 was removed a while ago. windows-2022 is now the oldest available Windows runner.
| cpp = true | ||
| std = true | ||
| run = true | ||
|
|
||
| [[target]] | ||
| target = "i686-pc-windows-gnu" | ||
| os = "ubuntu-latest" | ||
| cpp = true |
There was a problem hiding this comment.
As I memtioned in #1755 (comment), cargo --features=re2 causes a NULL pointer deference now. Not sure about the underlying reason, or whether switching back to self-compiled/patched mingw compilers would solve it. However, #1755 also runs into failing CI as far as I can see, even the same it seems.
Removing cpp = true => no --features=re2 tests, fixes things. Of course I can re-enable it if someone wants to debug and find a different solution.
@Emilgardis Naturally, rerunning all jobs again (like with a new |
This does not fix a particular issue, but is an alignment with all other Dockerfiles, so we can count on the exact same package sources in all cases. Signed-off-by: MichaIng <micha@dietpi.com>
Emilgardis
left a comment
There was a problem hiding this comment.
I want to merge this, @cross-rs/maintainers fyi
|
Thank you @MichaIng for continuing this. Ill let this be for a bit more and then merge 😊 |
|
Yeah maybe someone else finds time to look through the code comments, as some polishing could be done, or things solved in different ways. But can be also done in follow-ups, if issues appear. Maybe trigger a rerun of failed CI jobs, just to be sure all is green as expected? 🙂 |
|
I thought I started a rerun but apparently not, thank you :) The changes are good and bring the project to a working state, and small fixes or refactoring can be worked on afterwards |
|
This one I meant: https://github.com/cross-rs/cross/actions/runs/26971729604 |
|
/ci try --target zig |
This comment has been minimized.
This comment has been minimized.
|
I'm aware thank you, reran zig explicitly because the action will checkout the old code pre a2921b5 |
|
Rerunning jobs from a workflow always pulls the latest code from the same branch. The only thing that remains on old code state is the workflow YAML files themselves. Hence, triggering a new workflow should only be needed when those are changed, and AFAIK as well when secrets are changed. |
This comment has been minimized.
This comment has been minimized.
|
Luckily it is the PR ref, which made dev-test cycles on my fork a lot easier 😄. I still felt a bit guilty to cause GitHub so much runner load and traffic, as some targets required a lot of iterations to find out and solve all issues 😅. EDIT: This rate limiting towards GitHub runners is really an issue. I wished VPS providers would whitelist or relax this for GitHub IPs. On my main project, we check for latest software versions once a day, also to check whether related projects are still alive, and whether URL structures or filename schemes changed. A particular one fails in like 70% of cases with 502 or 403, even that it is a tiny plain text document with like 20 characters body that is requested, causing probably less traffic/load then the error response, and of course a retry is then needed. And that rate limiting is not in the hands of the software dev/maintainer, but the VPS provider he uses 😞. |


This is a continuation of #1580 with CI all turned green.