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

backintime-qt script uses non-absolute python3 call #1316

Closed
eddgrant opened this issue Sep 28, 2022 · 26 comments
Closed

backintime-qt script uses non-absolute python3 call #1316

eddgrant opened this issue Sep 28, 2022 · 26 comments
Labels
Discussion decision or consensus needed

Comments

@eddgrant
Copy link

Hey folks,

This was baffling me for ages, but I had a flash of inspiration which led me to understand what was going on so I thought I'd share what I have found.

At some point recently the backintime GUI stopped working for me. I initially thought this might have been due to my recent upgrade to Ubuntu 22.04 LTS but none of the solutions I found helped my particular issue.

When I run /usr/bin/backintime-qt I got the following error:

Traceback (most recent call last):
  File "/usr/share/backintime/qt/app.py", line 35, in <module>
    import qttools
  File "/usr/share/backintime/qt/qttools.py", line 21, in <module>
    from PyQt5.QtGui import (QFont, QColor, QKeySequence)
ModuleNotFoundError: No module named 'PyQt5'

My initial thought was that I was somehow missing the PyQt5 package so I checked using pip3 freeze | grep PyQt5 and it confirmed that I had PyQt5 installed:

pip3 freeze | grep PyQt5
PyQt5==5.15.6
PyQt5-sip==12.9.1

I then looked at the contents of /usr/bin/backintime-qt and noticed that it has the #!/bin/sh shebang line. Furthermore when it calls out to python (python3 -Es ${APP_PATH}/app.py "$@") it doesn't specify the path to the python binary.

I think what was happening in my case was that my pyenv configuration was being initialised in the process in which backintime-qt was being run. When it initialised pyenv inserts various "shims" in to the PATH environment variable, with the intention of providing access to a specific Python runtime. The backintime-qt script seems to be expecting to use the system python runtime.

To test this I replaced the line which calls out to python with the following, which ensures that it calls my system python runtime:

/usr/bin/python3 -Es ${APP_PATH}/app.py "$@"

Lo and behold this resolves the issue and the backintime GUI now works for me again.

I'm not sure what a "proper" fix might look like here, as I'm aware that hard coding the path to the python runtime might not be portable across all Linux distros. I did wonder if there was a way to ensure the backintime-qt script was run in a "non-interactive" way, in the hope this might avoid pyenv and other similar tools from polluting backintime's environment, but I didn't have any luck when testing this.

Anyway, thought this was worth sharing and hope this helps someone.

P.S. BackInTime is a fabulous tool, much better than Ubuntu's default backup tool. Thanks for the time and effort spent on it! 🙌

@emtiu emtiu added Discussion decision or consensus needed Bug Python-Specific only for certain versions of Python labels Sep 28, 2022
@emtiu
Copy link
Member

emtiu commented Sep 28, 2022

Thanks a lot for sharing! We're still getting settled in with a new maintenance team, but we'll take a look at what's going on, and how your personal hotfix may improve things for everyone :)

@buhtz
Copy link
Member

buhtz commented Sep 28, 2022

@eddgrant Awesome. Thank you so much for reporting and analysing this.

I'm not well experienced with virtual environments (e.g. pyenv) because I don't trust them. :D But I have an idea how they work. I will try to reproduce this in a VM.

If I understood you correct you run backintime in a virtuell environment? How can this happen? Or somehow the sh run in the environment?

btw: Maybe a /usr/bin/env python3 -Es ${APP_PATH}/app.py "$@" will help here.

@emtiu Didn't we had other issues with importing PyQt5?

When migrating to the "src" Layout (far away in the future) this won't happen because python (or setuptools and pip) do create the entry point scripts them selfs.

EDIT: To improve diagnosis. Can you please add a line before the last line in your backintime-qt script. Modify this part

fi

python3 -Es ${APP_PATH}/app.py "$@"

into this please

fi

python3 -c "import sys;print(sys.executable)"
python3 -Es ${APP_PATH}/app.py "$@"

This should print the path to the used python interpreter binary on stdout.

@emtiu
Copy link
Member

emtiu commented Sep 28, 2022

@emtiu Didn't we had other issues with importing PyQt5?

From a quick search, that would be:

So, this one looks unique to me at first glance.

@buhtz
Copy link
Member

buhtz commented Sep 28, 2022

I think what was happening in my case was that my pyenv configuration

How did you installed pyenv? Did you build and installed it direct from github? It seems to me not installable via pip.

Reading further into the docu of pyenv it seems to me that it is a bit exotic and works different then other virtenv systems I know. The virtual environment doesn't exists only in a specific project folder but is active all the time no mater where you are in the file system. The shims are activated globally. But maybe I misunderstand here something. Maybe you can bring some light into it.

