-
Notifications
You must be signed in to change notification settings - Fork 182
chore: upgrade go-f3 to v0.8.8
#5825
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
Conversation
WalkthroughDependency versions were updated in the Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
f3-sidecar/go.mod (1)
133-133:golang.org/x/expbumped forward – rungo vet ./...before mergeUnlike the other downgrades,
x/expmoved forward (dcc06ee1d476), and the package is still experimental. Ensure no new breaking API is pulled in by dependent packages.
Runninggo vetor the repo’s CI linter should catch any surprises.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
f3-sidecar/go.sumis excluded by!**/*.suminterop-tests/src/tests/go_app/go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
f3-sidecar/go.mod(7 hunks)interop-tests/src/tests/go_app/go.mod(6 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). (8)
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: rubocop
- GitHub Check: All lint checks
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: Build MacOS
🔇 Additional comments (4)
interop-tests/src/tests/go_app/go.mod (2)
7-7: ipfs/boxo upgraded – double-check cross-module alignment
ipfs/boxois now pinned tov0.33.0here, whilef3-sidecar/go.modwas pulled back tov0.30.0. Having two major-compatible but diverging minor versions in the same workspace can lead to two copies of the package landing in the final build, bloating binaries and risking subtle ABI mismatches (e.g.init()side effects).
Confirm that both modules are built in isolation, or converge to one version (preferred: the newerv0.33.0) with:go work sync # if you use Go workspaces go mod tidy -e go list -m all | grep boxo
28-114: Multiple indirect downgrades – verify you’re not discarding security patchesThe following libraries were rolled back:
github.com/go-logr/logr→v1.4.2github.com/klauspost/cpuid/v2→v2.2.10github.com/libp2p/go-flow-metrics→v0.2.0github.com/pion/*packages (logging,rtp,sdp)github.com/prometheus/common→v0.64.0go.opentelemetry.io/otel{,/metric,/trace}→v1.35.0Downgrades often surface because of transitive constraints introduced by another dependency bump (here:
go-f3/boxo). Before merging, run:go list -m -json all | jq -r 'select(.Replace==null) | "\(.Path) \(.Version)"' > versions.txt…and compare against the previous lockfile to ensure no CVE-fix versions are lost.
If the resolver chose these older tags only to satisfy an outdated indirect constraint, consider using
go get -u=patch ./... && go mod tidyto pick the newest patch releases within the chosen minor lines.f3-sidecar/go.mod (2)
6-6: go-f3 bumped to v0.8.8 – LGTMThe main objective of the PR is fulfilled; no API-breaking changes are expected between 0.8.7 → 0.8.8. Good catch keeping the sidecar on the latest patch.
124-126: OpenTelemetry rolled back to v1.35.0 – confirm exporter/runtime parityDowngrading OTEL core/metric/trace from
v1.37.x→v1.35.0may break compatibility with:
- instrumented binaries already deployed with 1.37 resource schema
- collectors running >= 1.36 that expect newer proto definitions
Make sure all components (collectors, sidecars, in-process exporters) share the same minor line, or explicitly vendor the older proto files.
| github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect | ||
| github.com/huin/goupnp v1.3.0 // indirect | ||
| github.com/ipfs/boxo v0.32.0 // indirect | ||
| github.com/ipfs/boxo v0.30.0 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ipfs/boxo pinned to v0.30.0 – conflicts with test module
This sidecar module now depends on an older boxo than the interop test module (v0.33.0). Resolving two different minor versions across a workspace will lead to duplicated symbols.
Recommend aligning to the newer tag unless go-f3 requires the older one. If the older tag is mandatory, add an explicit replace directive in the test module to avoid ambiguity.
🤖 Prompt for AI Agents
In f3-sidecar/go.mod at line 47, the dependency on github.com/ipfs/boxo is
pinned to v0.30.0, which conflicts with the interop test module using v0.33.0.
To fix this, update the version of github.com/ipfs/boxo in this go.mod file to
v0.33.0 to align with the test module. If the older version is required by
go-f3, instead add a replace directive in the test module's go.mod to explicitly
resolve the version conflict and avoid duplicated symbols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hanabi1224 is it actionable or not useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hanabi1224 is it actionable or not useful?
@LesnyRumcajs I've run into some dependency hell of go-libp2p, upgrading everything does not compile.
| github.com/libp2p/go-libp2p v0.42.0 | ||
| github.com/libp2p/go-libp2p-kad-dht v0.33.1 | ||
| github.com/libp2p/go-libp2p-pubsub v0.14.1 | ||
| github.com/libp2p/go-libp2p-pubsub v0.14.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
PubSub minor upgrade – ensure libp2p core stays compatible
go-libp2p-pubsub is now v0.14.2 while go-libp2p core is held at v0.42.0. The pubsub changelog notes optional feature-flags that rely on go-libp2p >= v0.43.
If you hit build-time symbol mismatches, bump core as well:
- github.com/libp2p/go-libp2p v0.42.0
+ github.com/libp2p/go-libp2p v0.43.0Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In f3-sidecar/go.mod at line 15, the go-libp2p-pubsub module was upgraded to
v0.14.2, which may require go-libp2p core to be at least v0.43 due to new
feature flags. To fix potential build-time symbol mismatches, update the
go-libp2p core dependency version to v0.43 or higher in the go.mod file to
maintain compatibility with the pubsub module.
Summary of changes
Changes introduced in this pull request:
go-f3to v0.8.8Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit