Skip to content

Conversation

@guillaumemichel
Copy link
Contributor

@guillaumemichel guillaumemichel commented May 21, 2025

Context

Depends on

Notes

This PR is only removing direct dependencies. Some indirect dependency on github.com/benbjohnson/clock are remaining, but they are trickier to remove, since some are circular dependencies (e.g go-libp2p-pubsub depends on go-libp2p/net/p2p/connmgr here).

I suggest we migrate the dependency iteratively, first remove direct dependencies from go-libp2p, after the release migrate libraries to use the latest go-libp2p and filecoin-project/go-clock, and finally update go-libp2p to use the latest libs depending on its latest version.

@guillaumemichel guillaumemichel merged commit 97f29c5 into master May 22, 2025
11 checks passed
@guillaumemichel guillaumemichel deleted the go-clock-migration branch May 22, 2025 08:23
@MarcoPolo
Copy link
Collaborator

I previously thought this only affected our tests. Now I realize this affects libp2p users using the connmgr. This means downstream users now must add filecoin-project/go-clock as a dependency. I don't think this is good.

I suggest we:

  1. Revert this change.
  2. Make a new package in core/clock that has the Clock interface we are already using elsewhere that fixes some subtle footguns in the go-clock library .
  3. Update our usages of go-clock to use that new package and consolidate our existing usages.

cc @guillaumemichel

@guillaumemichel
Copy link
Contributor Author

downstream users now must add filecoin-project/go-clock as a dependency. I don't think this is good.

Why? They already depended on github.com/benbjohnson/clock, so this doesn't add a new dependency.

Update our usages of go-clock to use that new package and consolidate our existing usages.

I am not familiar with the go-libp2p Clock interface, but you say it has feature parity for our use cases with github.com/benbjohnson/clock?

@MarcoPolo
Copy link
Collaborator

Why? They already depended on github.com/benbjohnson/clock, so this doesn't add a new dependency.

Even if it was simply a 1:1 copy, it changes the imports. It's effectively a new dependency. There also may be users that don't want to import from the filecoin-project org.

I am not familiar with the go-libp2p Clock interface, but you say it has feature parity for our use cases with github.com/benbjohnson/clock?

It should, or, if not, should be very easy to add missing methods.

@MarcoPolo
Copy link
Collaborator

Note that I think this is a blocker for the next release.

@MarcoPolo MarcoPolo mentioned this pull request Feb 22, 2025
19 tasks
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.

move away from benjohnson/clock

3 participants