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

Bump dependencies #3072

Closed
wants to merge 7 commits into from
Closed

Bump dependencies #3072

wants to merge 7 commits into from

Conversation

KGrewal1
Copy link

This PR bumps the version of many dependencies as well as refactoring Cargo.toml so that the bulk of dependencies are in the top level [workspace.dependencies] for easier maintenance.

No code changes were needed except for the removal of .unwrap() calls as strip-ansi-escapes changed return type to Vec from Result<Vec> : the remaining crates that may be updated would require more significant changes.

This was motivated as currently the cargo.lock file contains yanked crates and has warning

warning: the following packages contain code that will be rejected by a future version of Rust: daemonize v0.4.1

As the MSRV of Zellij is already being update to 1.75, this is not effected.

On build the warning

warning: the following packages contain code that will be rejected by a future version of Rust: nom v5.1.2, terminfo v0.7.3

still appears: these are transitive dependencies of termwiz for which zellij is already on the newest version

@imsnif
Copy link
Member

imsnif commented Jan 19, 2024

Hi - I really appreciate your work on this, but I'm afraid I can't accept this PR.

Over the years in this and other projects, I have seen countless occasions where even a patch version of a dependency would break a sufficiently complex project in subtle and unexpected ways. Not because anyone did something malicious or wrong, but because very often the creator and consumer of a library have different ideas about how it should behave.

So while I think semantic versioning is a good way to communicate intent, I do not want to rely on it as a blind guide for mass upgrades.

In this project, I upgrade dependencies either when we absolutely have to (security issues), when we need features/performance from the new versions or when the old versions would prevent us from doing things in the ecosystem due to bugs/issues with cargo or crates.io's dependency tree resolution. In anyway though - it is never in a sweeping manner and always in a targeted deliberate fashion.

@imsnif imsnif closed this Jan 19, 2024
@KGrewal1
Copy link
Author

I don't think SemVer actually could impact this differently however (as cargo already actively considers all SemVer changes valid for package resolution without the lock file and the lock file contains yanked packages curreently)

@imsnif
Copy link
Member

imsnif commented Jan 19, 2024

Which is the reason we tell people to install with --locked.

@KGrewal1
Copy link
Author

Which is the reason we tell people to install with --locked.

but specifically installation with locked now results in

warning: package `ahash v0.7.6` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
warning: package `cpufeatures v0.2.2` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
warning: package `crossbeam-channel v0.5.4` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
warning: package `hermit-abi v0.3.1` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked

specifically for crossbeam this removes a race condition bug, for hermit and ahash its unclear and for cpufeatures to work around a CPUID bug in std

@imsnif
Copy link
Member

imsnif commented Jan 19, 2024

You have my answer. Thanks again for your efforts.

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

2 participants