Avoid build package becoming unavailable during session run

We create a hard link (if possible, otherwise copy of the
built package) inside a temp folder. This way subsequent sessions are
free to remove dist folder and we also guarantee that the built package
during the build is what tox environments use (even if the package
gets rebuilt in the meantime).

Author: gaborbernat

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
 -   repo: https://github.com/ambv/black
-    rev: 18.6b4
+    rev: 18.9b0
     hooks:
     -   id: black
         args: [--line-length=99, --safe]

changelog/1026.feature.rst

@@ -0,0 +1,9 @@
+Added ``temp_dir`` folder configuration (defaults to ``{toxworkdir}/.tmp``) that contains tox
+temporary files. Package builds now create a hard link (if possible, otherwise copy - notably
+Windows Python 2.7) to the built file, and feed that file downstream (e.g. for pip to install it).
+The hard link is removed at the end of the run (what it points though is kept inside ``distdir``).
+This ensures that a tox session operates on the same package it built, even if a parallel tox run
+builds another version. Note ``distdir`` will contain only the last built package in such cases.
+ - by :user:`gaborbernat`
+
+

doc/config.rst

@@ -43,6 +43,13 @@ Global settings are defined under the ``tox`` section as:
 
    Directory for tox to generate its environments into, will be created if it does not exist.
 
+.. conf:: temp_dir ^ PATH ^ {toxinidir}/.tmp
+
+   Directory where to put tox temporary files. For example: we create a hard link (if possible,
+   otherwise new copy) in this directory for the project package. This ensures tox works correctly
+   when having parallel runs (as each session will have its own copy of the project package - e.g.
+   the source distribution).
+
 .. conf:: skipsdist ^ true|false ^ false
 
    Flag indicating to perform the packaging operation or not. Set it to ``true`` when using tox for
@@ -59,10 +66,11 @@ Global settings are defined under the ``tox`` section as:
    Directory where the packaged source distribution should be put. Note this is cleaned at the start of
    every packaging invocation.
 
-.. conf:: distshare ^ PATH ^ {toxworkdir}/distshare
+.. conf:: distshare ^ PATH ^ {homedir}/.tox/distshare
 
    Folder where the packaged source distribution will be moved, this is not cleaned between packaging
-   invocations.
+   invocations. On Jenkins (exists ``JENKINS_URL`` or ``HUDSON_URL`` environment variable)
+   the default path is ``{toxworkdir}/distshare``.
 
 .. conf:: envlist ^ comma separated values
 

src/tox/config.py

@@ -1024,6 +1024,7 @@ def __init__(self, config, ini_path, ini_data):  # noqa
 
         reader.addsubstitutions(distdir=config.distdir)
         config.distshare = reader.getpath("distshare", dist_share_default)
+        config.temp_dir = reader.getpath("temp_dir", "{toxworkdir}/.tmp")
         reader.addsubstitutions(distshare=config.distshare)
         config.sdistsrc = reader.getpath("sdistsrc", None)
         config.setupdir = reader.getpath("setupdir", "{toxinidir}")

src/tox/package.py

@@ -1,7 +1,9 @@
 import json
+import os
 import sys
 import textwrap
 from collections import namedtuple
+from itertools import chain
 
 import pkg_resources
 import py
@@ -38,11 +40,40 @@ def get_package(session):
         except Timeout:
             report.verbosity0("lock file {} present, will block until released".format(lock_file))
             lock.acquire()
-        return acquire_package(config, report, session)
+        package = acquire_package(config, report, session)
+        return create_session_view(package, config.temp_dir, report)
     finally:
         lock.release(force=True)
 
 
