Defend against parallel package builds via a file lock

Author: gaborbernat

changelog/1026.bugfix.rst

@@ -0,0 +1 @@
+intermittent failures with ``--parallel--safe-build``, instead of mangling with the file paths now uses a lock to make the package build operation thread safe and is now on by default (``--parallel--safe-build`` is now deprecated) - by :user:`gaborbernat`

setup.py

@@ -80,6 +80,7 @@ def main():
             "six >= 1.0.0, <2",
             "virtualenv >= 1.11.2",
             "toml >=0.9.4",
+            "filelock >= 3.0.0, <4",
         ],
         extras_require={
             "testing": [

src/tox/_pytestplugin.py

@@ -4,6 +4,7 @@
 import sys
 import textwrap
 import time
+from collections import OrderedDict
 from fnmatch import fnmatch
 
 import py
@@ -408,12 +409,30 @@ def mock_venv(monkeypatch):
 
     # object to collect some data during the execution
     class Result(object):
-        def __init__(self):
+        def __init__(self, session):
             self.popens = []
-            self.cwd = None
-            self.session = None
-
-    res = Result()
+            self._popen = session.popen
+
+            # collect all popen calls
+            def popen(cmd, **kwargs):
+                # we don't want to perform installation of new packages,
+                # just replace with an always ok cmd
+                if "pip" in cmd and "install" in cmd:
+                    cmd = ["python", "-c", "print({!r})".format(cmd)]
+                activity_id = session._actions[-1].id
+                activity_name = session._actions[-1].activity
+                try:
+                    ret = self._popen(cmd, **kwargs)
+                except tox.exception.InvocationError as exception:  # pragma: no cover
+                    ret = exception  # pragma: no cover
+                finally:
+                    self.popens.append((activity_id, activity_name, kwargs.get("env"), ret, cmd))
+                return ret
+
+            monkeypatch.setattr(session, "popen", popen)
+            self.session = session
+
+    res = OrderedDict()
 
     # convince tox that the current running virtual environment is already the env we would create
     class ProxyCurrentPython:
@@ -468,28 +487,11 @@ def tox_runenvreport(venv, action):
     prev_build = tox.session.build_session
 
     def build_session(config):
-        res.session = prev_build(config)
-        res._popen = res.session.popen
-        monkeypatch.setattr(res.session, "popen", popen)
-        return res.session
+        session = prev_build(config)
+        res[id(session)] = Result(session)
+        return session
 
     monkeypatch.setattr(tox.session, "build_session", build_session)
-
-    # collect all popen calls
-    def popen(cmd, **kwargs):
-        # we don't want to perform installation of new packages, just replace with an always ok cmd
-        if "pip" in cmd and "install" in cmd:
-            cmd = ["python", "-c", "print({!r})".format(cmd)]
-        activity_id = res.session._actions[-1].id
-        activity_name = res.session._actions[-1].activity
-        try:
-            ret = res._popen(cmd, **kwargs)
-        except tox.exception.InvocationError as exception:  # pragma: no cover
-            ret = exception  # pragma: no cover
-        finally:
-            res.popens.append((activity_id, activity_name, kwargs.get("env"), ret, cmd))
-        return ret
-
     return res
 
 

src/tox/config.py

@@ -8,7 +8,6 @@
 import shlex
 import string
 import sys
-import uuid
 import warnings
 from collections import OrderedDict
 from fnmatch import fnmatchcase
@@ -419,7 +418,8 @@ def tox_addoption(parser):
         "--parallel--safe-build",
         action="store_true",
         dest="parallel_safe_build",
-        help="ensure two tox builds can run in parallel",
+        help="(deprecated) ensure two tox builds can run in parallel "
+        "(uses a lock file in the tox workdir with .lock extension)",
     )
     parser.add_argument(
         "--installpkg",
@@ -1018,20 +1018,16 @@ def __init__(self, config, ini_path, ini_data):  # noqa
         if override:
             for name in config.indexserver:
                 config.indexserver[name] = IndexServerConfig(name, override)
-        unique_id = str(uuid.uuid4())
 
         reader.addsubstitutions(toxworkdir=config.toxworkdir)
         config.distdir = reader.getpath("distdir", "{toxworkdir}/dist")
-        self._make_thread_safe_path(config, "distdir", unique_id)
 
         reader.addsubstitutions(distdir=config.distdir)
         config.distshare = reader.getpath("distshare", dist_share_default)
-        self._make_thread_safe_path(config, "distshare", unique_id)
         reader.addsubstitutions(distshare=config.distshare)
         config.sdistsrc = reader.getpath("sdistsrc", None)
         config.setupdir = reader.getpath("setupdir", "{toxinidir}")
         config.logdir = config.toxworkdir.join("log")
-        self._make_thread_safe_path(config, "logdir", unique_id)
 
         self.parse_build_isolation(config, reader)
         config.envlist, all_envs = self._getenvdata(reader, config)
@@ -1076,12 +1072,6 @@ def parse_build_isolation(self, config, reader):
                     name, "{}{}".format(testenvprefix, name), reader._subs, config
                 )
 
-    def _make_thread_safe_path(self, config, attr, unique_id):
-        if config.option.parallel_safe_build:
-            path = getattr(config, attr)
-            value = py.path.local(path.dirname).join("{}-{}".format(path.basename, unique_id))
-            setattr(config, attr, value)
-
     @staticmethod
     def ensure_requires_satisfied(specified):
         missing_requirements = []

src/tox/package.py

@@ -6,6 +6,7 @@
 import pkg_resources
 import py
 import six
+from filelock import FileLock, Timeout
 
 import tox
 from tox.config import DepConfig, get_py_project_toml
@@ -27,6 +28,22 @@ def get_package(session):
     if config.skipsdist:
         report.info("skipping sdist step")
         return None
+    lock_file = str(
+        session.config.toxworkdir.join("{}.lock".format(session.config.isolated_build_env))
+    )
+    lock = FileLock(lock_file)
+    try:
+        try:
+            lock.acquire(0.0001)
+        except Timeout:
+            report.verbosity0("lock file {} present, will block until released".format(lock_file))
+            lock.acquire()
+        return acquire_package(config, report, session)
+    finally:
+        lock.release(force=True)
+
+
+def acquire_package(config, report, session):
     if not config.option.sdistonly and (config.sdistsrc or config.option.installpkg):
         path = config.option.installpkg
         if not path:

tests/unit/session/test_session.py

@@ -2,7 +2,6 @@
 import re
 import sys
 import textwrap
-import uuid
 from threading import Thread
 
 import pytest
@@ -84,31 +83,6 @@ def test_minversion(cmd, initproj):
     assert result.ret
 
 
-def test_tox_parallel_build_safe(initproj, cmd, mock_venv):
-    initproj(
-        "env_var_test",
-        filedefs={
-            "tox.ini": """
-                          [tox]
-                          envlist = py
-                          [testenv]
-                          skip_install = true
-                          commands = python --version
-                      """
-        },
-    )
-    result = cmd("--parallel--safe-build")
-
-    for path, base in (
-        (result.session.config.distdir, "dist-"),
-        (result.session.config.logdir, "log-"),
-        (result.session.config.distshare, "distshare-"),
-    ):
-        basename = path.basename
-        assert basename.startswith(base)
-        assert uuid.UUID(basename[len(base) :], version=4)
-
-
 def test_skip_sdist(cmd, initproj):
     initproj(
         "pkg123-0.7",

tests/unit/test_package.py

@@ -1,4 +1,5 @@
 import re
+import traceback
 
 import py
 import pytest
@@ -254,3 +255,64 @@ def test_dist_exists_version_change(mock_venv, initproj, cmd):
 
     result = cmd("-e", "py")
     assert result.ret == 0, result.out
+
+
+def test_tox_parallel_build_safe(initproj, cmd, mock_venv, monkeypatch):
+    initproj(
+        "env_var_test",
+        filedefs={
+            "tox.ini": """
+                          [tox]
+                          envlist = py
+                          install_cmd = python -m -c 'print("ok")' -- {opts} {packages}'
+                          [testenv]
+                          commands = python --version
+                      """
+        },
+    )
+    import tox.package
+    import threading
+
+    triggered = threading.Event()
+    event = threading.Event()
+
+    result = {}
+
+    def run(name):
+        try:
+            outcome = cmd("--parallel--safe-build")
+            result[name] = outcome
+        except Exception as exception:
+            result[name] = exception, traceback.format_exc()
+
+    with monkeypatch.context() as m:
+        prev_build_package = tox.package.build_package
+
+        def build_package(config, report, session):
+            triggered.set()
+            event.wait()
+            return prev_build_package(config, report, session)
+
+        m.setattr(tox.package, "build_package", build_package)
+        t1 = threading.Thread(target=run, args=("t1",))
+        t1.start()
+        triggered.wait()
+
+    t2 = threading.Thread(target=run, args=("t2",))
+    t2.start()
+
+    t2.join(timeout=0.1)  # 100 ms should be enough to build the package
+    assert t2.is_alive()  # if still alive means something (our lock) prevented two parallel builds
+    event.set()
+    t1.join()
+    t2.join()
+    assert result
+    for val in result.values():
+        if isinstance(val, tuple):
+            assert False, "{!r}\n{}".format(val[0], val[1])
+    err = "\n".join((result["t1"].err, result["t2"].err)).strip()
+    out = "\n".join((result["t1"].out, result["t2"].out)).strip()
+    assert not err
+    lock_file = py.path.local().join(".tox", ".package.lock")
+    msg = "lock file {} present, will block until released".format(lock_file)
+    assert msg in out

tox.ini

@@ -141,7 +141,7 @@ include_trailing_comma = True
 force_grid_wrap = 0
 line_length = 99
 known_first_party = tox,tests
-known_third_party = apiclient,docutils,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six,sphinx,toml
+known_third_party = apiclient,docutils,filelock,git,httplib2,oauth2client,packaging,pkg_resources,pluggy,py,pytest,setuptools,six,sphinx,toml
 
 [testenv:release]
 description = do a release, required posarg of the version number