Repository: rclone/rclone
Pull request #7776 (2024-04-17 14:46:49)

Fix golangci-lint issues reported on different OS
#### What is the purpose of this change?

Fixes the following issues from running golangci-lint on Windows, as well as a couple of more:

```
cmd\serve\nfs\nfs_unsupported.go:13:30: var-declaration: should drop = nil from declaration of var Command; it is the zero value (revive)
var Command *cobra.Command = nil
                             ^
cmd\serve\nfs\nfs_unsupported.go:12:1: exported: comment on exported var Command should be of the form "Command ..." (revive)
// For unsupported platforms we just put nil
^
lib\buildinfo\osversion_windows.go:100:6: func `getRegistryVersionInt` is unused (unused)
func getRegistryVersionInt(name string) int {
     ^
lib\file\mkdir_windows_test.go:38:15: Error return value of `f.Close` is not checked (errcheck)
        defer f.Close()
                     ^
lib\file\preallocate_windows.go:95:32: exported: comment on exported const FSCTL_SET_SPARSE should be of the form "FSCTL_SET_SPARSE ..." (revive)
        FSCTL_SET_SPARSE = 0x000900c4 // Control code to set or clears the FILE_ATTRIBUTE_SPARSE_FILE attribute of a file.
                                      ^
lib\file\preallocate_windows.go:59:10: unnecessary conversion (unconvert)
                uintptr(out.Fd()),
                       ^
lib\file\preallocate_windows.go:62:10: unnecessary conversion (unconvert)
                uintptr(unsafe.Sizeof(fsSizeInfo)),
                       ^
lib\file\preallocate_windows.go:78:10: unnecessary conversion (unconvert)
                uintptr(out.Fd()),
                       ^
lib\file\preallocate_windows.go:81:10: unnecessary conversion (unconvert)
                uintptr(unsafe.Sizeof(allocInfo)),
                       ^
lib\file\preallocate_windows.go:85:25: unnecessary conversion (unconvert)
                if e1 == syscall.Errno(windows.ERROR_DISK_FULL) || e1 == syscall.Errno(windows.ERROR_HANDLE_DISK_FULL) {
                                      ^
lib\terminal\hidden_windows.go:17:19: Error return value of `showWindow.Call` is not checked (errcheck)
                        showWindow.Call(hwnd, 0)
                                       ^
fs\fserrors\retriable_errors_windows.go:27:2: var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)
        WSAHOST_NOT_FOUND syscall.Errno = 11001
        ^
fs\fserrors\retriable_errors_windows.go:28:2: var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)
        WSATRY_AGAIN      syscall.Errno = 11002
        ^
backend\local\remove_windows.go:15:2: exported: exported const ERROR_SHARING_VIOLATION should have comment (or a comment on this block) or be unexported (revive)
        ERROR_SHARING_VIOLATION syscall.Errno = 32
        ^
fs\accounting\token_bucket.go:43:20: func `(*buckets)._isOff` is unused (unused)
func (bs *buckets) _isOff() bool {
                   ^
fs\log\redirect_stderr_windows.go:24:15: SA1019: syscall.Syscall has been deprecated since Go 1.18: Use SyscallN instead. (staticcheck)
        r0, _, e1 := syscall.Syscall(procSetStdHandle.Addr(), 2, uintptr(stdhandle), uintptr(handle), 0)
```


#### Was the change discussed in an issue or in the forum before?

<!--
Link issues and relevant forum posts here.
-->

#### Checklist

- [ ] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-new-feature-or-bug-fix).
- [ ] I have added tests for all changes in this PR if appropriate.
- [ ] I have added documentation for the changes if appropriate.
- [ ] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [ ] I'm done, this Pull Request is ready for review :-)


ncw: Do you think we should run golangci-lint on windows/macOS too?

Actually I don't think we need to as this seems to work

  GOOS=windows golangci-lint run ./...

We can probably wedge that into the `lint` workflow somehow...
albertony: Maybe we could.. However the running time maybe an issue? At least if we end up running it once for each of the supported GOOS values... PR checks already takes some 15-20 minutes, which is considerable. The linting something above 40s of that it seems, and of that again e.g. 11s for installing golangci-lint in the runner and 31s for running it. Running it multiple times in the same agent will hopefully run faster, assuming its clever enough to reuse cached results. Worth a little experiment, yes.
albertony: I also noticed the following golangci-options while digging a bit into it earlier today, if it slows the checks down too much it could perhaps be an option to configure it to only check actual changed code when triggered by PRs? I haven't tested how it works, though.

```
  -n, --new                            Show only new issues: if there are unstaged changes or untracked files, only those changes are analyzed, else only changes in HEAD~ are analyzed.
                                       It's a super-useful option for integration of golangci-lint into existing large codebase.
                                       It's not practical to fix all existing issues at the moment of integration: much better to not allow issues in new code.
                                       For CI setups, prefer --new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate unstaged files before golangci-lint runs.
      --new-from-rev REV               Show only new issues created after git revision REV
```
albertony: I've been thinking, some of the issues may not be from golangci-lint, actually. I ran it from within VSCode at one point, and then it reports some issues from gopls as well, so I can look into that and revert - if some of them looks closer to false positives.
ncw: > Maybe we could.. However the running time maybe an issue? At least if we end up running it once for each of the supported GOOS values... PR checks already takes some 15-20 minutes, which is considerable. The linting something above 40s of that it seems, and of that again e.g. 11s for installing golangci-lint in the runner and 31s for running it.

The checks run in parallel so as long as we don't make the lint step take more that say 10 mins it won't affect the total running time.

I think the lint runs quite quickly so I don't think we'll need to enable the diff check.

You can always try the golangci lint binary from the command line to see if it matches vscode


albertony: I took a step back and re-ran the linting, and removed most of the fixes which were not from golangci-lint. Left a few of them, though, as I found they were quite unquestionable, as well as the go mod tidy commit.

