Skip to content

Move binary-only dependencies under a feature #1937

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

willcrichton
Copy link
Contributor

@willcrichton willcrichton commented Nov 25, 2022

Problem

When mdBook is used as a library for preprocessors, preprocessors need far less functionality than for the main mdbook binary. However, even with default-features = false, mdbook currently has 91 transitive dependencies.

Solution

This PR reduces the number of transitive dependencies from 91 to 46 with default-features = false, while maintaining enough API surface for preprocessors to function. It introduces a new default "full" feature that activates several dependencies like chrono and handlebars. This feature then gates the compilation of several modules and functions.

To reduce the usage of #[cfg(feature = "full")], this PR factors out the MDBook data structure from the book module into a new module manage (better names welcome), while leaving the Book data structure (essential for preprocessors) inside the book module. Specifically the set of renamings are:

  • book/book.rs -> book/mod.rs
  • book/mod.rs -> manage/mod.rs
  • book/init.rs -> manage/init.rs

Alternatives

In theory, a cleaner alternative would be to factor the monolithic mdbook project into a Cargo workspace: one crate for the core API that would be used by preprocessors, and one crate for building the mdbook binary. However, this is a much larger change that requires messing with CI, publishing multiple packages, etc. so this PR seemed like a simpler way to achieve the same desired outcome for now.

Notes

Note there's still some TODOs for documentation --- I will add the docs if it is decided to go ahead with this PR.

@willcrichton willcrichton force-pushed the prune-preprocessor-deps branch 3 times, most recently from 694a75f to 318d5b0 Compare November 25, 2022 16:43
@willcrichton willcrichton force-pushed the prune-preprocessor-deps branch from 318d5b0 to aac8fac Compare November 25, 2022 16:57
@ehuss
Copy link
Contributor

ehuss commented Nov 25, 2022

Thanks for taking a look at this. I've been thinking of going the multiple-package route in the 0.5 release.

Unfortunately I don't think a change like this can be made in 0.4 since it is a breaking change.

Would you be interested in helping come up with an outline of what it might look like split into separate packages?

@willcrichton
Copy link
Contributor Author

Yes I would be interested! You can treat this PR as a proof-of-concept for one possible factoring.

Is that discussion happening anywhere in particular?

@ehuss
Copy link
Contributor

ehuss commented Nov 26, 2022

#1234. I haven't given it too much thought recently. I think it would be good to identify the use cases ("preprocessor user", "renderer user", non-CLI driver (like rustbook), and try to minimize those. I was thinking maybe something like the "preprocessor user" just gets some basic structs (Preprocessor, SerializedBook or something like that) with serde impls, so it can be as lightweight as possible. That would likely entail doing a little survey of existing extensions to see what they use (and why they use it).

@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 2025

☔ The latest upstream changes (possibly #2681) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants