refacto: extract the core crate from the CLI

[poliorcetics]

This builds on #240 to extract a buffrs-core crate.

This allows separating what we want as CLI vs what we want as a lib clearly and makes tests clearer IMO by separating CLI/E2E tests from API tests

It also makes separating deps easier

[mara-schulke]

Hi @poliorcetics I agree with the fact that we pull in some unused dependencies in the current state of the crate but I disagree with the splitting:

• It won't really solve the problem as you still have this feature flag mess (you probably would need to add more granular flags to solve this)
• Users don't depend on buffrs anymore in their production code (I think since 0.6)
• It violates the "no io rule" in crate-inlined tests

So iiuc and the goal is to minimize the dependency footprint buffrs has when being used as a build dependencies I think the most meaningless ways to achieve this are:

• Checking deps and where they are used (and if we can strip them)
• Identifying disjoint set of dependencies that relate to features

[mara-schulke]

So my hint here was basically, we will save something by this layout! But it's mostly tokio and clap and I think this save can be achieved with less noise (e.g. having to republish crates etc.)

Also bare in mind that we solved most of the annoyances of the original issue but it was not properly documented (which is my bad). Eg. we removed the dependencies on openssl and libgit.

[poliorcetics]

It violates the "no io rule" in crate-inlined tests

What is that rule ? This is the first time I hear of it. Also, I'm unclear on how it is violated if it wasn't before since I didn't move any test, only changed paths to make them work with the new layout.

It won't really solve the problem as you still have this feature flag mess (you probably would need to add more granular flags to solve this)

I'm not really doing this for dependencies, though I certainly think having a clear cut separation is good, I'm doing this for separation of concerns: the CLI shouldn't have access at all to anything private in the lib and with a workspace that's very clear.

Checking deps and where they are used

Cargo always download all the deps, regardless of specified target/features (to make offline builds much easier but also because it can't resolve final features before downloading all crates) so having two crates is effectively the only way to ensure clap is not downloaded for the lib. Marking it optional is not enough.

Users don't depend on buffrs anymore in their production code (I think since 0.6)

Even then, I still prefer workspaces to a single monolithic crate

[mara-schulke]

Regarding the "no-i-rule" (sorry I should have elaborated yesterday), I am specifically referring to the fact that:

• In rust tests within the src directory are meant to be used for unit tests (https://doc.rust-lang.org/book/ch11-03-test-organization.html) not for integration tests -> testing logic, not end to end workflows.
• Test best practices include separating test suites depending on their runtime requirements (eg. only program logic in unit tests, public api testing with io in integration tests and full simulation in end to end tests)

Some links for the above:

• https://blog.logrocket.com/how-to-organize-rust-tests/
• https://effective-rust.com/testing.html#summary
• https://www.testrail.com/blog/the-7-sins-of-unit-testing/ (nr 4)

FWIW I'm nitpicky here but I do think that this is important nonetheless.

[mara-schulke]

I'm not really doing this for dependencies, though I certainly think having a clear cut separation is good, I'm doing this for separation of concerns: the CLI shouldn't have access at all to anything private in the lib and with a workspace that's very clear.

Well, this was my concern: I agree that this setup is an alternative way of structuring the repository but if it has no immediate purpose for users I would prefer to invest efforts into meaningful improvements, like documentation, bug fixes or feature development over endless refactoring the code base in an idempotent way. I'm not trying to cut you down here but I am concerned about a few things that the nature of your changes entail:

• This mr essentially bricks all existing mrs as it shuffles all of the code around
• It's a breaking change for users (they have to now look into the fact why there is no new version of buffrs anymore ~> find out it was renamed)
• We would need to publish two crates instead of one (this seems simple but is annoying for referencing docs.rs, CI, usage statistics etc)
• Bricks existing code that may depend on CLI code (pub mod commands)
• Suddenly it becomes "unclear" whether to put code in crate a or crate b (unless you clearly document the mental model behind the split). People could and would probably write "library" code in the binary package)

All of the above are pain points without a clear user focused benefit. The initial issue this pr is referring to (#140) is essentially already solved to a good portion and as you said untouched by the change.

I get the fact that our personal opinions differ here but I would just urge the point that it is unsatisfying to ship changes that don't do anything and create a lot of work.

[poliorcetics]

Regarding the "no-i-rule" (sorry I should have elaborated yesterday), I am specifically referring to the fact that: (snip)

I still don't see how that rule is broken now if it wasn't before since I just moved an already existing lib.rs so if it's broken now, it was broken before without issues.

Regarding the rest I highly disagree that it's useless. I would even argue that non-workspaced crates should be banned in non-learning projects that are intended to grow because it causes endless pains in the Cargo.toml and prevents refactoring to separate concerns in various crates (e.g: the registry situation previously was very annoying to work with imo)

Anyway, we'll have to agree to disagree on (dis)advantages, no need to rehash them back and forth for 10 more comments ^^