Skip to content

Conversation

djc
Copy link
Member

@djc djc commented Sep 8, 2025

No description provided.

@djc djc requested review from cpu, ctz and complexspaces September 8, 2025 14:52
Copy link
Member

@ctz ctz left a comment

Choose a reason for hiding this comment

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

Context: #161

@complexspaces
Copy link
Collaborator

complexspaces commented Sep 8, 2025

Thanks for opening this PR! For some context I tried doing this locally when I did the same for arboard (1Password/arboard#201) a few weeks ago. I found that it was basically impossible to get the version bump to cleanly apply for the sake of CI testing though, so I dropped the idea for the time being.

Due to rustls-native-certs (via schannel) being stuck on an older version of windows-sys with no range flexibility, the cargo update ... command used in CI to force an update to check version compatibility wasn't moving to the right version, and explicitly telling it to bump a revision of windows-sys via windows-sys@... didn't work as another result. You can see that in the relevant Clippy job for this PR its not checking the new version either due to this resolution constriction.

I wasn't confident moving forward with a PR as I didn't have the time to go manually check all the type and function signatures we use for breaking changes.

@complexspaces
Copy link
Collaborator

Unrelated to the testing problem but it looks like they finally fixed the definition of CERT_CHAIN_PARA in windows-sys 0.60-0.61, although it doesn't take Windows 7 compatibility into account like our definition does.

@djc
Copy link
Member Author

djc commented Sep 9, 2025

Fair, I'll prod the schannel folks a bit.

@djc djc force-pushed the windows-sys-0.61 branch from ce0c711 to 547c723 Compare September 9, 2025 09:38
@djc
Copy link
Member Author

djc commented Sep 9, 2025

schannel 0.1.28 has been released with a bump to windows-sys 0.61. Added a commit that updates the Cargo.lock to include the new stuff.

@djc djc force-pushed the windows-sys-0.61 branch from 547c723 to b7af4c3 Compare September 9, 2025 09:41
@djc djc force-pushed the windows-sys-0.61 branch from b7af4c3 to 740bd23 Compare September 9, 2025 14:30
@complexspaces
Copy link
Collaborator

It looks like there were some int -> ptr type changes in the latest windows-sys changes. If you'd like I can try and port my arboard abstraction trait(s) for that over to this repository tonight.

@djc
Copy link
Member Author

djc commented Sep 9, 2025

It looks like there were some int -> ptr type changes in the latest windows-sys changes. If you'd like I can try and port my arboard abstraction trait(s) for that over to this repository tonight.

Sounds good to me!

@complexspaces
Copy link
Collaborator

CI looks happy with the trait EnginePtr type inference shim, so this looks good to go now.

I will refrain from approving since I pushed my own code to the branch though 😄. Feel free to double check it makes sense to you and then consider this comment a 2nd approval for merge if all looks well.

@djc
Copy link
Member Author

djc commented Sep 10, 2025

Squashed your additional commit and made some stylistic tweaks.

LGTM. ✅

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