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

Check min versions of transitive depencencies in CI #1115

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

nepalez
Copy link
Contributor

@nepalez nepalez commented Sep 20, 2023

To prevent problems like #1090 maybe it makes sense to check transitive dependencies in the CI.

I also had to fix another dependency openssl restricted from 0.10.29 -> 0.10.36 (the latest one is 0.10.55)

@nepalez nepalez marked this pull request as draft September 20, 2023 17:03
@nepalez nepalez force-pushed the ci-minimal-versions branch 6 times, most recently from 2f81410 to 500b412 Compare September 20, 2023 21:19
@nepalez nepalez marked this pull request as ready for review September 20, 2023 21:23
@Jarema
Copy link
Member

Jarema commented Sep 21, 2023

That is something we had on our list, thanks for doing this!
Will take a look.

@Jarema
Copy link
Member

Jarema commented Sep 29, 2023

@nepalez this has a conflict as I merged your CI improvements PR :)

@nepalez
Copy link
Contributor Author

nepalez commented Sep 29, 2023

I rebased the branch over the freshy master, and also fixed the problem with openssl.

The fix is simple: after downgrading to minimal dependencies with -Zminimal-dependencies flag, I upgraded the openssl-sys and openssl back to prevent the problem that for the legacy version of openssl-sys there're no corresponding libssl-dev package installed on ubuntu-latest anymore.

I could instead try extracting the proper version of libssl-dev from the crate and then downgrade the installed package... but it looks like a huge overkill. We can stop on checking the other dependencies, leaving the openssl unchecked because... well, this pair of crates are widely used and it's less possible they contain some invalid dependencies under the hood.

Copy link
Collaborator

@caspervonb caspervonb left a comment

Choose a reason for hiding this comment

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

lgtm, leaving it to @Jarema discretion to merge or not.

@Jarema Jarema merged commit 3c9b6cb into nats-io:main Nov 9, 2023
11 checks passed
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.

3 participants