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

Repo Consolidation: Transports #1187

Closed
marten-seemann opened this issue Sep 14, 2021 · 10 comments · Fixed by #1465
Closed

Repo Consolidation: Transports #1187

marten-seemann opened this issue Sep 14, 2021 · 10 comments · Fixed by #1465
Assignees
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/maintenance Work required to avoid breaking changes or harm to project's status quo P0 Critical: Tackled by core team ASAP

Comments

@marten-seemann
Copy link
Contributor

The proposal is to move all transport-related repos to go-libp2p.
To be clear, this consolidation does NOT mean that only these transports are supported. We will keep the interfaces and the methods to add new or custom handshakes / muxers / transports. libp2p will remain as extensible as it is now.

Repositories

Security Handshakes:

Muxers:

Transports:

Others:

@marten-seemann marten-seemann added exp/expert Having worked on the specific codebase is important P1 High: Likely tackled by core team if no one steps up kind/maintenance Work required to avoid breaking changes or harm to project's status quo effort/days Estimated to take multiple days, but less than a week labels Sep 14, 2021
@Stebalien
Copy link
Member

I'd also move the resuseport "transport" package.

But, in short

Do it! 🎉

@Stebalien Stebalien reopened this Sep 21, 2021
@marten-seemann
Copy link
Contributor Author

What's our policy on preserving git histories? For circuitv2, we preserved the history using git filter-tree. We can do that as well for these repositories, but the price we'll pay is that we'll end up with a lot of commits that won't compile, as interfaces have changed over time (making it difficult to git bisect later).

@Stebalien
Copy link
Member

I try to preserve history wherever possible. I'm fine with commits that don't compile. Generally, you'd just browse master's history, and all of those should compile.

@marten-seemann
Copy link
Contributor Author

Here's how the directory structure could look like.

libp2p repo consolidation

Transports

go-reuseport-transport => p2p/transport/transport/go-reuseport-transport
go-tcp-transport => p2p/transport/transport/tcp
go-libp2p-quic-transport => p2p/transport/transport/quic
go-ws-transport => p2p/transport/ransport/websocket

transport/transport is kind of ugly!

Security

go-libp2p-tls => p2p/transport/security/tls
go-libp2p-noise => p2p/transport/security/noise

Muxers

go-libp2p-yamux => p2p/transport/muxer/yamux
go-libp2p-yamux => p2p/transport/muxer/yamux

Misc

go-libp2p-swarm => p2p/transport/swarm
go-libp2p-transport-upgrader => p2p/transport/upgrader
go-conn-security-multistream => p2p/transport/conn-security-multistream

@vyzo @Stebalien thoughts?

@vyzo
Copy link
Contributor

vyzo commented Apr 19, 2022

why do we need the extra transport?

We could have transport/{tcp, ...} and put the swarm in net?

@marten-seemann
Copy link
Contributor Author

marten-seemann commented Apr 19, 2022

why do we need the extra transport?

It seemed weird to have a subdirectory for the muxers, but not for the transports. Maybe there's a less generic term than "transport" that we can use? Any ideas?

We could have transport/{tcp, ...} and put the swarm in net?

I like that!

Here's the updated table:

Transports

go-reuseport-transport => p2p/transport/transport/go-reuseport-transport
go-tcp-transport => p2p/transport/transport/tcp
go-libp2p-quic-transport => p2p/transport/transport/quic
go-ws-transport => p2p/transport/ransport/websocket

transport/transport is kind of ugly!

Security

go-libp2p-tls => p2p/transport/security/tls
go-libp2p-noise => p2p/transport/security/noise

Muxers

go-libp2p-yamux => p2p/transport/muxer/yamux
go-libp2p-yamux => p2p/transport/muxer/yamux

go-yamux and go-mplex will not be moved.

Net

go-libp2p-swarm => p2p/net/swarm
go-libp2p-transport-upgrader => p2p/net/upgrader
go-conn-security-multistream => p2p/net/conn-security-multistream

@marten-seemann marten-seemann added P0 Critical: Tackled by core team ASAP and removed P1 High: Likely tackled by core team if no one steps up labels Apr 19, 2022
This was referenced Apr 19, 2022
@Stebalien
Copy link
Member

Overall, a big thumbs-up from me.

go-reuseport-transport

Hm. That's more of a helper, not a transport. It probably belongs somewhere else? Maybe p2p/net?

transport/transport

  1. We could get rid of the top-level transport. I.e., p2p/security, etc...
  2. We could rename transport/transport to transport/connection.

@marten-seemann
Copy link
Contributor Author

marten-seemann commented Apr 22, 2022

Thank you. Here's the updated table:

Transports

go-tcp-transport => p2p/transport/tcp
go-libp2p-quic-transport => p2p/transport/quic
go-ws-transport => p2p/transport/websocket

Security

go-libp2p-tls => p2p/security/tls
go-libp2p-noise => p2p/security/noise

Muxers

go-libp2p-yamux => p2p/muxer/yamux
go-libp2p-mplex => p2p/muxer/mplex

go-yamux and go-mplex will not be moved.

Net

go-libp2p-swarm => p2p/net/swarm
go-libp2p-transport-upgrader => p2p/net/upgrader
go-conn-security-multistream => p2p/net/conn-security-multistream
go-reuseport-transport => p2p/net/reuseport

@BigLep
Copy link
Contributor

BigLep commented May 17, 2022

Note: this will go out in #1371

@marten-seemann
Copy link
Contributor Author

v0.20.0 was released. Transports are now consolidated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/maintenance Work required to avoid breaking changes or harm to project's status quo P0 Critical: Tackled by core team ASAP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants