Skip to content

Conversation

@kolyshkin
Copy link
Collaborator

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:

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 add

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.

@kolyshkin kolyshkin marked this pull request as ready for review September 26, 2024 06:24
@kolyshkin
Copy link
Collaborator Author

Checked that is solves the warning issue, and no attempts to save or restore cache is done.

Copy link
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

left one comment about a comment, but mostly thinking out loud, and I'm fine with taking this as-is for sure

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

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>
@kolyshkin kolyshkin merged commit 46235e8 into moby:main Sep 27, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants