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

Caching should use go.mod, not go.sum #478

Open
peterbourgon opened this issue May 9, 2024 · 8 comments
Open

Caching should use go.mod, not go.sum #478

peterbourgon opened this issue May 9, 2024 · 8 comments
Labels
feature request New feature or request to improve the current logic

Comments

@peterbourgon
Copy link

go.sum is an append-only log of checksums, used to verify the integrity of modules downloaded during builds. It's essentially a manifest file (shasums) and not any kind of lock file (Cargo.lock). It doesn't represent the dependencies of the corresponding module in any meaningful sense. This dependabot issue goes into more detail.

Cache keys for Go modules need to be based on the (normalized) content of go.mod, not go.sum, in order to be useful.

@aparnajyothi-y
Copy link

Hello @peterbourgon, Thank you for creating this issue and we will look into it :)

@aparnajyothi-y aparnajyothi-y self-assigned this Jun 4, 2024
@matthewhughes934
Copy link

go.sum is an append-only log of checksums

Note: go.sum will be pruned as dependencies are removed if you run go mod tidy (from: https://go.dev/ref/mod#go-sum-files):

go mod tidy will add missing hashes and will remove unnecessary hashes from go.sum.

used to verify the integrity of modules downloaded during builds

Is this not a suitable for a file to be used as a cache key? if some new file needs to be downloaded that the cache should be updated to include that new file.

@peterbourgon
Copy link
Author

peterbourgon commented Jun 7, 2024

Is this not a suitable for a file to be used as a cache key? if some new file needs to be downloaded that the cache should be updated to include that new file.

Unfortunately not, no.

Again, go.sum isn't a lock file, and doesn't (necessarily) represent the actual dependencies used by the module. In fact, it doesn't even need to be committed! It exists purely to verify any dependencies fetched as part of the build process.

The go.sum file contains cryptographic hashes of the module’s direct and indirect dependencies ... The go.sum file may contain hashes for multiple versions of a module. The go command may need to load go.mod files from multiple versions of a dependency in order to perform minimal version selection. go.sum may also contain hashes for module versions that aren’t needed anymore.

Just use go.mod and the problem is solved.

And don't take my word for it: github.blog, etc.

lukaszcl added a commit to smartcontractkit/chainlink-github-actions that referenced this issue Aug 14, 2024
Update caching and use go.mod instead of go.sum as hashFile as discussed in actions/setup-go#478 (comment)
lukaszcl added a commit to smartcontractkit/chainlink-github-actions that referenced this issue Aug 16, 2024
…mework/run-tests actions (#191)

* Update caching in setup-go GHA

Update caching and use go.mod instead of go.sum as hashFile as discussed in actions/setup-go#478 (comment)

* Remove cache_key_id and use cache_version

This makes sure that the same cache key is used everywhere unless cache_version is changed.

* Remove download mods step

This is unnecessary step. The mods are downloaded anyway and then cached at the end of a job

* Do not cache go builds by default

Go builds can be fast and do not rely on network.
This will reduce cache size.

* Restore cache_key_id

* Update related GHA

* Use local actions for run-tests

* Update cache keys

* Fix

* Revert using local actions

* Bump

* Fix

* Bring back test_download_vendor_packages_command

* Remove cache_version as cache_key_id can include this

* Make test_download_vendor_packages_command optional

* Bump

* Bump
@aparnajyothi-y
Copy link

Hello @peterbourgon,

Thank you once again for creating this issue. We have analyzed using go.mod instead of go.sum for caching and identified the following key points:

  • The go.mod file declares a project's dependencies. Any changes here represent changes to the project's dependencies, necessitating a cache update.
  • The go.sum file logs all downloaded modules and includes checksums for integrity verification but doesn't represent the actual dependencies used by the project.

We will check the feasibility of the requested implementation and consider it as a feature request once we receive some feedback.

@aparnajyothi-y aparnajyothi-y added feature request New feature or request to improve the current logic and removed bug Something isn't working labels Aug 22, 2024
@aparnajyothi-y aparnajyothi-y removed their assignment Aug 22, 2024
@peterbourgon
Copy link
Author

Thank you!

@xeger
Copy link

xeger commented Sep 9, 2024

The caching performed by actions/setup-go is ineffective at caching gocache and gomodcache contents with my project and this may be one contributing factor. I stress that I don't know this for sure.

All I know is that when I cache the gocache directory myself using actions/cache, I benefit from significantly faster build, test and lint performance.

I do see evidence that setup-go is effectively caching some or all of gomodcache, so it seems my issue is mostly limited to gocache contents (which govern the behavior of go install, golangci-lint and go test).

@remyleone
Copy link

I also notice that when running go test or go build that many files are downloaded each time. I don't think that setup-go is effective. For people that are working at GitHub, do you have analytics about the github action where setup-go is present?

@Mago16
Copy link

Mago16 commented Sep 10, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

No branches or pull requests

6 participants