*: Move `libp2p` meta crate into its own directory by thomaseizinger · Pull Request #3012 · libp2p/rust-libp2p

thomaseizinger · 2022-10-12T01:43:20Z

Description

Currently, our top-level Cargo.toml manifest represents a crate AND a workspace. This causes surprising behaviour (e.g. #2949 ) where we need to explicitly pass --workpace to every command to run it on the entire workspace and not just the meta crate.

My moving the meta crate into its own directory, the root manifest file is a virtual manifest only and thus, every cargo command will automatically default to running on the entire workspace.

I am setting this as draft for now. Will audit any broken links once there is consensus on doing this.

Links to any relevant issues

Open Questions

What is the best name for this directory?

Open tasks

Audit codebase for any broken links in markdowns etc

Change checklist

I have performed a self-review of my own code
~~I have made corresponding changes to the documentation~~
~~I have added tests that prove my fix is effective or that my feature works~~
~~A changelog entry has been made in the appropriate crates~~

mxinden

I am fine with this. I am guessing that this is also the conventional way of doing things? Any big projects out there that does what we do today?

mxinden · 2022-10-14T19:10:25Z

core/Cargo.toml

 base64 = "0.13.0"
 criterion = "0.4"
-libp2p = { path = "..", features = ["full"] }
+libp2p = { path = "../meta/", features = ["full"] }


Suggested change

libp2p = { path = "../meta/", features = ["full"] }

libp2p = { path = "../libp2p/", features = ["full"] }

Would it not make sense to name the folder like the crate name? Tokio seems to do just that. What do other projects do?

https://github.com/tokio-rs/tokio/tree/master/tokio

Would you rename all the other directories to their actual crate name too?

It might be a bit of repetition but I think I'd prefer just straight up using the crate name for all directories.

The directory hierarchy is also something I am not too sure about but that is orthogonal :)

Would you rename all the other directories to their actual crate name too?

Given that each of them are sub-crates of libp2p, I think it is fine to keep them as is.

thomaseizinger · 2022-10-15T03:57:09Z

I am fine with this. I am guessing that this is also the conventional way of doing things? Any big projects out there that does what we do today?

I don't know of any but also haven't done any explicit research. For me, having a dedicated virtual manifest feels cleaner :)

mergify · 2022-11-14T10:02:29Z

This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏

mxinden · 2022-11-14T10:04:12Z

I don't really have an opinion on both the introduction of a pure workspace crate and the rename of the folders. Off the top of my head I would say it is not worth the change.

That said, I think @thomaseizinger you feel strongly about it. Thus happy to move forward unless anyone has objections.

thomaseizinger · 2022-11-23T11:23:43Z

What about a layout with:

a top-level crates directory, containing all the released crates
a top-level lib directory, containing misc stuff like prost-codec that could at some point live outside our repository (if the rate of changes gets low enough)
a top-level testing directory, containing crates like the muxer test harness and in the future swarm-test
a top-level examples directory as per Move all examples to a common location #3111

elenaf9 · 2022-12-10T18:34:33Z

What about a layout with:

a top-level crates directory, containing all the released crates

Would crates then still contain the subfolders transports, muxers, protocols, ..?

a top-level lib directory, containing misc stuff like prost-codec that could at some point live outside our repository (if the rate of changes gets low enough)

But prost-codec is also released as a crate on crates.io, right? I would find it a bit confusing to differentiate between what belongs to lib and what to crates.

I also don't feel strongly about this.
If we decide to change the repo structure I have a slight preference to how the PR currently implements it: libp2p meta crate and libp2p-* for sub-crates. Adding the top-level testing directory also sounds good to me.

thomaseizinger · 2022-12-10T20:44:47Z

a top-level lib directory, containing misc stuff like prost-codec that could at some point live outside our repository (if the rate of changes gets low enough)

But prost-codec is also released as a crate on crates.io, right? I would find it a bit confusing to differentiate between what belongs to lib and what to crates.

The difference would be:

directly libp2p related crates
misc crates we happen to maintain but are more generic and only in the workspace to allow for quicker iteration

I also don't feel strongly about this.
If we decide to change the repo structure I have a slight preference to how the PR currently implements it: libp2p meta crate and libp2p-* for sub-crates. Adding the top-level testing directory also sounds good to me.

Thanks for adding opinion. Despite being slightly redundant, I also prefer the direct mapping of directory to crate name.

thomaseizinger · 2022-12-15T01:32:08Z

a top-level crates directory, containing all the released crates

Would crates then still contain the subfolders transports, muxers, protocols, ..?

I am not sure. I always found this only semi helpful. Even though I understand the categorization, my brain primarily works in names (e.g. I want to modify gossipsub, yamux etc), so having to then think about which category that is in to pick the right directory is a bit odd. It is also not as easy to search (at least within Intellij) because after focusing the directory tree, I first have to fuzzy-match one of the top level directories before I can start with the protocol / component name.

From that perspective, my vote would be: No, those directories would be gone.

thomaseizinger · 2022-12-15T01:32:34Z

a top-level lib directory, containing misc stuff like prost-codec that could at some point live outside our repository (if the rate of changes gets low enough)

A better name for lib could be external?

mxinden · 2022-12-19T11:51:16Z

a top-level crates directory, containing all the released crates

Would crates then still contain the subfolders transports, muxers, protocols, ..?

I am not sure. I always found this only semi helpful. Even though I understand the categorization, my brain primarily works in names (e.g. I want to modify gossipsub, yamux etc), so having to then think about which category that is in to pick the right directory is a bit odd. It is also not as easy to search (at least within Intellij) because after focusing the directory tree, I first have to fuzzy-match one of the top level directories before I can start with the protocol / component name.

From that perspective, my vote would be: No, those directories would be gone.

I find the directories helpful, especially for newcomers, as they provide structure to the otherwise chaotic flood of crates that libp2p-* provides.

thomaseizinger · 2022-12-20T02:38:53Z

a top-level crates directory, containing all the released crates

Would crates then still contain the subfolders transports, muxers, protocols, ..?

I am not sure. I always found this only semi helpful. Even though I understand the categorization, my brain primarily works in names (e.g. I want to modify gossipsub, yamux etc), so having to then think about which category that is in to pick the right directory is a bit odd. It is also not as easy to search (at least within Intellij) because after focusing the directory tree, I first have to fuzzy-match one of the top level directories before I can start with the protocol / component name.
From that perspective, my vote would be: No, those directories would be gone.

I find the directories helpful, especially for newcomers, as they provide structure to the otherwise chaotic flood of crates that libp2p-* provides.

Sorry to question this so bluntly but is this just a guess or actual feedback from newcomers? I can see why the number of crates could be overwhelming for newcomers, however, I am not sure whether a directory structure is the best way to help here. Personally, I'd find a UML diagram that visualises the dependencies much more useful. Such a diagram could be auto-generated with a script by inspecting cargo metadata and placed in the README.md of the proposed crates/ directory.

I would like to push #3072 (comment) forward in the near future, so we will get a few new crates where it is not obvious, which directory they should go in. As we untangle some of the existing abstractions, I think this directory structure will also need revisiting.

mxinden · 2023-01-17T13:01:03Z

Sorry to question this so bluntly but is this just a guess or actual feedback from newcomers?

No worries. Please question every word I write.

It is my intuition that the directory structure makes it easier for newcomers to find their way around. That said, it is an intuition only and I don't remember my own experience getting familiar with the structure.

- Moved with libp2p#3012 - Dependabot always shows root `CHANGELOG.md` in pull request on dependent repository

- Moved with #3012 - Dependabot always shows root `CHANGELOG.md` in pull request on dependent repository Pull-Request: #3583.

More libp2p meta crate into its own directory

23768b8

thomaseizinger requested review from elenaf9 and mxinden October 12, 2022 01:43

thomaseizinger added 2 commits October 14, 2022 19:06

Merge branch 'master' into meta-crate-directory

e35f2b0

Only compile libp2p crate for wasm

ebc4d7b

thomaseizinger marked this pull request as ready for review October 14, 2022 08:24

mxinden reviewed Oct 14, 2022

View reviewed changes

thomaseizinger added 7 commits October 17, 2022 10:39

Rename meta crate dir to libp2p

76f83dd

Fix links in README.md

8198c1c

Split main changelog

2da0afa

Fix changelog formatting

e51e639

Fix links in changelog

f197b7f

Merge branch 'master' into meta-crate-directory

cbcd75a

Rename directories to full crate name

686c717

thomaseizinger requested a review from mxinden October 17, 2022 00:10

thomaseizinger added 2 commits October 17, 2022 18:16

Merge branch 'master' into meta-crate-directory

8c7ae53

Merge branch 'master' into meta-crate-directory

a3bf3de

thomaseizinger closed this Mar 2, 2023

thomaseizinger mentioned this pull request Mar 2, 2023

chore: move libp2p crate to a dedicated directory #3536

Merged

4 tasks

mxinden added a commit to mxinden/rust-libp2p that referenced this pull request Mar 10, 2023

docs: Reintroduce root level changelog file

471dc0a

- Moved with libp2p#3012 - Dependabot always shows root `CHANGELOG.md` in pull request on dependent repository

mxinden mentioned this pull request Mar 10, 2023

docs: Reintroduce root level changelog file #3583

Merged

4 tasks

mergify bot pushed a commit that referenced this pull request Mar 10, 2023

docs: Reintroduce root level changelog file

69af850

- Moved with #3012 - Dependabot always shows root `CHANGELOG.md` in pull request on dependent repository Pull-Request: #3583.

thomaseizinger deleted the meta-crate-directory branch April 26, 2023 16:38

	libp2p = { path = "../meta/", features = ["full"] }
	libp2p = { path = "../libp2p/", features = ["full"] }

Conversation

thomaseizinger commented Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Links to any relevant issues

Open Questions

Open tasks

Change checklist

Uh oh!

mxinden left a comment

Choose a reason for hiding this comment

Uh oh!

mxinden Oct 14, 2022

Choose a reason for hiding this comment

Uh oh!

thomaseizinger Oct 15, 2022

Choose a reason for hiding this comment

Uh oh!

thomaseizinger Oct 15, 2022

Choose a reason for hiding this comment

Uh oh!

mxinden Nov 14, 2022

Choose a reason for hiding this comment

Uh oh!

thomaseizinger commented Oct 15, 2022

Uh oh!

mergify bot commented Nov 14, 2022

Uh oh!

mxinden commented Nov 14, 2022

Uh oh!

thomaseizinger commented Nov 23, 2022

Uh oh!

elenaf9 commented Dec 10, 2022

Uh oh!

thomaseizinger commented Dec 10, 2022

Uh oh!

thomaseizinger commented Dec 15, 2022

Uh oh!

thomaseizinger commented Dec 15, 2022

Uh oh!

mxinden commented Dec 19, 2022

Uh oh!

thomaseizinger commented Dec 20, 2022

Uh oh!

mxinden commented Jan 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomaseizinger commented Oct 12, 2022 •

edited

Loading