'share' directory install logic broken with wheels / PEP517 build

[mgorny]

When installing electrum via modern package managers, the share directory ends up inside site-packages rather than in the correct path, e.g.:

$ pip install --root=/tmp/z .
[...]
$ find /tmp/z | grep share
/tmp/z/usr/share
/tmp/z/usr/share/man
/tmp/z/usr/share/man/man1
/tmp/z/usr/share/man/man1/qr.1
/tmp/z/usr/lib/python3.10/site-packages/home/mgorny/.local/share
/tmp/z/usr/lib/python3.10/site-packages/home/mgorny/.local/share/icons
/tmp/z/usr/lib/python3.10/site-packages/home/mgorny/.local/share/icons/electrum.png
/tmp/z/usr/lib/python3.10/site-packages/home/mgorny/.local/share/applications
/tmp/z/usr/lib/python3.10/site-packages/home/mgorny/.local/share/applications/electrum.desktop
/tmp/z/usr/lib/python3.10/site-packages/electrum/gui/kivy/theming/light/share.png

AFAIK using absolute paths with data_files is invalid, and the wheel format doesn't support installing via absolute paths at all (only relative to sys.prefix).

[SomberNight]

Seems to be a duplicate of https://github.com/spesmilo/electrum/issues/5227 Although your report adds some more context.

What do you suggest?

I guess we could just give up and not try to put the .desktop file and the icon anywhere. i.e.

