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.

Sign up for GitHub

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

Jump to bottom

muxer: remove support for mplex #2498

Merged
merged 1 commit into from
Aug 18, 2023
Merged

muxer: remove support for mplex #2498

merged 1 commit into from
Aug 18, 2023

Conversation

marten-seemann
Copy link
Contributor

For libp2p/specs#553.

Kubo recently removed mplex support: ipfs/kubo#9958.

sukunrt
sukunrt approved these changes Aug 17, 2023
View reviewed changes
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

🥳

@nisdas
Copy link

nisdas commented Aug 17, 2023

For any other downstream users who need mplex, how would they go about using go-libp2p with it ? From what I understand we will be unable to update Prysm to the next go-libp2p release which contains this PR as we are still required to support mplex.

@sukunrt
Copy link
Member

sukunrt commented Aug 17, 2023 •
edited
Loading

@nisdas I'm not aware of past conversations regarding this, so apologies if this is incorrect.
The removed package(github.com/libp2p/go-libp2p/p2p/muxer/mplex) looks self contained to me. Can you maintain a fork of that specific package?

@nisdas
Copy link

nisdas commented Aug 17, 2023

@sukunrt That would require me to maintain a separate fork of go-libp2p. I would like to avoid maintaining a fork of go-libp2p if possible here

@sukunrt
Copy link
Member

sukunrt commented Aug 17, 2023

I haven't looked at this closely, but this should be possible:

  1. Copy the files in package github.com/libp2p/go-libp2p/p2p/muxer/mplex to a new repo.
  2. Rename the package to the repo path.
  3. Pass in the new package mplex muxer to libp2p option func Muxer(name string, muxer network.Multiplexer) Option

So you should be able to only fork the package and not entire go-libp2p.

@marten-seemann
Copy link
Contributor Author

@sukunrt is right. We built libp2p in such a way that it’s modular: you can bring your own muxer implementation without the need to upstream it.

If that doesn’t work, we did something wrong.

@nisdas
Copy link

nisdas commented Aug 17, 2023

Thank you @sukunrt @marten-seemann , I will take a look at including mplex separately then in a separate repo/package

@marten-seemann marten-seemann merged commit 1a038ef into master Aug 18, 2023
18 checks passed
@marten-seemann marten-seemann mentioned this pull request Aug 21, 2023
Closed
3 tasks
@Jorropo Jorropo mentioned this pull request Aug 21, 2023
Closed
3 tasks
@sukunrt
Copy link
Member

sukunrt commented Aug 30, 2023

@nisdas
we've moved this package to https://github.com/libp2p/go-libp2p-mplex/
you can just change your import path for the muxer.

@nisdas
Copy link

nisdas commented Sep 4, 2023

Perfect, thanks a lot ! @sukunrt

jyellick added a commit to bobanetwork/op-erigon that referenced this pull request Sep 25, 2023
@jyellick
Upgrade libp2p (enables go 1.21 support)
754cdd8 
This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead.  Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.
jyellick added a commit to bobanetwork/op-erigon that referenced this pull request Sep 25, 2023
@jyellick
Upgrade libp2p (enables go 1.21 support)
838fc02 
This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead.  Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.
@jyellick jyellick mentioned this pull request Sep 25, 2023
Merged
jyellick added a commit to bobanetwork/op-erigon that referenced this pull request Sep 25, 2023
@jyellick
Upgrade libp2p (enables go 1.21 support)
b536156 
This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead.  Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.
Giulio2002 pushed a commit to erigontech/erigon that referenced this pull request Sep 29, 2023
@jyellick
Upgrade libp2p (enables go 1.21 support) (#8288)
5654ba0 
Closes #8078 

This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead. Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.

Co-authored-by: Jason Yellick <jason@enya.ai>
lemenkov pushed a commit to lemenkov/erigon that referenced this pull request Oct 2, 2023
@jyellick @lemenkov
Upgrade libp2p (enables go 1.21 support) (erigontech#8288)
408dc14 
Closes erigontech#8078 

This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead. Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.

Co-authored-by: Jason Yellick <jason@enya.ai>
lemenkov pushed a commit to fedora-ethereum/erigon that referenced this pull request Oct 4, 2023
@jyellick @lemenkov
Upgrade libp2p (enables go 1.21 support) (erigontech#8288)
e953638 
Closes erigontech#8078 

This change is primarily intended to support go 1.21, but as a
side-effect requires updating libp2p, which in turn triggers an update
of golang.org/x/exp which creates quite a bit of (simple) churn in the
slice sorting.

This change introduces a new `cmp.Compare` function which can be used to
return an integer satisfying the compare interface for slice sorting.

In order to continue to support mplex for libp2p, the change references
github.com/libp2p/go-libp2p-mplex instead. Please see the PR at
libp2p/go-libp2p#2498 for the official usptream
comment that indicates official support for mplex being moved to this
location.

Co-authored-by: Jason Yellick <jason@enya.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sukunrt sukunrt sukunrt approved these changes

@mxinden mxinden mxinden approved these changes

@MarcoPolo MarcoPolo Awaiting requested review from MarcoPolo

@p-shahi p-shahi Awaiting requested review from p-shahi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marten-seemann @nisdas @sukunrt @mxinden