Skip to content

update semver compat. deps, fix cbindgen CI diff check #559

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

Merged
merged 4 commits into from
Apr 13, 2025

Conversation

cpu
Copy link
Member

@cpu cpu commented Apr 12, 2025

Notably fixes two dev-dep RUSTSEC issues:

Along the way I also dug into the main CI failure that's been flagging a cbindgen diff for the rustls.h header. I believe this is an upstream regression and so to fix it we have to temporarily pin our nightly version. I've opened an upstream issue for this (rust-lang/rust#139715).

@cpu cpu changed the title WIP: update semver deps, fix cbindgen diff WIP: update semver compat. deps, fix cbindgen diff Apr 12, 2025
@cpu cpu changed the title WIP: update semver compat. deps, fix cbindgen diff WIP: update semver compat. deps Apr 12, 2025
@cpu
Copy link
Member Author

cpu commented Apr 12, 2025

wip: try pinning nightly rust?

This seems to have done the trick, but is an unsatisfying solution.

@djc
Copy link
Member

djc commented Apr 12, 2025

Where do the guards come from? Is that related to the nightly somehow? Maybe there is a bug in a specific nightly? I agree that pinning the nightly is unsatisfying if we don't understand why it helps but in general pinning nightly seems like a good way to make CI more deterministic.

@cpu
Copy link
Member Author

cpu commented Apr 12, 2025

Where do the guards come from? Is that related to the nightly somehow?

The guards come from mapping cargo features to #define's. That in turn relies on expanding the crate content with specific features enabled. It's controlled in the cbindgen.toml file:

[defines]
"feature = read_buf" = "DEFINE_READ_BUF"
"feature = aws-lc-rs" = "DEFINE_AWS_LC_RS"
"feature = ring" = "DEFINE_RING"
"feature = fips" = "DEFINE_FIPS"
[parse.expand]
crates = ["rustls-ffi"]
features = ["read_buf", "aws-lc-rs", "ring", "fips"]

The parse.expand part requires nightly: mozilla/cbindgen#1015 We require the parse.expand part because of the macro we use for the numeric enums since #322

@cpu cpu force-pushed the cpu-semver-deps branch from b254541 to ceab284 Compare April 12, 2025 16:02
@cpu cpu changed the title WIP: update semver compat. deps update semver compat. deps, fix cbindgen CI diff check Apr 12, 2025
@cpu cpu marked this pull request as ready for review April 12, 2025 16:05
@cpu
Copy link
Member Author

cpu commented Apr 12, 2025

cpu marked this pull request as ready for review now

I think this is good to go. I've opened an upstream issue for what I believe is the nightly regression that broke our workflow. Depending on how that's triaged we can revisit the change here.

dependencies = [
"crossbeam-channel",
"crossbeam-epoch",
"crossbeam-utils",
"once_cell",
"loom",
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure loom is not supposed to be a lib dependency. IIRC most users guard with a cfg flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

cargo tree | grep loom and cargo tree --invert loom both produce no results. I assume this is something transitive and not our issue?

Copy link
Member Author

@cpu cpu Apr 13, 2025

Choose a reason for hiding this comment

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

It looks like it's hickory-resolver -> moka -> loom ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cpu cpu merged commit c911c07 into rustls:main Apr 13, 2025
46 checks passed
@cpu cpu deleted the cpu-semver-deps branch April 13, 2025 12:47
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