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

caches: Do we really need them anymore? The current implementation conflicts with automatic caching in latest setup-go. #863

Closed
2 tasks done
tinyzimmer opened this issue Sep 26, 2023 · 4 comments · Fixed by #1024
Labels
area: cache enhancement New feature or request

Comments

@tinyzimmer
Copy link

Welcome

  • Yes, I understand that the GitHub action repository is not the repository of golangci-lint itself.
  • Yes, I've searched similar issues on GitHub and didn't find any.

Your feature request related to a problem? Please describe.

The actions/setup-go action which is a requirement of this action already, has automatic caching now. When using that feature with skip-pkg-cache: false it causes many annotations to appear like the following screenshot.

image

Describe the solution you'd like.

Would it be possible to just have this action store it's caches in the same directories the actions/setup-go action uses? Then things should just work out of the box I assume.

Describe alternatives you've considered.

Setting skip-pkg-cache: true when using the latest setup-go actions.

Additional context.

Not much. But happy to provide.

@tinyzimmer tinyzimmer changed the title caches: Do we really need them anymore? caches: Do we really need them anymore? The current implementation conflicts with automatic caching in latest setup-go. Sep 26, 2023
@tinyzimmer
Copy link
Author

On second thought. The combination of telling setup-go to not cache and then using this action regularly is another solution. I guess I'm wondering if for simpler cases there should be a similar cache: false for this action in case one wants to use the default one from setup-go.

@richchurcher
Copy link

We're seeing this as well. Related to #387 and #807 (among probably others).

@navijation
Copy link
Contributor

I believe the cache does not simply contain Go's build artifacts: it also caches the analyzer facts / reporting for each package and reuses cached outputs when the dependency closure for a package doesn't change.

It might warrant creating an option to skip the go build cache specifically though.

@richchurcher
Copy link

richchurcher commented Oct 28, 2023

Yes, or even potentially just writing to a path that won't conflict. You could even provide an option to summarise the exit 2 from tar as "some cache paths could not be written due to the file already existing", since not doing it generates screeds of warnings/errors. It's also worth noting that our very rough testing showed that jobs where these errors occurred were also slower than those which completed without error.

@ldez ldez added enhancement New feature or request area: cache labels Feb 16, 2024
strantalis added a commit to opentdf/platform that referenced this issue Mar 13, 2024
github-merge-queue bot pushed a commit to opentdf/platform that referenced this issue Mar 13, 2024
* chore: cleanup local golangci

* remove different golangci-config

* add merge group step to push step

* upgrade golangci to 1.56

* update golangci action

* try skip cache golangci/golangci-lint-action#863

* try single golangci-lint

* don't set only new issues in config

* Update checks.yaml

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>

* log context

* point to commit

* only new issues on pr or merge group

---------

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>
tech-guru42 added a commit to tech-guru42/TDF that referenced this issue Jun 3, 2024
* chore: cleanup local golangci

* remove different golangci-config

* add merge group step to push step

* upgrade golangci to 1.56

* update golangci action

* try skip cache golangci/golangci-lint-action#863

* try single golangci-lint

* don't set only new issues in config

* Update checks.yaml

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>

* log context

* point to commit

* only new issues on pr or merge group

---------

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>
passion-127 added a commit to passion-127/TDF that referenced this issue Jun 6, 2024
* chore: cleanup local golangci

* remove different golangci-config

* add merge group step to push step

* upgrade golangci to 1.56

* update golangci action

* try skip cache golangci/golangci-lint-action#863

* try single golangci-lint

* don't set only new issues in config

* Update checks.yaml

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>

* log context

* point to commit

* only new issues on pr or merge group

---------

Co-authored-by: Dave Mihalcik <dmihalcik@virtru.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cache enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants