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

go: lower Go requirement to Go 1.18 #37

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 51 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,52 @@ on:
- cron: "30 10 * * 0"

jobs:
test-windows:
test-build:
strategy:
fail-fast: false
matrix:
os:
- windows-latest
- ubuntu-latest
- macos-latest
go-version:
- "1.18"
- "1.19"
- "1.20"
- "1.21"
- "1.22"
- "1.23"
- "^1"
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
- name: go build check
run: go build ./...
- name: go test build check
run: go test -run none ./...

test-windows:
strategy:
fail-fast: false
matrix:
go-version:
- "1.18"
- "1.20"
- "1.21"
- "oldstable"
- "stable"
Comment on lines +49 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

This is somewhat problematic, here's why.

A cache that actions/setup-go creates adds a Go version to the source data for the cache key (or maybe to the cache key itself -- I don't remember exactly). Now, if you change the go version manually (say from 1.22 to 1.23), the existing cache is instantly invalidated. But a string like "stable" or "oldstable" never changes, so when a new minor Go version is released, your CI jobs can still be stuck using an old one from the cache.

The same problem exists when a new Go patch version is released, and it is mitigated by setting the check-latest: true input for action/setup-go. I believe it also helps when you use stable/oldstable, and in this case it becomes a must-have, if you really want to test Go 1.24.0 once it comes out, not when a cache expires.

I might not be entirely correct here but at the very least it makes sense to keep an eye on your CI runs.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah, damn. I assumed this was more like a thin alias that wouldn't cause those kinds of issues. I'll add check-latest: true, hopefully that mitigates the risk.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It seems that the aliases are supposed to imply check-latest: true according to the docs:

Note: using these aliases will result in same version as using corresponding minor release with check-latest input set to true

But I'll add it anyway.

runs-on: windows-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
- name: mkdir gocoverdir
# We can only use -test.gocoverdir for Go >= 1.20.
if: ${{ matrix.go-version != '1.18' && matrix.go-version != '1.19' }}
run: |
# mktemp --tmpdir -d gocoverdir.XXXXXXXX
function New-TemporaryDirectory {
Expand All @@ -42,8 +73,15 @@ jobs:
$GOCOVERDIR = (New-TemporaryDirectory -Prefix "gocoverdir")
echo "GOCOVERDIR=$GOCOVERDIR" >>"$env:GITHUB_ENV"
- name: unit tests
run: go test -v -cover '-test.gocoverdir' "$env:GOCOVERDIR" ./...
run: |
if (Test-Path 'env:GOCOVERDIR') {
go test -v -cover '-test.gocoverdir' "$env:GOCOVERDIR" ./...
} else {
go test -v ./...
}
- name: upload coverage
# We can only use -test.gocoverdir for Go >= 1.20.
if: ${{ matrix.go-version != '1.18' && matrix.go-version != '1.19' }}
uses: actions/upload-artifact@v4
with:
name: coverage-${{ runner.os }}-${{ github.job }}-${{ strategy.job-index }}
Expand All @@ -57,24 +95,30 @@ jobs:
- ubuntu-latest
- macos-latest
go-version:
- "1.18"
- "1.20"
- "1.21"
- "1.22"
- "^1"
- "oldstable"
- "stable"
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
- name: mkdir gocoverdir
# We can only use -test.gocoverdir for Go >= 1.20.
if: ${{ matrix.go-version != '1.18' && matrix.go-version != '1.19' }}
run: |
GOCOVERDIR="$(mktemp --tmpdir -d gocoverdir.XXXXXXXX)"
echo "GOCOVERDIR=$GOCOVERDIR" >>"$GITHUB_ENV"
- name: go test
run: go test -v -cover -timeout=30m -test.gocoverdir="$GOCOVERDIR" ./...
run: go test -v -timeout=30m ${GOCOVERDIR:+-cover -test.gocoverdir="$GOCOVERDIR"} ./...
- name: sudo go test
run: sudo go test -v -cover -timeout=30m -test.gocoverdir="$GOCOVERDIR" ./...
run: sudo go test -v -timeout=30m ${GOCOVERDIR:+-cover -test.gocoverdir="$GOCOVERDIR"} ./...
- name: upload coverage
# We can only use -test.gocoverdir for Go >= 1.20.
if: ${{ matrix.go-version != '1.18' && matrix.go-version != '1.19' }}
uses: actions/upload-artifact@v4
with:
name: coverage-${{ runner.os }}-${{ github.job }}-${{ strategy.job-index }}
Expand All @@ -89,7 +133,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: "^1"
go-version: "stable"
- name: download all coverage
uses: actions/download-artifact@v4
with:
Expand Down
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,27 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased] ##

### Compatibility ###
- The minimum Go version requirement for `filepath-securejoin` is now Go 1.18
(we use generics internally).

For reference, `[email protected]` somewhat-arbitrarily bumped the
Go version requirement to 1.21.

While we did make some use of Go 1.21 stdlib features (and in principle Go
versions <= 1.21 are no longer even supported by upstream anymore), some
downstreams have complained that the version bump has meant that they have to
do workarounds when backporting fixes that use the new `filepath-securejoin`
API onto old branches. This is not an ideal situation, but since using this
library is probably better for most downstreams than a hand-rolled
workaround, we now have compatibility shims that allow us to build on older
Go versions.
- Lower minimum version requirement for `golang.org/x/sys` to `v0.18.0` (we
need the wrappers for `fsconfig(2)`), which should also make backporting
patches to older branches easier.

## [0.3.5] - 2024-12-06 ##

### Fixed ###
- `MkdirAll` will now no longer return an `EEXIST` error if two racing
processes are creating the same directory. We will still verify that the path
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
module github.com/cyphar/filepath-securejoin

go 1.21
go 1.18

require (
github.com/stretchr/testify v1.9.0
golang.org/x/sys v0.28.0
golang.org/x/sys v0.18.0
)

require (
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
Expand Down
18 changes: 18 additions & 0 deletions gocompat_errors_go120.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//go:build linux && go1.20

// Copyright (C) 2024 SUSE LLC. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package securejoin

import (
"fmt"
)

// wrapBaseError is a helper that is equivalent to fmt.Errorf("%w: %w"), except
// that on pre-1.20 Go versions only errors.Is() works properly (errors.Unwrap)
// is only guaranteed to give you baseErr.
func wrapBaseError(baseErr, extraErr error) error {
return fmt.Errorf("%w: %w", extraErr, baseErr)
}
26 changes: 26 additions & 0 deletions gocompat_errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//go:build linux

// Copyright (C) 2024 SUSE LLC. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package securejoin

import (
"errors"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGoCompatErrorWrap(t *testing.T) {
baseErr := errors.New("base error")
extraErr := errors.New("extra error")

err := wrapBaseError(baseErr, extraErr)

require.Error(t, err)
assert.ErrorIs(t, err, baseErr, "wrapped error should contain base error")
assert.ErrorIs(t, err, extraErr, "wrapped error should contain extra error")
}
38 changes: 38 additions & 0 deletions gocompat_errors_unsupported.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//go:build linux && !go1.20

// Copyright (C) 2024 SUSE LLC. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package securejoin

import (
"fmt"
)

type wrappedError struct {
inner error
isError error
}

func (err wrappedError) Is(target error) bool {
return err.isError == target
}

func (err wrappedError) Unwrap() error {
return err.inner
}

func (err wrappedError) Error() string {
return fmt.Sprintf("%v: %v", err.isError, err.inner)
}

// wrapBaseError is a helper that is equivalent to fmt.Errorf("%w: %w"), except
// that on pre-1.20 Go versions only errors.Is() works properly (errors.Unwrap)
// is only guaranteed to give you baseErr.
func wrapBaseError(baseErr, extraErr error) error {
return wrappedError{
inner: baseErr,
isError: extraErr,
}
}
32 changes: 32 additions & 0 deletions gocompat_generics_go121.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//go:build linux && go1.21

// Copyright (C) 2024 SUSE LLC. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package securejoin

import (
"slices"
"sync"
)

func slices_DeleteFunc[S ~[]E, E any](slice S, delFn func(E) bool) S {
return slices.DeleteFunc(slice, delFn)
}

func slices_Contains[S ~[]E, E comparable](slice S, val E) bool {
return slices.Contains(slice, val)
}

func slices_Clone[S ~[]E, E any](slice S) S {
return slices.Clone(slice)
}

func sync_OnceValue[T any](f func() T) func() T {
return sync.OnceValue(f)
}

func sync_OnceValues[T1, T2 any](f func() (T1, T2)) func() (T1, T2) {
return sync.OnceValues(f)
}
Loading
Loading