-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Reduce accidental dependency duplication #8060
Comments
@zhiburt would there be any potential breaking changes bumping |
As I remember there will be small ones; |
After running
And some dependencies already updated:
|
# Description Relative: #8060 While investigating, I found we need to update notify, which is a good step to remove some duplicate dependencies. As title, here are some goods and bads after updating: ## Good keep dependency up to date, and remove duplidate dependency(cfg-if, winapi) in Cargo.lock. ## Bad Introduce some breaking changes: After updating to notify v5, I found that we have to remove `Rename` events. But I've testing under notify v4, and it doesn't work good if we running the following command on MacOS: ``` touch a mv a b ``` It fires file create event, but no file rename event. So `rename` event is not really reliable, so I think it's ok for us to remove `Rename` events. The reason to remove `--debounce-ms` flag: It's not provided by defualt file watcher, we can use [PollWatcher](https://docs.rs/notify/latest/notify/poll/struct.PollWatcher.html), but it scans filesystem, which is really expensive. So I just remove the flag. # User-Facing Changes 1. `--debounce-ms` flag is removed 2. no longer watch `Rename` event. # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date.
close? #8060 Quite a bit of refactoring took place. I believe a few improvements to collapse/expand were made. I've tried to track any performance regressions and seems like it is fine. I've noticed something different now with default configuration path or something in this regard? So I might missed something while testing because of this. Requires some oversight. --------- Signed-off-by: Maxim Zhiburt <[email protected]>
Apart from `polars` (only used with `--features dataframe`) and the dev-dependencies our deps use `indexmap 2.0`. Thus the default or `extra` `cargo build` will reduce deps. This also will help deduplicating `hashbrown` and `ahash`. For nushell#8060
# Description Apart from `polars` (only used with `--features dataframe`) and the dev-dependencies our deps use `indexmap 2.0`. Thus the default or `extra` `cargo build` will reduce deps. This also will help deduplicating `hashbrown` and `ahash`. For #8060 - Bump `indexmap` to 2.0 - Remove unneeded `serde` feature from `indexmap` # User-Facing Changes None
# Description Simplify the dependencies. There are two different versions of `terminal_size` that nushell directly depends on. Related: #8060
# Description Simplify the dependencies. There are two different versions of `terminal_size` that nushell directly depends on. Related: nushell#8060
# Description `which` 5.0.0 is already in the dependency tree, so remove v4.4.2 Related: nushell#8060
Windows FFIThere're three major crates for Windows FFI that we're using:
The
The |
Thanks for the upstream work @YizhePKU ! Agreed that the situation around |
Problem
We currently have a number of dependencies for which we pull in different versions of the same crate as dependencies in our
cargo tree
have conflicting semver requirements. This causes multiple compilation runs, slows down linking, and causes binary bloat.This is on top of crates that functionally duplicate the solution to the same problem (e.g. multiple crates responsible for ANSI coloring or wrappers around the system e.g. for libc or windows system calls)
Aims
As a first goal we should strive to reduce this version duplication by taking a hard look at dependencies or semver-specs in our crates that cause them.\nEither we eliminate them, choose better versions on our side (as long as this doesn't preclude important bugfixes), or be good stewards and try to contribute upstream version bumps.
Output of
cargo tree -d
This returns dependencies which appear with multiple versions and their dependency tree. Also check the
Cargo.lock
(last updated 2024-03-11 with
--depth 3
)The text was updated successfully, but these errors were encountered: