elvish
elvish copied to clipboard
The CI only runs lint checks (`make all-checks`) on Ubuntu
I recently rewrote my script that automates tasks, such as building Elvish and running its unit-tests, on all of my systems from Bash to Elvish. That made it much easier to add another task: running make all-checks
on each system. That revealed an unexpected error:
Linting Elvish...
macpro 2.767 seconds (1.0x)
fedora37 3.113 seconds (1.1x)
fedora28 13.886 seconds (4.5x)
osuse 14.110 seconds (1.0x)
ubuntu32 14.456 seconds (1.0x)
freebsd 14.564 seconds (1.0x)
ubuntu64 15.015 seconds (1.0x)
msys 24.078 seconds (1.6x)
./tools/check-fmt-go.sh
Go files need these changes:
None!
./tools/check-fmt-md.sh
Markdown files that need changes:
None!
./tools/check-disallowed.sh
codespell
go vet ./...
staticcheck ./...
pkg\sys\eunix\termios_32bitflag.go:5:6: type termiosFlag is unused (U1000)
make: *** [Makefile:41: most-checks] Error 1
Exception: make exited with 2
[tty 17]:1:1: make all-checks
That error isn't being caught by the CI (continuous integration) environment because it only runs make all-checks
on Ubuntu and not Windows (or another non-Linux OS). I'll open a pull-request that fixes the cause of that error. This issue is to discuss whether it makes sense to modify the CI config to run make all-checks
on every OS target given the additional cost of doing so.
P.S., Fixing the build constraints reveals another problem. A generated file only used on Windows is out of date:
Linting Elvish...
macpro 2.868 seconds (1.0x)
fedora37 3.195 seconds (1.1x)
fedora28 13.189 seconds (4.1x)
osuse 13.527 seconds (1.0x)
ubuntu32 13.750 seconds (1.0x)
freebsd 13.973 seconds (1.0x)
ubuntu64 14.191 seconds (1.0x)
msys 40.497 seconds (2.9x)
./tools/check-fmt-go.sh
Go files need these changes:
None!
./tools/check-fmt-md.sh
Markdown files that need changes:
None!
./tools/check-disallowed.sh
codespell
go vet ./...
staticcheck ./...
./tools/check-gen.sh
======================================================================
Generated Go code is out of date. See
https://github.com/elves/elvish/blob/master/CONTRIBUTING.md#generated-code
======================================================================
M pkg/sys/ewindows/ztypes_windows.go
make: *** [Makefile:44: all-checks] Error 1
Exception: make exited with 2
[tty 25]:1:1: make all-checks
The reason for the ./tools/check-gen.sh
failure is because the generated file, pkg/sys/ewindows/ztypes_windows.go, contains this line:
// cgo.exe -godefs C:\Users\xiaq\on\elvish\pkg\sys\ewindows\types.go
It should apparently contain this line:
// cgo.exe -godefs types.go
It is not necessary to run go vet
or staticcheck
on multiple platforms. Setting GOOS
and GOARCH
is sufficient.
The check-gen
script does need to be run on multiple platforms.
The other checks do not depend on GOOS
or GOARCH
.
Playing devil's advocate: It still seems the simplest, easiest to understand and maintain, option is to simply run make all-checks
on every platform in the CI test matrix rather than just Ubuntu. Doing so should be cheap and protects against future changes that might invalidate the current assumptions vis-a-vis make most-checks
being platform independent. On the other hand having to install codespell
and other tools on every CI platform to run checks that are platform independent is wasteful. Which argues for modifying the CI config to run ./tools/check-gen.sh
on every platform in the CI matrix and modifying the "checks:" CI job to run make most-checks
rather than make all-checks
. That is more efficient but also more fragile (at least in theory).
I am ambivalent about which approach is preferable. However, one of the changes proposed above is needed because the CI environments should catch any error I would find running the same commands on my local systems.