The removed gopls reported issues was mainly [unusedparams](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/unusedparams#unusedparam) issues like this:

 ```diff
-func lChtimes(_ string, _ time.Time, _ time.Time) error {
+func lChtimes(name string, atime time.Time, mtime time.Time) error {
        // Does nothing
        return nil
 }
 ```
 
I assume golangci-lint would have reported them as well, if we hadn't had this configuration:

https://github.com/rclone/rclone/blob/c09426bcfef45dcfc0aa2e4ac0ef272c52be6c58/.golangci.yml#L91-L92

Mainly because there would be a lot of such issues reported, probably.
ncw: Nice! What is up with netbsd and solaris I wonder?



> I took a step back and re-ran the linting, and removed most of the fixes which were not from golangci-lint. Left a few of them, though, as I found they were quite unquestionable, as well as the go mod tidy commit.
> 
> The removed gopls reported issues was mainly [unusedparams](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/unusedparams#unusedparam) issues like this:
[...]
> I assume golangci-lint would have reported them as well, if we hadn't had this configuration:
> 
> https://github.com/rclone/rclone/blob/c09426bcfef45dcfc0aa2e4ac0ef272c52be6c58/.golangci.yml#L91-L92
> 
> Mainly because there would be a lot of such issues reported, probably.

Can you get vscode to use the the golangci-lint config that we supply in the root of the rclone source repo?

I didn't enable unusedparams as in a project like rclone we implement the same interface many, many times and I think it is actually useful to keep the function definitions constant rather than change some of them to have `_` in.


albertony: > Can you get vscode to use the the golangci-lint config that we supply in the root of the rclone source repo?

Yes it does that. VSCode with go extension uses gopls for various language features and staticcheck specifically for linting by default, and its a simple setting to switch from staticcheck to golangci-lint. However the gopls, used for intellisense and a lot of things, also seem to do some lint-like checks. So its that gopls check that reports the unusedparams I mentioned. If I explicitely run the linting feature of VSCode it comes reports no issues. But when the gopls checks kick in they report additional issues, and all are mixed in a single "Problems" panel, thats why I "blamed" it on golangci-lint at first. I may look into the gopls vs lint settings at some point, but I think I'll just live with it for now..

> I didn't enable unusedparams as in a project like rclone we implement the same interface many, many times and I think it is actually useful to keep the function definitions constant rather than change some of them to have _ in.

Yes, I concluded on that too, that parameter names may indeed add relevant information even if not used.
albertony: > What is up with netbsd and solaris I wonder?

I don't know.. 😕 I suspect something to do with caching. In addition to the separate module cache step, which I suggest removing with https://github.com/rclone/rclone/pull/7779, the setup-go action has caching of modules and build output, and the golangci-lint action has caching of modules, build output - and lint results.
ncw: > > What is up with netbsd and solaris I wonder?
> 
> I don't know.. 😕 I suspect something to do with caching. In addition to the separate module cache step, which I suggest removing with #7779, the setup-go action has caching of modules and build output, and the golangci-lint action has caching of modules, build output - and lint results.

I wonder if golangci-lint is coping with caching results for different architectures OK? You can set cache keys for github runs but not sure that is relevant here since everything is in one run.

> Edit: In the build logs (also existing builds) there are from time to time a lot of "Cannot open: File exists" entries which is related to caching as well. Many discussions on similar things over at https://github.com/golangci/golangci-lint-action.

Can try `skip-cache: true` I suppose - that is suggested in an issue.
 
> Edit: I also added setup-go action to the lint job. The [golangci-lint-action docs](https://github.com/golangci/golangci-lint-action?tab=readme-ov-file#compatibility) states:
> 
> > v4.0.0+ requires an explicit setup-go installation step before using this action: uses: actions/setup-go@v5. The skip-go-installation option has been removed.
> 
> But it obviously worked without it as well... 🤔

Odd! But your change sounds correct :-)


albertony: I just added a commit which I think fixes the issues by some small adjustments to build tags in a couple of tests etc. Seems that fixed it actually, so maybe not caching after all.

Unfortunately I also added linting on dragonfly, so that introduced a lot of errors again.. 😆 But not too hard to fix I think, with some more adjustments to build tags.

But I don't know for which GOOS we do want to run the linting? All we have build tags for in code (linux, windows, darwin, freebsd, netbsd, openbsd, dragonfly, plan9, js, solaris)? All that we build during the PR checks (linux, darwin, windows)? All that we have builds for on download page (linux, windows, darwin, freebsd, netbsd, openbsd, plan9, solaris - but not dragonfly or js)?
albertony: All green in https://github.com/rclone/rclone/actions/runs/8737234254, with goos linux, windows, darwin, freebsd, netbsd, openbsd, dragonfly, plan9, solaris, but skipped js as that fails with "Running error: context loading failed: no go files to analyze: running `go mod tidy` may solve the problem".

But that run was with all cache hits on golangci-lint-action caches. I have manually deleted the caches now, and re-running it to see how that behaves..
albertony: > But that run was with all cache hits on golangci-lint-action caches. I have manually deleted the caches now, and re-running it to see how that behaves..

That worked too: https://github.com/rclone/rclone/actions/runs/8737533920/job/23974720264

However, in the automatic Post processing steps, where the cache saving happens, the first post-step that runs, which is for the last lint step that ran, "Post Code quality test (Solaris)", saves the cache:

```
Cache Size: ~12 MB (12671557 B)
Cache saved successfully
Saved cache for golangci-lint from paths '/home/runner/.cache/golangci-lint' in 4932ms
```

While the rest of the post steps does nothing - because all have the same cache key:

```
Failed to save: Unable to reserve cache with key golangci-lint.cache-2833-820432f836f6c171106fffde2baea2a6de7ef88e, another job may be creating this cache. More details: Cache already exists. Scope: refs/pull/7776/merge, Key: golangci-lint.cache-2833-820432f836f6c171106fffde2baea2a6de7ef88e, Version: a8bb4d051abedb2e113db79282817e72e10ab797944320637e9cb6fad8ce5a5c
Saved cache for golangci-lint from paths '/home/runner/.cache/golangci-lint' in 2237ms
```

Next I'm running yet another build: https://github.com/rclone/rclone/actions/runs/8738200411/job/23976751398
This will pick up the saved cache, download it and extract into place:

```
Cache Size: ~12 MB (12671557 B)
  /usr/bin/tar -xf /home/runner/work/_temp/9[15](https://github.com/rclone/rclone/actions/runs/8738200411/job/23976751398#step:4:16)02d34-5e6a-492d-acf5-a9db0301603e/cache.tzst -P -C /home/runner/work/rclone/rclone --use-compress-program unzstd
  Received 12671557 of 12671557 (100.0%), 12.1 MBs/sec
  Cache restored successfully
  Restored cache for golangci-lint from key 'golangci-lint.cache-2833-820432f836f6c[17](https://github.com/rclone/rclone/actions/runs/8738200411/job/23976751398#step:4:18)1106fffde2baea2a6de7ef88e' in 3989ms
```

And when done, the post steps does nothing:

```
Cache hit occurred on the primary key golangci-lint.cache-2833-820432f836f6c171106fffde2baea2a6de7ef88e, not saving cache.
```

And this will be done again for each of the lint steps, downloading the same cache, extracting over the existing identical one.

So to conclude:
- We can let golangci-lint-action cache the lint results as it does by default.
- With multiple golangci-lint-action steps in a row, the stored cache contains the results from all of them (all GOOS variants) in the same cache.
- This seem to work fine, although does not seem to be a use case explicitly considered, e.g.
  - When cache can be reused it will unnecessarily be restored over again for each step.
  - When cache could not be used then the linting results is attempted to be stored back for each step, but only the first succeeds.

Maybe there is a trick to force use of individual caches, however as long as this works we save cache storage (which is under pressure: "Approaching total cache storage limit (14.47 GB of 10 GB Used)", and some time (for cache upload at least, but it already repeats the cache restores so no change there).

So I think the remaining question is for which GOOS we should do the linting?

albertony: The current approach, repeating golangci-lint-action steps with different GOOS, is based on:
- https://github.com/golangci/golangci-lint-action/issues/742
ncw: > I just added a commit which I think fixes the issues by some small adjustments to build tags in a couple of tests etc. Seems that fixed it actually, so maybe not caching after all.

:-)

> Unfortunately I also added linting on dragonfly, so that introduced a lot of errors again.. 😆 But not too hard to fix I think, with some more adjustments to build tags.

:-(
 
> But I don't know for which GOOS we do want to run the linting? All we have build tags for in code (linux, windows, darwin, freebsd, netbsd, openbsd, dragonfly, plan9, js, solaris)? All that we build during the PR checks (linux, darwin, windows)? All that we have builds for on download page (linux, windows, darwin, freebsd, netbsd, openbsd, plan9, solaris - but not dragonfly or js)?

Good question. I think `linux`, `darwin`, `windows`, `freebsd` will hit > 99% of the rclone users so any more we are into diminishing returns. Most of the build tags are used to disable features which don't build on a given OS. I think if you added `openbsd` to that list then you'd hit `backend/local/metadata_unix.go` and `lib/diskusage/diskusage_openbsd.go` which are the only files I could find which doesn't build under the first 4.

So if we can afford the runtime for 5 linters `linux`, `darwin`, `windows`, `freebsd` & `openbsd` will hit all the files.

If we only have 4 then I'd choose `linux`, `darwin`, `windows`, `freebsd` being the most popular OSes.

Will this stop on the first lint failure? The reason I ask that is that the lint action also puts comments on the PRs so if the user makes a normal sort of dev mistake then all the linters will pick it up and there will be N comments on the PR. So I think it should probably stop on the first linter to fail.
albertony: Sounds good.

> Will this stop on the first lint failure?

No. I intended it not to. 

> all the linters will pick it up and there will be N comments on the PR.

But that is a good point. Maybe I can push a build error to see, but you're probably right, and then I agree it should abort on first linter.
albertony: Done:
- Aborting lint job after first failed (GOOS-specific) lint step
- Removed the unnecessary GOOS linters, leaving the big 5: linux, darwin, windows, freebsd & openbsd
   - Even when I had all the 9 linters they all finished within the time it took for all the builds, e.g. 11-13m, while mac_amd64 build may be as slow as 16-18m, so with 5 linters I think we can be pretty sure these finish before all the builds.

Unfortunately:

I just noticed that it seems the golangci-lint executions always takes about the same time now, like 1 minute or so, even when it reports it found a cache - in which case it previously executed like 5 seconds. Probably the changing of GOOS invalidates the cache in the sense that it needs to rebuild regardless. Not sure how easy that is to fix, I fear some custom caching hacks is needed, like completely disabling the automatic caching from both setup-go and golangci-lint-action and adding an explicit cache action step for each of the golangci-lint-action steps. The alternative is to run without cache, just disable it if it is true it does not have any real effect, and accept it being a bit slow as long as it is not limiting the entire build. Would be unfortunate, but exactly how much and how acceptable that would be I don't know? I will investigate this cache topic later, like tomorrow.
albertony: I think I've solved the last caching issue of the lint job.

To test I manually deleted relevant caches, and forced a build which did the entire lint job in about 6m (1m35s on each lint step), which includes saving a new cache. Then forced another build, and the entire lint job now took 46s (2s on each lint step). I think the current solution should be solid. Note that there may be some strange effects as long as this PR does the lint job differently than master and other PRs, because they may generate same cache keys and end up reusing each others caches, with unexpected content.

@ncw Let me know what you think of it now.

Patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 5c1136d5d..3a939f19d 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -226,18 +226,50 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Code quality test
-        uses: golangci/golangci-lint-action@v4
-        with:
-          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: latest
-
-      # Run govulncheck on the latest go version, the one we build binaries with
       - name: Install Go
         uses: actions/setup-go@v5
         with:
           go-version: '>=1.22.0-rc.1'
           check-latest: true
+          cache: false
+
+      - name: Code quality test (Linux)
+        uses: golangci/golangci-lint-action@v4
+        with:
+          version: latest
+          skip-cache: false # Caching enabled (which is default) on this first lint step only, it handles complete cache of build, go modules and golangci-lint analysis which was necessary to get all lint steps to properly take advantage of it
+
+      - name: Code quality test (Windows)
+        uses: golangci/golangci-lint-action@v4
+        env:
+          GOOS: "windows"
+        with:
+          version: latest
+          skip-cache: true
+
+      - name: Code quality test (macOS)
+        uses: golangci/golangci-lint-action@v4
+        env:
+          GOOS: "darwin"
+        with:
+          version: latest
+          skip-cache: true
+
+      - name: Code quality test (FreeBSD)
+        uses: golangci/golangci-lint-action@v4
+        env:
+          GOOS: "freebsd"
+        with:
+          version: latest
+          skip-cache: true
+
+      - name: Code quality test (OpenBSD)
+        uses: golangci/golangci-lint-action@v4
+        env:
+          GOOS: "openbsd"
+        with:
+          version: latest
+          skip-cache: true
 
       - name: Install govulncheck
         run: go install golang.org/x/vuln/cmd/govulncheck@latest
diff --git a/backend/azureblob/azureblob_unsupported.go b/backend/azureblob/azureblob_unsupported.go
index b5ccf8643..50fcea34c 100644
--- a/backend/azureblob/azureblob_unsupported.go
+++ b/backend/azureblob/azureblob_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || solaris || js
 
+// Package azureblob provides an interface to the Microsoft Azure blob object storage system
 package azureblob
diff --git a/backend/azurefiles/azurefiles_unsupported.go b/backend/azurefiles/azurefiles_unsupported.go
index f261676fd..d2723e62f 100644
--- a/backend/azurefiles/azurefiles_unsupported.go
+++ b/backend/azurefiles/azurefiles_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || js
 
+// Package azurefiles provides an interface to Microsoft Azure Files
 package azurefiles
diff --git a/backend/cache/cache_unsupported.go b/backend/cache/cache_unsupported.go
index 8ce32ee66..8c94aaf26 100644
--- a/backend/cache/cache_unsupported.go
+++ b/backend/cache/cache_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || js
 
+// Package cache implements a virtual provider to cache existing remotes.
 package cache
diff --git a/backend/hdfs/hdfs_unsupported.go b/backend/hdfs/hdfs_unsupported.go
index 02305691b..a14c7725c 100644
--- a/backend/hdfs/hdfs_unsupported.go
+++ b/backend/hdfs/hdfs_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9
 
+// Package hdfs provides an interface to the HDFS storage system.
 package hdfs
diff --git a/backend/http/http.go b/backend/http/http.go
index 862376d4f..ce03bb9bd 100644
--- a/backend/http/http.go
+++ b/backend/http/http.go
@@ -89,6 +89,10 @@ that directory listings are much quicker, but rclone won't have the times or
 sizes of any files, and some files that don't exist may be in the listing.`,
 			Default:  false,
 			Advanced: true,
+		}, {
+			Name:    "no_escape",
+			Help:    "Do not escape URL metacharacters in path names.",
+			Default: false,
 		}},
 	}
 	fs.Register(fsi)
@@ -100,6 +104,7 @@ type Options struct {
 	NoSlash  bool            `config:"no_slash"`
 	NoHead   bool            `config:"no_head"`
 	Headers  fs.CommaSepList `config:"headers"`
+	NoEscape bool            `config:"no_escape"`
 }
 
 // Fs stores the interface to the remote HTTP files
@@ -326,6 +331,11 @@ func (f *Fs) NewObject(ctx context.Context, remote string) (fs.Object, error) {
 
 // Join's the remote onto the base URL
 func (f *Fs) url(remote string) string {
+	if f.opt.NoEscape {
+		// Directly concatenate without escaping, no_escape behavior
+		return f.endpointURL + remote
+	}
+	// Default behavior
 	return f.endpointURL + rest.URLPathEscape(remote)
 }
 
diff --git a/backend/local/about_unix.go b/backend/local/about_unix.go
index 24134c794..f8dce54e4 100644
--- a/backend/local/about_unix.go
+++ b/backend/local/about_unix.go
@@ -23,9 +23,9 @@ func (f *Fs) About(ctx context.Context) (*fs.Usage, error) {
 	}
 	bs := int64(s.Bsize) // nolint: unconvert
 	usage := &fs.Usage{
-		Total: fs.NewUsageValue(bs * int64(s.Blocks)),         // quota of bytes that can be used
-		Used:  fs.NewUsageValue(bs * int64(s.Blocks-s.Bfree)), // bytes in use
-		Free:  fs.NewUsageValue(bs * int64(s.Bavail)),         // bytes which can be uploaded before reaching the quota
+		Total: fs.NewUsageValue(bs * int64(s.Blocks)),         //nolint: unconvert // quota of bytes that can be used
+		Used:  fs.NewUsageValue(bs * int64(s.Blocks-s.Bfree)), //nolint: unconvert // bytes in use
+		Free:  fs.NewUsageValue(bs * int64(s.Bavail)),         //nolint: unconvert // bytes which can be uploaded before reaching the quota
 	}
 	return usage, nil
 }
