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.

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

ci: Build ROM release from versioned git ref #1604

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benjamindoron
Copy link
Contributor

Address #1464

@jhand2 jhand2 requested review from jhand2 and zhalvorsen July 9, 2024 04:54
zhalvorsen
zhalvorsen previously approved these changes Jul 9, 2024
Comment on lines 19 to 22
# Generate ROM and Image Bundle Binary
cargo run --manifest-path=builder/Cargo.toml --bin image -- --rom-no-log $WORKSPACE_DIR/caliptra-rom.bin --fw $WORKSPACE_DIR/image-bundle.bin
# Copy ROM ELF
cp -a target/riscv32imc-unknown-none-elf/firmware/caliptra-rom $WORKSPACE_DIR/caliptra-rom.elf
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand why the things in this file need to be moved to the action definition.

I think the desire with these scripts is that someone can run them locally to build the expected release artifacts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose that's valid. The intention was to build the ROM with the chosen tag first, before checking out HEAD and building some more. I wanted to avoid an actual git checkout command here, which would mean calling this has side effects.

Other solutions include passing git commit hashes to this script, and running git checkout here (if the CI is okay with it, and people won't mind the side effects that calling it will have to their working directory, it's an option). Alternatively, we could pass arguments to the script to determine which build stage to run: ROM, FMC/RT, or both. Then the CI/developer is responsible for making any changes in between.

- uses: actions/checkout@v3
with:
submodules: 'true'
ref: 'rom-1.0.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to ability to build all the fw at a specific revision (e.g. via configuration when manually running the job). But I think we still want the automated nightly release to build from head. Otherwise we won't catch any regressions in ROM until we go to build a new release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I think that I got a little lost in all the workflow files. Shouldn't that be handled by build-test.yml? And if this workflow file doesn't generate the release, which one does? This is the only file to call build_release.sh.

@benjamindoron benjamindoron force-pushed the build_versioned_release_rom branch 6 times, most recently from 778574a to ee1925e Compare August 13, 2024 19:01
@benjamindoron benjamindoron force-pushed the build_versioned_release_rom branch 3 times, most recently from 70bf54b to bd121d2 Compare August 22, 2024 22:20
Copy link

linux-foundation-easycla bot commented Sep 11, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: benjamindoron / name: Benjamin Doron (456b551)

mhatrevi pushed a commit that referenced this pull request Nov 18, 2024
mhatrevi pushed a commit that referenced this pull request Nov 19, 2024
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.

3 participants