Merge pull request #1257 from msarahan/fix_deadlock_in_create_env

Fix deadlock in create env

.travis.yml

@@ -28,7 +28,7 @@ install:
   - conda update -q --all
   - conda install -q --force --no-deps conda requests
   - conda install -q pip pytest requests jinja2 patchelf flake8 mock python=$TRAVIS_PYTHON_VERSION pyflakes=1.1
-  - conda install -q anaconda-client pip pytest-cov numpy
+  - conda install -q anaconda-client pip pytest-cov numpy pytest-timeout
   - conda install -c conda-forge -q perl
   - pip install pytest-cov pytest-xdist pytest-capturelog filelock
   - if [[ "$CANARY" == "true" ]]; then

appveyor.yml

@@ -59,7 +59,7 @@ install:
   - python -c "import sys; print(sys.version)"
   - python -c "import sys; print(sys.executable)"
   - python -c "import sys; print(sys.prefix)"
-  - conda install -q pip pytest pytest-cov jinja2 patch flake8 mock requests
+  - conda install -q pip pytest pytest-cov pytest-timeout jinja2 patch flake8 mock requests
   - conda install -q pyflakes=1.1 pycrypto posix m2-git anaconda-client numpy
   - conda install -c conda-forge -q perl
   # this is to ensure dependencies

conda.recipe/meta.yaml

@@ -34,6 +34,7 @@ test:
   requires:
     - pytest
     - pytest-cov
+    - pytest-timeout
     # Optional: you can use pytest-xdist to run the tests in parallel
     # - pytest-env  # [win]
     # - pytest-xdist

conda_build/api.py

@@ -19,7 +19,7 @@
 import sys as _sys
 
 # make the Config class available in the api namespace
-from conda_build.config import Config, get_or_merge_config
+from conda_build.config import Config, get_or_merge_config, DEFAULT_PREFIX_LENGTH as _prefix_length
 
 
 def _ensure_list(recipe_arg):
@@ -225,6 +225,21 @@ def inspect_objects(packages, prefix=_sys.prefix, groupby='filename'):
     return inspect_objects(packages, prefix=prefix, groupby=groupby)
 
 
+def inspect_prefix_length(packages, min_prefix_length=_prefix_length):
+    from conda_build.tarcheck import check_prefix_lengths
+    packages = _ensure_list(packages)
+    prefix_lengths = check_prefix_lengths(packages, min_prefix_length)
+    if prefix_lengths:
+        print("Packages with binary prefixes shorter than %d characters:"
+                % min_prefix_length)
+        for fn, length in prefix_lengths.items():
+            print("{0} ({1} chars)".format(fn, length))
+    else:
+        print("No packages found with binary prefixes shorter than %d characters."
+                % min_prefix_length)
+    return len(prefix_lengths) == 0
+
+
 def create_metapackage(name, version, entry_points=(), build_string=None, build_number=0,
                        dependencies=(), home=None, license=None, summary=None,
                        config=None):

conda_build/build.py

@@ -4,7 +4,6 @@
 from __future__ import absolute_import, division, print_function
 
 from collections import deque
-import errno
 import fnmatch
 from glob import glob
 import io
@@ -18,7 +17,6 @@
 import subprocess
 import sys
 import tarfile
-import time
 
 # this is to compensate for a requests idna encoding error.  Conda is a better place to fix,
 #   eventually
@@ -390,50 +388,57 @@ def create_env(prefix, specs, config, clear_cache=True):
         if value:
             specs.append('%s@' % feature)
 
-    for d in config.bldpkgs_dirs:
-        if not isdir(d):
-            os.makedirs(d)
-        update_index(d, config)
     if specs:  # Don't waste time if there is nothing to do
+        for d in config.bldpkgs_dirs:
+            if not isdir(d):
+                os.makedirs(d)
+            update_index(d, config)
         with path_prepended(prefix):
             index = get_build_index(config=config, clear_cache=True)
 
-            warn_on_old_conda_build(index)
-
             cc.pkgs_dirs = cc.pkgs_dirs[:1]
             dirname = os.path.join(cc.root_dir, 'pkgs')
