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

MSRV policy #965

Open
martinfrances107 opened this issue Jan 13, 2023 · 12 comments
Open

MSRV policy #965

martinfrances107 opened this issue Jan 13, 2023 · 12 comments

Comments

@martinfrances107
Copy link
Contributor

martinfrances107 commented Jan 13, 2023

PR #963 strove to fix all the notifications reported by

 cargo outdated

We finished by reducing the patch to ONLY updating the packages that could be fixed without a formal policy decision.

I think we have a 'living' policy, something not written down.

"All our dependencies should bump, to the latest, and as a fallback be bumped to the a version that is AT LEAST supported."

Formalising this will be disruptive, so this issue is a request for feedback..

  1. Write a PR that fixes any problems that were causing build failures
  2. pull in a MSRV policy from an existing project

I am going to work in (1) NOW

[All credit to lnicola. This issue is largely a write up of his reviews in that PR]

Today after #963 was merged below is the "outdated" report.

My immediate want is de-duplicate this list to only include packages once
( generic_array, has32 etc. )

Name                               Project  Compat   Latest   Kind    Platform
----                               -------  ------   ------   ----    --------
arbitrary                          1.1.3    ---      1.2.2    Normal  ---
as-slice->generic-array            0.12.4   0.13.3   Removed  Normal  ---
as-slice->generic-array            0.13.3   ---      Removed  Normal  ---
as-slice->generic-array            0.14.6   0.13.3   Removed  Normal  ---
as-slice->stable_deref_trait       1.2.0    ---      Removed  Normal  ---
atomic-polyfill->critical-section  1.1.1    Removed  ---      Normal  ---
generic-array->typenum             1.16.0   ---      Removed  Normal  ---
generic-array->version_check       0.9.4    ---      Removed  Build   ---
generic-array->version_check       0.9.4    Removed  Removed  Build   ---
heapless->as-slice                 0.1.5    ---      Removed  Normal  ---
heapless->atomic-polyfill          0.1.11   Removed  ---      Normal  cfg(target_arch = "avr")
heapless->generic-array            0.14.6   ---      Removed  Normal  ---
heapless->hash32                   0.1.1    ---      0.2.1    Normal  ---
heapless->hash32                   0.2.1    0.1.1    ---      Normal  ---
heapless->rustc_version            0.4.0    Removed  ---      Build   ---
heapless->spin                     0.9.4    Removed  ---      Normal  cfg(target_arch = "x86_64")
lock_api->autocfg                  1.1.0    Removed  ---      Build   ---
lock_api->scopeguard               1.1.0    Removed  ---      Normal  ---
rstar                              0.8.4    ---      0.9.3    Normal  ---
rstar                              0.9.3    0.8.4    ---      Normal  ---
rstar->heapless                    0.6.1    ---      0.7.16   Normal  ---
rstar->heapless                    0.7.16   0.6.1    ---      Normal  ---
rstar->pdqselect                   0.1.0    ---      Removed  Normal  ---
rustc_version->semver              1.0.16   Removed  ---      Normal  ---
spin->lock_api                     0.4.9    Removed  ---      Normal  ---
@lnicola
Copy link
Member

lnicola commented Jan 13, 2023

I think that table might be misleading because it shows optional and transitive dependencies. cargo outdated -R will show only direct dependencies, while cargo tree -d will show the duplicate ones. We're not compiling three different versions of generic-array, as it might appear.

On our side, arbitrary is the only remaining thing to bump. And you can see it has no outdated deps.

@lnicola
Copy link
Member

lnicola commented Jan 13, 2023

PS: we could probably drop support for that old rstar, and I'm not sure what we're using the previous version of geo-types for (called gt-prev in the manifest).

@martinfrances107
Copy link
Contributor Author

martinfrances107 commented Jan 13, 2023

Thanks for all the comments, that is indeed where my focus was shifting.. it is really helpfully to know when others share or don't share what I am thinking...

@lnicola
Copy link
Member

lnicola commented Jan 13, 2023

I think your concerns are valid in general, but not really applicable today.

Next steps would probably be to drop 1.58 from CI and update rust-version in the manifests, then bump arbitrary, then maybe drop the old rstar.

@martinfrances107
Copy link
Contributor Author

martinfrances107 commented Jan 13, 2023

I appreciate what you say about

cargo outdated -R

My context : -

I have written 3 crate/libraries which all depend of "geo". When I use those libraries to write a application, well the dependency tree looks overly inflated and compile performance is just a nightmare.

Last month, two levels downstream - I took this approach, and de-duped my own library -- It was a big improvement .. but still not a fix.

so two levels down cargo outdated .. is a real truth teller.

and when you say

but not really applicable today.

I think differently, I know this can be helpful.

When I analyse my own problem I know I can only make real progress until I move this PR forward #908 .

That PR will limit this project to "Rust 2021 Edition"

But hey each PR should have a singular focus

@michaelkirk
Copy link
Member

Thanks for moving us into the ✨Future✨ @martinfrances107!

PS: we could probably drop support for that old rstar, and I'm not sure what we're using the previous version of geo-types for (called gt-prev in the manifest).

I think we decided this would entail a breaking bump to geo-types. Though we pretty often and eagerly bump the geo crate, we're more conservative with geo-types. geo-types is a relatively widely used crate that we encourage library maintainers to integrate with. Whenever we bump it, that change needs to ripple across the ecosystem. We're like a guest staying in their home, so we try to be polite.

That said, if there's a good reason to bump it, we should. Is the optional old version of rstar causing some kind of problem?

@lnicola
Copy link
Member

lnicola commented Jan 13, 2023

I don't think so, only confusion. Maybe we can schedule it for the next breaking release?

@lnicola
Copy link
Member

lnicola commented Jan 13, 2023

I have written 3 crate/libraries which all depend of "geo". When I use those libraries to write a application, well the dependency tree looks overly inflated and compile performance is just a nightmare.

What duplicate dependencies does geo bring into your libraries?

@martinfrances107
Copy link
Contributor Author

Before I act, I just want to give the rationale, to firm up the "probably" in this quote

we could probably drop support for that old rstar,

Copied from the code base, here is the explanation for the rstar / namespaced-features

[features]
# Prefer `use-rstar` feature rather than enabling rstar directly.
# rstar integration relies on the optional approx crate, but implicit features cannot yet enable other features.
# See: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#namespaced-features

When I follow the link I see

Namespaced features has been stabilized in the 1.60 release

So ignoring my drives, the wider case is a maintenance patch is to simplify the use of rstar

@lnicola
Copy link
Member

lnicola commented Jan 14, 2023

Would that improve the build times or reduce the number of dependencies?

@martinfrances107
Copy link
Contributor Author

Would that improve the build times or reduce the number of dependencies?

In the cold light of day, syncing up version numbers in my libraries solved all the issues, but I could ONLY do that as I am the maintainer of those 3 libraries, in the general case the problem is intractable and persists until someone works their way upstream, through several open source communities/projects ... a tough ask.

The only valid response to this problem ... is known,

"In your own code base be extremely circumspect when pulling in a new dependency, make sure the community is vibrant, and has well considered MSRV policy.

I am not a security expert but the other motivation for the change I am advocating is "Simplified" is better/ "more secure" if only because now needless features increase the attack surface, and or make the security review more error prone.

Along those lines, If I remember correctly floppy disk drive support in Linux was considered working and stable - until a zero day was spotted .. then response was just drop the whole floppy module.

@martinfrances107
Copy link
Contributor Author

#968 we now have a candidate PR ...

Opps I need to update the change log, at the very least...

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

No branches or pull requests

3 participants