+def create_session_view(package, temp_dir, report):
+    """once we build a package we cannot return that directly, as a subsequent call
+    might delete that package (in order to do its own build); therefore we need to
+    return a view of the file that it's not prone to deletion and can be removed when the
+    session ends
+    """
+    exists = [
+        i.basename[len(package.basename) + 1 :]
+        for i in temp_dir.listdir(fil="{}.*".format(package.basename))
+    ]
+    file_id = max(chain((0,), (int(i) for i in exists if six.text_type(i).isnumeric())))
+    session_package = temp_dir.join("{}.{}".format(package.basename, file_id + 1))
+    # if we can do hard links do that, otherwise just copy
+    operation = "links"
+    if hasattr(os, "link"):
+        os.link(str(package), str(session_package))
+    else:
+        operation = "copied"
+        package.copy(session_package)
+    common = session_package.common(package)
+    report.verbosity1(
+        "cwd {} package {} {} to {}".format(
+            common, common.bestrelpath(session_package), operation, common.bestrelpath(package)
+        )
+    )
+    return session_package
+
+
 def acquire_package(config, report, session):
     if not config.option.sdistonly and (config.sdistsrc or config.option.installpkg):
         path = config.option.installpkg

src/tox/result.py

@@ -1,9 +1,8 @@
 import json
 import socket
+import subprocess
 import sys
 
-import py
-
 import tox
 
 
@@ -44,24 +43,18 @@ def __init__(self, reportlog, name, dict):
         self.name = name
         self.dict = dict
 
-    def set_python_info(self, pythonexecutable):
-        pythonexecutable = py.path.local(pythonexecutable)
-        out = pythonexecutable.sysexec(
+    def set_python_info(self, python_executable):
+        cmd = [
+            python_executable,
             "-c",
-            "import sys; "
-            "print(sys.executable);"
-            "print(list(sys.version_info)); "
-            "print(sys.version)",
-        )
-        lines = out.splitlines()
-        executable = lines.pop(0)
-        version_info = eval(lines.pop(0))
-        version = "\n".join(lines)
-        self.dict["python"] = {
-            "executable": executable,
-            "version_info": version_info,
-            "version": version,
-        }
+            "import sys; import json;"
+            "print(json.dumps({"
+            "'executable': sys.executable,"
+            "'version_info': list(sys.version_info),"
+            "'version': sys.version}))",
+        ]
+        result = subprocess.check_output(cmd, universal_newlines=True)
+        self.dict["python"] = json.loads(result)
 
     def get_commandlog(self, name):
         return CommandLog(self, self.dict.setdefault(name, []))

src/tox/session.py

@@ -13,6 +13,7 @@
 import subprocess
 import sys
 import time
+from contextlib import contextmanager
 
 import pkg_resources
 import py
@@ -448,7 +449,20 @@ def runcommand(self):
         elif self.config.option.listenvs_all:
             self.showenvs(all_envs=True, description=verbosity)
         else:
-            return self.subcommand_test()
+            with self.cleanup():
+                return self.subcommand_test()
+
+    @contextmanager
+    def cleanup(self):
+        self.config.temp_dir.ensure(dir=True)
+        try:
+            yield
+        finally:
+            for tox_env in self.venvlist:
+                if hasattr(tox_env, "package"):
+                    if tox_env.package.exists():
+                        self.report.verbosity2("cleanup {}".format(tox_env.package))
+                        tox_env.package.remove()
 
     def _copyfiles(self, srcdir, pathlist, destdir):
         for relpath in pathlist:

tests/unit/test_package.py

@@ -1,5 +1,6 @@
 import re
 import traceback
+from functools import partial
 
 import py
 import pytest
@@ -262,57 +263,110 @@ def test_tox_parallel_build_safe(initproj, cmd, mock_venv, monkeypatch):
         "env_var_test",
         filedefs={
             "tox.ini": """
-                          [tox]
-                          envlist = py
-                          install_cmd = python -m -c 'print("ok")' -- {opts} {packages}'
-                          [testenv]
-                          commands = python --version
+                  [tox]
+                  envlist = py
+                  install_cmd = python -m -c 'print("ok")' -- {opts} {packages}'
+                  [testenv]
+                  commands = python --version
                       """
         },
     )
