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

misc. dependency updates #23

Merged
merged 7 commits into from
Aug 29, 2023
Merged

misc. dependency updates #23

merged 7 commits into from
Aug 29, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Aug 25, 2023

Before diving into more meaningful work I wanted to update some dependencies (possibly a good time to consider adding a Dependabot config?). This branch updates all of the deps and dev deps (with the exception of jni (see #22), and semver compatible dependencies). Likely easiest to review commit-by-commit.

Additionally rolls in a small .gitignore tweak for CLion users and a tweak to the CI config to make it easier to run CI without opening a PR or making config changes.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Increasing the requirements within a semver-compatible range is unnecessary, so I'd avoid most of these bumps.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@cpu
Copy link
Member Author

cpu commented Aug 25, 2023

Increasing the requirements within a semver-compatible range is unnecessary, so I'd avoid most of these bumps.

Good point 😅 I'll remove those.

@complexspaces
Copy link
Collaborator

I agree with @djc on some of these bumps, especially for tokio. The CI changes look fine though 👍.

While it shouldn't be the sole decision making process, its become increasingly hard at work to make some of these increases without mass dependency duplication. So, leaving the ranges wider makes it easier to match ones that have more compatible sub-dependencies. For example this seems to duplicate socket2 and windows-sys and would push those downstream as well.

Helpful for Jetbrains IDE users.
This makes it easier to test CI before opening a PR without needing to
make changes to the CI config, simply name your branch with a `_dev`
suffix.
Additionally resolves use of deprecated fn.
Fixes one breaking change related to the `TLS_SERVER_ROOTS` type.
Also fix one removal of a deprecated fn. `with_min_level` was calling
`with_max_level` behind the scenes, so we switch to doing that ourselves
now that `with_min_level` is removed.
@cpu
Copy link
Member Author

cpu commented Aug 25, 2023

Fixed up to remove the semver compatible bumps.

@complexspaces
Copy link
Collaborator

@cpu Do you mind also running cargo update -p rustls before merging to drop the duplicate rustls-webpki dependency from the lockfile? Otherwise CI will need to build it twice every time.

Additionally resolves a warning from the use of the deprecated
`add_server_trust_anchors` fn.
@cpu
Copy link
Member Author

cpu commented Aug 25, 2023

@complexspaces Ahhh! Yes, done ☑️

I was prev. confused why after re-arranging some of the commits I had to remove the deprecated add_server_trust_anchors fix I had landed in the first go, but it was related to this duplication and the lockfile rustls version.

All sorted now, thank you.

Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

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

I sanity checked these changes against another codebase and confirmed nothing blew up. LGTM 😄

@cpu cpu merged commit 5462d83 into rustls:main Aug 29, 2023
11 checks passed
@cpu cpu deleted the cpu-dep-updates branch August 29, 2023 13:04
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

3 participants