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

fix: adjust liboqs pkgconfig version detection to new scheme #246

Merged

Conversation

tranzystorekk
Copy link
Contributor

Noticed this part wasn't touched after the versioning update, this works correctly with it.

@thomwiggers
Copy link
Contributor

Do we need to do a patch release for this or are we good?

Relatedly I'm thinking we should probably split out the vendored stuff in its own crate, because I suppose we might want to separately bump oqs, oqs-sys's minimum requirement and the default-included code...

@tranzystorekk
Copy link
Contributor Author

The old code tries to match pkgconfig with a nonsense version range, which doesn't break anything AFAIU, but also plain doesn't work as intended, so a patch seems desirable.

As for your second point, I'm not sure I follow, or maybe I don't see the benefit of separating the vendored code.

@thomwiggers
Copy link
Contributor

Imagine liboqs publishes version 0.9.1, then I would like to package oqs-sys version 0.9.x+liboqs-0.9.1. But that would also bump the minimum requirement for pkgconfig even if it should still be compatible with liboqs 0.9.0.

Moving the vendored code into a separate crate would allow Cargo's dependency resolver to resolve such conflicts. I suppose that alternatively we could get rid of the version autodetection but that means we have yet another version code point to keep track of when updating...

@thomwiggers thomwiggers added this pull request to the merge queue Oct 19, 2023
@tranzystorekk
Copy link
Contributor Author

But that would also bump the minimum requirement for pkgconfig even if it should still be compatible with liboqs 0.9.0.

Huh, I haven't thought about that, but my reasoning was if we declare liboqs-0.9.1 we should vendor or accept via pkgconfig only versions that include at least fixes from that baseline version, so patch numbers >=1.

@thomwiggers
Copy link
Contributor

Patch releases could also include improvements, such as new implementations, though. It's a bit tricky.

Anyway I don't care about this too much, we'll see if the OS vendors yell at us down the line.

Merged via the queue into open-quantum-safe:main with commit 8ff539b Oct 19, 2023
10 checks passed
@tranzystorekk tranzystorekk deleted the fix-version-detection branch October 19, 2023 08:27
@tranzystorekk
Copy link
Contributor Author

Looks like 0.9.1 wasn't released on crates.io, presumably a simple omission?

@thomwiggers
Copy link
Contributor

I was waiting on the merge queue and then entered my weekend. I have just released it.

(I should ideally set up CI to do all of this for me... but that's a bunch of engineering effort.)

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