@eddgrant
Copy link
Author

eddgrant commented Sep 28, 2022

@buhtzz , here's the extra diagnostic info you asked for:

~ backintime-qt
/home/egrant/.pyenv/versions/3.9.9/bin/python3
Traceback (most recent call last):
  File "/usr/share/backintime/qt/app.py", line 35, in <module>
    import qttools
  File "/usr/share/backintime/qt/qttools.py", line 21, in <module>
    from PyQt5.QtGui import (QFont, QColor, QKeySequence)
ModuleNotFoundError: No module named 'PyQt5'

Looks like its using my pyenv default Python runtime (which it calls "global"), which is currently set to 3.9.9:

~ pyenv global
3.9.9

@eddgrant
Copy link
Author

I think my pyenv installation probably pre-dates the automatic installer. IIRC I installed pyenv simply by cloning the git repo e.g.
git clone https://github.com/pyenv/pyenv.git ~/.pyenv

How did you installed pyenv? Did you build and installed it direct from github? It seems to me not installable via pip.

@eddgrant
Copy link
Author

eddgrant commented Sep 28, 2022

@buhtzz I think your understanding of how pyenv works sounds correct (at least to me!). As I understand it pyenv is initialised when a shell is started, which in turn executes its initialisation file (.zshrc, .bashrc etc) which contains statements to initialise pyenv. Once pyenv is initialised it figures out which python runtime is desired (shell, local or global) and injects the relevant shims in to the environment.

@buhtz
Copy link
Member

buhtz commented Sep 28, 2022

Referring to your headline I would say it isn't the responsibility of an application to point to the correct (specific) interpreter. BIT says it needs Python version 3 and then take what the operating system (or shell) does offer.

In your case you kind of "hack" a bit and manage multiple python interpreters of different versions beside the operating systems itself. PyEnv "steal" the responsibility from the OS.

IMHO the reasons for that Issue is that you misused PyEnv. Use PyEnv to get the correct interpreter and its site-packages. Then it should work. Maybe PyEnv is able to point to the original OS own interpreter, too.

Background

PyEnv isn't a virtual environment. It is just kind of a Python interpreter switcher enabling you to install multiple Python interpreters of different versions all with there own site-packages. You can switch between them. The selection is globally in the current shell valid. It is not restricted to a specific folder.

My opinion

Because of that I vote for Close.

Consequences

Despite that your report is valuable. It points me to an improvement for our diagnostics module.

@aryoda
Copy link
Contributor

aryoda commented Oct 2, 2022

@eddgrant Excellent problem analysis - thanks a lot for sharing your knowledge here!

pyenv explicitly declares to be able to

... change the global Python version on a per-user basis.

I fully understand that manipulating the search path (like virtenv does here)

  • has a major impact on every tools that calls python3 without a hard-coded path to the OS-packaged version
    with sometimes fatal results (like non-installed packages) - this is how Trojans try to get called ;-)
  • has its uses cases (eg. when you want to intentionally inject a newer python version for testing purposes)

and that users doing this are fully responsible for the impact.

But: IMHO we should prevent starting the wrong (non-OS-packages) python version for packaged BiT versions to reduce support efforts.

I don't know how, but the OS package maintainers could possibly be able to solve this (it is a packaging issue IMHO).

@buhtzz How about discussing this eg. with the DEBIAN package maintainer of BiT as a starting point, AFAIR you have an account there. What do you think?

@buhtz
Copy link
Member

buhtz commented Oct 3, 2022

I see no need to discuss this with distro maintainers. It is not easy to argument here in my non-native English. 😄

The use case described here violates the concept of responsibilities. A userland application with dependencies to extern components (packages, libraries or an inpterprete like python2/3) never specifies the path to them explicit.
The operating system is responsible to offer the correct interpreter.
The interpreter is responsible to offer the correct packages.

When you "hack" that concept via a python-interpreter-switcher like pyenv or with virtual environments like virtuelevn that you have to take care of that edge case yourself and can't blame the system or the application for that.

This is Issues is not a bug. It is a misuse of pyenv. I assume that @eddgrant has some problems to understand how pyenv should be used. Switch to the correct python interpreter with pyenv and it should work with BIT. How to do this is up to the pyenv user.

It would violate a lot of concepts if BIT would specify an explicit path to the python interpreter.

  • In more modern and currently official recommended python project layouts ("src-layout") using setup.cfg or project.toml to setup the project and its "entry points" (starting scripts installed to bin-folders) the interpreter is not specified that way.
  • It violates platform independence not only between Linux and Windows or Mac. There are Linux distros that will throw "python3" away in the future (e.g. Debian) and switch to "python" because "Python" is "Python version 3" by default.
  • It could be problematic in testing environments (e.g. TravisCI) because they also using special python interpreters.

