Feature-gate all dependencies

[tomaka]

Close #1364
cc @thomaseizinger

I removed the CommonTransport struct, because I would otherwise need to provide eight different implementations of it (one for each combination of tcp/dns/websocket).

[thomaseizinger]

Awesome! :)

[romanb]

So the takeaway from the discussion in #1364 is that the project is moving towards a single libp2p crate with feature flags? For a user, I'm not sure I understand why enabling feature flags is much better than listing individual crates as dependencies. I can see how releases may be simplified if all versions are anyway supposed to be in lock-step, i.e. if there is actually not much desire to develop and release the currently separate crates individually.

A potential problem that came to mind with a single crate is that I remembered feature flags to be unified between dependencies and dev-dependencies (i.e. rust-lang/cargo#1796). However, that seems to have been recently at least partly addressed (rust-lang/cargo#7916). So maybe this is no longer a concern moving forward.

[thomaseizinger]

For a user, I'm not sure I understand why enabling feature flags is much better than listing individual crates as dependencies.

I see two differences:

1. If you have to list the dependencies yourself, you have to manually keep the versions in sync which is a) tedious and b) doesn't work with tools like dependabot for example as it updates the dependencies one by one.
2. Due to the current design of the derive(NetworkBehaviour) macro, you have to depend on the libp2p metacrate as it assumes those imports paths to exist. That means you are bringing in all dependencies through the meta-crate anyway which makes listing them individually pointless.

A potential problem that came to mind with a single crate is that I remembered feature flags to be unified between dependencies and dev-dependencies (i.e. rust-lang/cargo#1796).

That is a good point and definitely something to consider! Glad to see it is being fixed. I don't see how it would make a difference though as the current state is that you are bringing in all of libp2p anyway.

The exception to that are libraries that are built on top of single protocol crates and don't use the custom derive. Point (1) from above still applies to them though.

[tomaka]

For now it's just about feature-flagging all the crates, not unifying them into one.

But if we're on the topic of unifying crate, to me the major advantage is how much more understandable the crate could become. We could for example put all the implementations of Transport (TcpConfig, DnsConfig, ...) in a transport module alongside with the Transport trait itself.

[romanb]

For now it's just about feature-flagging all the crates, not unifying them into one.

But if we're on the topic of unifying crate, to me the major advantage is how much more understandable the crate could become. We could for example put all the implementations of Transport (TcpConfig, DnsConfig, ...) in a transport module alongside with the Transport trait itself.

Maybe, I just think it should not be forgotten that it comes with a significant loss of flexibility in releases, which I understand libp2p did not actually make use of so far, so maybe it doesn't matter that much. I'm thinking about situations where e.g. some parts of the code have undergone refactorings that should not yet be released, other parts need important patch releases. If everything is under a single version, that can be quite a constraint in the sense that nothing can go into master that is not ready to be released. A master / develop or similar branch split could help with that, of course.

In any case, I have no objections to these feature flags themselves.

[thomaseizinger]

A master / develop or similar branch split could help with that, of course.

Shameless plug: I just wrote a blogpost the other day, how a GitFlow like model allows for a lot of automation around releases / hotfixes. Might be helpful if you are considering that moving to that :)

In any case, I am excited about these changes!

[tomaka]

Can I ping for reviews/opinions? 🙏

[mxinden]

This looks good to me from a technical standpoint. I don't have an opinion on the actual process improvements.

[thomaseizinger]

Can I ping for reviews/opinions? 🙏

Not sure if this was also directed towards me but the implementation looks good to me. Tested this branch on our end and works flawlessly: -60 dependencies in total 👌