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

Pin RefinedRust to a specific version #70

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Conversation

lgaeher
Copy link
Collaborator

@lgaeher lgaeher commented Aug 9, 2024

Description of the changes

Currently, we do not fix a particular RefinedRust version. Since RefinedRust has regular breaking changes, fixing a particular version and only updating it together with new verification results/when the need arises will reduce the maintenance burden.

For this, I included RefinedRust as a submodule, as this is how we pin existing dependencies as well.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Formal verification
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactorization (non-breaking change which improves code quality)

How to test this PR?

It's sufficient to check whether CI passes.

@lgaeher lgaeher requested a review from wojciechozga August 9, 2024 10:13
Signed-off-by: Lennard Gäher <l.gaeher@posteo.de>
@lgaeher lgaeher force-pushed the lennard/pin-refinedrust branch from 77c30b7 to e607b6d Compare August 9, 2024 10:14
@wojciechozga
Copy link
Member

wojciechozga commented Aug 9, 2024

Thank you for the PR. I am fine with pinning RefinedRust. However, the current build pipeline fails and we have to fix it first. The error indicates lack of disk space during the build. Probably, CI initializes all git submodules, including newly added RefinedRust, and then reaches max space size when it tries to build the test VM image. Reconfiguring the build configuration, so that it does not initialize the RefinedRust git module when building the security monitor, should fix the issue.

@lgaeher
Copy link
Collaborator Author

lgaeher commented Aug 9, 2024

Yeah, that makes sense. I'm not sure the changes in this PR are the root cause for the failure though. After this run failed, I re-ran the build pipeline on current main, and it fails with the same message: https://github.com/IBM/ACE-RISCV/actions/runs/10014257449/job/28567659391

I'm not sure what may have caused this, since the same job ran fine a couple of weeks ago.

@wojciechozga
Copy link
Member

Right, probably Github actions reduces disk space for the pipeline jobs. We can then decrease the test VM image size. Let me try it out.

…m size

Signed-off-by: Wojciech Ozga <woz@zurich.ibm.com>
@lgaeher
Copy link
Collaborator Author

lgaeher commented Aug 9, 2024

Seems like this is a problem others are experiencing as well: actions/runner-images#10386

Some people report that switching to ubuntu-24.04 works. Or we could wait until this is fixed.

Signed-off-by: Wojciech Ozga <woz@zurich.ibm.com>
Signed-off-by: Wojciech Ozga <woz@zurich.ibm.com>
@wojciechozga
Copy link
Member

I have changed CI base image to Ubuntu-24.04 and, indeed, it works fine. Looks like we can merge this PR now.

@wojciechozga wojciechozga merged commit ceccd81 into main Aug 12, 2024
4 checks passed
@wojciechozga wojciechozga deleted the lennard/pin-refinedrust branch August 12, 2024 08:34
@lgaeher
Copy link
Collaborator Author

lgaeher commented Aug 12, 2024

Thanks!

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