@eddgrant You should be able to start BIT with your pyenv when you select a python interpreter where PyQt5 is installed in its environment.

I vote for close because this is not a BIT bug but a misuse of on external tool.

But: IMHO we should prevent starting the wrong (non-OS-packages) python version for packaged BiT versions to reduce support efforts.

There is no usual way for BIT to know what is "wrong". But I will add the used python binary path to the diagnostics in the next days.

I don't know how, but the OS package maintainers could possibly be able to solve this (it is a packaging issue IMHO).

They still do because they are responsible for what "python3" in there system means.

@aryoda
Copy link
Contributor

aryoda commented Oct 3, 2022

I think @eddgrant opened this issue more with the intention to share his insights and less to urge a fix.

Nevertheless if I grep -i python3 /usr/bin/* I can find a lot of shebangs with hard coded python3 paths so I think
it could be solved by either using an absolute path injected by the package maintainer or a generic "find distro-packaged python" function in the starter script at our (BiT) side.

...
/usr/bin/asan_symbolize-9:#!/usr/bin/env python3
/usr/bin/backintime:python3 -Es $APP_PATH/backintime.py "$@"
/usr/bin/backintime-askpass:python3 -Es $APP_PATH/askpass.py "$@"
/usr/bin/backintime-qt:python3 -Es ${APP_PATH}/app.py "$@"
/usr/bin/chardet3:#! /usr/bin/python3
/usr/bin/chardetect3:#! /usr/bin/python3
/usr/bin/check-language-support:#! /usr/bin/python3
/usr/bin/chrome-gnome-shell:#!/usr/bin/python3
...

I think a simple if in the shell script could heavily improve the situation, eg. (not tested!):

# ...
python_starter="/usr/bin/python3"
if [ ! -f $python_starter ]; then
    python_starter="python3"
    echo "Unexpected python3 installation path detected (virtual env activated?). BiT may not work as expected"
fi

$python_starter -Es ~/dev/backintime/qt/app.py "$@"

Edit: If this shouldn't use the correct python version the package maintainers of BiT will come back to us anyhow ;-)

Edit 2: BiT common (CLI) is also affected by this issue...

Edit 3: Should we also add the path to the active python file to our diagnostics output (which python3)?

@buhtz
Copy link
Member

buhtz commented Oct 3, 2022

I assume that @eddgrant has some problems to understand how pyenv should be used.

@eddgrant I would like to apologize for this. After I had read that again, it struck me that this could be understood very impolitely. It was not meant that way and can possibly be explained with my rather rough English. Although the latter may also not always be the excuse.

@buhtz
Copy link
Member

buhtz commented Oct 3, 2022

As a disclaimer I have to say that my opinion is based on experience not on expertise. 😄 It is just how I understood the relationship between OS, envrionment, userland applications etc. I don't have hard facts here to support my opinion. So maybe I'm totally wrong here.

I still don't see an advantage in modifying the starter script. I'm scared about a lot of upcoming side effects. And this situation described by @eddgrant is an unusual and rare use case. And you can use BIT in virtenv and pyenv when you have a correct setup of such special envrionments.

Edit: If this shouldn't use the correct python version the package maintainers of BiT will come back to us anyhow ;-)

I would suggest to contact distro maintainers before modify something and ask them for an opinion. Maybe we could open an Issue or contact them directly. I would suggest to open an official debian report and mail to the maintainers after one week of no response.
Because of my English I would recommend that someone else does formulate the debian report and then mail it to me and I can open it via "reportbug".

Edit 3: Should we also add the path to the active python file to our diagnostics output (which python3)?

See #1318. It is IMHO implemented.

    "python-setup": {
        "python": "3.9.2 default Feb 28 2021 17:03:44 CPython GCC 10.2.1 20210110",
        "python-executable": "/usr/bin/python3",
        "python-executable-symlink": true,
        "python-executable-resolved": "/usr/bin/python3.9",

@aryoda
Copy link
Contributor

aryoda commented Oct 3, 2022

OK, so let's settle this with "watch status" (don't change anything for now)
to observe if more issues arise due to non-standard python installations.

The new --diagnostics feature with your PR #1318 to show the active python executable should allow us to diagnose these types of problems in issues now...

@emtiu emtiu changed the title backintime-qt doesn't guarantee that system python will be used backintime-qt script uses non-absolute python3 call Oct 6, 2022
@emtiu emtiu removed Bug Python-Specific only for certain versions of Python labels Oct 6, 2022
@emtiu
Copy link
Member

emtiu commented Oct 6, 2022

Thanks for a thorough discussion, everyone. And thanks @buhtzz for explaining to @eddgrant that you meant no insult :)

I agree with @buhtzz that it's a sane default for a caller script to simply call python3. Where does it lead? That's defined by the distro/system first, the system admin second, and the user third. If any of them change the destination of python3 by modifying $PATH or inserting symlinks, then it's their responsibility to check that python software still works as expected.

We might put /usr/bin/python3 in there, with python3 as a fallback, and it probably wouldn't break things (though I'm not willing to bet my bike on it ;)). But I don't think it's necessary, since really, a system/user should always have python3 point to somewhere that makes sense. I also happen to know that FreeBSD will always have /usr/local/bin/python3 instead of /usr/bin/python3, so that would be a "miss".

I also agree with @aryoda that we should keep this Issue open for reference. Python is a lively language, and conditions might change. Again, thanks @eddgrant for sharing!

@aryoda
Copy link
Contributor

aryoda commented Oct 7, 2022

@emtiu Do you have an idea for a new label with a self-explanatory name ("watchlist" or better) for this type of issue status?

@emtiu
Copy link
Member

emtiu commented Oct 7, 2022

IMHO, Discussion decision or consensus needed fits the bill well enough. Do you think we might add "Reference" for this case?

@aryoda
Copy link
Contributor

aryoda commented Oct 7, 2022

@emtiu I would interpret "Discussion" as an active discourse or waiting for an answer to a question.
What I search is something like "resubmission" since the issue sleeps until it is waked up again due to a similar issue or finally closed after the resubmission period.

Let's keep "Discussion", it is a good filter to be revisited again from time to time...

@aryoda
Copy link
Contributor

aryoda commented Oct 10, 2022

I just stumbled across the solution how to inject the OS-specific python path into all launcher scripts and configurations:

Via ./configure in common and qt.

This is what package maintainers call when preparing a package (together with make and make install to be ready to package the installed files).

If we extend the existing logic to inject python2 vs. python3 by also injecting the absolute path (eg. found with which python3)
it should work universally (there may be a better way than which I may look into the clever logic of the another configuration tool (autoconf for C is very clever AFAIR):

backintime/common/configure

Lines 133 to 141 in b4dd2d1

#patch python command
#use 'python' or 'python3' to start Python Version 3.x
case $PYTHON in
--python) PYVERSION="" ;;
--python3) PYVERSION="3";;
esac
sed -e "s/^python3\? /python${PYVERSION} /g" \
-e "s/^ssh-agent python3\? /ssh-agent python${PYVERSION} /g" \
-i $USR_BIN_FILES

@emtiu
Copy link
Member

emtiu commented Oct 10, 2022

That sounds like it should work. It would be a lot of effort to test, and we can't cover all bases, but it seems reasonable.

@aryoda
Copy link
Contributor

aryoda commented Oct 10, 2022

Yes, too much efforts for now, let' stick to the to be revisited again from time to time... plan to learn how much other users are impacted at all.

BTW: You will find me quite often posting "how it could be fixed" or at least "how does the current implementation work" comments even for non-HIGH issues because I am trying to understand the BiT full code base by digging into issues. This does not imply "must be fixed [at all or right now]" unless the fix is trivial and low-risk...

@aryoda
Copy link
Contributor

aryoda commented Oct 10, 2022

Related to (already closed) issue #677 (comment)
where the issue reason was using a non-distro-standard python version...

@aryoda
Copy link
Contributor

aryoda commented Oct 14, 2022

FYI:

OpenSuse 15.3 uses this command in the installation script of the BiT package (v1.2.1):

https://build.opensuse.org/package/view_file/openSUSE:Leap:15.3/backintime/backintime.spec?expand=1

# Fix shebangs
sed -i 's|/usr/bin/env python|#!/usr/bin/python|g' common/askpass.py

@buhtz buhtz added this to the 1.3.5 / 1.4.0 milestone Mar 19, 2023
@buhtz
Copy link
Member

buhtz commented Jan 9, 2024

Let's review this issue after #1575 (migration to Python build system) is done.

@aryoda
Copy link
Contributor

aryoda commented Jan 21, 2024

I think we should close this issue after a cooling-off period of a few weeks since we have merged PR #1602 to solve this:

It is possible to configure an absolute path to python3 with ./configure --python=<path>.

By default we still use no absolute path for the BiT starter scripts...

@buhtz
Copy link
Member

buhtz commented Mar 28, 2024

Closing this ticket based on the comment above. Feel free to reopen
if the problem is still relevant. Thank you for your efforts. If you have
any further questions, ideas or encounter any other issues, please
don't hesitate to let us know.

Best regards,

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

No branches or pull requests

4 participants