Update Docker images to Ubuntu-24.04 and Cargo dependencies [continuation]#1779
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.
| [[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 comment has been minimized.
This comment has been minimized.
|
Thank you, going to merge this! (without CI since we've already done CI :) ) |
|
and thank you @JanDiederich and @itpetey for helping with this! |


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