-
Notifications
You must be signed in to change notification settings - Fork 75
Bump libloading version to 0.8.6 #261
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
Conversation
The kryoptic CI failure is addressed with #258. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 thanks!
Kryoptic is not happy 😿 |
I think the test is overly tied to specific versions. Converting them to check if the version is greater than some number is better. The minor check can be dropped IMO. As a compromise I think the version could be just |
As mentioned in the previous comment, the kryoptic failure is fixed in #258, which is now merged so merging this should be green (but I can rebase it here if you prefer). |
The version numbers should not change that fast with the pace Oasis works on them. So I would be for keeping them as they are now. Actually, we have now the PKCS#11 3.2 in kryoptic now, while rust-cryptoki is still on 3.1 but with the backward compatibility, it should not matter functionally (just the this test complains). |
Please do. I work in security so I know "red means bad, green means good" 🫠 😉
Ah, right, this is for PKCS#11 version 🤦♂️ still, changing the test when the backend returns higher numbers seems kind of off 🤔 |
Signed-off-by: Jakub Jelen <[email protected]>
done
Actually, its a pkcs11 interface version. There is only 3.0 and 3.2 now. I took this test failing as a good feedback that the version is properly propagated to the calling application (as for now it is the only indication as we do not have anything else to test) and that I do not mess up in the future for example by not enabling the (currently non-default) pkcs11_3_2 feature of kryoptic in Fedora build in the future. Doing the comparison on greater_equal or something will match a lot of bad numbers, which is also something I would like to avoid. Just printing would make it really So the only reasonable option I can find would be just to allow 3.0 and 3.2 values right now. This will also allow the tests to work for other people who are not on 3.2 yet. I will do it in separate PR as this is a bit off-topic here. |
Resolves the discussion that started in the following PR: parallaxsecond#261 (comment) Signed-off-by: Jakub Jelen <[email protected]>
Filled #262. |
Resolves the discussion that started in the following PR: parallaxsecond#261 (comment) Signed-off-by: Jakub Jelen <[email protected]>
Resolves the discussion that started in the following PR: parallaxsecond#261 (comment) Signed-off-by: Jakub Jelen <[email protected]>
Resolves the discussion that started in the following PR: parallaxsecond#261 (comment) For now, only PKCS#11 3.0 and 3.2 interface versions are defined so just take these two instead of depending on one of them. Signed-off-by: Jakub Jelen <[email protected]>
the libloading version 0.7.0 is a bit dated and makes downstream packaging (for example for Fedora) more complicated. I did not see any huge incompatible changes locally (but it might need some more love for windows -- hope the CI has a coverage).