Open luigi-discoup opened 1 year ago
The cache key name includes the OTP version and the Elixir version, so the cache will be updated when those versions change. Still, I agree this is probably not quite correct, the key name should probably also include a hash of mix.lock. Otherwise some extra work will be done on each subsequent build after dependencies are changed. It looks like all the CI examples have this same issue. In practice it may not slow down the jobs too much but I agree it does not seem ideal to offer an example like this.
I'm curious for @Nezteb's take just in case I'm missing something here as he added the Gitlab example relatively recently and it uses mix.lock
in the _build
cache but not the PLT cache.
@jeremyjh yes the chache name includes OTP and Elixir version and it works well for core PLTs, but project specific PLT need extra work at every run.
I've just configured my repo in this way and it seems to work:
mix.exs
defmodule MyApp.MixProject do
use Mix.Project
def project do
[
...
dialyzer: [plt_core_path: "priv/plts"],
...
]
end
...
end
.github/workflows/build.yaml
name: Elixir build
...
jobs:
checks:
name: Analyze and test
steps:
- name: Git clone the repository
uses: actions/checkout@v3
- name: Set up Elixir
id: beam
uses: erlef/setup-beam@v1
with:
elixir-version: "1.14.4"
otp-version: "25"
- name: Build cache
uses: actions/cache@v3
with:
path: |
deps
_build
key: ${{ runner.os }}-mix-${{ hashFiles('**/mix.lock') }}
restore-keys: ${{ runner.os }}-mix-
- name: Install dependencies
run: mix deps.get
- name: Restore PLT cache
id: plt_cache
uses: actions/cache/restore@v3
with:
key: ${{ runner.os }}-${{ steps.beam.outputs.elixir-version }}-${{ steps.beam.outputs.otp-version }}-plt
restore-keys: ${{ runner.os }}-${{ steps.beam.outputs.elixir-version }}-${{ steps.beam.outputs.otp-version }}-plt
path: priv/plts
- name: Create PLTs
if: steps.plt_cache.outputs.cache-hit != 'true'
run: mix dialyzer --plt
- name: Save PLT cache
id: plt_cache_save
uses: actions/cache/save@v3
if: steps.plt_cache.outputs.cache-hit != 'true'
with:
key: ${{ runner.os }}-${{ steps.beam.outputs.elixir-version }}-${{ steps.beam.outputs.otp-version }}-plt
path: priv/plts
- name: Compile code
run: mix compile
- name: Check code format
run: mix format --check-formatted
- name: Run credo
run: mix credo --strict --all
- name: Run dialyzer
run: mix dialyzer --format github
- name: Run tests
run: mix test
...
Now, my dialyzer setp last 4s and it outputs PLT is up to date!
he added the Gitlab example relatively recently and it uses
mix.lock
in the_build
cache but not the PLT cache.
Yeahhh, there aren't really any better keys that I can think of unless you use every single source file in a project as part of the key. At first glance I think that'd be slow and not a good use of a build cache. 😅 In an ideal world, these build artifacts (including the PLTs) aren't changing that much between branches, so that shouldn't be necessary.
EDIT: Removed the stuff I got wrong.
I'll also ping @akasprzok to verify since he wrote the original blog post that led to me to update the GitHub Actions example and create a new one for GitLab CI. 😄
EDIT: I'm wrong, because plt_file
is the project PLT, not the Erlang/Elixir PLT 🤦♂️. After rereading the dialyxir
docs, we could probably update the CI portion to suggest using:
dialyzer: [
plt_core_path: "priv/plts/core.plt",
plt_local_path: "priv/plts/local.plt"
]
Instead of:
dialyzer: [
plt_file: {:no_warn, "priv/plts/dialyzer.plt"}
]
I'm currently traveling but I'll take another look at this when I'm back!
@Nezteb I'm thinking just adding mix.lock
to the file key on the dialyzer step might do it:
@jeremyjh Ah that makes sense.
Would it also be worth getting rid of the plt_file
example since it's technically deprecated?
I can start a PR later today. 😄
plt_file
is not deprecated for usage in CI, just for local development; , or at least that is why it supports :no_warn
. There is some discussion in this old issue on why plt_file
is supported this way and why some may prefer it; I think otherwise you have to cache your mix home folder or put core files in _build but this may just be a matter of preference that's been replicated.
Thanks for the ping!
The problem I kept running into was that I'd push a change, and dialyzer would fail 9 minutes later (bigish code base). Then I'd push the fix, and have to wait another 9 minutes for all green.
Simply getting rid of that second 9 minutes was pretty big for me.
Adding mix.lock to the key does make sense I think.
Simply adding the mix.lock
to the key IMHO is incorrect. As far as I understand, the mix dialyzer --plt
generates only core PLT files (so a basic set of OTP applications, as well as all of the Elixir standard libraries) thus is correct to bind that step to only OTP and Elixir version. Correct me if I'm wrong.
@Nezteb I made some test and plt_file: {:no_warn, "priv/plts/project.plt"}
and plt_local_path: "priv/plts/project.plt"
seems to have different behaviour.
The former will generate the file priv/plts/project.plt
while the latter generates the folder priv/plts/project.plt/
within which there are generated PLT files. The same goes for plt_core_path
, it should point to a folder, not a file.
Furthermore, it's better to store the cache at the end too, in order to cache project-level PLT.
My proposal is to use the cache mechanism described in the current github action example to handle only the core PLT files (the ones generated by mix dialyzer --plt
), leaving the project-local PLTs inside _build
folder and cache that with the simple actions/cache@v3
action.
As far as I understand, the
mix dialyzer --plt
generates only core PLT files
@luigi-discoup I tested this, and... it depends. 😅
plt_file
followed by mix dialyzer --plt
With this config;
dialyzer: [
plt_file: {:no_warn, "priv/plts/project.plt"}
]
Running rm -rf _build && rm -rf priv/plts && mix dialyzer --plt
produces these PLTs:
❯ tree priv
priv
├── plts
│ ├── project.plt
│ └── project.plt.hash
And there are no PLTs in _build
.
plt_core_path
followed by mix dialyzer --plt
With this config;
dialyzer: [
plt_core_path: "priv/plts/core"
]
Running rm -rf _build && rm -rf priv/plts && mix dialyzer --plt
produces these PLTs:
❯ tree _build/dev
_build/dev
├── dialyxir_erlang-25.1.1_elixir-1.14.1_deps-dev.plt
├── dialyxir_erlang-25.1.1_elixir-1.14.1_deps-dev.plt.hash
❯ tree priv
priv
├── plts
│ ├── core
│ ├── dialyxir_erlang-25.1.1.plt
│ └── dialyxir_erlang-25.1.1_elixir-1.14.1.plt
plt_core_path
and plt_local_path
followed by mix dialyzer --plt
With this config;
dialyzer: [
plt_core_path: "priv/plts/core",
plt_local_path: "priv/plts/local"
]
Running rm -rf _build && rm -rf priv/plts && mix dialyzer --plt
produces these PLTs:
❯ tree priv
priv
├── plts
│ ├── core
│ │ ├── dialyxir_erlang-25.1.1.plt
│ │ └── dialyxir_erlang-25.1.1_elixir-1.14.1.plt
│ └── local
│ ├── dialyxir_erlang-25.1.1_elixir-1.14.1_deps-dev.plt
│ └── dialyxir_erlang-25.1.1_elixir-1.14.1_deps-dev.plt.hash
And there are no PLTs in _build
.
For each set of config, I see the following available CI caching options:
priv/plts
based on mix.lock
and Erlang/Elixir versions.
_build
based on mix.lock
and cache priv/plts
based on Erlang/Elixir versions.
priv/plts/core
based on Erlang/Elixir versions and priv/plts/local
based on mix.lock
.
The current CI examples are using option 1, while options 2 and 3 seem more ideal. The only difference between them would depend on if you want to cache your project PLTs alongside your regular build or separately in their own directory.
Thoughts?
@Nezteb great job! Thank you. I've only tested configuration 1
and 2
, and my current CI implements scenario 2.
For the configuration 2
, you said
And there are no PLTs in
_build
.
but it seems that there actually are PLTs in _build
, right?
I think that configuration 2
or 3
can fulfill different needs, but the 2
is the easier to implement since you have to configure only plt_core_path
and use only 2 separate caches configuration, one for core PLTs and one for the _build
.
With the configuration 3
you have to set both plt_core_path
and plt_local_path
, cache core PLTS, local PLTS and _build
folder, although in github action you can specify multiple path in you cache configuration with:
...
- name: Build cache
uses: actions/cache@v3
with:
path: |
deps
_build
priv/plts/local
key: ${{ runner.os }}-mix-${{ hashFiles('**/mix.lock') }}
restore-keys: ${{ runner.os }}-mix-
...
maybe the 3
is more explicit?
Anyway, I vote for the 2
:)
For the configuration
2
, you saidAnd there are no PLTs in
_build
.but it seems that there actually are PLTs in
_build
, right?
Whoops yeah that was just a bad copy-paste on my part. I edited the comment. 😅
Anyway, I vote for the
2
:)
I'm inclined to agree! I'll update my docs PR to reflect that. 😄
Precheck
Environment
Elixir & Erlang/OTP versions (elixir --version):
Elixir 1.14.4 (compiled with Erlang/OTP 25)
Which version of Dialyxir are you using? (cat mix.lock | grep dialyxir):
"dialyxir": {:hex, :dialyxir, "1.3.0", "fd1672f0922b7648ff9ce7b1b26fcf0ef56dda964a459892ad15f6b4410b5284", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "00b2a4bcd6aa8db9dcb0b38c1225b7277dca9bc370b6438715667071a304696f"},
Question
From the Continuous Integration documentation page, I see that I have to put
plt_file: {:no_warn, "priv/plts/dialyzer.plt"}
in thedialyzer
project property and configure the GitHub Actions cache as described here, but I'm not sure about this configuration.Using the GitHub Actions cache configuration provided, the PLT cache will be stored only once, and every subsequent changes in source code or dependencies will be analyzed again and never stored in cache so the project specific PLT wil be generated every time. Am I right?
As far as I understand, reading the PLT section, a better configuration would be to put
plt_core_path: "priv/plts"
indialyzer
configuration instead ofplt_file
, leaving the local PLT file in its default location_build/env/dialyze_erlang-[OTP Version]_elixir-[Elixir Version]_deps-dev.plt
and caching the_build
folder separatly.