Merge ~cjwatson/launchpad:no-system-site-packages into launchpad:master

Proposed by Colin Watson on 2020-02-12
Status: Needs review
Proposed branch: ~cjwatson/launchpad:no-system-site-packages
Merge into: launchpad:master
Diff against target: 188 lines (+88/-17)
6 files modified
Makefile (+8/-7)
doc/pip.txt (+0/-1)
setup-requirements.txt (+0/-7)
setup.py (+1/-2)
system-packages.txt (+32/-0)
utilities/link-system-packages.py (+47/-0)
Reviewer Review Type Date Requested Status
Ioana Lasc 2020-02-12 Approve on 2020-03-18
Review via email: mp+378946@code.launchpad.net

Commit message

Stop using --system-site-packages

Description of the change

We've been using "virtualenv --system-site-packages" to ensure that Launchpad has access to some system-installed packages that can't or shouldn't be installed as a normal Python dependency for various reasons. This strategy has always had some problems, but we've been able to deal with them.

However, with recent versions of pip and Python 3, the situation gets worse. https://github.com/pypa/pip/issues/6264 describes the problem: pip attempts to isolate itself from site-packages when building wheels, leaving only its temporary one; but within a virtualenv created with --system-site-packages it removes the virtualenv's site-packages but not the system one (because virtualenv patches distutils.sysconfig.get_python_lib). It's not easy to see how pip might reliably and portably escape the virtualenv in order to find the system's site-packages directory.

