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

Regression in OpenSSL 3.0.2 vs. 1.1.1 on Android/NDK builds #427

Open
jfinkhaeuser opened this issue May 12, 2022 · 12 comments
Open

Regression in OpenSSL 3.0.2 vs. 1.1.1 on Android/NDK builds #427

jfinkhaeuser opened this issue May 12, 2022 · 12 comments

Comments

@jfinkhaeuser
Copy link

On very few platforms, namely linux-armv4 and linux-ppc, the generated-config directory contains a reference to an atomic library in the 3.0.2 version, but not in the 1.1.1 version.

$ grep atomic $(find subprojects/openssl-3.0.2/generated-config/ -type f -name meson.build)
subprojects/openssl-3.0.2/generated-config/archs/linux-armv4/asm/meson.build:  'atomic',
subprojects/openssl-3.0.2/generated-config/archs/linux-armv4/no-asm/meson.build:  'atomic',
subprojects/openssl-3.0.2/generated-config/archs/linux-ppc/asm/meson.build:  'atomic',
subprojects/openssl-3.0.2/generated-config/archs/linux-ppc/no-asm/meson.build:  'atomic',

The main meson.build decides that the Android NDK is Linux, which for most intents and purposes is entirely reasonable. However, the Linux NDK does not provide an atomic library; therefore, the NDK build breaks for these platforms.

As a suggestion, split openssl_libraries into required and optional libraries, so that this distinction can be made at compile time.

@eli-schwartz
Copy link
Member

@nazar-pc

@nazar-pc
Copy link
Contributor

Those files are auto-generated.

These indicate that the change is probably an upstream issue:
openssl/openssl#16551
openssl/openssl#16584
openssl/openssl#18059

@jfinkhaeuser
Copy link
Author

They're not generated upstream, though.

@nazar-pc
Copy link
Contributor

Sure, but I'd like to first confirm that those platforms are even supported by OpenSSL 3.0 properly before trying to figure out what is wrong in the Meson wrap.

@amyspark
Copy link
Contributor

Sure, but I'd like to first confirm that those platforms are even supported by OpenSSL 3.0 properly before trying to figure out what is wrong in the Meson wrap.

👋 As of nodejs/node#42616, linux-ppc got totaled from Node.

As for the -latomic entry, you'll find it in https://github.com/nodejs/node/blob/b6d62f7fad43802a0820ab6c1945b556468c6bab/deps/openssl/openssl/Configurations/10-main.conf#L702-L704 -- linux-armv4 inherits from that configuration in OpenSSL 3.

@jfinkhaeuser
Copy link
Author

I would love to understand what this means for me, but I think that's more for the others, right?

@nazar-pc
Copy link
Contributor

I think that means linux-armv4 and linux-ppc should be removed unless someone actually using those will step up to maintain it. I have no personal interest in those platforms.

@jfinkhaeuser
Copy link
Author

The platforms are supported by OpenSSL just fine, to get back to the earlier question. It's node that's introducing the problem, and node isn't part of OpenSSL.

I think the right way would be to support OpenSSL without the use of node. Node helps a lot, but it also adds a lot of cruft that isn't needed for OpenSSL compilation. OpenSSL's Configure script is quite robust, and produces a (Perl) configuration that can be used to generate whatever is necessary (node does that), so about 95% of the required knowledge is already part of OpenSSL. The other 5% is mapping meson's understanding of platforms to those that OpenSSL uses, and node can serve as a good inspiration for that - but that's just a lookup table, nothing that requires a dependency.

TL;DR is, I think this is a case of how you adapted OpenSSL biting you, and it's probably worth a re-think.

@nazar-pc
Copy link
Contributor

From what I understand no one was brave enough to tackle it for a long time and I found an interesting workaround that allows us to have OpenSSL with Meson.

If you can improve it or make less hacky I think everyone will be even more happy.

@jfinkhaeuser
Copy link
Author

I understand.

I have a ton on my plate for the next few weeks, but since I will need to get NDK builds sooner or later, I think it's something I can tackle then. In the meantime, this has helped me figure things out a bit better, so thank you as well!

@jfinkhaeuser
Copy link
Author

I think I will have to decline working on this. It's not that I really do not want to - it's more that I found a workaround that provides some other benefits as well. Since I wanted to provide packages anyway, I've been looking into conan - and with conan wrapping meson, I can provide the dependencies from conan, and use subprojects only as a fallback/for development purposes. That serves me better in the long run.

FWIW, the conan recipe builds OpenSSL just fine on android. But it's also ~750 lines long, and still just wraps OpenSSL's own build system: https://github.com/conan-io/conan-center-index/blob/master/recipes/openssl/3.x.x/conanfile.py

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

No branches or pull requests

4 participants