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

Fix msrv: Run msrv checks with minimal versions #1541

Merged
merged 3 commits into from
Aug 24, 2024
Merged

Conversation

NobodyXu
Copy link
Contributor

Since it is possible to for dependencies to bump MSRV in patch release, msrv checking should use the minimal versions supported by gix.

Users cares deeply about msrv can also pin them to the minimal versions to maintain their current msrv.

This would have also help discover bug in #1538

Since it is possible to for dependencies to bump MSRV in patch release, msrv checking should use the minimal versions supported by gix.

Users cares deeply about msrv can also pin them to the minimal versions to maintain their current msrv.
Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the initiative! I can use any help I can get to get this dependency-monster under control.

@Byron
Copy link
Owner

Byron commented Aug 23, 2024

Even though it was initially reproducing the gix-object + winnow error, now it seems to fail in another crate entirely.

Is this something you can figure out? I tried, but it doesn't make sense after all.

@Byron Byron added help wanted Extra attention is needed feedback requested labels Aug 23, 2024
@NobodyXu
Copy link
Contributor Author

I think the version of yoke might be too old?

Since the error is from icu_locid_transform I suppose try bumping it, if latest version doesn't work then I think it's report a bug in upstream.

This seems to be more compatible to nightly versions of the compiler.
@Byron Byron removed help wanted Extra attention is needed feedback requested labels Aug 23, 2024
@NobodyXu
Copy link
Contributor Author

error: failed to run custom build command for `libz-ng-sys v1.1.8`

Caused by:
  process didn't exit successfully: `D:\a\gitoxide\gitoxide\target\debug\build\libz-ng-sys-c79adf8887a64bd5\build-script-build_zng` (exit code: 101)
  --- stderr
  thread 'main' panicked at C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\cmake-0.1.44\src\lib.rs:783:22:
  Visual studio version detected but this crate doesn't know how to generate cmake files for it, can the `cmake` crate be updated?

looks like libz-ng is too old?

Byron added a commit to Byron/flate2-rs that referenced this pull request Aug 23, 2024
…test releases.

That way, those compiling with `-Zminimal-versions` have higher chances of it to work.

See Byron/gitoxide#1541 for reference.
@Byron
Copy link
Owner

Byron commented Aug 23, 2024

Thanks a lot for the encouragement!

I ended up running the cargo +nightly update -Zminimal-versions to reproduce it locally.

However, now it seems to show drift in the flate2 crate, which apparently fails to build on Windows 2022, and I opened a PR against it.

This shows that minimal-versions is also debugging the entire dependency tree, and I don't know if I like that burden responsibility 😅.

@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 23, 2024

This shows that minimal-versions is also debugging the entire dependency tree, and I don't know if I like that burden responsibility 😅.

Yeah it's somewhat transitive, I think we can reduce this burden by running -Zminimal-versions in more places, i.e. in flate2, url, etc

If more crates start using it, then it would be detected early on instead of passing it to downstream.

@NobodyXu
Copy link
Contributor Author

@Byron I opened rust-lang/flate2-rs#425 to add -Zminimal-versions to flate2

@Byron
Copy link
Owner

Byron commented Aug 23, 2024

Thanks - once it fails I plan to put my 'fix' from the other PR on top of it, bump the version, merge and create a new release. Then this PR should be unblocked (or is blocked by something else :D).

Byron added a commit to NobodyXu/flate2-rs that referenced this pull request Aug 23, 2024
…test releases.

That way, those compiling with `-Zminimal-versions` have higher chances of it to work.

See Byron/gitoxide#1541 for reference.
@Byron Byron added the blocked Issue can't progress due to external dependencies label Aug 23, 2024
@Byron
Copy link
Owner

Byron commented Aug 23, 2024

Needs rust-lang/flate2-rs#425 to continue, which needs minimal-version checks for libz-sys along with a fix to raise the minimally required vcpkg.

Byron added a commit to NobodyXu/flate2-rs that referenced this pull request Aug 24, 2024
…test releases.

That way, those compiling with `-Zminimal-versions` have higher chances of it to work.

See Byron/gitoxide#1541 for reference.
@Byron Byron removed the blocked Issue can't progress due to external dependencies label Aug 24, 2024
@Byron Byron merged commit a64d94e into Byron:main Aug 24, 2024
18 checks passed
@NobodyXu NobodyXu deleted the patch-2 branch August 24, 2024 14:10
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.

2 participants