-            with filelock.SoftFileLock(os.path.join(dirname, ".conda_lock"),
-                                                    timeout=config.timeout):
+            lock_file = os.path.join(dirname, ".conda_lock")
+
+            lock = filelock.SoftFileLock(lock_file)
+            lock.acquire(timeout=config.timeout)
+
+            try:
                 actions = plan.install_actions(prefix, index, specs)
                 plan.display_actions(actions, index)
-                try:
-                    if on_win:
-                        for k, v in os.environ.items():
-                            os.environ[k] = str(v)
-                    plan.execute_actions(actions, index, verbose=config.debug)
-                except OSError as e:
-                    if e.errno == errno.ENOENT:
-                        time.sleep(5)
-                        plan.execute_actions(actions, index, verbose=config.debug)
-                except SystemExit as exc:
-                    if "too short in" in str(exc) and config.prefix_length > 80:
-                        log.warn("Build prefix failed with prefix length {0}."
-                                .format(config.prefix_length))
-                        log.warn("Error was: ")
-                        log.warn(str(exc))
-                        log.warn("One or more of your package dependencies needs to be rebuilt "
-                                "with a longer prefix length.")
-                        log.warn("Falling back to legacy prefix length of 80 characters.")
-                        log.warn("Your package will not install into prefixes > 80 characters.")
-                        config.prefix_length = 80
-
-                        # Set this here and use to create environ
-                        #   Setting this here is important because we use it below (symlink)
-                        prefix = config.build_prefix
-                        create_env(prefix, specs, config=config,
-                                   clear_cache=clear_cache)
-                    else:
-                        raise
+                if on_win:
+                    for k, v in os.environ.items():
+                        os.environ[k] = str(v)
+                plan.execute_actions(actions, index, verbose=config.debug)
+            except SystemExit as exc:
+                if "too short in" in str(exc) and config.prefix_length > 80:
+                    log.warn("Build prefix failed with prefix length {0}."
+                            .format(config.prefix_length))
+                    log.warn("Error was: ")
+                    log.warn(str(exc))
+                    log.warn("One or more of your package dependencies needs to be rebuilt "
+                            "with a longer prefix length.")
+                    log.warn("Falling back to legacy prefix length of 80 characters.")
+                    log.warn("Your package will not install into prefixes > 80 characters.")
+                    config.prefix_length = 80
+
+                    # Set this here and use to create environ
+                    #   Setting this here is important because we use it below (symlink)
+                    prefix = config.build_prefix
+                    lock.release()
+                    if os.path.isfile(lock_file):
+                        os.remove(lock_file)
+                    create_env(prefix, specs, config=config,
+                                clear_cache=clear_cache)
+                else:
+                    lock.release()
+                    if os.path.isfile(lock_file):
+                        os.remove(lock_file)
+                    raise
+            finally:
+                lock.release()
+                if os.path.isfile(lock_file):
+                    os.remove(lock_file)
         warn_on_old_conda_build(index)
 
     # ensure prefix exists, even if empty, i.e. when specs are empty

conda_build/cli/main_build.py

@@ -12,6 +12,8 @@
 import sys
 import warnings
 
+import filelock
+
 import conda_build.api as api
 import conda_build.build as build
 from conda_build.cli.main_render import (set_language_env_vars, RecipeCompleter,
@@ -259,6 +261,10 @@ def main():
     except RuntimeError as e:
         print(str(e))
         sys.exit(1)
+    except filelock.Timeout as e:
+        print("File lock could on {0} not be obtained.  You might need to try fewer builds at once."
+              "  Otherwise, run conda clean --lock".format(e.lock_file))
+        sys.exit(1)
     except Exception as e:
         print_issue_message(str(e))
         sys.exit(1)

conda_build/cli/main_inspect.py

@@ -138,6 +138,23 @@ def parse_args(args):
         help="The channel to test. The default is %(default)s."
     )
 
+    prefix_lengths = subcommand.add_parser(
+        "prefix-lengths",
+        help="""Inspect packages in given path, finding those with binary
+            prefixes shorter than specified""",
+        description=linkages_help,
+    )
+    prefix_lengths.add_argument(
+        'folder',
+        help='folder containing packages to inspect.',
+    )
+    prefix_lengths.add_argument(
+        '--min-prefix-length', '-m',
+        help='Minimum length.  Only packages with prefixes below this are shown.',
+        default=api.Config().prefix_length,
+        type=int,
+    )
+
     args = p.parse_args(args)
     return p, args
 

conda_build/config.py

@@ -23,6 +23,8 @@
 # conda_build.config.config.build_prefix, as that won't reflect any mutated
 # changes.
 
+DEFAULT_PREFIX_LENGTH = 255
+
 
 class Config(object):
     __file__ = __path__ = __file__
@@ -63,7 +65,8 @@ def env(lang, default):
             self.CONDA_NPY = int(self.CONDA_NPY.replace('.', '')) or None
 
         self._build_id = kwargs.get('build_id', getattr(self, '_build_id', ""))
-        self._prefix_length = kwargs.get("prefix_length", getattr(self, '_prefix_length', 255))
+        self._prefix_length = kwargs.get("prefix_length", getattr(self, '_prefix_length',
+                                                                  DEFAULT_PREFIX_LENGTH))
         # set default value (not actually None)
         self._croot = kwargs.get('croot', getattr(self, '_croot', None))
 

conda_build/tarcheck.py

@@ -1,11 +1,11 @@
 from __future__ import absolute_import, division, print_function
 
-import glob
 import json
-import os
 from os.path import basename
 import tarfile
 
+from conda_build.utils import codec
+
 
 def dist_fn(fn):
     if fn.endswith('.tar'):
@@ -65,6 +65,8 @@ def prefix_length(self):
                 # lines not conforming to the split
                 except ValueError:
                     continue
+                if hasattr(file_type, 'decode'):
+                    file_type = file_type.decode(codec)
                 if file_type == 'binary':
                     prefix_length = len(prefix)
                     break
@@ -78,8 +80,7 @@ def check_all(path):
     x.t.close()
 
 
-def check_prefix_lengths(folder, min_prefix_length=255):
-    files = glob.glob(os.path.join(folder, "*.tar.bz2"))
+def check_prefix_lengths(files, min_prefix_length=255):
     lengths = {}
     for f in files:
         length = TarCheck(f).prefix_length()

conda_build/utils.py

@@ -107,6 +107,8 @@ def copy_into(src, dst, config, symlinks=False):
                 dst_fn = dst
 
             try:
+                if not os.path.isdir(os.path.dirname(dst_fn)):
+                    os.makedirs(os.path.dirname(dst_fn))
                 shutil.copy2(src, dst_fn)
             except shutil.Error:
                 log.debug("skipping {0} - already exists in {1}".format(os.path.basename(src), dst))

tests/test_build.py

@@ -6,8 +6,11 @@
 import os
 import sys
 
-from conda_build import build
+import pytest
+
+from conda_build import build, api
 from conda_build.metadata import MetaData
+from conda_build.utils import rm_rf, on_win
 
 from .utils import testing_workdir, test_config, metadata_dir
 
@@ -49,3 +52,27 @@ def test_build_preserves_PATH(testing_workdir, test_config):
     ref_path = os.environ['PATH']
     build.build(m, test_config)
     assert os.environ['PATH'] == ref_path
+
+
+@pytest.mark.timeout(60)
+@pytest.mark.skipif(on_win, reason=("Windows binary prefix replacement (for pip exes)"
+                                    " not length dependent"))
+def test_env_creation_with_short_prefix_does_not_deadlock(caplog):
+    test_base = os.path.expanduser("~/cbtmp")
+    config = api.Config(croot=test_base, anaconda_upload=False, verbose=True)
+    recipe_path = os.path.join(metadata_dir, "has_prefix_files")
+    fn = api.get_output_file_path(recipe_path, config=config)
+    if os.path.isfile(fn):
+        os.remove(fn)
+    config.prefix_length = 80
+    try:
+        api.build(recipe_path, config=config)
+        pkg_name = os.path.basename(fn).replace("-1.0-0.tar.bz2", "")
+        assert not api.inspect_prefix_length(fn, 255)
+        config.prefix_length = 255
+        build.create_env(config.build_prefix, specs=["python", pkg_name], config=config)
+    except:
+        raise
+    finally:
+        rm_rf(test_base)
+    assert 'One or more of your package dependencies needs to be rebuilt' in caplog.text()