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 OpenSSL version check for 3.x.x #2650

Merged
merged 2 commits into from
May 13, 2022
Merged

Conversation

s-ludwig
Copy link
Member

See #2647

@s-ludwig
Copy link
Member Author

@dd86k, can you test if this fixes the build on your machine?

@dd86k
Copy link

dd86k commented May 11, 2022

Hi @s-ludwig

After cloning 0.9.4+commit.56.g86e94971 (from master) and having it added to dub with add-local, does not seem to work. Do say if I'm doing it wrong.

Here's the output I get under my Ubuntu 22.04 LTS VM (libssl-dev is at version 3.0.2-0ubuntu1.1):

output.log

@s-ludwig
Copy link
Member Author

Thanks for trying it out! It looks like the dub add-local part has worked as expected, but it seems like that particular commit doesn't contain the changes here. I couldn't find the commit g86e94971, so I guess that could be a merge commit generated by a git pull? Can you try again with git checkout fix_openssl_3_version_check, or use https://github.com/vibe-d/vibe.d/archive/939b13cd5244d7ac4f52e238d925ca1a6c3de00c.zip directly?

@dd86k
Copy link

dd86k commented May 12, 2022

Oh, my bad, forgot to checkout to fix_openssl_3_version_check, the description is now at vibe-d-0.9.4+commit.57.g939b13cd.

Here is the new output (I get the same results using ldc2):

output-dmd.log

Hope this is more helpful.

@s-ludwig
Copy link
Member Author

I've added some 3.x specific wrappers. If this doesn't work, I'll look into officially adding 3.x as a CI test.

@dd86k
Copy link

dd86k commented May 12, 2022

ERR_raise seems to be the only undefined reference left:

/usr/bin/ld: /tmp/.dub/build/vibe-d-0.9.4+commit.58.g7349013a/openssl-debug-linux.posix-x86_64-dmd_v2.099.1-530F7268A1390F57959C8EAB656E0DF5/libvibe-d_tls.a(openssl_1d3_448.o): in function `_D4vibe6stream7openssl11setSSLErrorFNbNeAyaQdiQgZv':
/home/dd/dev/vibe-test/../vibe.d/tls/vibe/stream/openssl.d:1411: undefined reference to `ERR_raise'
collect2: error: ld returned 1 exit status
Error: linker exited with status 1
/usr/bin/dmd failed with exit code 1.

@s-ludwig
Copy link
Member Author

Okay, ERR_raise is actuallyERR_raise_data under the surface, pushed another fix.

@Geod24
Copy link
Contributor

Geod24 commented May 13, 2022

Tested locally:

tls/vibe/stream/openssl.d(299,45): Error: no property `GEN_DNS` for type `deimos.openssl.x509v3.GENERAL_NAME_st`
tls/vibe/stream/openssl.d(318,46): Error: no property `GEN_IPADD` for type `deimos.openssl.x509v3.GENERAL_NAME_st`
tls/vibe/stream/openssl.d(1173,8): Error: no property `GEN_DNS` for type `deimos.openssl.x509v3.GENERAL_NAME_st`
tls/vibe/stream/openssl.d(1178,8): Error: no property `GEN_IPADD` for type `deimos.openssl.x509v3.GENERAL_NAME_st`
tls/vibe/stream/openssl.d(1186,33): Error: function `deimos.openssl.x509v3.GENERAL_NAMES_free(stack_st_GENERAL_NAME* a)` is not callable using argument types `(STACK_OF!(GENERAL_NAME_st)*)`
tls/vibe/stream/openssl.d(1186,33):        cannot pass argument `gens` of type `STACK_OF!(GENERAL_NAME_st)*` to parameter `stack_st_GENERAL_NAME* a`
tls/vibe/stream/openssl.d(1191,33): Error: no property `GEN_DNS` for type `deimos.openssl.x509v3.GENERAL_NAME_st`

Cherry-picking the first commit in #2652 fixes the GEN_{DNS,IPADD} issue. Looking into the last one, might push to your branch @s-ludwig .

@s-ludwig
Copy link
Member Author

Okay, so I've installed an Ubuntu 22.04 VM and tested this successfully now. I couldn't reproduce the GEN_* issues, though, did you maybe force compilation with the Deimos 2.x bindings instead of staying on 1.x?

@s-ludwig s-ludwig force-pushed the fix_openssl_3_version_check branch from 05aa2c8 to c79cffb Compare May 13, 2022 09:23
@Geod24
Copy link
Contributor

Geod24 commented May 13, 2022

I couldn't reproduce [...]

Right, I edited my dub.selections.json to test some changes. Tested locally, LGTM.

@Geod24 Geod24 merged commit c38182c into master May 13, 2022
@Geod24 Geod24 deleted the fix_openssl_3_version_check branch May 13, 2022 10:07
@Geod24
Copy link
Contributor

Geod24 commented May 13, 2022

Could you make a new release ?

@s-ludwig
Copy link
Member Author

Let's try to also get the two URL PRs in (or at least #2653 for now), I'll prepare the change log in the meantime.

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