-
Notifications
You must be signed in to change notification settings - Fork 865
[ci] Remove old dependencies from apt-requirements.txt
#27605
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
Conversation
77709d8
to
58f1998
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming everything works (which has presumably just been checked in CI), this looks like a really excellent change.
Our CI containers use an earlier OT snapshot to pre-install some packages. So this isn't really a good indication apart the jobs that run on GitHub-hosted runners only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the removed packages, could you try to test in CI by explicitly apt remove
them?
a75d04e
to
df0c6c3
Compare
One minor nit, probably also worth removing packages from the markdown list in |
I believe all of these are things that are now handled within Bazel so we don't need to download them on the system anymore. Signed-off-by: James Wainwright <[email protected]>
df0c6c3
to
dc16ce6
Compare
CI passes with explicitly removing those packages (with an autoremove as well) so I think this is safe |
dc16ce6
to
376fab4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing out. This is the run: https://github.com/lowRISC/opentitan/actions/runs/16296028670
I believe all of these are things that are now handled within Bazel so we don't need to download them on the system anymore.