+    # we try to recreate the following situation
+    # t1 starts and performs build
+    # t2 starts, but is blocked from t1 build lock to build
+    # t1 gets unblocked, t2 can now enter
+    # t1 is artificially blocked to run test command until t2 finishes build
+    #  (parallel build package present)
+    # t2 package build finishes both t1 and t2 can now finish and clean up their build packages
     import tox.package
     import threading
 
-    triggered = threading.Event()
-    event = threading.Event()
+    t1_build_started = threading.Event()
+    t1_build_blocker = threading.Event()
+    t2_build_started = threading.Event()
+    t2_build_finished = threading.Event()
 
-    result = {}
+    invoke_result = {}
 
-    def run(name):
+    def invoke_tox_in_thread(thread_name):
         try:
-            outcome = cmd("--parallel--safe-build")
-            result[name] = outcome
+            result = cmd("--parallel--safe-build", "-vv")
         except Exception as exception:
-            result[name] = exception, traceback.format_exc()
+            result = exception, traceback.format_exc()
+        invoke_result[thread_name] = result
+
+    prev_build_package = tox.package.build_package
 
     with monkeypatch.context() as m:
-        prev_build_package = tox.package.build_package
 
         def build_package(config, report, session):
-            triggered.set()
-            event.wait()
+            t1_build_started.set()
+            prev_run_test_env = tox.session.Session.runtestenv
+
+            def run_test_env(self, venv, redirect=False):
+                t2_build_finished.wait()
+                return prev_run_test_env(self, venv, redirect)
+
+            session.runtestenv = partial(run_test_env, session)
+
+            t1_build_blocker.wait()
             return prev_build_package(config, report, session)
 
         m.setattr(tox.package, "build_package", build_package)
-        t1 = threading.Thread(target=run, args=("t1",))
+
+        t1 = threading.Thread(target=invoke_tox_in_thread, args=("t1",))
         t1.start()
-        triggered.wait()
+        t1_build_started.wait()
 
-    t2 = threading.Thread(target=run, args=("t2",))
-    t2.start()
+    with monkeypatch.context() as m:
+
+        def build_package(config, report, session):
+            t2_build_started.set()
+            try:
+                return prev_build_package(config, report, session)
+            finally:
+                t2_build_finished.set()
 
-    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()
+        m.setattr(tox.package, "build_package", build_package)
+
+        t2 = threading.Thread(target=invoke_tox_in_thread, args=("t2",))
+        t2.start()
+
+        # t2 should get blocked by t1 build lock
+        t2_build_started.wait(timeout=0.1)
+        assert not t2_build_started.is_set()
+
+        t1_build_blocker.set()  # release t1 blocker -> t1 can now finish
+        # t1 at this point should block at run test until t2 build finishes
+        t2_build_started.wait()
+
+    t1.join()  # wait for both t1 and t2 to finish
     t2.join()
-    assert result
-    for val in result.values():
+
+    # all threads finished without error
+    for val in invoke_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()
+
+    # output has no error
+    err = "\n".join((invoke_result["t1"].err, invoke_result["t2"].err)).strip()
+    out = "\n".join((invoke_result["t1"].out, invoke_result["t2"].out)).strip()
     assert not err
+
+    # when the lock is hit we notify
     lock_file = py.path.local().join(".tox", ".package.lock")
     msg = "lock file {} present, will block until released".format(lock_file)
     assert msg in out
+
+    # intermediate packages are removed at end of build
+    t1_package = invoke_result["t1"].session.getvenv("py").package
+    t2_package = invoke_result["t1"].session.getvenv("py").package
+    assert t1 != t2
+    assert not t1_package.exists()
+    assert not t2_package.exists()
+
+    # the final distribution remains
+    dist_after = invoke_result["t1"].session.config.distdir.listdir()
+    assert len(dist_after) == 1
+    sdist = dist_after[0]
+    assert t1_package != sdist