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

CI: caching: closer match work/CI guarantees #2536

Merged
merged 9 commits into from
Dec 27, 2021

Conversation

Anton-Latukha
Copy link
Collaborator

@Anton-Latukha Anton-Latukha commented Dec 24, 2021

  • Would note we have a cabal v2-build all --enable-tests --enable-benchmark problem - that command requires the level of dependency consistency guarantees that PR CI does not checks.

test workflow does essentially cabal v2-build --enable-tests on matrix by target.
bench workflow does cabal v2-build --enable-benchmarks by target.

caching used all --enable-tests --enable-benchmarks - requiring total consistency.

These changes are to make caching able to work when test & bench deps
versions are inconsistent (which is how CI & people use them - separately).

  • "Workaround to CI platform" needs to be removed either way, since I found out post hooks does not run if any of (even marked continue-on-error: true) steps fail.

@Anton-Latukha Anton-Latukha added CI Continuous integration build tool: cabal labels Dec 24, 2021
@Anton-Latukha Anton-Latukha marked this pull request as ready for review December 24, 2021 23:59
@Anton-Latukha Anton-Latukha marked this pull request as draft December 26, 2021 21:41
`cabal v2-build all --enable-tests --enable-benchmarks` inferres 1 version per dep
keeping all targets.

People (frequently) & CI do `test` & `bench` separately.

So `all tests` & `all bench` deps may not end up.

Even current code does not match the CI guarantees, as all plugins get `test`
separately, so their deps can not match-up.

`caching` should not assume guarantees bigger then provided.
This workaround was not addressing the CI behaviour.
@Anton-Latukha Anton-Latukha changed the title CI: caching: dodge all && test && benchmark dep inconsistencies CI: caching: closer match work/CI guarantees Dec 27, 2021
@Anton-Latukha
Copy link
Collaborator Author

I reformulated the PR to address the most important stuff in it.

To match the process completely - we would need to run the v2-build --enable-tests command for every plugin separately, so it matches how test does it.

@Anton-Latukha
Copy link
Collaborator Author

[skip circleci]

.github/workflows/test.yml Outdated Show resolved Hide resolved
Co-authored-by: Javier Neira  <atreyu.bbb@gmail.com>
Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Dec 27, 2021

Nice review.

Would belabor additionally: now CI would start to cache benchmarks deps only for the configuration that is used in bench. That, on update of GHC in bench to update caching. But in that way we both deduplicate that build-cache over several workflows & cache only used benchmarks dependencies.

@Anton-Latukha Anton-Latukha marked this pull request as ready for review December 27, 2021 17:49
@Anton-Latukha Anton-Latukha added the merge me Label to trigger pull request merge label Dec 27, 2021
@Anton-Latukha
Copy link
Collaborator Author

After the recent merge HLint check works.

@mergify mergify bot merged commit f47bb47 into haskell:master Dec 27, 2021
@Anton-Latukha Anton-Latukha deleted the 2021-12-24-upd-caching branch December 28, 2021 11:37
drsooch pushed a commit to drsooch/haskell-language-server that referenced this pull request Dec 29, 2021
* CI: caching: do `bench` & `test` separately

`cabal v2-build all --enable-tests --enable-benchmarks` inferres 1 version per dep
keeping all targets.

People (frequently) & CI do `test` & `bench` separately.

So `all tests` & `all bench` deps may not end up.

Even current code does not match the CI guarantees, as all plugins get `test`
separately, so their deps can not match-up.

`caching` should not assume guarantees bigger then provided.

* CI: caching: rm workaround

This workaround was not addressing the CI behaviour.

* CI: {caching, test, bench}: output `freeze` or warning

* CI: {caching, test, bench}: m v2-update unification

* CI: caching: do bench caching only for what gets used

Efficient use of available space.

* CI: caching: fx benchmark caching step

Co-authored-by: Javier Neira  <atreyu.bbb@gmail.com>

* CI: {caching, test, bench}: `haskell/actions/setup` does the update

* CI: caching: m fx

Co-authored-by: Javier Neira  <atreyu.bbb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build tool: cabal CI Continuous integration merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants