Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump CI to use supported go version, fix a single Go 1.20 issue in mount pkg; rm uneeded errorlint annotations #129

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Aug 25, 2023

  1. Bump Go to supported version, other CI components.
  2. Bump go to 1.17 in */go.mod.
  3. Bump golang.org/x/sys to v0.11.0. v.0.1.0.
  4. mount: fix an errorlint issue with Go 1.20+.
  5. mount: rm unneeded errorlint annotations (thanks to Allow bare errors from golang.org/x/sys/unix polyfloyd/go-errorlint#47).
  6. sequential: fix a formatting issue (file is not gofumpt'ed)

@kolyshkin kolyshkin changed the title Bump CI to use supported go version, fix a single Go 1.20 issue in mount pkg Bump CI to use supported go version, fix a single Go 1.20 issue in mount pkg; rm uneeded errorlint annotations Aug 25, 2023
.github/workflows/test.yml Outdated Show resolved Hide resolved
mount/mount_unix.go Outdated Show resolved Hide resolved
@kolyshkin
Copy link
Collaborator Author

@thaJeztah ptal

mount/go.mod Outdated
@@ -4,5 +4,5 @@ go 1.17

require (
github.com/moby/sys/mountinfo v0.6.2
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a
golang.org/x/sys v0.11.0
Copy link
Member

Choose a reason for hiding this comment

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

if we don't need anything newer, I prefer follow Go module's "minimum version selection", and use the oldest tagged version (v0.1.0), and leave it up to users of this module to pick the version they want

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, PTAL @thaJeztah

@thaJeztah
Copy link
Member

@kolyshkin are you ok with picking a "minimum" version for golang.org/x/sys, or did you have a specific reason to require the latest version?

@kolyshkin
Copy link
Collaborator Author

@kolyshkin are you ok with picking a "minimum" version for golang.org/x/sys, or did you have a specific reason to require the latest version?

Yes, I don't think we have a requirement for a specific version here. Will update.

Since Go 1.20,

> [t]he fmt.Errorf function now supports multiple occurrences of the %w
> format verb, which will cause it to return an error that wraps all of
> those error operands.

This change, together with newer errorlint, causes the following
warning:

> mount_unix.go:74:56: non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
> 					return fmt.Errorf("%w (possible cause: %s)", err, suberr)
>					                                                  ^

As we still want to support Go 1.17, use explicit string for suberr,
and add a TODO to switch to %w once we switch to Go 1.20+.

Fixes: 487129d
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Test with supported Go releases (1.20.x and 1.21.x). Also leave Go
   1.17.x since it costs us little to keep supporting it.

2. Bump golangci-lint to the latest released version (v1.55.1).

3. Bump actions/setup-go to v4.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It is already that way for sequential, and Go 1.17 is the oldest version
we use in CI, so let's bring other modules go.mod in sync.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is the oldest tagged version of x/sys. This allows to follow Go
module's "minimum version selection", leaving it up to users of this
module to pick the version they want.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Remove the annotation that is no longer needed.

Unfortunately, switch on a bare unix error (in mountinfo) still needs to
be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#54

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following CI error:

> sequential_windows.go:114: File is not `gofumpt`-ed (gofumpt)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Collaborator Author

@thaJeztah ptal

1 similar comment
@kolyshkin
Copy link
Collaborator Author

@thaJeztah ptal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants