-
Notifications
You must be signed in to change notification settings - Fork 40
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
[v1.1.0] Add OPENSSL prefix for sk_* macros #35
[v1.1.0] Add OPENSSL prefix for sk_* macros #35
Conversation
I think in general there should be a separate branch for 1.1.x here with a different major version (e.g. "2.0.0+openssl.1.1.0"), but having this available in the meantime is definitely a good idea. The configuration approach for vibe-d:tls is also a good point. The current version selection is indeed a bit obscure. |
… version constants. See #1958 and D-Programming-Deimos/openssl#35.
deimos/openssl/stack.d
Outdated
int function(const(void)*, const(void)*) OPENSSL_sk_set_cmp_func(_STACK* sk, ExternC!(int function(const(void)*, const(void)*)) c); | ||
_STACK* OPENSSL_sk_dup(_STACK* st); | ||
void OPENSSL_sk_sort(_STACK* st); | ||
int OPENSSL_sk_is_sorted(const(_STACK)* st); |
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.
Shouldn't the above functions only be in the DeimosOpenSSLv1.1
block? As far as I could see they don't exist in the earlier versions.
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.
It would be more correct, but IMHO having them defined nonetheless might simplify downstream usage (by removing the need to version if only stack
is used for example). It seemed to me this approach has upside without visible downside, but I can version it out if you prefer.
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.
The question is a bit how backwards compatibility works for OpenSSL. As long as changes are just either permanently removing symbols, or adding new ones with different names, we could try to go this route and support a sliding range of versions. Just as soon as they'd change signatures or things like that we'd be screwed. It also makes it difficult to properly integrate new versions using a three-way diff or by running htod over the latest C headers.
Of course, if these points can be ruled out, having all declarations at once would be the preferable solution.
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.
Hum you're right, I moved that code in the version
block.
This should allow Vibe.d to compile, provided they compiler with the right flag.
aac0d01
to
1b270cb
Compare
Currently Vibe.d doesn't compile with OpenSSL v1.1.0. This is the default on ArchLinux, for example.
This is by no means a complete support for v1.1.0 but should allow Vibe.d to compile if they provide the right flag at compilation.
@s-ludwig: Does that approach works for you ? Will the
tls
subpackage need aconfigutation
? IIRC it should.Man page