search

motioneye-project / motioneye

A web frontend for the motion daemon.
GNU General Public License v3.0
3.97k stars 651 forks source link

Why do the installation instructions encourage "sudo pip install" #3066

Open robvdl opened 1 week ago

robvdl commented 1 week ago

First of all, I personally don't run this project (I run a zoneminder installation), but I am a Python developer and a friend came across it and installed it.

I was a bit shocked to see "sudo pip install" instructions, especially because that is going away in future versions of pip.

https://stackoverflow.com/questions/61452582/why-is-using-sudo-pip-a-bad-idea

https://pages.charlesreid1.com/dont-sudo-pip/

Why are we encouraging this sort of installation? Why can't we recommend users to create a virtualenv instead and teach good practices?

I don't want to come across negative, but "sudo pip install" is always a bad idea.

MichaIng commented 1 week ago

motionEye has a low number of dependencies without any specific version requirements. So IMO a venv/virtualenv does not add any benefit. If anyone is running into any conflicts with the system-wide installation, I would be interested to know, since it must be a very rare and special case.

However, you are right that Python generally moves towards venv only, recet Debian and Ubuntu install flag files with their distro packages which prevent pip from installing system-wide, without adding another flag or config option. So yes, we should switch to venv in our install instructions.

Please open a PR for the readme, if someone finds time. Also the install tests should be updated accordingly.

robvdl commented 1 week ago

I dunno if I agree with the "does not add any benefits" because my friend did run into conflicts. I'll have a look at it when I can, but I'm going to try zoneminder with that setup instead.

There has to be a better way to install it. But I thought that we got over that "sudo pip install" thing years ago, almost nobody does that anymore.

MichaIng commented 1 week ago

Did you even read the second half of my comment?

robvdl commented 1 week ago

You're taking about the --user flag?

MichaIng commented 1 week ago

No, that is still prevented on Debian and Ubuntu. This part of my comment is what I mean:

So yes, we should switch to venv in our install instructions.

Please open a PR for the readme, if someone finds time. Also the install tests should be updated accordingly.

It is one more package python3-venv and two more commands to set it up and load it. The problem is that the systemd service needs to be changed as well. Not yet sure how to do this best. We would need to detect the venv in the linux_init script, if present, and in that case load it and get the meyectl path accordingly.

robvdl commented 1 week ago

I'll have to have a good read of the code first to get an understanding of the installation, it looks like motioneye_init.py invokes a shell script which makes me wonder what the point of that is and why not just run the shell script directly. But there may have been a reason.

Makes me wonder "who not just make install" when there already is a Makefile there in the project root.

MichaIng commented 1 week ago

Here is the shell script and systemd unit: https://github.com/motioneye-project/motioneye/tree/dev/motioneye/extra

I was wondering as well what the Python script purpose is. At least it is installed into PATH. However now it comes in handy, since it assures the shell script is executed within the venv, so that we can know the location of meyectl.

The Makefile is for localization only. It is indeed misleading there. I already wanted to move it into the l10n dir. But workflows need to be adjusted as well then.