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

Upgrade all dependencies mentioned by dependabot #230

Merged
merged 9 commits into from
Oct 10, 2022

Conversation

patowen
Copy link
Collaborator

@patowen patowen commented Aug 13, 2022

Summary

This PR upgrades the following crates (comments in parentheses):

  • metrics (complicated)
  • ash-window (mechanical)
  • hecs (mechanical)
  • nalgebra (mechanical. Was blocked by a performance regression in nalgebra, but this has now been fixed.)
  • winit (mechanical)
  • approx (trivial)
  • rand (trivial)
  • rand_distr (trivial)
  • rand_pcg (trivial)
  • rcgen (trivial)
  • tracing (trivial)
  • tracing-subscriber (trivial, apart from changing "chrono" feature to "time")
  • webpki (trivial)
  • metrics-core (removed, not upgraded, since "metrics" no longer uses it)

One semi-unrelated change is adding debug-assertions = true in the top-level Cargo.toml. Debug assertions are only enabled by default in non-optimized builds, so for them to ever be triggered, they need to be enabled manually. I discovered this while debugging the nalgebra upgrade.

The below sections summarize the code changes for each crate.

Although a lot of crates were upgraded, I don't believe the diff is too complicated to review. However, the individual commits are relatively clean, so looking at these may help simplify the code review. I did create the PR under the assumption that the commits would be squashed, though.

metrics

This is the most complicated change, as the API was significantly adjusted in the metrics crate. The timing! macro was removed and replaced with a histogram! macro. The histogram! macro both "registers" a histogram and records to it. I found the documentation of the crate a bit lacking, but my understanding is that registration is meant to be idempotent.

I made registration practically a no-op, just returning an object that had enough information to implement "recording" the same way as it was implemented before. Fortunately, the PR diff highlights this well.

One other major change is that the metrics crate uses f64 instead of u64 now, and the conversion of an Interval to an f64 using this crate uses a unit of seconds, so I had the calls to histogram! use seconds, while I had the hdrhistogram use nanoseconds as before.

Fortunately, besides switching metrics_core::Key to metrics::Key, I didn't need to change the Recorder struct itself.

Another minor note: the std feature option was removed in version 0.18.0.

ash-window

The required parameters for ash_window::enumerate_required_extensions and ash_window::create_surface have changed to use RawDisplayHandle and RawWindowHandle objects directly instead of relying on traits. This required adding an additional dependency: raw_window_handle. Implementing this change just involved following the example provided in the ash_window git repo.

hecs

The main change in the hecs crate was the replacement of get::<T> and get_mut::<T> with get::<&T> and get::<&mut T> respectively. I believe this change was introduced in 0.8.0.

nalgebra

The most repetitive code modification was changing na::RealField to na::RealField + Copy, as the Copy trait was removed from na::RealField, and we depended on that in a lot of places. I systematically made this change in all usages. This was a breaking change in 0.29.0.

Another code change was replacing some occurrences of na::U3 with 3, which is due to version 0.26.0 introducing the usage of const generics.

The final code change was that nalgebra is now imported in Cargo.toml as itself instead of na to make its procedural macros introduced in 0.27.0 usable. An extern crate declaration was added to continue allowing the use of na. Actually using these macros is out of scope for this PR.

Unfortunately, 0.26.0 also came with a performance regression in nalgebra that makes Hypermine laggy with a 5x CPU slowdown on my computer. The fix is in dimforge/nalgebra#1140, which is now present in 0.31.2.

winit

set_cursor_grab takes an enum insteasd of a boolean, but the change is relatively straightforward. The code for grabbing the cursor came straight from the method documentation. ash-window was not compatible with winit 0.27.1, but patch 0.27.2 added in backwards compatibility, making the upgrade possible.

tracing-subscriber

The "chrono" feature option was made unavailable, and based on tokio-rs/tracing#1646, I believe it was replaced with "time". The justification was that the maintenance status of "chrono" was uncertain, causing support for it to be dropped.

All other crates

For all other crates, only cargo.toml was modified, making the upgrade easy. Due to interdependence between crates, I believe some had to be updated at the same time (like nalgebra and rand), but the upgrade process itself was trivial.

@patowen patowen changed the title Upgrade more dependencies Upgrade most dependencies mentioned by dependabot Aug 13, 2022
client/Cargo.toml Outdated Show resolved Hide resolved
@patowen patowen changed the title Upgrade most dependencies mentioned by dependabot Upgrade all dependencies mentioned by dependabot Aug 15, 2022
nalgebra's proc macros are unhygenic and hardcode the package name as
nalgebra when looking up functions, so using the matrix! and vector!
macros require the ability to reference nalgebra with its full name.
@patowen patowen marked this pull request as ready for review October 9, 2022 21:08
@patowen
Copy link
Collaborator Author

patowen commented Oct 9, 2022

Now that nalgebra has been updated, this is finally ready to review and merge.

@Ralith Ralith merged commit 0cd7626 into Ralith:master Oct 10, 2022
@patowen
Copy link
Collaborator Author

patowen commented Oct 10, 2022

Thanks for merging!

@patowen patowen deleted the upgrade-more-dependencies branch December 18, 2022 21:12
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