Skip to content

ci/gha: enable go caching#48729

Merged
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:go-cache
Oct 24, 2024
Merged

ci/gha: enable go caching#48729
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:go-cache

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

- What I did

actions/setup-go complains:

Restore cache failed: Dependencies file is not found in
/home/runner/work/moby/moby. Supported file pattern: go.sum

Let's give it one to chew.

- How I did it

- How to verify it

Look in GHA warnings, the one quoted above should be gone.

Some CI jobs might suddenly become faster thanks to go caches.

- Description for the changelog

ci/gha: fix go caching

- A picture of a cute animal (not mandatory but encouraged)

actions/setup-go complains:

> Restore cache failed: Dependencies file is not found in
> /home/runner/work/moby/moby. Supported file pattern: go.sum

Let's give it one to chew.

Signed-off-by: Kir Kolyshkin <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member

Oh! I recall we had some discussion about this in #47428 (comment)

Do we know what it's trying to restore? Given that we use vendoring, I'm wondering what module cache it would try to use outside of that. If there's none, perhaps we need to disable the cache 🤔

It also restores the go build cache (go env GOCACHE). Basically it's using the hash of go.sum file to make a cache key, so the cache is invalidated whenever go.sum changes.
I don't see any meaningful impact on the jobs execution time though. So maybe we can just disable that..

FWIW, I'm not against doing this, but perhaps only to make the action happy (as it may not bring much gain from a performance perspective otherwise?)

@thaJeztah thaJeztah added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/testing labels Oct 23, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Oct 23, 2024
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

but nudging @vvoland as well 🤗

@thaJeztah thaJeztah requested a review from vvoland October 23, 2024 10:27
Copy link
Copy Markdown
Contributor

@vvoland vvoland left a comment

Choose a reason for hiding this comment

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

I tried it some time ago (#47428) and didn't notice any meaningful speedup.

But it doesn't hurt, so LGTM

@thaJeztah
Copy link
Copy Markdown
Member

👍 yeah, let's bring it in then

I was also wondering if we should have a target to run it in a container instead of using the go-setup action, but I guess that may be hella slow on Windows, so probably not great.

@thaJeztah thaJeztah merged commit 5aaceef into moby:master Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants