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 install script only uses python / python3 even if other versions are available #1574

Closed
tguruswamy opened this issue Dec 3, 2023 · 10 comments
Assignees
Labels
Discussion decision or consensus needed Distro-Specific only for certain distributions, desktop environments or display servers External depends on others/upstream Infrastructure Python-Specific only for certain versions of Python

Comments

@tguruswamy
Copy link
Contributor

The install scripts (configure) assume that /usr/bin/python{3} are the correct binary to use for running backintime, and error out if that version is < 3.8. On systems with multiple python versions (e.g. Red Hat Enterprise Linux, SUSE Enterprise Linux/openSUSE Leap, Debian 10, Ubuntu LTS) this means backintime cannot be installed even if an appropriate python version is available (openSUSE Leap has python36 and python311, for example). More generally it means backintime cannot be reliably managed on systems with custom python installs, such as from conda or pyenv.

The install script should simply accept the full path to a python binary to be used, not only python / python3.
(Even better would be to use a standard python build system).

@buhtz
Copy link
Member

buhtz commented Dec 3, 2023

Dear Tejas,

thanks for your report and your feedback.

First of all: We do plan to migrate to a "standard python build system" and eliminate the make system (#1575). But this will take time and currently we do not have a schedule for this.

The install scripts (configure) assume that /usr/bin/python{3} are the correct binary to use for running backintime

Can you point us to the specific location please and give us a line number. I was not able to find it in configure.

and error out if that version is < 3.8

That is correct. We (as upstream) do support Python 3.8 and not older.

On systems with multiple python versions (e.g. Red Hat Enterprise Linux, SUSE Enterprise Linux/openSUSE Leap, Debian 10, Ubuntu LTS) this means backintime cannot be installed even if an appropriate python version is available

I think you mix up some things here. I might be mistaken but Debian and Ubuntu do not offer multiple Python versions. Am I wrong? I don't know about the other distros but...

(openSUSE Leap has python36 and python311, for example)

If this really is the case then please open a ticket there. Then the "openSUSE Leap" maintainer can contact us at upstream and tell us how we could modify BIT to fit SUSEs needs. Then we can discuss if this solution will violate other rules.

More generally it means backintime cannot be reliably managed on systems with custom python installs, such as from conda or pyenv.

That is an often misunderstanding of conda/pyenv etc. A productive system do not have multiple Python versions. Conda & Co are for testing and development environments. BIT is an end-user application intended to run on a productive system.

The install script should ...

As I told the configure do not point to a full path of the python binary. It just checks the version of the systems python.
I need more details to understand the real problem here.
If my understanding improved then I might be able to offer you a solution that do not violate our needs but also fit your needs.

But be aware that we won't invest to much effort in the current build system. As you pointed out yourself a migration to modern python build system would solve all this issues. And we will migrate someday. I still work on some issues in context of that migration.

@buhtz buhtz self-assigned this Dec 3, 2023
@buhtz buhtz added Distro-Specific only for certain distributions, desktop environments or display servers Infrastructure Discussion decision or consensus needed Feedback needs user response, may be closed after timeout without a response External depends on others/upstream Python-Specific only for certain versions of Python Low relevant, but not urgent labels Dec 3, 2023
@aryoda
Copy link
Contributor

aryoda commented Dec 3, 2023

The install scripts (configure) assume that /usr/bin/python{3} are the correct binary to use for running backintime

Can you point us to the specific location please and give us a line number. I was not able to find it in configure.

I have just checked our files and could neither find a hard-coded python[3] path in our configure scripts in common and qt nor in the generated Makefiles
(but found hard-coded py.test-3 paths for the unittest* make targets which is not perfect IMHO and may cause eg. problems with unit testing during the installation on ARCH Linux).

The configure script uses the default python interpreter in the search path (which should correspond to the which python3 path and is IMHO in sync with the packaged python version).

@tguruswamy
Copy link
Contributor Author

tguruswamy commented Dec 9, 2023

First of all: We do plan to migrate to a "standard python build system" and eliminate the make system (#1575). But this will take time and currently we do not have a schedule for this.

Great to hear, it is the right move long term.

The install scripts (configure) assume that /usr/bin/python{3} are the correct binary to use for running backintime

Can you point us to the specific location please and give us a line number. I was not able to find it in configure

Sorry, my mistake, my statement is inaccurate. Remove the /usr/bin. The install scripts assume that only python or python3 are valid interpreter names (first one found on $PATH). But this is not a reliable assumption.

I think you mix up some things here. I might be mistaken but Debian and Ubuntu do not offer multiple Python versions. Am I wrong? I don't know about the other distros but...

Yes, I'm afraid this isn't true. Parallel python installations are quite common. For example, Debian Testing at this moment offers both python3.11 and python3.12. And it is also very standard on LTS/enterprise distros like RHEL, CentOS, and openSUSE Leap/SLE.

If this really is the case then please open a ticket there. Then the "openSUSE Leap" maintainer can contact us at upstream and tell us how we could modify BIT to fit SUSEs needs.

I am the relevant openSUSE Leap package maintainer. This is my ticket. Hopefully my suggestion is clear after this discussion. We have all the required dependencies of backintime available in openSUSE Leap 15.5, including python3 > 3.6 available, but it cannot be built (without patching) because the relevant binary is not called "python3".

That is an often misunderstanding of conda/pyenv etc. A productive system do not have multiple Python versions. Conda & Co are for testing and development environments. BIT is an end-user application intended to run on a productive system.

Productive systems really do have multiple python installs (c.f. the distro list discussed previously). Another possibility is restrictive/low privilege environments which often rely on a python version installed via conda or pyenv only for a specific user. All of these use cases will be solved automatically when moving to a setuptools-based build system, but until then I propose the quick workaround of accepting the full path to a python binary in the configure script. If you are open to implementing this, I may be able to take a look.

@tguruswamy tguruswamy changed the title Backintime install script defaults to /usr/bin/python3 even if other versions are available Backintime install script only uses python / python3 even if other versions are available Dec 9, 2023
@buhtz
Copy link
Member

buhtz commented Dec 9, 2023

Dear tgurusmwamy,
thanks for your feedback. I won't argue with a distro maintainer. 😄 If you say there are good reasons having multiple Python versions in a productive environment I have to believe.

About your proposed workaround: Do you want the configure script accept a path to a python binary or the entry-point-scripts (e.g. /usr/bin/backintime-qt, /usr/bin/backintime-qt_poolkit, etc) ?

Did I understand you correct that you would prepare a PR for this? It might be the smartest choice because you understand better what you need. In my current understanding I see no no reason why we shouldn't accept such a solution. "Make it so." 🚀

@buhtz buhtz added this to the 1.4.2 (upcoming release) milestone Dec 9, 2023
@buhtz buhtz removed Feedback needs user response, may be closed after timeout without a response Low relevant, but not urgent labels Dec 9, 2023
@buhtz
Copy link
Member

buhtz commented Jan 5, 2024

FYI: We plan a new release round about 15th January.

@aryoda
Copy link
Contributor

aryoda commented Jan 9, 2024

Related to #1316

@aryoda
Copy link
Contributor

aryoda commented Jan 21, 2024

I just wanted to propose to close this issue but then I saw two issues:

  1. We have a possibly security critical semantic change in the qt code part:

    By default the absolute path to python3 is not used anymore:

    > ./configure
    Replacement of python path with "python3" successful.
    All OK. Now run:
        make
        sudo make install
    
    > make
    #man pages
    for i in $(ls -1 man/C/); do case $i in *.gz|*~) continue;; *) gzip -n --best -c man/C/$i > man/C/${i}.gz;; esac; done
    
    > git diff net.launchpad.backintime.serviceHelper.service
    diff --git a/qt/net.launchpad.backintime.serviceHelper.service b/qt/net.launchpad.backintime.serviceHelper.service
    index aceb0c25..82beb9ba 100644
    --- a/qt/net.launchpad.backintime.serviceHelper.service
    +++ b/qt/net.launchpad.backintime.serviceHelper.service
    @@ -1,4 +1,4 @@
     [D-BUS Service]
     Name=net.launchpad.backintime.serviceHelper
    -Exec=/usr/bin/python3 -Es /usr/share/backintime/qt/serviceHelper.py
    +Exec=python3 -Es /usr/share/backintime/qt/serviceHelper.py
     User=root
    

    Using an absolute path by default is IMHO better for security reasons...

