lsd-rs / lsd

The next gen ls command
Apache License 2.0
13.05k stars 425 forks source link

update various dependencies #938

Closed decathorpe closed 8 months ago

decathorpe commented 10 months ago

I'm the maintainer of lsd for Fedora Linux, and some outdated dependencies are making maintenance increasingly difficult.

We have this patched for almost two years in Fedora and it has not caused issues.

Same here, this patch has been in the Fedora package for a while.

This dependency seems to have been added recently, not sure why an old version was chosen.

Same here, this was added recently but 0.1 was used instead of 0.2, not sure why.

Using old versions of git2 is not a good idea, since the bundled libgit2 C library often has CVE issues.

The "users" crate is unmaintained. The "uzers" crate is an API-compatible fork that also fixes some bugs and security issues.

The current dependency (v0.5) is reeeeeally old. Not sure why this was never updated.

Same here, predicates v1 is reaally old.

Not sure why strange x.0.* style dependencies were used here. It's holding back various updates for both url and xdg crates, and makes maintaining lsd in Fedora more difficult. We have built lsd against the latest versions of all three crates forever, and it has not caused issues.

muniu-bot[bot] commented 10 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: decathorpe

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/lsd-rs/lsd/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
decathorpe commented 10 months ago

PS: I'm not sure why you're keeping Cargo.lock pinned to really old versions. It looks like "cargo update" hasn't been run in ages. I'm sure there's tons of bugfixes (and potentially also security fixes) that happened between the versions pinned in Cargo.lock and the latest compatible versions.

codecov-commenter commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (89659e4) 85.76% compared to head (f6dec60) 84.51%. Report is 10 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #938 +/- ## ========================================== - Coverage 85.76% 84.51% -1.26% ========================================== Files 51 51 Lines 5001 5068 +67 ========================================== - Hits 4289 4283 -6 - Misses 712 785 +73 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zwpaper commented 9 months ago

Hi @decathorpe, thanks so much for raising this, it would definitely help a lot!

I found predicates v3 need msrv 1.69, I have updated the msrv in master, can you help rebase your PR onto it?

decathorpe commented 9 months ago

Ah, the MSRV is only checked in CI - it might be a good idea to set package.rust-version in Cargo.toml as well?

Thanks for taking a look, I'll rebase my PR.

zwpaper commented 8 months ago

hi @decathorpe can you help to update the msrv to 1.70, so that we can pass the CI

decathorpe commented 8 months ago

I've bumped the MSRV in the GitHub action and in Cargo.toml.

I also removed the hand-written MSRV check that was present in build.rs that is redundant with defining package.rust-version in Cargo.toml.

zwpaper commented 8 months ago

thanks so much for your help @decathorpe!

zwpaper commented 8 months ago

btw, I have sent you an invitation for the lsd-rs org package maintainers team, welcome aboard if you would like to 😀

decathorpe commented 8 months ago

Great, thank you :)