Skip to content

Fix CI #97

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

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Fix CI #97

wants to merge 26 commits into from

Conversation

Tomer-Eliahu
Copy link

@Tomer-Eliahu Tomer-Eliahu commented Apr 23, 2025

This is a partial fix to the CI.
It should go some way into closing issue #93.
The CI should now pass.
Note that as the CI here is fairly complex that I made just the minimal changes needed to help it pass.

The changes made were as follows:

  • The CI now uses rust 1.63.0 instead of 1.62.0. This is needed as otherwise the test script (ci/script.sh) fails as package cc v1.2.19 cannot be built because it requires rustc 1.63.0 or newer.

  • Instead of using current nightly the CI now uses an old version of nightly: nightly-2022-08-12. This is needed because in the test script some checks like "check presence of the rust_begin_unwind symbol" fail on current nightly. I think this is due to compilation differences between when this was first made to current nightly. Since Rust 1.63.0 came out Aug 11 2022, we use nightly from the day after.

  • In ci/script.sh (the test script) renamed "nightly" to "nightly-2022-08-12".

  • Moved installing the old versions of rust and nightly until after installing mdbook and cargo-binutils. If we don't do this then installing them fails (they need dependencies which in turn need a more recent version of rust). Note all GitHub runners come with the latest stable version of Rust pre-installed, and it is that version we use to install mdbook and cargo-binutils.

  • Changed the runner to run on ubuntu-latest as version 20.04 is no longer supported.

  • Updated actions/cache to v4 from v1 as v1 is no longer supported.

  • 4 files in the ci folder under .cargo were renamed from config (no file extension) to config.toml. This was needed but now linkchecker in the test script complains that it cannot find the old config (no file extension) files. This is just a warning though. This also makes mdbook build complain so an additional fix might be needed here.

  • Added a test_ci workflow file. This is what I used to make sure the CI passes. It is just the same as the new ci.yaml (but with these comments about what and why certain changes were made). It can be used to manually test the CI. I can delete this if it is not wanted.

  • Updated bors.toml to build (1.63.0) from build (1.62.0). I am not familiar with bors so maybe something different is needed here.

Note: In ci/script.sh there is a specific check that needs to be fixed manually:

     #FIXME: This fails on nightly-2022-08-12, but we need at least rust 1.63.0 or other things fail. This needs to be fixed manually.
     # # exception handling
     # NOTE(nightly) this will require nightly until core::arch::arm::udf is stabilized
     if [ $RUST_VERSION = FIXME ]; then
         pushd exceptions

@Tomer-Eliahu Tomer-Eliahu requested a review from a team as a code owner April 23, 2025 22:44
Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Thank you!

  1. Could you remove the bors.toml file? that does not work anymore. I have removed it now from the branch protection rules.
  2. If you add workflow_dispatch: to the ci.yaml there is no need for the duplicate test_ci.yml, right? Could you remove it if so?
  3. Could you also add .toml in the book as well in places like {{#include ../ci/smallest-no-std/.cargo/config}}? That should get rid of the warnings.
  4. It would be great to use a more recent current nightly (and also stable Rust) here if we could get rid of the rust_begin_unwind symbol check but I do not know what it is needed for.
  5. I do not know what this is needed for either:
Note: In ci/script.sh there is a specific check that needs to be fixed manually:

     #FIXME: This fails on nightly-2022-08-12, but we need at least rust 1.63.0 or other things fail. This needs to be fixed manually.
     # # exception handling
     # NOTE(nightly) this will require nightly until core::arch::arm::udf is stabilized
     if [ $RUST_VERSION = FIXME ]; then
         pushd exceptions

Maybe @adamgreig knows about 4. and 5. ?

@Tomer-Eliahu
Copy link
Author

  1. Could you remove the bors.toml file? that does not work anymore. I have removed it now from the branch protection rules.
  2. If you add workflow_dispatch: to the ci.yaml there is no need for the duplicate test_ci.yml, right? Could you remove it if so?
  3. Could you also add .toml in the book as well in places like {{#include ../ci/smallest-no-std/.cargo/config}}? That should get rid of the warnings.

No problem! done.

@Tomer-Eliahu
Copy link
Author

I stumbled on #94 (comment) and noticed it was still a problem so I fixed it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants