launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-no-sqlite into launchpad:master.

Commit message:
Stop linking in sqlite/_sqlite on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/389203

cscvs hasn't yet been ported to Python 3, and requires the old sqlite module.  This may take some time to fix.  To avoid this blocking other Python 3 work in the meantime, only link in the sqlite and _sqlite system-installed packages on Python 2.

Rather than inventing a whole new syntax for this or splitting up the system-packages.txt file, the simplest approach seemed to be to use the PEP 508 environment marker syntax.  This also involved changing how optional packages are specified.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-no-sqlite into launchpad:master.

diff --git a/system-packages.txt b/system-packages.txt
index 5fe7918..0ae5029 100644
--- a/system-packages.txt
+++ b/system-packages.txt
@@ -4,9 +4,12 @@
 # changes in system libraries they depend on, or frequent security updates
 # managed by the distribution's security team).
 #
-# Package names that end with "?" are optional, in that link-system-packages
-# will not fail if they are missing; this should be reserved for packages
-# only used by the test suite.
+# The syntax is based on that of PEP 508 requirements, although only
+# name-based requirements are permitted, possibly with environment markers,
+# and the names are top-level package names rather than distribution names.
+# Lines that begin with "[optional]" are optional, in that
+# link-system-packages will not fail if they are missing; this should be
+# reserved for packages only used by the test suite.
 
 # Used by launchpad-buildd.
 apt
@@ -23,14 +26,14 @@ convoy
 GeoIP
 
 # lp.testing.html5browser
-gi?
+[optional] gi
 
 # lp.services.fields, lp.services.spriteutils
 PIL
 
 # Dependency of cscvs.
-sqlite
-_sqlite
+sqlite; python_version < "3"
+_sqlite; python_version < "3"
 
 # Used by bzr-git and bzr-svn.
 tdb
diff --git a/utilities/link-system-packages.py b/utilities/link-system-packages.py
index 18b863b..56e9b95 100755
--- a/utilities/link-system-packages.py
+++ b/utilities/link-system-packages.py
@@ -13,6 +13,11 @@ import importlib
 import os.path
 import re
 
+# Importing this from the vendored version in pkg_resources is a bit dodgy
+# (using packaging.markers directly would be better), but we want to
+# minimise our dependencies on packages outside the virtualenv.
+from pkg_resources.extern.packaging.markers import Marker
+
 
 def link_module(name, virtualenv_libdir, optional=False):
     try:
@@ -47,12 +52,16 @@ def main():
         line = re.sub(r"#.*", "", line).strip()
         if not line:
             continue
-        if line.endswith("?"):
-            line = line[:-1]
-            optional = True
-        else:
-            optional = False
-        link_module(line, args.virtualenv_libdir, optional=optional)
+        match = re.match(
+            r"^(\[optional\])?\s*([A-Za-z_][A-Za-z0-9_]*)(?:\s*;\s*(.*))?",
+            line)
+        if not match:
+            raise ValueError("Parse error: %s" % line)
+        optional = bool(match.group(1))
+        name = match.group(2)
+        if match.group(3) and not Marker(match.group(3)).evaluate():
+            continue
+        link_module(name, args.virtualenv_libdir, optional=optional)
 
 
 if __name__ == "__main__":