I think we should fix this for the upcoming release...

Edit: This code is responsible for loosing the absolute path:

backintime/qt/configure

Lines 123 to 124 in d30b9a0

if [ -n "$(sed -e "s#^python3\? #${PYTHON} #gw /dev/stdout" -i $USR_BIN_FILES)" ] \
&& [ -n "$(sed -e "s#^Exec=/usr/bin/python3\? #Exec=${PYTHON} #gw /dev/stdout" -i $DBUS_SERVICE_FILES)" ]

Edit 2: Using the absolute path for the service helper dbus service and a relative path for the GUI and CLI may explain problems where the service helper is not working (at all)...

@tguruswamy
Copy link
Contributor Author

As you say I agree the path should be consistent between the cli and service helper.
It was not consistent in the past, so this would be a change in behavior.

If it's ok to make the path absolute by default everywhere all one would have to do is change this line (and for consistency, also in common/configure):

backintime/qt/configure

Lines 13 to 14 in d30b9a0

#set default options
PYTHON="python3"

@aryoda
Copy link
Contributor

aryoda commented Jan 22, 2024

OK, I will provide a PR tomorrow to use absolute paths everywhere (consistently compared to the past)

@aryoda
Copy link
Contributor

aryoda commented Jan 22, 2024

@tguruswamy Fixed with PR #1614 (if you want to review - I am not a bash guru ;-)

@aryoda aryoda closed this as completed in d583cf8 Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion decision or consensus needed Distro-Specific only for certain distributions, desktop environments or display servers External depends on others/upstream Infrastructure Python-Specific only for certain versions of Python
Projects
None yet
Development

No branches or pull requests

3 participants