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

py310-311: Re-include openssl 1.x until all py-spk migrated over #5818

Merged
merged 2 commits into from
Jul 22, 2023

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Jul 22, 2023

Description

py310-311: Re-include openssl 1.x until all py-spk migrated over

Fixes #5800

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@th0ma7 th0ma7 mentioned this pull request Jul 22, 2023
10 tasks
spk/python310/Makefile Outdated Show resolved Hide resolved
@th0ma7 th0ma7 merged commit bb4770c into SynoCommunity:master Jul 22, 2023
@th0ma7 th0ma7 deleted the revert-openssl3 branch July 22, 2023 17:36
@hgy59
Copy link
Contributor

hgy59 commented Jul 22, 2023

@th0ma7 I doubt that this migration path will work.
If you build python with openssl 1.1 and 3.x you are not sure whether, when building a package for sickchill, the cryptography wheel will be linked against openssl 1.1 or 3.x.
But you must ensure that the cryptography wheel delivered with the python packages link against openssl 3.x

The problem is, that packages depending on cryptography wheel from installed python, install the wheels once into the venv at installation time.
If we cannot guarantee that the cryptography wheel links against openssl 3.x, we will never be able to get rid of openssl 1.1 libraries.

My suggestion was that this error exists only in packages that do not include cryptography as cross compiled wheel but install it from site-packages of installed python.
But I got the same error in homeassistant like #5800 (comment) when updating python.
Though this could be fixed by adding export LD_LIBRARY_PATH=${SYNOPKG_PKGDEST}/lib to service-setup as the homeassistant package includes the cryptography wheel and it's dependencies (i.e. libssl.so.1.1)

If we cannot force cryptography wheel to depend on openssl 3.x I propose to update all packages depending on cryptography wheel now instead of building python packages with openssl 1.1 and 3.x.

@mreid-tt
Copy link
Contributor

So, should this PR be reverted?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Jul 22, 2023

@hgy59 you got an excellent point and in fact I am totally in agreement with your last statement: the real fix is to re-release all python packages that has a relationship towards openssl, which is mostly due to cryptography. Further as we can now easily package newer cryptography versions.

This being said, this PR really only is a "patch" so packages that aren't yet updated continue working (e.g. still able to find appropriate openssl.so.1.1 file), that's it. Honestly this only is a temporary fix in order to have something "working" ASAP so online packages do no break than waiting for a full-fledge fix, further as the openssl3 only version was downloaded and already caused issues.

@mreid-tt I don't believe it's needed although I'll create a new PR to enforce using openssl3 only for all python packages who may require it, thus bumping all package versions.

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.

4 participants