chore(deps): bump go deps #6135
Conversation
WalkthroughBumps Go toolchain and Docker builder to 1.25.x, updates numerous Go dependencies across f3-sidecar and interop-tests, and centralizes f3-sidecar logging initialization into shared utilities (setLogLevels/checkError), removing repetitive per-component SetLogLevel calls. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor OS
participant Main as main.go
participant FFI as ffi_impl.go (init)
participant Utils as utils.go
OS->>Main: process start
Main->>Utils: checkError(setLogLevels())
Utils-->>Main: ok / err
OS->>FFI: package init
FFI->>Utils: checkError(setLogLevels())
Utils-->>FFI: ok / err
FFI->>FFI: initialize GoF3NodeImpl
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
🔇 Additional comments (5)
Comment |
0079287 to
b215b50
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
f3-sidecar/utils.go (2)
12-13: Prefer standard library error wrapping over xerrors.The
golang.org/x/xerrorspackage is experimental and primarily intended for Go versions prior to 1.13 to provide error wrapping semantics. Since this project uses Go 1.25.2, you should use the standard library'sfmt.Errorfwith%wfor error wrapping instead.Based on learnings.
Apply this diff to use standard library error handling:
import ( "context" + "fmt" "os" "path/filepath" "time" "github.com/filecoin-project/go-f3/gpbft" "github.com/ipfs/go-cid" leveldb "github.com/ipfs/go-ds-leveldb" logging "github.com/ipfs/go-log/v2" - "golang.org/x/xerrors" )And update the error wrapping in
setLogLevel:func setLogLevel(name string, level string) error { if err := logging.SetLogLevel(name, level); err != nil { - return xerrors.Errorf("%s %w", name, err) + return fmt.Errorf("failed to set log level for %s: %w", name, err) } return nil }
51-56: Improve error message clarity.The error message format
"%s %w"simply concatenates the component name with the error, which may not be clear to users. Consider providing more context about what operation failed.This improvement is included in the previous refactor suggestion.
f3-sidecar/go.mod (1)
19-19: Reconsider xerrors dependency addition.The
golang.org/x/xerrorspackage has been added to support error wrapping inf3-sidecar/utils.go. However, as mentioned in the review of that file, this experimental package is unnecessary on Go 1.25 since the standard library provides equivalent functionality viafmt.Errorfwith%w.If you adopt the standard library error wrapping suggested for
f3-sidecar/utils.go, you can remove this dependency:github.com/libp2p/go-libp2p-pubsub v0.15.0 github.com/stretchr/testify v1.11.1 golang.org/x/crypto v0.42.0 - golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
f3-sidecar/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workinterop-tests/src/tests/go_app/go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
Dockerfile(1 hunks)f3-sidecar/ffi_impl.go(1 hunks)f3-sidecar/go.mod(6 hunks)f3-sidecar/main.go(1 hunks)f3-sidecar/utils.go(2 hunks)interop-tests/src/tests/go_app/go.mod(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: All lint checks
🔇 Additional comments (7)
Dockerfile (1)
10-10: LGTM! Base image aligned with toolchain upgrade.The base image update to golang:1.25-bookworm is consistent with the Go toolchain bump to 1.25.2 in the go.mod files.
f3-sidecar/utils.go (2)
58-71: LGTM! Centralized logging configuration.The
setLogLevelsfunction provides a clean, centralized approach to configuring component log levels. The explicit muting ofdht/RtRefreshManagerto prevent terminal issues is well-documented.
81-84: Retain CGO safety overrides Disablinginvalidptr=0,cgocheck=0remains necessary to avoid “invalid pointer found on stack” panics when using rust2go-generated CGO/unsafe.Pointer calls.f3-sidecar/ffi_impl.go (1)
8-12: LGTM! Clean refactoring to centralized utilities.The
initfunction now properly delegates logging configuration to the centralizedsetLogLevels()utility with consistent error handling viacheckError(). This eliminates code duplication and improves maintainability.f3-sidecar/main.go (1)
13-13: LGTM! Consistent with centralized logging approach.The initialization now uses the centralized
setLogLevels()utility, maintaining consistency withf3-sidecar/ffi_impl.goand eliminating duplicate logging configuration code.f3-sidecar/go.mod (1)
3-20: Fix govulncheck errors and confirm dependency updates
go mod verifypassed; core deps remain at intended versions with no newer patches available.- Breaking changes reviewed:
•go-state-typesbump only updatesx/sync.
•go-datastorerequires Go 1.24 (project uses 1.25).
•go-libp2pcontains non-breaking minor enhancements.govulncheckaborted (ffi_impl.go: undefined: GoF3NodeImpl); fix the build error or exclude FFI code and rerun to ensure no known vulnerabilities.interop-tests/src/tests/go_app/go.mod (1)
3-14: Fix govulncheck errors and re-run vulnerability scangovulncheck fails at bitswap_impl.go:24:2 (“undefined: GoBitswapNodeImpl”) and kad_impl.go:19:2 (“undefined: GoKadNodeImpl”); resolve these build errors or use modules-only mode before re-running govulncheck to ensure no critical vulnerabilities remain.
b215b50 to
c928d4f
Compare
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Chores
Refactor
Tests