Skip to content
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.

Sign up for GitHub

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

Rust: Add no_std CI #1648

Merged
merged 8 commits into from
Sep 5, 2023
Merged

Rust: Add no_std CI #1648

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c964521
Rust: Add no_std CI
pinkforest Sep 4, 2023
5836bf5
Merge into rust.yml
pinkforest Sep 4, 2023
b7bf75c
Remove no_std.yml
pinkforest Sep 4, 2023
e0d830b
The shellscripts rely on CWD ../
pinkforest Sep 4, 2023
176bc42
Add job id into group
pinkforest Sep 4, 2023
236e7a8
Fix a mistaken path for shellscript
pinkforest Sep 4, 2023
ec7b500
Move concurrency setting to root
pinkforest Sep 4, 2023
d05f291
Fix yaml syntax and revert group name
pinkforest Sep 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,47 @@ on:
schedule:
- cron: '0 0 1 * *'

defaults:
run:
working-directory: fiat-rust

jobs:
test-rust:

runs-on: ubuntu-latest

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}-${{ github.job }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this doesn't work the way I hoped, and still cancels the job
Let's try

Suggested change
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}-${{ github.job }}
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}-${{ job.name }}

Or else just manually disambiguate them?

Copy link
Contributor Author

@pinkforest pinkforest Sep 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think it needs to be at root level

cancel-in-progress: true

steps:
- uses: actions/checkout@v3
- name: cargo build
run: (cd fiat-rust; cargo build --verbose)
run: cargo build --verbose
- name: cargo test
run: (cd fiat-rust; cargo test --verbose)
- name: cargo test (no_std)
run: (cd fiat-rust; cargo test --no-default-features --verbose)
run: cargo test --verbose
- name: curve25519-dalek test
run: etc/ci/test-fiat-rust-curve25519-dalek.sh
run: (cd .. ; etc/ci/test-fiat-rust-curve25519-dalek.sh)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you instead make the scripts robust to cwd changes by sticking in a line like

fiat-crypto/etc/ci/github-actions-make.sh

Line 4 in dba0b22

cd "$(dirname "$0")/../.." || exit 7

in the script itself?

Copy link
Contributor Author

@pinkforest pinkforest Sep 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I recommend to do that in a follow-up PR where I will remove those shellscripts together and it will be natively github action - or a bit more idiomatic w/o shellscripts

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, sounds good. In that case I think you'll need to fix the cd ..; ../etc/... that's a couple lines down though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh thanks fixed 💜

- name: crate-crypto/Ed448-Goldilocks test
run: etc/ci/test-fiat-rust-crate-crypto-Ed448-Goldilocks.sh
run: (cd .. ; ../etc/ci/test-fiat-rust-crate-crypto-Ed448-Goldilocks.sh)

build-nostd:
name: Build on no_std target (thumbv7em-none-eabi)
runs-on: ubuntu-latest

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}-${{ github.job }}
cancel-in-progress: true

steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@master
with:
toolchain: stable
targets: thumbv7em-none-eabi
- uses: taiki-e/install-action@cargo-hack
# No default features build
- name: no_std / no feat fiat-crypto
run: cargo build -p fiat-crypto --target thumbv7em-none-eabi --release --no-default-features
- name: no_std / cargo hack fiat-crypto
run: cargo hack build -p fiat-crypto --target thumbv7em-none-eabi --release --each-feature --exclude-features default,std
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this supposed to complete in 0.00s?
Run cargo build -p fiat-crypto --target thumbv7em-none-eabi --release --no-default-features
Compiling fiat-crypto v0.2.2 (/home/runner/work/fiat-crypto/fiat-crypto/fiat-rust)
Finished release [optimized] target(s) in 6.75s
0s
Run cargo hack build -p fiat-crypto --target thumbv7em-none-eabi --release --each-feature --exclude-features default,std
cargo hack build -p fiat-crypto --target thumbv7em-none-eabi --release --each-feature --exclude-features default,std
shell: /usr/bin/bash -e {0}
env:
CARGO_INCREMENTAL: 0
CARGO_TERM_COLOR: always
info: running cargo build --release --target thumbv7em-none-eabi --no-default-features on fiat-crypto (1/1)
Finished release [optimized] target(s) in 0.00s

Copy link
Contributor Author

@pinkforest pinkforest Sep 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes because it only builds it and it uses incremental compilation - it will take more when there are features