ci: checkout first, disable go caching#160
Merged
Conversation
Collaborator
Author
|
Checked that is solves the warning issue, and no attempts to save or restore cache is done. |
thaJeztah
approved these changes
Sep 26, 2024
Member
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
left one comment about a comment, but mostly thinking out loud, and I'm fine with taking this as-is for sure
tianon
approved these changes
Sep 27, 2024
Member
tianon
left a comment
There was a problem hiding this comment.
LGTM (a comment would be nice though, maybe even just a pointer to this PR for the trivial context reminder?)
First, checking out the code should be done before installing Go.
Second, since we don't have top-level go.mod, actions/setup-go
complains:
> Restore cache failed: Dependencies file is not found in /home/runner/work/sys/sys. Supported file pattern: go.sum
One way to solve this would be to
cache-dependency-path: "*/go.sum"
parameter to actions/setup-go. Alas it won't work because not all
modules here have go.mod, and * in GHA is not what you think it is
(i.e. not a part of shell glob pattern, but rather "every directory"),
and as a result it complains about missing go.sum files.
Another way is to list all the paths explicitly, which is not good from
the maintainability perspective (someone will definitely forgot to add
a go.sum path).
Yet another way is to add a step which lists all go.sum. It works
something like this:
- name: Find go.sum files
id: gosum
run: |
echo 'files<<EOF' >> "$GITHUB_OUTPUT"
git ls-files '*/go.sum' >> "$GITHUB_OUTPUT"
echo 'EOF' >> "$GITHUB_OUTPUT"
...
- name: Install Go
uses: actions/setup-go@v5
with:
cache-dependency-path: ${{ steps.gosum.outputs.files }}
But given the fact that caching is not doing much here (as we don't have
a lot of code) it's becoming too complicated with not much to gain.
So, let's just disable Go caching, so the warning above will go away.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
f652093 to
abc10f0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an alternative to #155.
First, checking out the code should be done before installing Go.
Second, since we don't have top-level
go.mod, actions/setup-go complains:One way to solve this would be to add
parameter to
actions/setup-go. Alas it won't work because not all modules here have go.mod, and * in GHA is not what you think it is (i.e. not a part of shell glob pattern, but rather "every directory"), and as a result it complains about missing go.sum files.Another way is to list all the paths explicitly, which is not good from the maintainability perspective (someone will definitely forgot to add a go.sum path).
Yet another way is to add a step which lists all go.sum. It works something like this:
But given the fact that caching is not doing much here (as we don't have a lot of code) it's becoming too complicated with not much to gain.
So, let's just disable Go caching, so the warning above will go away.