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
Rust svdtools for CI #701
Rust svdtools for CI #701
Conversation
9ef124b
to
d2cb5b5
Compare
bors try |
🔒 Permission denied Existing reviewers: click here to make burrbull a reviewer |
@adamgreig Can't test mmaps as they action uses old version of ci. |
.github/workflows/ci.yaml
Outdated
on: | ||
push: | ||
branches: [ staging, trying, master ] | ||
branches: [ staging, trying, master, bors/staging, bors/trying ] |
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.
What's bors/staging
and bors/trying
for? I think bors only pushes to plain staging
and trying
?
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 this PR, it will be good to get CI swapped over to the Rust tools.
I don't think the caching is doing what I want - the old caching actions were to cache the svd2rust binary (and could also cache the svdtools binary); I don't really care about caching the crate's own dependencies as they're extremely small and quick to build, while building svd2rust from source every time takes ages. I think the rust-cache action used here would not cache svd2rust and only cache things like bare-metal and cortex-m, which isn't as useful.
.github/workflows/ci.yaml
Outdated
- name: Copy svd2rust to cache directory | ||
if: steps.cache-cargo.outputs.cache-hit != 'true' | ||
working-directory: ${{ matrix.crate }} | ||
key: svdtools-${{ env.SVDTOOLS_VERSION }}-svd2tust-${{ env.SVD2RUST_VERSION }} |
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.
key: svdtools-${{ env.SVDTOOLS_VERSION }}-svd2tust-${{ env.SVD2RUST_VERSION }} | |
key: svdtools-${{ env.SVDTOOLS_VERSION }}-svd2rust-${{ env.SVD2RUST_VERSION }} |
195: encode dimIndex ranges r=adamgreig a=burrbull cc `@adamgreig` Partially related to stm32-rs/stm32-rs#701 (comment) Co-authored-by: Andrey Zgarbul <zgarbul.andrey@gmail.com>
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! I think this is pretty close now but I have a couple of questions about the cache:
-
Right now cargo reinstalls svdtools, svd2rust, and form every time the job runs, because that step isn't disabled if the cache was present. We could add an
if
to that step that skips it if the cache was loaded, like it used to have, otherwise we have to recompile svd2rust every time. The rust-cache action has acache-hit
output that should work for this just like the old step. -
Right now the nightlies job uses form, but uses the same cache key as the CI job, so if we did this it might load a cache that doesn't have form installed. Maybe it's better to have all the jobs install svdtools, svd2rust, and form, and then share a cache key, and only re-install when changed, so we have a single cache with all three tools in it. Or use a separate cache key for the nightlies job.
On second question I doesn't have answer as I haven't test it.
|
Ah, cool, that makes sense! Very neat. I think this means the nightlies job will reinstall form every time instead of using the cached one, so maybe we should install form in the main CI job too so that it will be present in the cache. It adds a little time when the cache is rebuilt, but shouldn't be too bad normally. I don't think we need to put the form version in the key or anything. |
bors try |
So. You are right. Now it rebuilds form each time. Needs to clean cache somehow. |
tryBuild succeeded: |
Ah, that's annoying. Maybe just put "form-0.8.0" into the cache key after all, since that will trigger it to build it properly now? |
bors try |
tryBuild succeeded: |
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 again!
bors merge
Build succeeded: |
No description provided.