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 Go (1.21.3), golangci-lint (1.55.1), fix go mod tidy #557

Merged
merged 7 commits into from
Nov 3, 2023

Conversation

mitjat
Copy link
Collaborator

@mitjat mitjat commented Nov 2, 2023

This PR:

  • Bumps the Go version used to build Nexus
  • Bumps the version of the linter, and updates code and linting rules to make the new linter happy
  • Updates CI so it can detect go mod tidy issues. Those were previously only checked-for in the release process, thus allowing us to check in code that would later fail to release.

The backstory is a little convoluted:

  1. Release v0.1.18 failed to produce a docker image because goreleaser first runs go mod tidy as a hook, which failed with
github.com/oasisprotocol/nexus/analyzer/uncategorized imports
        github.com/ethereum/go-ethereum/crypto imports
        github.com/btcsuite/btcd/btcec/v2/ecdsa tested by
        github.com/btcsuite/btcd/btcec/v2/ecdsa.test imports                                                       
        github.com/btcsuite/btcd/chaincfg/chainhash: ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
        github.com/btcsuite/btcd v0.20.1-beta (/home/mitjat/go/pkg/mod/github.com/btcsuite/btcd@v0.20.1-beta/chaincfg/chainhash)
        github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 (/home/mitjat/go/pkg/mod/github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.1)
  1. A sort-of fix was to add an explicit dependency in an attempt to enforce a specific version: go get github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.1. After this, nexus builds and go mod tidy is happy, but the latter also removes the chainhash entry from go.mod, and a second invocation of go mod tidy fails. This smells buggy.

  2. It turns out that bumping the Go version resolves the buggy go mod tidy behavior. Go v1.21 is also used by oasis-core and @ptrus recently suggested we use it in nexus too, so it's a good idea to upgrade anyway. However, this also requires us to bump the linter version (the old linter does not support Go v1.21), which in turn requires us to resolve some new linter warnings. This PR does that.

Developers: After this goes in, you might need to update your local env accordingly:

# Download go v1.21
go install golang.org/dl/go1.21.3 && go1.21.3 download

# Make it the default
sudo ln -s -f $GOPATH/bin/go1.21.3 /usr/bin/go

# Make it the "Oasis default" (used by some Makefile targets)
(edit ~/.bashrc or similar, update OASIS_GO variable)

# Install new golangci-lint
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.1

@Andrew7234 Andrew7234 self-requested a review November 3, 2023 01:15
Copy link
Collaborator

@Andrew7234 Andrew7234 left a comment

Choose a reason for hiding this comment

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

oh ok I see the slack messages/CI now

@mitjat mitjat force-pushed the mitjat/fix-imports--chainhash branch 3 times, most recently from dc42240 to 8155c6c Compare November 3, 2023 05:25
@mitjat mitjat force-pushed the mitjat/fix-imports--chainhash branch from 8155c6c to 7fddb1c Compare November 3, 2023 05:35
@mitjat mitjat changed the title Manually specify version of dependency github.com/btcsuite/btcd/chaincfg/chainhash Bump Go (1.21.3), golangci-lint (1.55.1) Nov 3, 2023
@mitjat mitjat force-pushed the mitjat/fix-imports--chainhash branch from 7fddb1c to dc6178b Compare November 3, 2023 05:59
@mitjat
Copy link
Collaborator Author

mitjat commented Nov 3, 2023

Additional data point: @peternose points out that oasis-core had the same issue in oasisprotocol/oasis-core#5268, and also resolved it by switching to a newer Go.

@mitjat mitjat merged commit fe976c3 into main Nov 3, 2023
6 checks passed
@mitjat mitjat deleted the mitjat/fix-imports--chainhash branch November 3, 2023 17:49
@mitjat mitjat changed the title Bump Go (1.21.3), golangci-lint (1.55.1) Bump Go (1.21.3), golangci-lint (1.55.1), fix go mod tidy Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants