Skip to content

chore: update dependencies to newer versions#740

Closed
ctron wants to merge 1 commit intoCycloneDX:mainfrom
ctron:feature/update_deps_1
Closed

chore: update dependencies to newer versions#740
ctron wants to merge 1 commit intoCycloneDX:mainfrom
ctron:feature/update_deps_1

Conversation

@ctron
Copy link
Contributor

@ctron ctron commented Jul 9, 2024

Mainly to update jsonschema -> reqwest -> hyper to a 1.x version.

@ctron ctron requested a review from a team as a code owner July 9, 2024 13:57
@ctron ctron force-pushed the feature/update_deps_1 branch 2 times, most recently from cd2585d to a17ed16 Compare July 9, 2024 14:40
@ctron
Copy link
Contributor Author

ctron commented Jul 9, 2024

Sorry, but I have no idea what to do about that nix flake issue.

@Shnatsel
Copy link
Contributor

Shnatsel commented Jul 9, 2024

I think a better idea is to just disable the resolve* features that pull in hyper. We really don't want to pull in the entirety of reqwest, hyper and tokio unconditionally from a serialization/deserialization crate.

Also that's definitely a no on the packageurl upgrade - that's certainly semver-breaking, and if we're breaking semver we should just switch to the purl crate which is much better all around.

@ctron
Copy link
Contributor Author

ctron commented Jul 9, 2024

I think a better idea is to just disable the resolve* features that pull in hyper. We really don't want to pull in the entirety of reqwest, hyper and tokio unconditionally from a serialization/deserialization crate.

Also that's definitely a no on the packageurl upgrade - that's certainly semver-breaking, and if we're breaking semver we should just switch to the purl crate which is much better all around.

Ok, I'll try to update the PR tomorrow.

@Shnatsel
Copy link
Contributor

The jsonschema crate pulls in a lot of dependencies and shouldn't be a runtime dependency at all. We should turn it into a dev-dependency. There is now an issue about it: #741

@ctron ctron force-pushed the feature/update_deps_1 branch from a17ed16 to c9abc59 Compare July 11, 2024 06:28
@ctron
Copy link
Contributor Author

ctron commented Jul 11, 2024

Ok, I changed it to the following:

  • packageurl is back to 0.3
  • the jsonschema is an optional dependency, enabled by default. As the validate method is a public method, that would break the API. This can be turned into off-by-default with the next breaking release.
  • Also, jsonschema has default-features=false, which is sufficient for this crate

Mainly to update jsonschema -> reqwest -> hyper to a 1.x version.

Also, this allows to disable the jsonschema dependency. Currently, it is
enabled by default to be compatible with previous versions. But this
can be changed in the next breaking release to exclude it by default.

Signed-off-by: Jens Reimann <ctron@dentrassi.de>
@ctron ctron force-pushed the feature/update_deps_1 branch from c9abc59 to 7cbcf22 Compare July 11, 2024 06:39
@Shnatsel
Copy link
Contributor

Shnatsel commented Jul 11, 2024

Turning jsonschema into an optional feature is a breaking change if someone was using the crate with default-features = false, but I don't think anyone has been doing that on account of the crate not having any features. So that sounds like a reasonable short-term fix.

The Nix flake CI is failing due to increased MSRV. It expects 1.70, while this PR bumps it to 1.74. 1.74 is very recent, and I think we would like to stick to 1.70 for now.

What is the motivation for the upgrades? We could consider bumping it if it's really worth it.

@Shnatsel
Copy link
Contributor

Since this appears to have stalled, I started pruning the dependency tree myself. See #744 and #746

@ctron
Copy link
Contributor Author

ctron commented Jul 16, 2024

Since this appears to have stalled, I started pruning the dependency tree myself. See #744 and #746

Sorry for that. Yes, that might be quicker.

@Shnatsel
Copy link
Contributor

Shnatsel commented Aug 6, 2024

We've dropped reqwest entirely in #744 and #750, so I'm going to go ahead and close this.

@Shnatsel Shnatsel closed this Aug 6, 2024
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