Update to Go 1.22 and Golangci-lint 1.56.2 (#104)

I recommend reviewing commits one by one

---

UDENG-2488

.github/workflows/qa-azure.yaml

@@ -39,7 +39,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.20"
+          go-version-file: go.mod
       - name: Prepare repo
         shell: powershell
         run: |

.github/workflows/qa.yaml

@@ -36,12 +36,12 @@ jobs:
     - name: Lint with mock back-end
       uses: golangci/golangci-lint-action@v4
       with:
-        version: v1.55.2
+        version: v1.56.2
         args:  --build-tags="gowslmock"
     - name: Lint with real back-end
       uses: golangci/golangci-lint-action@v4
       with:
-        version: v1.55.2
+        version: v1.56.2
     - name: Test with mocks
       shell: bash
       run: go test -tags="gowslmock" -shuffle=on

.golangci.yaml

@@ -9,25 +9,31 @@ linters:
   # linters to run in addition to default ones
   enable:
     - dupl
+    - dupword
     - durationcheck
     - errname
     - errorlint
     - exportloopref
     - forbidigo
     - forcetypeassert
     - gci
+    - gocheckcompilerdirectives
     - godot
     - gofmt
     - gosec
     - misspell
+    - nakedret
     - nolintlint
+    - protogetter
     - revive
+    - sloglint
+    - testifylint
     - thelper
     # - tparallel # Tests are non-parallel by default because they depend on the global state of WSL and the registry
     - unconvert
     - unparam
+    - usestdlibvars
     - whitespace
-    ##- wrapcheck # To think properly about it
 
 run:
   timeout: 5m
@@ -68,4 +74,4 @@ linters-settings:
   staticcheck:
     # Should be better for it to be autodetected
     # https://github.com/golangci/golangci-lint/issues/2234
-    go: "1.21"
+    go: "1.22.1"

distro_test.go

@@ -3,6 +3,7 @@ package gowsl_test
 import (
 	"context"
 	"fmt"
+	"os/exec"
 	"regexp"
 	"sync"
 	"testing"
@@ -32,7 +33,6 @@ func TestShutdown(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx, modifyMock := setupBackend(t, context.Background())
 			d1 := newTestDistro(t, ctx, rootFS)
@@ -78,7 +78,6 @@ func TestTerminate(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx, modifyMock := setupBackend(t, context.Background())
 			testDistro := newTestDistro(t, ctx, rootFS)
@@ -129,7 +128,6 @@ func TestDefaultDistro(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx, modifyMock := setupBackend(t, context.Background())
 
@@ -195,7 +193,6 @@ func TestDistroSetAsDefault(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx, modifyMock := setupBackend(t, context.Background())
 			if tc.wslexeError {
@@ -249,7 +246,6 @@ func TestDistroString(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			setupBackend(t, context.Background())
 
@@ -297,7 +293,6 @@ func TestGUID(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			if tc.registryInaccessible {
 				modifyMock(t, func(m *mock.Backend) {
@@ -385,7 +380,6 @@ func TestConfigurationSetters(t *testing.T) {
 	}
 
 	for name, tc := range tests {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			// This test has two phases:
 			// 1. Changes one of the default settings and asserts that it has changed, and the others have not.
@@ -506,7 +500,6 @@ func TestGetConfiguration(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx, modifyMock := setupBackend(t, context.Background())
 			if tc.syscallError {
@@ -530,11 +523,11 @@ func TestGetConfiguration(t *testing.T) {
 				return
 			}
 			require.NoError(t, err, "unexpected failure in GetConfiguration")
-			assert.Equal(t, c.Version, uint8(2))
-			assert.Equal(t, c.DefaultUID, uint32(0))
-			assert.Equal(t, c.InteropEnabled, true)
-			assert.Equal(t, c.PathAppended, true)
-			assert.Equal(t, c.DriveMountingEnabled, true)
+			assert.Equal(t, uint8(2), c.Version)
+			assert.Zero(t, c.DefaultUID)
+			assert.True(t, c.InteropEnabled)
+			assert.True(t, c.PathAppended)
+			assert.True(t, c.DriveMountingEnabled)
 
 			defaultEnvs := map[string]string{
 				"HOSTTYPE": "x86_64",
@@ -579,7 +572,6 @@ func TestDistroState(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			switch tc.action {
 			case none:
@@ -632,7 +624,15 @@ func asyncNewTestDistro(t *testing.T, ctx context.Context, rootFs string) wsl.Di
 
 	go func() {
 		defer wg.Done()
-		installDistro(t, ctx, d.Name(), loc, rootFs)
+
+		defer wslExeGuard(2 * time.Minute)()
+		cmd := fmt.Sprintf("$env:WSL_UTF8=1 ;  wsl --import %q %q %q", d.Name(), loc, rootFs)
+		//nolint:gosec // Code injection is not a concern in tests.
+		out, err := exec.Command("powershell.exe", "-Command", cmd).CombinedOutput()
+		if err != nil {
+			t.Logf("Setup: failed to register %q: %s", d.Name(), out)
+		}
+		// We cannot fail here because this is not the main test goroutine
 	}()
 
 	t.Cleanup(func() {

exec.go

@@ -353,6 +353,8 @@ func (c *Cmd) closeDescriptors(closers []io.Closer) {
 
 // readerDescriptor connects an arbitrary reader to an os pipe's writer,
 // and returns this pipe's reader as a file.
+//
+//nolint:nakedret // This function is mostly copied from the standard library
 func (c *Cmd) readerDescriptor(r io.Reader) (f *os.File, err error) {
 	// Based on exec/exec.go:stdin.
 	if r == nil {
@@ -399,6 +401,8 @@ func (c *Cmd) readerDescriptor(r io.Reader) (f *os.File, err error) {
 
 // writerDescriptor connects an arbitrary writer to an os pipe's reader,
 // and returns this pipe's writer as a file.
+//
+//nolint:nakedret // This function is mostly copied from the standard library
 func (c *Cmd) writerDescriptor(w io.Writer) (f *os.File, err error) {
 	// Based on exec/exec.go.
 	if w == nil {

exec_test.go

@@ -82,7 +82,6 @@ func TestCommandRun(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			d := realDistro
 			if tc.fakeDistro {
@@ -133,7 +132,7 @@ func TestCommandRun(t *testing.T) {
 			target := &exec.ExitError{}
 			if tc.wantExitCode != 0 {
 				require.ErrorAsf(t, err, &target, "Run() should have returned an ExitError")
-				require.Equal(t, target.ExitCode(), tc.wantExitCode, "returned error ExitError has unexpected Code status")
+				require.Equal(t, tc.wantExitCode, target.ExitCode(), "returned error ExitError has unexpected Code status")
 				return
 			}
 
@@ -245,7 +244,7 @@ func TestCommandStartWait(t *testing.T) {
 		target := &exec.ExitError{}
 		if tc.wantExitError != 0 {
 			require.ErrorAsf(t, err, &target, "Unexpected error type at time %s. Expected an ExitError.", whenToString(now))
-			require.Equal(t, target.ExitCode(), tc.wantExitError, "Unexpected value for ExitError.Code at time %s", whenToString(now))
+			require.Equal(t, tc.wantExitError, target.ExitCode(), "Unexpected value for ExitError.Code at time %s", whenToString(now))
 			return true
 		}
 
@@ -255,7 +254,6 @@ func TestCommandStartWait(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			ctx := context.Background()
 
@@ -408,7 +406,6 @@ func TestCommandOutPipes(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -494,7 +491,6 @@ func TestCommandOutput(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -521,7 +517,7 @@ func TestCommandOutput(t *testing.T) {
 
 			target := &exec.ExitError{}
 			require.ErrorAsf(t, err, &target, "Unexpected error type. Expected an ExitError.")
-			require.Equal(t, target.ExitCode(), tc.wantExitCode, "Unexpected value for ExitError.Code.")
+			require.Equal(t, tc.wantExitCode, target.ExitCode(), "Unexpected value for ExitError.Code.")
 
 			got := strings.ReplaceAll(string(target.Stderr), "\r\n", "\n")
 			require.Equal(t, tc.wantStderr, got, "Unexpected contents in stderr")
@@ -565,7 +561,6 @@ func TestCommandCombinedOutput(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -596,7 +591,7 @@ func TestCommandCombinedOutput(t *testing.T) {
 
 			target := &exec.ExitError{}
 			require.ErrorAsf(t, err, &target, "Unexpected error type. Expected an ExitError.")
-			require.Equal(t, target.ExitCode(), tc.wantExitCode, "Unexpected value for ExitError.Code.")
+			require.Equal(t, tc.wantExitCode, target.ExitCode(), "Unexpected value for ExitError.Code.")
 		})
 	}
 }
@@ -641,7 +636,6 @@ print("Your text was", v)
 '`
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			if tc.text == "" {
 				tc.text = "Hello, wsl!"

go.mod

@@ -1,6 +1,8 @@
 module github.com/ubuntu/gowsl
 
-go 1.21
+go 1.22.0
+
+toolchain go1.22.1
 
 require golang.org/x/sys v0.18.0
 

internal/backend/windows/win32_windows.go

@@ -8,7 +8,6 @@ import (
 	"math"
 	"os"
 	"strings"
-	"sync"
 	"syscall"
 	"unsafe"
 
@@ -259,45 +258,27 @@ func callDll(proc *syscall.LazyProc, args ...uintptr) (uint32, error) {
 func processEnvVariables(cStringArray **char, len uint64) map[string]string {
 	stringPtrs := unsafe.Slice(cStringArray, len)
 
-	env := make(chan struct {
-		key   string
-		value string
-	})
-
-	wg := sync.WaitGroup{}
+	env := make(map[string]string)
 	for _, cStr := range stringPtrs {
-		cStr := cStr
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			goStr := stringCtoGo(cStr, 32768)
-			idx := strings.Index(goStr, "=")
-			env <- struct {
-				key   string
-				value string
-			}{
-				key:   strings.Clone(goStr[:idx]),
-				value: strings.Clone(goStr[idx+1:]),
-			}
-			windows.CoTaskMemFree(unsafe.Pointer(cStr))
-		}()
-	}
+		goStr := stringCtoGo(cStr, 32768)
+
+		idx := strings.Index(goStr, "=")
+		if idx == -1 {
+			continue
+		}
 
-	// Cleanup
-	go func() {
-		wg.Wait()
-		windows.CoTaskMemFree(unsafe.Pointer(cStringArray))
-		close(env)
-	}()
+		variable := goStr[:idx]
+		value := goStr[idx+1:]
 
-	// Collecting results
-	m := map[string]string{}
+		env[variable] = value
 
-	for kv := range env {
-		m[kv.key] = kv.value
+		// Free the string
+		windows.CoTaskMemFree(unsafe.Pointer(cStr))
 	}
 
-	return m
+	// Free the array
+	windows.CoTaskMemFree(unsafe.Pointer(cStringArray))
+	return env
 }
 
 // stringCtoGo converts a null-terminated *char into a string

internal/flags/flags_test.go

@@ -36,7 +36,6 @@ func TestUnpackFlags(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(fmt.Sprintf("input_0x%x", int(tc.input)), func(t *testing.T) {
 			t.Parallel()
 			got := flags.Unpack(tc.input)
@@ -64,7 +63,6 @@ func TestPackFlags(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(fmt.Sprintf("expects_0x%x", int(tc.wants)), func(t *testing.T) {
 			t.Parallel()
 			got, _ := tc.input.Pack()

internal/state/state_test.go

@@ -27,7 +27,6 @@ func TestStateFromString(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
@@ -62,7 +61,6 @@ func TestString(t *testing.T) {
 	}
 
 	for name, tc := range testCases {
-		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 

mock/wslexe.go

@@ -42,7 +42,7 @@ func (backend *Backend) Terminate(distroName string) error {
 
 	guid, key := backend.findDistroKey(distroName)
 	if guid == "" {
-		return errors.New("Bla bla bla this is localized text, don't assert on it.\nError code: Wsl/Service/WSL_E_DISTRO_NOT_FOUND")
+		return errors.New("This is localized text, don't assert on it.\nError code: Wsl/Service/WSL_E_DISTRO_NOT_FOUND")
 	}
 
 	return key.state.Terminate()