Merge pull request #443 from betatim/subdir-tweak

[MRG] Switch repository used to test sub-directory support

repo2docker/app.py

@@ -35,7 +35,7 @@ from .buildpacks import (
 from . import contentproviders
 from .utils import (
     ByteSpecification, maybe_cleanup, is_valid_docker_image_name,
-    validate_and_generate_port_mapping
+    validate_and_generate_port_mapping, chdir
 )
 
 
@@ -704,48 +704,48 @@ class Repo2Docker(Application):
             self.fetch(self.repo, self.ref, checkout_path)
 
             if self.subdir:
-                checkout_path = os.path.join(checkout_path, self.subdir).rstrip('/')
+                checkout_path = os.path.join(checkout_path, self.subdir)
-                if not os.path.exists(checkout_path):
+                if not os.path.isdir(checkout_path):
-                    self.log.error('Subdirectory %s does not exist', self.subdir, extra=dict(phase='failure'))
+                    self.log.error('Subdirectory %s does not exist',
+                                   self.subdir, extra=dict(phase='failure'))
                     sys.exit(1)
 
-            os.chdir(checkout_path)
+            with chdir(checkout_path):
+                for BP in self.buildpacks:
+                    bp = BP()
+                    if bp.detect():
+                        picked_buildpack = bp
+                        break
+                else:
+                    picked_buildpack = self.default_buildpack()
 
-            for BP in self.buildpacks:
+                picked_buildpack.appendix = self.appendix
-                bp = BP()
-                if bp.detect():
-                    picked_buildpack = bp
-                    break
-            else:
-                picked_buildpack = self.default_buildpack()
 
-            picked_buildpack.appendix = self.appendix
+                self.log.debug(picked_buildpack.render(),
+                               extra=dict(phase='building'))
 
-            self.log.debug(picked_buildpack.render(),
+                if self.build:
-                           extra=dict(phase='building'))
+                    build_args = {
+                        'NB_USER': self.user_name,
+                        'NB_UID': str(self.user_id)
+                    }
+                    self.log.info('Using %s builder\n', bp.__class__.__name__,
+                                  extra=dict(phase='building'))
 
-            if self.build:
+                    for l in picked_buildpack.build(self.output_image_spec,
-                build_args = {
+                        self.build_memory_limit, build_args):
-                    'NB_USER': self.user_name,
+                        if 'stream' in l:
-                    'NB_UID': str(self.user_id)
+                            self.log.info(l['stream'],
-                }
+                                          extra=dict(phase='building'))
-                self.log.info('Using %s builder\n', bp.__class__.__name__,
+                        elif 'error' in l:
-                              extra=dict(phase='building'))
+                            self.log.info(l['error'], extra=dict(phase='failure'))
-
+                            sys.exit(1)
-                for l in picked_buildpack.build(self.output_image_spec,
+                        elif 'status' in l:
-                    self.build_memory_limit, build_args):
+                                self.log.info('Fetching base image...\r',
-                    if 'stream' in l:
+                                              extra=dict(phase='building'))
-                        self.log.info(l['stream'],
+                        else:
-                                      extra=dict(phase='building'))
+                            self.log.info(json.dumps(l),
-                    elif 'error' in l:
-                        self.log.info(l['error'], extra=dict(phase='failure'))
-                        sys.exit(1)
-                    elif 'status' in l:
-                            self.log.info('Fetching base image...\r',
                                           extra=dict(phase='building'))
-                    else:
-                        self.log.info(json.dumps(l),
-                                      extra=dict(phase='building'))
 
         if self.push:
             self.push_image()

repo2docker/utils.py

@@ -1,5 +1,6 @@
 from contextlib import contextmanager
 from functools import partial
+import os
 import re
 import shutil
 import subprocess
@@ -51,6 +52,21 @@ def execute_cmd(cmd, capture=False, **kwargs):
             raise subprocess.CalledProcessError(ret, cmd)
 
 
+@contextmanager
+def chdir(path):
+    """Change working directory to `path` and restore it again
+
+    This context maanger is useful if `path` stops existing during your
+    operations.
+    """
+    old_dir = os.getcwd()
+    os.chdir(path)
+    try:
+        yield
+    finally:
+        os.chdir(old_dir)
+
+
 @contextmanager
 def maybe_cleanup(path, cleanup=False):
     """Delete the directory at passed path if cleanup flag is True."""

tests/test_clone_depth.py

@@ -6,7 +6,11 @@ container requires a specific repository and commit to be checked out,
 and that is the only thing that is tested.
 
 """
+import os
 import subprocess
+
+from tempfile import TemporaryDirectory
+
 from repo2docker.app import Repo2Docker
 
 
@@ -16,88 +20,103 @@ URL = "https://github.com/binderhub-ci-repos/repo2docker-ci-clone-depth"
 def test_clone_depth():
     """Test a remote repository, without a refspec"""
 
-    app = Repo2Docker()
+    with TemporaryDirectory() as d:
-    argv = [URL]
+        app = Repo2Docker()
-    app.initialize(argv)
+        argv = [URL]
-    app.debug = True
+        app.initialize(argv)
-    app.run = False
+        app.build = False
-    app.cleanup_checkout = False
+        app.run = False
-    app.start()  # This just build the image and does not run it.
+        # turn of automatic clean up of the checkout so we can inspect it
+        # we also set the work directory explicitly so we know where to look
+        app.cleanup_checkout = False
+        app.git_workdir = d
+        app.start()
 
-    # Building the image has already put us in the cloned repository directory
+        cmd = ['git', 'rev-parse', 'HEAD']
-    cmd = ['git', 'rev-parse', 'HEAD']
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
-    assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
+        cmd = ['git', 'rev-list', '--count', 'HEAD']
-    cmd = ['git', 'rev-list', '--count', 'HEAD']
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        assert p.stdout.strip() == b'1'
-    assert p.stdout.strip() == b'1'
+        with open(os.path.join(d, 'COMMIT')) as fp:
-    with open('COMMIT') as fp:
+            assert fp.read() == '100\n'
-        assert fp.read() == '100\n'
 
 
 def test_clone_depth_full():
     """Test a remote repository, with a refspec of 'master'"""
 
-    app = Repo2Docker()
+    with TemporaryDirectory() as d:
-    argv = ['--ref', 'master', URL]
+        app = Repo2Docker()
-    app.initialize(argv)
+        argv = ['--ref', 'master', URL]
-    app.debug = True
+        app.initialize(argv)
-    app.run = False
+        app.run = False
-    app.cleanup_checkout = False
+        app.build = False
-    app.start()  # This just build the image and does not run it.
+        # turn of automatic clean up of the checkout so we can inspect it
+        # we also set the work directory explicitly so we know where to look
+        app.cleanup_checkout = False
+        app.git_workdir = d
+        app.start()
 
-    # Building the image has already put us in the cloned repository directory
+        # Building the image has already put us in the cloned repository directory
-    cmd = ['git', 'rev-parse', 'HEAD']
+        cmd = ['git', 'rev-parse', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
+        assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
-    cmd = ['git', 'rev-list', '--count', 'HEAD']
+        cmd = ['git', 'rev-list', '--count', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'100'
+        assert p.stdout.strip() == b'100'
-    with open('COMMIT') as fp:
+        with open(os.path.join(d, 'COMMIT')) as fp:
-        assert fp.read() == '100\n'
+            assert fp.read() == '100\n'
 
 
 def test_clone_depth_full2():
     """Test a remote repository, with a refspec of the master commit hash"""
 
-    app = Repo2Docker()
+    with TemporaryDirectory() as d:
-    argv = ['--ref', '703322e', URL]
+        app = Repo2Docker()
+        argv = ['--ref', '703322e', URL]
 
-    app.initialize(argv)
+        app.initialize(argv)
-    app.debug = True
+        app.run = False
-    app.run = False
+        app.build = False
-    app.cleanup_checkout = False
+        # turn of automatic clean up of the checkout so we can inspect it
-    app.start()  # This just build the image and does not run it.
+        # we also set the work directory explicitly so we know where to look
+        app.cleanup_checkout = False
+        app.git_workdir = d
+        app.start()
 
-    # Building the image has already put us in the cloned repository directory
+        # Building the image has already put us in the cloned repository directory
-    cmd = ['git', 'rev-parse', 'HEAD']
+        cmd = ['git', 'rev-parse', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
+        assert p.stdout.strip() == b'703322e9c6635ba1835d3b92eafbabeca0042c3e'
-    cmd = ['git', 'rev-list', '--count', 'HEAD']
+        cmd = ['git', 'rev-list', '--count', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'100'
+        assert p.stdout.strip() == b'100'
-    with open('COMMIT') as fp:
+        with open(os.path.join(d, 'COMMIT')) as fp:
-        assert fp.read() == '100\n'
+            assert fp.read() == '100\n'
 
 
 def test_clone_depth_mid():
     """Test a remote repository, with a refspec of a commit hash halfway"""
 
-    app = Repo2Docker()
+    with TemporaryDirectory() as d:
-    argv = ['--ref', '8bc4f21', URL]
+        app = Repo2Docker()
+        argv = ['--ref', '8bc4f21', URL]
 
-    app.initialize(argv)
+        app.initialize(argv)
-    app.debug = True
+        app.run = False
-    app.run = False
+        app.build = False
-    app.cleanup_checkout = False
+        # turn of automatic clean up of the checkout so we can inspect it
-    app.start()  # This just build the image and does not run it.
+        # we also set the work directory explicitly so we know where to look
+        app.cleanup_checkout = False
+        app.git_workdir = d
+        app.start()
 
-    # Building the image has already put us in the cloned repository directory
+        # Building the image has already put us in the cloned repository directory
-    cmd = ['git', 'rev-parse', 'HEAD']
+        cmd = ['git', 'rev-parse', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'8bc4f216856f86f6fc25a788b744b93b87e9ba48'
+        assert p.stdout.strip() == b'8bc4f216856f86f6fc25a788b744b93b87e9ba48'
-    cmd = ['git', 'rev-list', '--count', 'HEAD']
+        cmd = ['git', 'rev-list', '--count', 'HEAD']
-    p = subprocess.run(cmd, stdout=subprocess.PIPE)
+        p = subprocess.run(cmd, stdout=subprocess.PIPE, cwd=d)
-    assert p.stdout.strip() == b'50'
+        assert p.stdout.strip() == b'50'
-    with open('COMMIT') as fp:
+        with open(os.path.join(d, 'COMMIT')) as fp:
-        assert fp.read() == '50\n'
+            assert fp.read() == '50\n'

tests/test_subdir.py

@@ -1,28 +1,38 @@
 """
 Test if the subdirectory is correctly navigated to
 """
+import os
 import logging
-from os.path import abspath, dirname
 
 import pytest
 from repo2docker.app import Repo2Docker
 
-# This is the path to the repo2docker git repository that this file exists in.
+TEST_REPO = "https://github.com/binderhub-ci-repos/repo2docker-subdir-support"
-repo_path = dirname(dirname(abspath(__file__)))
 
 
 def test_subdir(run_repo2docker):
-    argv = ['--subdir', 'tests/conda/simple', repo_path]
+    # Build from a subdirectory
+    # if subdir support is broken this will fail as the instructions in the
+    # root of the test repo are invalid
+    cwd = os.getcwd()
+
+    argv = ['--subdir', 'a directory', TEST_REPO]
     run_repo2docker(argv)
 
+    # check that we restored the current working directory
+    assert cwd == os.getcwd(), "We should be back in %s" % cwd
+
 
 def test_subdir_invalid(caplog):
-    caplog.set_level(logging.INFO, logger='Repo2Docker')
+    # test an error is raised when requesting a non existent subdir
+    #caplog.set_level(logging.INFO, logger='Repo2Docker')
 
     app = Repo2Docker()
-    argv = ['--subdir', 'tests/conda/invalid', repo_path]
+    argv = ['--subdir', 'invalid-sub-dir', TEST_REPO]
     app.initialize(argv)
     app.debug = True
+    # no build does not imply no run
+    app.build = False
     app.run = False
     with pytest.raises(SystemExit) as excinfo:
         app.start()  # Just build the image and do not run it.
@@ -31,4 +41,4 @@ def test_subdir_invalid(caplog):
     assert excinfo.value.code == 1
 
     # Can't get this to record the logs?
-    # assert caplog.text == "Subdirectory tests/conda/invalid does not exist"
+    #assert caplog.text == "Subdirectory tests/conda/invalid does not exist"