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

remove lindera_dictionary "build.rs"-only code/deps from full crate #459

Merged
merged 2 commits into from
Mar 11, 2025

Conversation

eeeebbbbrrrr
Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr commented Mar 10, 2025

Hi!

We (https://github.com/paradedb/paradedb/) have a dependency on the top-level lindera crate and noticed that our project's dependency tree was pulling in other crates we don't use, such as reqwest and tokio (and many others).

After some cargo tree analysis we realized it was coming from lindera-dictionary -- specifically its "assets" module.

It turns out that "assets" is only used by Lindera Dictionary's build.rs, so...

This PR introduces a new feature flag for lindera-dictionary so the other Lindera crates can toggle this new build_rs feature flag only for their "build dependency" on lindera-dictionary.

Ultimately, this removes (at least) reqwest and all its dependencies, which was our goal.

There's probably a few different ways this could be accomplished, and I'm happy to rework this PR in any way that y'all deem correct for the Lindera project. I really just wanted to put this in front of y'all. It's a big bonus for us not to have these unused crates as part of our compiled library and runtime.

Unverified

This user has not yet uploaded their public signing key.
…across all crates

This adjusts all the Cargo.toml's so that `reqwest` doesn't end up in
the dependency tree of downstream consumers
@CLAassistant
Copy link

CLAassistant commented Mar 10, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@mosuka mosuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mosuka mosuka merged commit a3d0b96 into lindera:main Mar 11, 2025
8 checks passed
@mosuka
Copy link
Member

mosuka commented Mar 11, 2025

@eeeebbbbrrrr
Thank you for your pull request!

@eeeebbbbrrrr
Copy link
Contributor Author

Thanks for merging it, @mosuka! And thanks for Lindera!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants