-
Notifications
You must be signed in to change notification settings - Fork 3
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
[SOFT-438] Cache packages in CI #313
base: master
Are you sure you want to change the base?
Conversation
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.
Awesome job Kevin, the one other thing is please fix the status badge in the readme!
.github/workflows/main.yml
Outdated
- uses: actions/cache@v2 | ||
id: cache | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} |
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.
This only caches pip dependencies eh? You'll need to use a separate cache step with a separate id for each step you want to skip based on caching. The general rule as I see it is, whatever directories need to be cached go in path
, optionally line break separated if there's multiple of them:
path: |
~/foo
~/bar
Then the key should be created by concatenating the os (${{ runner.os }}
), the name of the thing you're caching, and then hashFiles
on any files that determine whether the cache should be invalidated. I.e., for pip
, any requirements.txt files in the repo could change what dependencies get installed, so we call hashFiles
on all the requirements.txt files.
.github/workflows/main.yml
Outdated
|
||
- name: Build x86 with clang | ||
run: | | ||
make build_all PLATFORM=x86 COMPILER=clang DEFINE="${DEFINES}" |
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.
Please add a newline at the end of the file!
.github/workflows/main.yml
Outdated
run: | | ||
make build_all PLATFORM=stm32f0xx DEFINES="${DEFINES}" | ||
make clean | ||
|
||
- name: Build x86 | ||
run: | | ||
make build_all PLATFORM=x86 DEFINE="${DEFINES}" | ||
make test_all PLATFORM=x86 DEFINE="${DEFINES}" |
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.
This line shouldn't be here eh?
.github/workflows/main.yml
Outdated
if: steps.cache-gcc.outputs.cache-hit != 'true' | ||
run: sudo apt-fast -y install gcc-6 | ||
|
||
if: steps.cache-clang.outputs.cache-hit != 'true' | ||
run: sudo apt-fast -y install clang-5.0 | ||
|
||
if: steps.cache-clang-format.outputs.cache-hit != 'true' | ||
run: sudo apt-fast -y install clang-format-5.0 | ||
|
||
if: steps.cache-libc6-i386.outputs.cache-hit != 'true' | ||
run: sudo apt-fast -y install libc6-i386 | ||
|
||
# for vcan module | ||
if: steps.cache-linux-module.outputs.cache-hit != 'true' | ||
run: sudo apt-fast -y install linux-modules-extra-$(uname -r) | ||
|
||
run: | |
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.
I'm fairly certain you can only have one if
and one run
per step - this is a limitation of the YAML format. So you'll need to split these into multiple steps, one per if/run.
.github/workflows/main.yml
Outdated
- uses: actions/cache@v2 | ||
id: cache-clang | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-clang | ||
|
||
- uses: actions/cache@v2 | ||
id: cache-clang-format | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-clang-format | ||
|
||
- uses: actions/cache@v2 | ||
id: cache-libc6-i386 | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-libc6-i386 | ||
|
||
- uses: actions/cache@v2 | ||
id: cache-linux-module | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-linux-module | ||
|
||
- uses: actions/cache@v2 | ||
id: cache-make | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-make |
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.
I'm not sure where clang, clang-format, make, etc are installed, but it's definitely not in ~/.cache/pip
! You'll need to figure out where the actual clang, make, etc binaries get downloaded and installed to and use those paths. This might involve reading the source of any actions in a uses
step (like the one that installs arm-none-eabi-gcc
) or running the commands on your machine and figuring out where things got installed to.
Also I notice there's no cache step for the installing arm-none-eabi-gcc step (uses: fiam-arm-none-eabi-gcc@v1
)? It installs the toolchain we need to build for STM32, and it takes almost two full minutes to do so - we'll definitely need to cache that step.
.github/workflows/main.yml
Outdated
- uses: actions/cache@v2 | ||
id: cache-gcc | ||
with: | ||
path: ~/.local/bin/gcc |
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.
The only thing this script does in ~/.local/bin
is set up some links that take a negligible amount of time to set up. I don't think you need to cache ~/.local/bin/gcc
- figure out where GCC is actually installed to and use that directory instead.
FYI if you think the failures might be because you need to clear the caches, you can change the relevant cache keys (like add a |
Ah, I meant the cache key rather than the id - the key is how it determines if it has a cache for a directory. |
Small additional request once you get the caching working: since you're updating the readme, could you update the paragraph about CI to talk about GitHub Actions instead of Travis? Thanks :) |
So I did a little bit of research, and here's a couple things you can try:
|
Let me know if the caching and the separation of make commands were done correctly 👍