diff --git a/backend/local/metadata_other.go b/backend/local/metadata_other.go
index c32c89f26..dc6a5a736 100644
--- a/backend/local/metadata_other.go
+++ b/backend/local/metadata_other.go
@@ -1,4 +1,4 @@
-//go:build plan9 || js
+//go:build dragonfly || plan9 || js
 
 package local
 
diff --git a/backend/local/remove_windows.go b/backend/local/remove_windows.go
index a04f4af66..70d65b63e 100644
--- a/backend/local/remove_windows.go
+++ b/backend/local/remove_windows.go
@@ -4,14 +4,10 @@ package local
 
 import (
 	"os"
-	"syscall"
 	"time"
 
 	"github.com/rclone/rclone/fs"
-)
-
-const (
-	ERROR_SHARING_VIOLATION syscall.Errno = 32
+	"golang.org/x/sys/windows"
 )
 
 // Removes name, retrying on a sharing violation
@@ -27,7 +23,7 @@ func remove(name string) (err error) {
 		if !ok {
 			break
 		}
-		if pathErr.Err != ERROR_SHARING_VIOLATION {
+		if pathErr.Err != windows.ERROR_SHARING_VIOLATION {
 			break
 		}
 		fs.Logf(name, "Remove detected sharing violation - retry %d/%d sleeping %v", i+1, maxTries, sleepTime)
diff --git a/backend/local/xattr_unsupported.go b/backend/local/xattr_unsupported.go
index e63c25492..20c86d07a 100644
--- a/backend/local/xattr_unsupported.go
+++ b/backend/local/xattr_unsupported.go
@@ -1,6 +1,7 @@
+// The pkg/xattr module doesn't compile for openbsd or plan9
+
 //go:build openbsd || plan9
 
-// The pkg/xattr module doesn't compile for openbsd or plan9
 package local
 
 import "github.com/rclone/rclone/fs"
diff --git a/backend/netstorage/netstorage.go b/backend/netstorage/netstorage.go
index d365918ca..108c03596 100755
--- a/backend/netstorage/netstorage.go
+++ b/backend/netstorage/netstorage.go
@@ -443,7 +443,7 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e
 	}
 
 	URL := f.url(dir)
-	files, err := f.netStorageDirRequest(ctx, dir, URL)
+	files, err := f.netStorageDirRequest(ctx, URL)
 	if err != nil {
 		return nil, err
 	}
@@ -932,7 +932,7 @@ func (f *Fs) netStorageStatRequest(ctx context.Context, URL string, directory bo
 }
 
 // netStorageDirRequest performs a NetStorage dir request
-func (f *Fs) netStorageDirRequest(ctx context.Context, dir string, URL string) ([]File, error) {
+func (f *Fs) netStorageDirRequest(ctx context.Context, URL string) ([]File, error) {
 	const actionHeader = "version=1&action=dir&format=xml&encoding=utf-8"
 	statResp := &Stat{}
 	if _, err := f.callBackend(ctx, URL, "GET", actionHeader, false, statResp, nil); err != nil {
diff --git a/backend/oracleobjectstorage/oracleobjectstorage_unsupported.go b/backend/oracleobjectstorage/oracleobjectstorage_unsupported.go
index 07d8fd242..ccbc39316 100644
--- a/backend/oracleobjectstorage/oracleobjectstorage_unsupported.go
+++ b/backend/oracleobjectstorage/oracleobjectstorage_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || solaris || js
 
+// Package oracleobjectstorage provides an interface to the OCI object storage system.
 package oracleobjectstorage
diff --git a/backend/qingstor/qingstor_unsupported.go b/backend/qingstor/qingstor_unsupported.go
index 209d909bc..e16a2e80c 100644
--- a/backend/qingstor/qingstor_unsupported.go
+++ b/backend/qingstor/qingstor_unsupported.go
@@ -3,4 +3,6 @@
 
 //go:build plan9 || js
 
+// Package qingstor provides an interface to QingStor object storage
+// Home: https://www.qingcloud.com/
 package qingstor
diff --git a/backend/sftp/sftp_unsupported.go b/backend/sftp/sftp_unsupported.go
index 8dc950374..67794bc4f 100644
--- a/backend/sftp/sftp_unsupported.go
+++ b/backend/sftp/sftp_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9
 
+// Package sftp provides a filesystem interface using github.com/pkg/sftp
 package sftp
diff --git a/backend/storj/storj_unsupported.go b/backend/storj/storj_unsupported.go
index 67f9ebdfd..c3c731594 100644
--- a/backend/storj/storj_unsupported.go
+++ b/backend/storj/storj_unsupported.go
@@ -1,3 +1,4 @@
 //go:build plan9
 
+// Package storj provides an interface to Storj decentralized object storage.
 package storj
diff --git a/cmd/cachestats/cachestats_unsupported.go b/cmd/cachestats/cachestats_unsupported.go
index 089ba751a..1ab469e91 100644
--- a/cmd/cachestats/cachestats_unsupported.go
+++ b/cmd/cachestats/cachestats_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || js
 
+// Package cachestats provides the cachestats command.
 package cachestats
diff --git a/cmd/ncdu/ncdu_unsupported.go b/cmd/ncdu/ncdu_unsupported.go
index e1f6430c2..34d99ed6c 100644
--- a/cmd/ncdu/ncdu_unsupported.go
+++ b/cmd/ncdu/ncdu_unsupported.go
@@ -3,4 +3,5 @@
 
 //go:build plan9 || js
 
+// Package ncdu implements a text based user interface for exploring a remote
 package ncdu
diff --git a/cmd/serve/ftp/ftp_unsupported.go b/cmd/serve/ftp/ftp_unsupported.go
index 7fa857560..f4c5415db 100644
--- a/cmd/serve/ftp/ftp_unsupported.go
+++ b/cmd/serve/ftp/ftp_unsupported.go
@@ -3,9 +3,10 @@
 
 //go:build plan9
 
+// Package ftp implements an FTP server for rclone
 package ftp
 
 import "github.com/spf13/cobra"
 
 // Command definition is nil to show not implemented
-var Command *cobra.Command = nil
+var Command *cobra.Command
diff --git a/cmd/serve/nfs/nfs_unsupported.go b/cmd/serve/nfs/nfs_unsupported.go
index a438c9748..c9b9f5bbf 100644
--- a/cmd/serve/nfs/nfs_unsupported.go
+++ b/cmd/serve/nfs/nfs_unsupported.go
@@ -8,5 +8,5 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// For unsupported platforms we just put nil
-var Command *cobra.Command = nil
+// Command is just nil for unsupported platforms
+var Command *cobra.Command
diff --git a/cmd/serve/sftp/sftp_unsupported.go b/cmd/serve/sftp/sftp_unsupported.go
index 9f0df6499..22e0673a7 100644
--- a/cmd/serve/sftp/sftp_unsupported.go
+++ b/cmd/serve/sftp/sftp_unsupported.go
@@ -3,9 +3,10 @@
 
 //go:build plan9
 
+// Package sftp implements an SFTP server to serve an rclone VFS
 package sftp
 
 import "github.com/spf13/cobra"
 
 // Command definition is nil to show not implemented
-var Command *cobra.Command = nil
+var Command *cobra.Command
diff --git a/fs/accounting/token_bucket.go b/fs/accounting/token_bucket.go
index 29204b5eb..5e5b597e1 100644
--- a/fs/accounting/token_bucket.go
+++ b/fs/accounting/token_bucket.go
@@ -40,7 +40,7 @@ type tokenBucket struct {
 // Return true if limit is disabled
 //
 // Call with lock held
-func (bs *buckets) _isOff() bool {
+func (bs *buckets) _isOff() bool { //nolint:unused // Don't include unused when running golangci-lint in case its on windows where this is not called
 	return bs[0] == nil
 }
 
diff --git a/fs/fserrors/enospc_error_notsupported.go b/fs/fserrors/enospc_error_notsupported.go
index 30f5a6b3c..f1f1e94de 100644
--- a/fs/fserrors/enospc_error_notsupported.go
+++ b/fs/fserrors/enospc_error_notsupported.go
@@ -2,7 +2,7 @@
 
 package fserrors
 
-// IsErrNoSpace() on plan9 returns false because
+// IsErrNoSpace on plan9 returns false because
 // plan9 does not support syscall.ENOSPC error.
 func IsErrNoSpace(cause error) (isNoSpc bool) {
 	isNoSpc = false
diff --git a/fs/fserrors/retriable_errors_windows.go b/fs/fserrors/retriable_errors_windows.go
index 58751c50f..17a62af5c 100644
--- a/fs/fserrors/retriable_errors_windows.go
+++ b/fs/fserrors/retriable_errors_windows.go
@@ -23,8 +23,8 @@ const (
 	WSAEHOSTUNREACH   syscall.Errno = 10065
 	WSAEDISCON        syscall.Errno = 10101
 	WSAEREFUSED       syscall.Errno = 10112
-	WSAHOST_NOT_FOUND syscall.Errno = 11001
-	WSATRY_AGAIN      syscall.Errno = 11002
+	WSAHOST_NOT_FOUND syscall.Errno = 11001 //nolint:revive // Don't include revive when running golangci-lint to avoid var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)
+	WSATRY_AGAIN      syscall.Errno = 11002 //nolint:revive // Don't include revive when running golangci-lint to avoid var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)
 )
 
 func init() {
diff --git a/fs/log/redirect_stderr_windows.go b/fs/log/redirect_stderr_windows.go
index 7a7e8cfc1..f20ede7b0 100644
--- a/fs/log/redirect_stderr_windows.go
+++ b/fs/log/redirect_stderr_windows.go
@@ -20,7 +20,7 @@ var (
 )
 
 func setStdHandle(stdhandle int32, handle syscall.Handle) error {
-	r0, _, e1 := syscall.Syscall(procSetStdHandle.Addr(), 2, uintptr(stdhandle), uintptr(handle), 0)
+	r0, _, e1 := syscall.SyscallN(procSetStdHandle.Addr(), uintptr(stdhandle), uintptr(handle))
 	if r0 == 0 {
 		if e1 != 0 {
 			return error(e1)
diff --git a/go.mod b/go.mod
index ffbeed00c..3ccdfc0d5 100644
--- a/go.mod
+++ b/go.mod
@@ -52,6 +52,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.0
 	github.com/prometheus/client_golang v1.18.0
 	github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8
+	github.com/rclone/gofakes3 v0.0.3-0.20240413171058-b7a9fdb78ddb
 	github.com/rfjakob/eme v1.1.2
 	github.com/rivo/uniseg v0.4.4
 	github.com/rogpeppe/go-internal v1.11.0
@@ -157,7 +158,6 @@ require (
 	github.com/prometheus/common v0.45.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/rasky/go-xdr v0.0.0-20170124162913-1a41d1a06c93 // indirect
-	github.com/rclone/gofakes3 v0.0.3-0.20240413171058-b7a9fdb78ddb // indirect
 	github.com/relvacode/iso8601 v1.3.0 // indirect
 	github.com/rs/xid v1.5.0 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
diff --git a/go.sum b/go.sum
index 80a4a1f7b..90d29fe8c 100644
--- a/go.sum
+++ b/go.sum
@@ -456,8 +456,6 @@ github.com/quic-go/qtls-go1-20 v0.4.1 h1:D33340mCNDAIKBqXuAvexTNMUByrYmFYVfKfDN5
 github.com/quic-go/quic-go v0.40.1 h1:X3AGzUNFs0jVuO3esAGnTfvdgvL4fq655WaOi1snv1Q=
 github.com/rasky/go-xdr v0.0.0-20170124162913-1a41d1a06c93 h1:UVArwN/wkKjMVhh2EQGC0tEc1+FqiLlvYXY5mQ2f8Wg=
 github.com/rasky/go-xdr v0.0.0-20170124162913-1a41d1a06c93/go.mod h1:Nfe4efndBz4TibWycNE+lqyJZiMX4ycx+QKV8Ta0f/o=
-github.com/rclone/gofakes3 v0.0.3-0.20240413163749-372c3e746e45 h1:qrH/G7j3Ru2E6tuKaNvpA+KD9uQSujTTMyQoonCJ7kU=
-github.com/rclone/gofakes3 v0.0.3-0.20240413163749-372c3e746e45/go.mod h1:L0VIBE0mT6ArN/5dfHsJm3UjqCpi5B/cdN+qWDNh7ko=
 github.com/rclone/gofakes3 v0.0.3-0.20240413171058-b7a9fdb78ddb h1:HJJ7XgRBfXew3EosVk45aGPJRY5wSTSpmAJqz8Kiw0w=
 github.com/rclone/gofakes3 v0.0.3-0.20240413171058-b7a9fdb78ddb/go.mod h1:L0VIBE0mT6ArN/5dfHsJm3UjqCpi5B/cdN+qWDNh7ko=
 github.com/relvacode/iso8601 v1.3.0 h1:HguUjsGpIMh/zsTczGN3DVJFxTU/GX+MMmzcKoMO7ko=
diff --git a/lib/buildinfo/osversion_windows.go b/lib/buildinfo/osversion_windows.go
index bcc903c36..0626d4227 100644
--- a/lib/buildinfo/osversion_windows.go
+++ b/lib/buildinfo/osversion_windows.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/shirou/gopsutil/v3/host"
 	"golang.org/x/sys/windows"
-	"golang.org/x/sys/windows/registry"
 )
 
 // GetOSVersion returns OS version, kernel and bitness
@@ -96,37 +95,3 @@ func getRegistryVersionString(name string) string {
 
 	return windows.UTF16ToString(regBuf[:])
 }
-
-func getRegistryVersionInt(name string) int {
-	var (
-		err     error
-		handle  windows.Handle
-		bufLen  uint32
-		valType uint32
-	)
-
-	err = windows.RegOpenKeyEx(windows.HKEY_LOCAL_MACHINE, regVersionKeyUTF16, 0, windows.KEY_READ|windows.KEY_WOW64_64KEY, &handle)
-	if err != nil {
-		return 0
-	}
-	defer func() {
-		_ = windows.RegCloseKey(handle)
-	}()
-
-	nameUTF16 := windows.StringToUTF16Ptr(name)
-	err = windows.RegQueryValueEx(handle, nameUTF16, nil, &valType, nil, &bufLen)
-	if err != nil {
-		return 0
-	}
-
-	if valType != registry.DWORD || bufLen != 4 {
-		return 0
-	}
-	var val32 uint32
-	err = windows.RegQueryValueEx(handle, nameUTF16, nil, &valType, (*byte)(unsafe.Pointer(&val32)), &bufLen)
-	if err != nil {
-		return 0
-	}
-
-	return int(val32)
-}
diff --git a/lib/file/preallocate_windows.go b/lib/file/preallocate_windows.go
index e1d0c1a01..928276c1b 100644
--- a/lib/file/preallocate_windows.go
+++ b/lib/file/preallocate_windows.go
@@ -55,10 +55,10 @@ func PreAllocate(size int64, out *os.File) error {
 
 	// Query info about the block sizes on the file system
 	_, _, e1 := ntQueryVolumeInformationFile.Call(
-		uintptr(out.Fd()),
+		out.Fd(),
 		uintptr(unsafe.Pointer(&iosb)),
 		uintptr(unsafe.Pointer(&fsSizeInfo)),
-		uintptr(unsafe.Sizeof(fsSizeInfo)),
+		unsafe.Sizeof(fsSizeInfo),
 		uintptr(3), // FileFsSizeInformation
 	)
 	if e1 != nil && e1 != syscall.Errno(0) {
@@ -74,14 +74,14 @@ func PreAllocate(size int64, out *os.File) error {
 
 	// Ask for the allocation
 	_, _, e1 = ntSetInformationFile.Call(
-		uintptr(out.Fd()),
+		out.Fd(),
 		uintptr(unsafe.Pointer(&iosb)),
 		uintptr(unsafe.Pointer(&allocInfo)),
-		uintptr(unsafe.Sizeof(allocInfo)),
+		unsafe.Sizeof(allocInfo),
 		uintptr(19), // FileAllocationInformation
 	)
 	if e1 != nil && e1 != syscall.Errno(0) {
-		if e1 == syscall.Errno(windows.ERROR_DISK_FULL) || e1 == syscall.Errno(windows.ERROR_HANDLE_DISK_FULL) {
+		if e1 == windows.ERROR_DISK_FULL || e1 == windows.ERROR_HANDLE_DISK_FULL {
 			return ErrDiskFull
 		}
 		return fmt.Errorf("preAllocate NtSetInformationFile failed: %w", e1)
@@ -90,10 +90,6 @@ func PreAllocate(size int64, out *os.File) error {
 	return nil
 }
 
-const (
-	FSCTL_SET_SPARSE = 0x000900c4 // Control code to set or clears the FILE_ATTRIBUTE_SPARSE_FILE attribute of a file.
-)
-
 // SetSparseImplemented is a constant indicating whether the
 // implementation of SetSparse actually does anything.
 const SetSparseImplemented = true
@@ -101,7 +97,7 @@ const SetSparseImplemented = true
 // SetSparse makes the file be a sparse file
 func SetSparse(out *os.File) error {
 	var bytesReturned uint32
-	err := syscall.DeviceIoControl(syscall.Handle(out.Fd()), FSCTL_SET_SPARSE, nil, 0, nil, 0, &bytesReturned, nil)
+	err := syscall.DeviceIoControl(syscall.Handle(out.Fd()), windows.FSCTL_SET_SPARSE, nil, 0, nil, 0, &bytesReturned, nil)
 	if err != nil {
 		return fmt.Errorf("DeviceIoControl FSCTL_SET_SPARSE: %w", err)
 	}
diff --git a/lib/kv/unsupported.go b/lib/kv/unsupported.go
index cf2f5d56a..c4bb0deff 100644
--- a/lib/kv/unsupported.go
+++ b/lib/kv/unsupported.go
@@ -1,5 +1,6 @@
 //go:build plan9 || js
 
+// Package kv provides key/value database.
 package kv
 
 import (
diff --git a/lib/terminal/hidden_windows.go b/lib/terminal/hidden_windows.go
index cd0a980b0..62eda1e3d 100644
--- a/lib/terminal/hidden_windows.go
+++ b/lib/terminal/hidden_windows.go
@@ -13,7 +13,7 @@ func HideConsole() {
 	if getConsoleWindow.Find() == nil && showWindow.Find() == nil {
 		hwnd, _, _ := getConsoleWindow.Call()
 		if hwnd != 0 {
-			showWindow.Call(hwnd, 0)
+			_, _, _ = showWindow.Call(hwnd, 0)
 		}
 	}
 }


Test patch:
diff --git a/backend/cache/utils_test.go b/backend/cache/utils_test.go
index c02d86ce9..9f85b0155 100644
--- a/backend/cache/utils_test.go
+++ b/backend/cache/utils_test.go
@@ -1,3 +1,6 @@
+//go:build !plan9 && !js
+// +build !plan9,!js
+
 package cache
 
 import bolt "go.etcd.io/bbolt"
diff --git a/fs/fserrors/error_syscall_test.go b/fs/fserrors/error_syscall_test.go
new file mode 100644
index 000000000..f2523b6fd
--- /dev/null
+++ b/fs/fserrors/error_syscall_test.go
@@ -0,0 +1,74 @@
+//go:build !plan9
+// +build !plan9
+
+package fserrors
+
+import (
+	"fmt"
+	"net"
+	"net/url"
+	"os"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// make a plausible network error with the underlying errno
+func makeNetErr(errno syscall.Errno) error {
+	return &net.OpError{
+		Op:     "write",
+		Net:    "tcp",
+		Source: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 123},
+		Addr:   &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 8080},
+		Err: &os.SyscallError{
+			Syscall: "write",
+			Err:     errno,
+		},
+	}
+}
+
+func TestWithSyscallCause(t *testing.T) {
+	for i, test := range []struct {
+		err           error
+		wantRetriable bool
+		wantErr       error
+	}{
+		{makeNetErr(syscall.EAGAIN), true, syscall.EAGAIN},
+		{makeNetErr(syscall.Errno(123123123)), false, syscall.Errno(123123123)},
+	} {
+		gotRetriable, gotErr := Cause(test.err)
+		what := fmt.Sprintf("test #%d: %v", i, test.err)
+		assert.Equal(t, test.wantErr, gotErr, what)
+		assert.Equal(t, test.wantRetriable, gotRetriable, what)
+	}
+}
+
+func TestWithSyscallShouldRetry(t *testing.T) {
+	for i, test := range []struct {
+		err  error
+		want bool
+	}{
+		{makeNetErr(syscall.EAGAIN), true},
+		{makeNetErr(syscall.Errno(123123123)), false},
+		{
+			wrap(&url.Error{
+				Op:  "post",
+				URL: "http://localhost/",
+				Err: makeNetErr(syscall.EPIPE),
+			}, "potato error"),
+			true,
+		},
+		{
+			wrap(&url.Error{
+				Op:  "post",
+				URL: "http://localhost/",
+				Err: makeNetErr(syscall.Errno(123123123)),
+			}, "listing error"),
+			false,
+		},
+	} {
+		got := ShouldRetry(test.err)
+		assert.Equal(t, test.want, got, fmt.Sprintf("test #%d: %v", i, test.err))
+	}
+}
diff --git a/fs/fserrors/error_test.go b/fs/fserrors/error_test.go
index ec8426660..21bdaf5cb 100644
--- a/fs/fserrors/error_test.go
+++ b/fs/fserrors/error_test.go
@@ -5,10 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"net/url"
-	"os"
-	"syscall"
 	"testing"
 	"time"
 
@@ -41,20 +38,6 @@ func wrap(err error, message string) error {
 
 var errUseOfClosedNetworkConnection = errors.New("use of closed network connection")
 
-// make a plausible network error with the underlying errno
-func makeNetErr(errno syscall.Errno) error {
-	return &net.OpError{
-		Op:     "write",
-		Net:    "tcp",
-		Source: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 123},
-		Addr:   &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 8080},
-		Err: &os.SyscallError{
-			Syscall: "write",
-			Err:     errno,
-		},
-	}
-}
-
 type myError1 struct {
 	Err error
 }
@@ -121,8 +104,6 @@ func TestCause(t *testing.T) {
 		{fmt.Errorf("potato: %w", errPotato), false, errPotato},
 		{fmt.Errorf("potato2: %w", wrap(errPotato, "potato")), false, errPotato},
 		{errUseOfClosedNetworkConnection, false, errUseOfClosedNetworkConnection},
-		{makeNetErr(syscall.EAGAIN), true, syscall.EAGAIN},
-		{makeNetErr(syscall.Errno(123123123)), false, syscall.Errno(123123123)},
 		{eNil1, false, eNil1},
 		{eNil2, false, eNil2.Err},
 		{myError1{io.EOF}, false, io.EOF},
@@ -151,27 +132,9 @@ func TestShouldRetry(t *testing.T) {
 		{fmt.Errorf("connection: %w", errUseOfClosedNetworkConnection), true},
 		{io.EOF, true},
 		{io.ErrUnexpectedEOF, true},
-		{makeNetErr(syscall.EAGAIN), true},
-		{makeNetErr(syscall.Errno(123123123)), false},
 		{&url.Error{Op: "post", URL: "/", Err: io.EOF}, true},
 		{&url.Error{Op: "post", URL: "/", Err: errUseOfClosedNetworkConnection}, true},
 		{&url.Error{Op: "post", URL: "/", Err: fmt.Errorf("net/http: HTTP/1.x transport connection broken: %v", fmt.Errorf("http: ContentLength=%d with Body length %d", 100663336, 99590598))}, true},
-		{
-			wrap(&url.Error{
-				Op:  "post",
-				URL: "http://localhost/",
-				Err: makeNetErr(syscall.EPIPE),
-			}, "potato error"),
-			true,
-		},
-		{
-			wrap(&url.Error{
-				Op:  "post",
-				URL: "http://localhost/",
-				Err: makeNetErr(syscall.Errno(123123123)),
-			}, "listing error"),
-			false,
-		},
 	} {
 		got := ShouldRetry(test.err)
 		assert.Equal(t, test.want, got, fmt.Sprintf("test #%d: %v", i, test.err))
diff --git a/fs/logger/logger_test.go b/fs/logger/logger_test.go
index 0e53563ce..d306cdd13 100644
--- a/fs/logger/logger_test.go
+++ b/fs/logger/logger_test.go
@@ -1,3 +1,6 @@
+//go:build !plan9
+// +build !plan9
+
 package logger_test
 
 import (
diff --git a/lib/file/mkdir_windows_test.go b/lib/file/mkdir_windows_test.go
index 5e6fdc60e..75e050bf7 100644
--- a/lib/file/mkdir_windows_test.go
+++ b/lib/file/mkdir_windows_test.go
@@ -34,7 +34,11 @@ func TestMkdirAll(t *testing.T) {
 	if err != nil {
 		t.Fatalf("create %q: %s", fpath, err)
 	}
-	defer f.Close()
+	defer func() {
+		if err := f.Close(); err != nil {
+			t.Fatalf("Close %q: %s", fpath, err)
+		}
+	}()
 
 	// Can't make directory named after file.
 	err = MkdirAll(fpath, 0777)
diff --git a/vfs/vfstest/write_other.go b/vfs/vfstest/write_other.go
new file mode 100644
index 000000000..f95c3efa1
--- /dev/null
+++ b/vfs/vfstest/write_other.go
@@ -0,0 +1,20 @@
+//go:build !linux && !darwin && !freebsd && !windows
+// +build !linux,!darwin,!freebsd,!windows
+
+package vfstest
+
+import (
+	"errors"
+	"runtime"
+	"testing"
+)
+
+// TestWriteFileDoubleClose tests double close on write
+func TestWriteFileDoubleClose(t *testing.T) {
+	t.Skip("not supported on " + runtime.GOOS)
+}
+
+// writeTestDup performs the platform-specific implementation of the dup() unix
+func writeTestDup(oldfd uintptr) (uintptr, error) {
+	return 0, errors.New("not supported on " + runtime.GOOS)
+}
diff --git a/vfs/vfstest/write_non_unix.go b/vfs/vfstest/write_windows.go
similarity index 92%
rename from vfs/vfstest/write_non_unix.go
rename to vfs/vfstest/write_windows.go
index 161354434..227e302a3 100644
--- a/vfs/vfstest/write_non_unix.go
+++ b/vfs/vfstest/write_windows.go
@@ -1,4 +1,4 @@
-//go:build !linux && !darwin && !freebsd
+//go:build windows
 
 package vfstest