Skip to content

Fix #9815: fix caching for quick-jobs CI (XDG, cache keys) #9845

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

Merged
merged 9 commits into from
Apr 2, 2024
Merged

Conversation

andreasabel
Copy link
Member

@andreasabel andreasabel commented Mar 26, 2024

Fix #9815:

  • Cache ~/.local/state/cabal instead of ~/.cabal/store
  • ~/.local/bin is used instead of ~/.cabal/bin and is already in the PATH
    (verify this by calling alex after installing it)

As I am passing by:

  • bump cache action to v4
  • double-quote $USER to keep actionlint happy
  • move if from shell-level to job-level
  • allow newest alex

UPDATE: This PR has been extended to

  • use preinstalled GHC and Cabal in this workflow (saves 90sec on each job)
  • clear the remains of the lexer goal in the Makefile (was removed in Add alex to build-tool-depends remove separate code gen #8980)
  • while I am add it, use .PHONY systematically in Makefile
  • fix caching for the quick-jobs
    • Use key from the build plan where possible
    • Use daily key for the doctest job
  • update a generated file to make "Meta checks" pass

@andreasabel
Copy link
Member Author

With caching restored, these jobs become several minutes faster, e.g.

E.g. for "Meta checks" most of the time is slandered in the "ghcup" step (1m 30s). @ulysses4ever : How about skipping this step and just use the preinstalled ghc and cabal?

@ulysses4ever : These jobs use a fixed cache key. This means the cache is never updated. At the same time, cabal update draws fresh dependencies, so the cached ones get more and more outdated. Is this intended? I'd expect a cache key to hash plan.json so that caches get replaced once a dependency is updated.

@andreasabel andreasabel added continuous-integration re: xdg Concerning the XDG directory structure labels Mar 26, 2024
@ulysses4ever
Copy link
Collaborator

@ulysses4ever : How about skipping this step and just use the preinstalled ghc and cabal?

Yes, sounds perfectly acceptable in the interest of quicker Quick Jobs.

@ulysses4ever : These jobs use a fixed cache key. This means the cache is never updated. At the same time, cabal update draws fresh dependencies, so the cached ones get more and more outdated. Is this intended? I'd expect a cache key to hash plan.json so that caches get replaced once a dependency is updated.

It's probably a blunder. I'm not fluent in the cache action, so if you could update it, that would be great.

Let's nuke the /usr/local/.ghcup/cache stuff while we're at it?

@andreasabel andreasabel force-pushed the ci-xdg branch 5 times, most recently from 773602c to c4edc3b Compare March 27, 2024 06:00
@andreasabel andreasabel changed the title Fix #9815: switch quick-jobs CI to XDG Fix #9815: fix caching for quick-jobs CI (XDG, cache keys) Mar 27, 2024
@andreasabel
Copy link
Member Author

When you start digging, as usual, you unearth all this bit-rot... Well, this PR got bigger than originally intended, see updated PR description on top.

@andreasabel
Copy link
Member Author

Before:
Screenshot 2024-03-27 at 07 07 06

After:
Screenshot 2024-03-27 at 07 06 26

@andreasabel andreasabel added the merge me Tell Mergify Bot to merge label Mar 27, 2024
andreasabel added a commit that referenced this pull request Mar 27, 2024
This is the same as #9845 but for the changelogs.yml workflow.
@@ -18,130 +18,135 @@ jobs:
meta:
name: Meta checks
runs-on: ubuntu-latest
env:
cabal_build: >-
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, that's the next level... I'm always worried about putting too much intelligence in the action code. Could this be a Makefile target too? (I see you made some updates to the Makefile).

I know, I'm stretching it, but some specification of what's supposed to happen inside the "quick jobs" workflow may help in future maintenance... Maybe a comment at the top of the file if not a whole section in CONTRIBUTING.md... OTOH it's always impossible to keep it up to date with the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I am aware that I am breaking the abstraction that the Makefile should offer, but I didn't see a way around it.
What I need here is a cache-key for some specific makefile goal(s), like e.g spdx templates. Putting this computation into the Makefile e.g. as spdx-templates-cache-hash would also be a bit awkward, and, I would need to invoke hashfiles from the Makefile (although this should be solvable).

I think we should document which goals of the Makefiles are in which workflows, so that these are changed in sync. E.g. the lexer goal was removed from the Makefile in #8980 but not from the workflow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. Future work then... Maybe a ticket.

andreasabel added a commit that referenced this pull request Mar 27, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Mar 29, 2024
Mikolaj pushed a commit that referenced this pull request Mar 29, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
andreasabel added a commit that referenced this pull request Apr 2, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
@mergify mergify bot merged commit 21c8780 into master Apr 2, 2024
@mergify mergify bot deleted the ci-xdg branch April 2, 2024 08:33
Mikolaj pushed a commit that referenced this pull request Apr 2, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
coot pushed a commit that referenced this pull request Apr 6, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
@ulysses4ever
Copy link
Collaborator

@andreasabel do you know if any of these fixes (or ones from #9849) can also be applied to the main workflow (validate) and worth it?

erikd pushed a commit to erikd/cabal that referenced this pull request Apr 22, 2024
This is the same as haskell#9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.
mergify bot pushed a commit that referenced this pull request May 13, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.

(cherry picked from commit af5d606)

# Conflicts:
#	.github/workflows/changelogs.yml
ulysses4ever pushed a commit that referenced this pull request May 13, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.

(cherry picked from commit af5d606)

# Conflicts:
#	.github/workflows/changelogs.yml
ulysses4ever pushed a commit to ulysses4ever/cabal that referenced this pull request May 14, 2024
This is the same as haskell#9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.

(cherry picked from commit af5d606)
ulysses4ever pushed a commit that referenced this pull request May 14, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.

(cherry picked from commit af5d606)
Mikolaj pushed a commit that referenced this pull request May 14, 2024
This is the same as #9845 but for the changelogs.yml workflow.

`changelog-d` currently has restrictive bound `base < 4.19` which we
need to ignore if we want to build on the latest GHC as shipped by the
GHA runner.

(cherry picked from commit af5d606)
@ulysses4ever
Copy link
Collaborator

CI-related backport

@ulysses4ever
Copy link
Collaborator

@mergify backport 3.12

Copy link
Contributor

mergify bot commented May 21, 2024

backport 3.12

✅ Backports have been created

@ulysses4ever
Copy link
Collaborator

Just a note for future us: it'd be helpful to keep the title of an issue in sync with its content. I just lost a day in CI hell because I wouldn't guess that the title "fix caching" is attached to a PR fixing actual CI failures like this one: https://github.com/haskell/cabal/pull/9845/files#diff-1d32b38ff76b89f854ace50e412f5b4bf3cadca31080b2d3104ef6937f7cccb7R5-R10

mergify bot added a commit that referenced this pull request May 23, 2024
…9845) (#10041)

* Fix #9815: switch quick-jobs CI to XDG

Fix #9815:
- Cache `~/.local/state/cabal` instead of `~/.cabal/store`
- `~/.local/bin` is used instead of `~/.cabal/bin` and is already in the PATH
  (verify this by calling `alex` after installing it)

As I am passing by:
- bump cache action to v4
- double-quote `$USER` to keep actionlint happy
- move `if` from shell-level to job-level
- allow newest `alex`

(cherry picked from commit e916cb5)

* CI quick-jobs: use preinstalled GHC and Cabal

(cherry picked from commit c209a82)

* Makefile: remove dead target 'lexer', use '.PHONY' systematically

The `lexer` target was removed in
#8980

(cherry picked from commit e600087)

* CI "Meta checks": correct cache key

(cherry picked from commit 56426e4)

* CI "Meta checks": print Haskell versions

(cherry picked from commit 9a311bd)

* CI "Doctest Cabal": daily refresh of cache

(cherry picked from commit ba6f6ff)

* CI "Check Field Syntax Reference": correct cache key

(cherry picked from commit 5949e3f)

* Update generated Cabal/src/Distribution/Simple/Build/Macros/Z.hs

Not sure why this was not up to date on master and still CI passed.
Maybe the content of this file is dependent on the GHC version we are
using to build the `get-cabal-macros` tool?

(cherry picked from commit 947860a)

* CI quick-jobs: entirely wipe ghcup directory rights workaround

(cherry picked from commit 5aa8afd)

* !fixup

---------

Co-authored-by: Andreas Abel <[email protected]>
Co-authored-by: Artem Pelenitsyn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
continuous-integration merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge re: xdg Concerning the XDG directory structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is something wrong with the cache in quick jobs?
4 participants