diff --git a/setup.py b/setup.py
index 6424087921..f077d0d174 100755
--- a/setup.py
+++ b/setup.py
@@ -30,26 +30,6 @@ version_spec = importlib.util.spec_from_file_location('version', 'electrum/versi
 version_module = version = importlib.util.module_from_spec(version_spec)
 version_spec.loader.exec_module(version_module)

-data_files = []
-
-if platform.system() in ['Linux', 'FreeBSD', 'DragonFly']:
-    parser = argparse.ArgumentParser()
-    parser.add_argument('--root=', dest='root_path', metavar='dir', default='/')
-    opts, _ = parser.parse_known_args(sys.argv[1:])
-    usr_share = os.path.join(sys.prefix, "share")
-    icons_dirname = 'pixmaps'
-    if not os.access(opts.root_path + usr_share, os.W_OK) and \
-       not os.access(opts.root_path, os.W_OK):
-        icons_dirname = 'icons'
-        if 'XDG_DATA_HOME' in os.environ.keys():
-            usr_share = os.environ['XDG_DATA_HOME']
-        else:
-            usr_share = os.path.expanduser('~/.local/share')
-    data_files += [
-        (os.path.join(usr_share, 'applications/'), ['electrum.desktop']),
-        (os.path.join(usr_share, icons_dirname), ['electrum/gui/icons/electrum.png']),
-    ]
-
 extras_require = {
     'hardware': requirements_hw,
     'gui': ['pyqt5'],
@@ -81,7 +61,7 @@ setup(
     # By specifying include_package_data=True, MANIFEST.in becomes responsible for both.
     include_package_data=True,
     scripts=['electrum/electrum'],
-    data_files=data_files,
+    data_files=[],
     description="Lightweight Bitcoin Wallet",
     author="Thomas Voegtlin",
     author_email="thomasv@electrum.org",

[mgorny]

Yes, removing it entirely is one possibility. It would work for us since we need to install it anyway, and we wouldn't have to remove the wrongly installed files, so there's a gain in it.

Alternatively, you could use /usr-relative paths, i.e. usr/share/.... Then it would work out of the box for system-wide installs, and it shouldn't break anything for other setups (except for installing an extra nonfunctional file).

[SomberNight]

I've tested installing the Electrum-4.2.1.tar.gz release distributable on a ~fresh ubuntu 20.04. That is, this is how what we currently have behaves.

// if the wheel python package is installed

case 1.1:

$ python3 -m pip install --user Electrum-4.2.1.tar.gz
(broken. files at:)
    /home/user/.local/lib/python3.8/site-packages/home/user/.local/share/applications/electrum.desktop
    /home/user/.local/lib/python3.8/site-packages/home/user/.local/share/icons/electrum.png

case 1.2:

$ python3 -m pip install Electrum-4.2.1.tar.gz
(broken. files at:)
    /usr/local/lib/python3.8/dist-packages/home/user/.local/share/applications/electrum.desktop
    /usr/local/lib/python3.8/dist-packages/home/user/.local/share/icons/electrum.png

case 1.3:

$ sudo python3 -m pip install Electrum-4.2.1.tar.gz
(broken. files at:)
    /usr/local/lib/python3.8/dist-packages/usr/share/applications/electrum.desktop
    /usr/local/lib/python3.8/dist-packages/usr/share/pixmaps/electrum.png

---

// if the wheel python package is NOT installed

case 2.1:

$ python3 -m pip install --user Electrum-4.2.1.tar.gz
(good result. files at:)
    /home/user/.local/share/applications/electrum.desktop
    /home/user/.local/share/icons/electrum.png

case 2.2:

$ python3 -m pip install Electrum-4.2.1.tar.gz
(fails to install)
      creating /usr/lib/python3.8/site-packages
      error: could not create '/usr/lib/python3.8/site-packages': Permission denied

case 2.3:

$ sudo python3 -m pip install Electrum-4.2.1.tar.gz
(good result. files at:)
    /usr/share/applications/electrum.desktop
    /usr/share/pixmaps/electrum.png

---

From https://setuptools.pypa.io/en/latest/deprecated/distutils/setupscript.html#installing-additional-files :

data_files specifies a sequence of (directory, files) pairs Each (directory, files) pair in the sequence specifies the installation directory and the files to install there. The directory should be a relative path. It is interpreted relative to the installation prefix (Python’s sys.prefix for system installations; site.USER_BASE for user installations). Distutils allows directory to be an absolute installation path, but this is discouraged since it is incompatible with the wheel packaging format. No directory information from files is used to determine the final location of the installed file; only the name of the file is used.

---

Alternatively, you could use /usr-relative paths, i.e. usr/share/.... Then it would work out of the box for system-wide installs, and it shouldn't break anything for other setups (except for installing an extra nonfunctional file).

I don't quite understand what you are suggesting here. Which case (see above) would that change the behaviour for? Do you mean something like this:

data_files = [
    (os.path.join('share', 'applications/'), ['electrum.desktop']),
    (os.path.join('share', icons_dirname), ['electrum/gui/icons/electrum.png']),
]

[mgorny]

Yes, roughly something like this. If I'm not mistaken, this would work for 1.3/2.3, and cause the remaining cases to install the file incorrectly (but should be harmless).

I don't think you can realistically assume systems without wheel these days. setup.py install is deprecated and I'm pretty sure pip is going to force PEP517 builds sooner than later (which means it'll implicitly install wheel).

[SomberNight]

Ok, I've tested with this now:

diff --git a/setup.py b/setup.py
index 6424087921..b4ddc916b6 100755
--- a/setup.py
+++ b/setup.py
@@ -33,21 +33,10 @@ version_spec.loader.exec_module(version_module)
 data_files = []

 if platform.system() in ['Linux', 'FreeBSD', 'DragonFly']:
-    parser = argparse.ArgumentParser()
-    parser.add_argument('--root=', dest='root_path', metavar='dir', default='/')
-    opts, _ = parser.parse_known_args(sys.argv[1:])
-    usr_share = os.path.join(sys.prefix, "share")
-    icons_dirname = 'pixmaps'
-    if not os.access(opts.root_path + usr_share, os.W_OK) and \
-       not os.access(opts.root_path, os.W_OK):
-        icons_dirname = 'icons'
-        if 'XDG_DATA_HOME' in os.environ.keys():
-            usr_share = os.environ['XDG_DATA_HOME']
-        else:
-            usr_share = os.path.expanduser('~/.local/share')
     data_files += [
-        (os.path.join(usr_share, 'applications/'), ['electrum.desktop']),
-        (os.path.join(usr_share, icons_dirname), ['electrum/gui/icons/electrum.png']),
+        (os.path.join('share', 'applications'),               ['electrum.desktop']),
+        (os.path.join('share', 'pixmaps'),                    ['electrum/gui/icons/electrum.png']),
+        (os.path.join('share', 'icons/hicolor/128x128/apps'), ['electrum/gui/icons/electrum.png']),
     ]

 extras_require = {

---

// with wheel installed

case 1.1:

$ python3 -m pip install --user Electrum-4.2.1.tar.gz
(good result. files at:)
    /home/user/.local/share/applications/electrum.desktop
    /home/user/.local/share/icons/hicolor/128x128/apps/electrum.png
    /home/user/.local/share/pixmaps/electrum.png

case 1.2:

$ python3 -m pip install Electrum-4.2.1.tar.gz
(pkg installed but data_files skipped:)
    ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: '/usr/local/share/applications'
    Consider using the `--user` option or check the permissions.

case 1.3:

$ sudo python3 -m pip install Electrum-4.2.1.tar.gz
(good result. files at:)
    /usr/local/share/applications/electrum.desktop
    /usr/local/share/icons/hicolor/128x128/apps/electrum.png
    /usr/local/share/pixmaps/electrum.png

---

// without wheel

case 2.1:

$ python3 -m pip install --user Electrum-4.2.1.tar.gz
(good result. files at:)
    /home/user/.local/share/applications/electrum.desktop
    /home/user/.local/share/icons/hicolor/128x128/apps/electrum.png
    /home/user/.local/share/pixmaps/electrum.png

case 2.2:

$ python3 -m pip install Electrum-4.2.1.tar.gz
(fails to install)
      creating /usr/lib/python3.8/site-packages
      error: could not create '/usr/lib/python3.8/site-packages': Permission denied

case 2.3:

$ sudo python3 -m pip install Electrum-4.2.1.tar.gz
(good result. files at:)
    /usr/local/share/applications/electrum.desktop
    /usr/local/share/icons/hicolor/128x128/apps/electrum.png
    /usr/local/share/pixmaps/electrum.png

---

So that looks very promising... I think I might commit that.