Update GHA for matrix build

[rpunt]

No description provided.

[rpunt]

Per suggestion in #97

[rpunt]

Example run at https://github.com/rpunt/go-diskfs/runs/8177713449?check_suite_focus=true

[deitch]

Let CI run.

[deitch]

CI is unhappy.

[yadutaf]

I'd love to see this merged!

If it helps, the build fix is trivial:

diff --git a/diskfs_unix.go b/diskfs_unix.go
index 31a94a4..3f364e1 100644
--- a/diskfs_unix.go
+++ b/diskfs_unix.go
@@ -1,5 +1,5 @@
-//go:build !darwin || linux || solaris || aix || freebsd || illumos || netbsd || openbsd || plan9
-// +build !darwin linux solaris aix freebsd illumos netbsd openbsd plan9
+//go:build linux || solaris || aix || freebsd || illumos || netbsd || openbsd || plan9
+// +build linux solaris aix freebsd illumos netbsd openbsd plan9
 
 package diskfs

A manual check with:

GOOS=darwin go build
GOOS=windows go build
GOOS=linux go build

Confirms that at least the build passes (not actually tested on target, though)

If it helps, I can open a PR for this fix, but it would be completely OK with me to just apply directly it on this PR!

[deitch]

Either fix is fine. I am happy to see this merged in once CI is clean.

[yadutaf]

@rpunt would you be interested in integrating this proposed fix in your PR ?

[rpunt]

@deitch just added to my branch and pushed, let's see how it does.

Thanks @yadutaf !

[deitch]

CI is unhappy about something. Also, should squash into a single commit.

[yadutaf]

It looks like the CI is unhappy because docker is not installed on the macos image. This somehow makes sense because the docker image would run in Linux VM under the hood, which is strictly equivalent to just run on Linux directly 😄

Since the Makefile and the test suite already make it easily possible to select whether we want to run the docker-based tests or not, maybe a good approach could be:

• linux: run make test
• macos / windows: run make unit-test (EDIT: and skip make image)

Would this approach make sense ? I'm not familiar with GitHub actions, so I'm not sure if this could be implemented.

[deitch]

We may not have much of a choice. Much as I would want make test to run on all platforms, it depends on the docker image TEST_IMAGE, which has utilities for validating the output.

OK, let's go with your approach:

• make image and make test on linux
• make unit-test on macOS/windows
• all of the rest on both

And squash the commits. The easiest may be to add an if statement to the last two lines:

      - name: unit-test
         run: make unit-test
      - name: image
         if: ${{ runner.os == "Linux" }}
         run: make image
       - name: test
         if: ${{ runner.os == "Linux" }}
         run: make test

You could use matrix.os == "ubuntu-latest" if you want, but that ties it to the matrix, which is fine.

[rpunt]

squashed and updated

[rpunt]

CI finally passed, with both the additions suggested for Docker images, and by skipping Windows runs for "vet" and "lint"

[deitch]

This looks good, except for the irrelevant Makefile changes. Some impelemntations might not like them either; arguably non-target-specific syntax is not supposed to be tab-indented. I have seen make versions where the following would get ignored:

ifeq ($(BUILDARCH),aarch64)
        BUILDARCH=arm64
endif

but not:

ifeq ($(BUILDARCH),aarch64)
BUILDARCH=arm64
endif

Either way, it is irrelevant to this PR, so please remove, and then we can merge in.