(On Python 2 we get away with this by luck, because distutils.sysconfig.get_python_lib returns '.../env/lib/python2.7/site-packages' but '.../env/local/lib/python2.7/site-packages' is on sys.path, and pip doesn't realise that they're the same thing due to symlinks; we don't get this stroke of luck on Python 3.)

Installing everything properly in the virtualenv is also difficult. Several of the ones that remain don't exist as wheels or in another form that pip can install. We could fake this up (I went so far as to prepare a script to convert the installed version of python-apt into a wheel), but this would incur a significant maintenance burden.

Instead, symlink packages from the system site-packages directory into the virtualenv in "make compile". It's conceivable that this might confuse pip slightly, but so far it doesn't seem to, and everything else works fine.

This shouldn't be landed until https://portal.admin.canonical.com/C124200 has been resolved and all production deployment targets have the python-tdb package installed.

To post a comment you must log in.
Ioana Lasc (ilasc) wrote :

LGTM

review: Approve
Colin Watson (cjwatson) wrote :

Currently blocked on https://portal.admin.canonical.com/C124836 to ensure that python-tdb is installed everywhere.

Unmerged commits

fdb69de... by Colin Watson on 2020-02-11

Stop using --system-site-packages

We've been using "virtualenv --system-site-packages" to ensure that
Launchpad has access to some system-installed packages that can't or
shouldn't be installed as a normal Python dependency for various
reasons. This strategy has always had some problems, but we've been
able to deal with them.

However, with recent versions of pip and Python 3, the situation gets
worse. https://github.com/pypa/pip/issues/6264 describes the problem:
pip attempts to isolate itself from site-packages when building wheels,
leaving only its temporary one; but within a virtualenv created with
--system-site-packages it removes the virtualenv's site-packages but not
the system one (because virtualenv patches
distutils.sysconfig.get_python_lib). It's not easy to see how pip might
reliably and portably escape the virtualenv in order to find the
system's site-packages directory.

(On Python 2 we get away with this by luck, because
distutils.sysconfig.get_python_lib returns
'.../env/lib/python2.7/site-packages' but
'.../env/local/lib/python2.7/site-packages' is on sys.path, and pip
doesn't realise that they're the same thing due to symlinks; we don't
get this stroke of luck on Python 3.)

Installing everything properly in the virtualenv is also difficult.
Several of the ones that remain don't exist as wheels or in another form
that pip can install. We could fake this up (I went so far as to
prepare a script to convert the installed version of python-apt into a
wheel), but this would incur a significant maintenance burden.

Instead, symlink packages from the system site-packages directory into
the virtualenv in "make compile". It's conceivable that this might
confuse pip slightly, but so far it doesn't seem to, and everything else
works fine.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/Makefile b/Makefile
2index 14a6f31..418cf98 100644
3--- a/Makefile
4+++ b/Makefile
5@@ -16,15 +16,14 @@ PIP_ENV := LC_ALL=C.UTF-8
6 # be reviewed/merged/deployed.
7 PIP_NO_INDEX := 1
8 PIP_ENV += PIP_NO_INDEX=$(PIP_NO_INDEX)
9-# Although --ignore-installed is slower, we need it to avoid confusion with
10-# system-installed Python packages. If we ever manage to remove the need
11-# for virtualenv --system-site-packages, then we can remove this too.
12-PIP_ENV += PIP_IGNORE_INSTALLED=1
13 PIP_ENV += PIP_FIND_LINKS="file://$(WD)/wheelhouse/ file://$(WD)/download-cache/dist/"
14
15 VIRTUALENV := $(PIP_ENV) virtualenv
16 PIP := PYTHONPATH= $(PIP_ENV) env/bin/pip --cache-dir=$(WD)/download-cache/
17
18+SITE_PACKAGES := \
19+ $$(env/bin/python -c 'from distutils.sysconfig import get_python_lib; print(get_python_lib())')
20+
21 TESTFLAGS=-p $(VERBOSITY)
22 TESTOPTS=
23
24@@ -250,7 +249,7 @@ $(PY): download-cache constraints.txt setup.py
25 rm -rf env
26 mkdir -p env
27 $(VIRTUALENV) \
28- --python=$(PYTHON) --system-site-packages --never-download \
29+ --python=$(PYTHON) --never-download \
30 --extra-search-dir=$(WD)/download-cache/dist/ \
31 --extra-search-dir=$(WD)/wheelhouse/ \
32 env
33@@ -270,6 +269,8 @@ compile: $(PY)
34 ${SHHH} utilities/relocate-virtualenv env
35 ${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
36 LPCONFIG=${LPCONFIG}
37+ $(PYTHON) utilities/link-system-packages.py \
38+ "$(SITE_PACKAGES)" system-packages.txt
39 ${SHHH} bin/build-twisted-plugin-cache
40 scripts/update-version-info.sh
41
42@@ -477,13 +478,13 @@ reload-apache: enable-apache-launchpad
43 TAGS: compile
44 # emacs tags
45 ctags -R -e --languages=-JavaScript --python-kinds=-i -f $@.new \
46- $(CURDIR)/lib $(CURDIR)/env/lib/$(PYTHON)/site-packages
47+ $(CURDIR)/lib "$(SITE_PACKAGES)"
48 mv $@.new $@
49
50 tags: compile
51 # vi tags
52 ctags -R --languages=-JavaScript --python-kinds=-i -f $@.new \
53- $(CURDIR)/lib $(CURDIR)/env/lib/$(PYTHON)/site-packages
54+ $(CURDIR)/lib "$(SITE_PACKAGES)"
55 mv $@.new $@
56
57 PYDOCTOR = pydoctor
58diff --git a/doc/pip.txt b/doc/pip.txt
59index b570ab8..23c024b 100644
60--- a/doc/pip.txt
61+++ b/doc/pip.txt
62@@ -372,6 +372,5 @@ Possible Future Goals
63 =====================
64
65 - Use wheels.
66-- No longer use system site-packages.
67 - No longer use make.
68 - Get rid of the sourcecode directory.
69diff --git a/setup-requirements.txt b/setup-requirements.txt
70index 11ef58a..e7472fc 100644
71--- a/setup-requirements.txt
72+++ b/setup-requirements.txt
73@@ -1,10 +1,3 @@
74 pip==20.0.2
75 setuptools==44.0.0
76 wheel==0.34.2
77-
78-# Install these early to avoid mismatches with a system-installed
79-# python-cffi-backend package.
80-# XXX cjwatson 2020-02-06: This would all be much easier if we could remove
81-# the need for virtualenv --system-site-packages.
82-cffi==1.12.2
83-pycparser==2.19
84diff --git a/setup.py b/setup.py
85index 84f146a..565163b 100644
86--- a/setup.py
87+++ b/setup.py
88@@ -187,8 +187,7 @@ setup(
89 'lpjsmin',
90 'Markdown',
91 'meliae',
92- # Pin version for now to avoid confusion with system site-packages.
93- 'mock==1.0.1',
94+ 'mock',
95 'oauth',
96 'oops',
97 'oops_amqp',
98diff --git a/system-packages.txt b/system-packages.txt
99new file mode 100644
100index 0000000..1737888
101--- /dev/null
102+++ b/system-packages.txt
103@@ -0,0 +1,32 @@
104+# System-installed Python packages to link into our virtualenv. This
105+# facility should be reserved for cases where installing them as a normal
106+# Python dependency is impossible or unreliable (perhaps due to frequent ABI
107+# changes in system libraries they depend on, or frequent security updates
108+# managed by the distribution's security team).
109+
110+# Used by launchpad-buildd.
111+apt
112+
113+# Used by Soyuz and other related code to parse Debian packages and
114+# repository index files, and to compare Debian version numbers.
115+apt_inst
116+apt_pkg
117+
118+# utilities/js-deps
119+convoy
120+
121+# lp.services.geoip.model
122+GeoIP
123+
124+# lp.testing.html5browser
125+gi
126+
127+# lp.services.fields, lp.services.spriteutils
128+PIL
129+
130+# Dependency of cscvs.
131+sqlite
132+_sqlite
133+
134+# Optional dependency of bzr-git and bzr-svn.
135+tdb
136diff --git a/utilities/link-system-packages.py b/utilities/link-system-packages.py
137new file mode 100755
138index 0000000..c760f49
139--- /dev/null
140+++ b/utilities/link-system-packages.py
141@@ -0,0 +1,47 @@
142+#! /usr/bin/python
143+
144+# Copyright 2020 Canonical Ltd. This software is licensed under the
145+# GNU Affero General Public License version 3 (see the file LICENSE).
146+
147+"""Link system-installed Python modules into Launchpad's virtualenv."""
148+
149+from __future__ import absolute_import, print_function, unicode_literals
150+
151+from argparse import ArgumentParser
152+from distutils.sysconfig import get_python_lib
153+import importlib
154+import os.path
155+import re
156+
157+
158+def link_module(name, virtualenv_libdir):
159+ module = importlib.import_module(name)
160+ path = module.__file__
161+ if os.path.basename(path).startswith("__init__."):
162+ path = os.path.dirname(path)
163+ system_libdir = get_python_lib(plat_specific=path.endswith(".so"))
164+ if os.path.commonprefix([path, system_libdir]) != system_libdir:
165+ raise RuntimeError(
166+ "%s imported from outside %s (%s)" % (name, system_libdir, path))
167+ target_path = os.path.join(
168+ virtualenv_libdir, os.path.relpath(path, system_libdir))
169+ if os.path.lexists(target_path) and os.path.islink(target_path):
170+ os.unlink(target_path)
171+ os.symlink(path, target_path)
172+
173+
174+def main():
175+ parser = ArgumentParser()
176+ parser.add_argument("virtualenv_libdir")
177+ parser.add_argument("module_file", type=open)
178+ args = parser.parse_args()
179+
180+ for line in args.module_file:
181+ line = re.sub(r"#.*", "", line).strip()
182+ if not line:
183+ continue
184+ link_module(line, args.virtualenv_libdir)
185+
186+
187+if __name__ == "__main__":
188+ main()

Subscribers

People subscribed via source and target branches