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

Should Calibre-Web's requirements.txt install cryptography ? #3183

Closed
holta opened this issue Oct 10, 2024 · 10 comments
Closed

Should Calibre-Web's requirements.txt install cryptography ? #3183

holta opened this issue Oct 10, 2024 · 10 comments
Labels

Comments

@OzzieIsaacs
Copy link
Collaborator

I could reproduce it with the newest nightly version and I‘ve added cryptography to the requirements file.
I‘ll check what the reason for this was

@holta
Copy link
Author

holta commented Oct 27, 2024

I‘ll check what the reason for this was

Awesome if you find out, Many Thanks @OzzieIsaacs !

@holta
Copy link
Author

holta commented Oct 27, 2024

@holta
Copy link
Author

holta commented Oct 27, 2024

@OzzieIsaacs' fix (c2ffa94) is tested on Raspberry Pi OS, many thanks to @EMG70 here:

@C0rn3j
Copy link

C0rn3j commented Oct 28, 2024

I don't get why python3-netifaces is installed outside of the venv.


This entire section should be removed, no sane operating system will let you install as --user nor --system today, you use a venv, otherwise you break the OS:

Ubuntu:

On some Ubuntu versions, you may encounter the error can't combine user with prefix. This is a [known bug](https://github.com/pypa/pip/issues/3826) and can be resolved by installing the requirements with the --system flag:
./venv/bin/python3 -m pip install --system -r requirements.txt

If the above has trouble pip installing cryptography

I don't get why this section exists.
If there is a problem, describe WHY there is a problem.
If it is a bug, link the bug report.
If there is not a bug reported yet, report it.
This looks like a downstream hack as is, even if it may actually be sensible.


cryptography is in requirements, so mentions to install it manually should be yeeted - c2ffa94

@holta
Copy link
Author

holta commented Oct 28, 2024

I don't get why python3-netifaces is installed outside of the venv.

I don't know the reason. But I know it's been important for a few years now. Is there perhaps a much better way!?

If the above has trouble pip installing cryptography

I don't get why this section exists. If there is a problem, describe WHY there is a problem. If it is a bug, link the bug report.

Personally, I don't agree, and here's why:

pip installation of the cryptography package (on Raspberry Pi OS especially, but also other OS's) has frequently been "broken", many different times (often for many weeks, sometimes more, for a variety of reasons) over the past half-decade.

While everyone would certainly love that pattern to end (!) it seems very premature to wipe out vital tips. A better plan (in my view) is to help people help themselves, by documenting valuable/proven workarounds that have (often!) worked extremely well.

🙏

@C0rn3j
Copy link

C0rn3j commented Oct 28, 2024

Is there perhaps a much better way!?

Installng it in the venv. venv and pip make sense to install on the system as they are required to setup the venv.

Rest should be installable in the venv.

pip installation of the cryptography package (on Raspberry Pi OS especially, but also other OS's) has frequently been "broken", many different times (often for many weeks, sometimes more, for a variety of reasons) over the past half-decade.

The project should pin a version that works when that happens, this should imo not be on the user.

I am not against having the workaround in the documentation, but it should be documented why it is there.

Something akin to "Sometimes the latest stable cryptography fails to install on specific architectures, if that happens and a bug has already been reported without a decent workaround, you can try using the library from the system"

I am not a project member, so what I want does not matter in the end :)

@holta
Copy link
Author

holta commented Oct 28, 2024

I am not against having the workaround in the documentation, but it should be documented why it is there.

I hope https://github.com/janeczku/calibre-web/wiki/Manual-Installation/_compare/0a20d71ec2af591cf4507a3f62e18ec597bd4acc...bc2892e4acc1b4b9a2bf92cf018f78047ead64a4 is an improvement.

@OzzieIsaacs
Copy link
Collaborator

OzzieIsaacs commented Oct 28, 2024

The Manual installation describes the procedure for installing the latest stable version 0.6.23. In the next release netifaces is replaced by netifaces-plus, as it was constantly causing problems during installation (especially on windows-> no wheel file available for python >3.10).
The only thing I can imagine whynetifaces needs to be installed outside the venv is the same problem on one of your linux installations.

Why did this happen?
"Cryptography" is used by calibre-web for a longer time to encrypt some of the external access passwords (email password, ldap login password ..). It was never listed as requirement, it was installed by chance, by installing "advocate". As Advocate is no longer maintained and netifaces is hard to install (at least on windows) I decided to include it in the calibre-web codebase.
I took the latest version from the master branch to include. I also took the needed additional packages from this version and I wasn't aware that the advocate version installed via pip and requirements file used a different set of packages (see following screenshot)
advocate_

Pyopenssl was installing cryptography before, but with the latest version it was not doing it anymore.

I merged my change, ... didn't run the tests, ... went on vacation, ... was happy and relaxed, ... forgot everything from before vacation .....

From my point of view the issue is solved and I close it now.

@jvonau
Copy link
Contributor

jvonau commented Dec 1, 2024

I don't get why python3-netifaces is installed outside of the venv.

I don't know the reason. But I know it's been important for a few years now. Is there perhaps a much better way!?

Just a little reminder of past adventures reference: iiab/iiab#3503 iiab/iiab#3538

If the above has trouble pip installing cryptography
I don't get why this section exists. If there is a problem, describe WHY there is a problem. If it is a bug, link the bug report.

Personally, I don't agree, and here's why:

pip installation of the cryptography package (on Raspberry Pi OS especially, but also other OS's) has frequently been "broken", many different times (often for many weeks, sometimes more, for a variety of reasons) over the past half-decade.

While everyone would certainly love that pattern to end (!) it seems very premature to wipe out vital tips. A better plan (in my view) is to help people help themselves, by documenting valuable/proven workarounds that have (often!) worked extremely well.

🙏

iiab/iiab#3498 One of my goals when developing software for easy redistribution and deployment was not to install any compilers for a smaller base image and not having ready access to compiles should something evil befall the installation. Let the bad actors work a bit harder should the machine get hacked, just my 2 cents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants