Repository: typesafehub/conductr-cli

Issue:
`bndl` produces bundles with differing digests despite same input.
Given the same input, we should produce a bundle with a constant digest. I believe the bug here is around timestamps. See below.

```bash
$ docker save nginx | sha256sum
bd4f716f157c4f0e7e4d3007fc38572a4b946884f1908d68520b88d8758caa4a  -
-> 0

~ $ docker save nginx | sha256sum
bd4f716f157c4f0e7e4d3007fc38572a4b946884f1908d68520b88d8758caa4a  -
-> 0

# Note digests are the same for two executions
```
 -vs-
```bash
$ docker save nginx | bndl --no-shazar | sha256sum
72493678617a8c4588ba98c64adb4649853ad914a130b147977a5e381cf5d54c  -
-> 0

$ docker save nginx | bndl --no-shazar | sha256sum
5e42b7b441d9a2dbda9343047b59446de9b6ce2616e97baef3d13b037ec92382  -
-> 0

# Note digests are different for two executions
```

Discussion:

Patch:
diff --git a/conductr_cli/bndl_create.py b/conductr_cli/bndl_create.py
index 1e40b09..050f089 100644
--- a/conductr_cli/bndl_create.py
+++ b/conductr_cli/bndl_create.py
@@ -1,15 +1,18 @@
-from conductr_cli.bndl_oci import oci_image_bundle_conf, oci_image_extract_manifest_config, oci_image_unpack
+from conductr_cli import bndl_oci
+from conductr_cli.bndl_oci import oci_image_bundle_conf, oci_image_unpack
 from conductr_cli.bndl_docker import docker_unpack
-from conductr_cli.bndl_utils import detect_format_dir, detect_format_stream
+from conductr_cli.bndl_utils import detect_format_dir, detect_format_stream, first_mtime
 from conductr_cli.constants import BNDL_PEEK_SIZE, IO_CHUNK_SIZE
 from conductr_cli.shazar_main import dir_to_zip, write_with_digest
 from io import BufferedReader, BytesIO
+import arrow
 import logging
 import os
 import shutil
 import sys
 import tarfile
 import tempfile
+import time
 import zipfile
 
 
@@ -21,7 +24,7 @@ def bndl_create(args):
 
         return 2
 
-    buff_in = BufferedReader(sys.stdin.buffer, IO_CHUNK_SIZE)
+    buff_in = BufferedReader(sys.stdin.buffer, IO_CHUNK_SIZE) if args.source is None else None
 
     if args.format is None:
         if args.source is None:
@@ -100,18 +103,27 @@ def bndl_create(args):
 
             return 2
 
-        oci_manifest, oci_config = oci_image_extract_manifest_config(oci_image_dir, args.tag)
+        oci_manifest, oci_config = bndl_oci.oci_image_extract_manifest_config(oci_image_dir, args.tag)
         bundle_conf = oci_image_bundle_conf(args, component_name, oci_manifest, oci_config)
         bundle_conf_name = os.path.join(args.name, 'bundle.conf')
         bundle_conf_data = bundle_conf.encode('UTF-8')
 
+        # bundle data timestamps can vary based on how it's acquired (docker save, our own resolver, etc) so we
+        # deterministically set the mtime of the files to be based on the OCI config 'created' value if available
+
+        if oci_config and 'created' in oci_config:
+            mtime = arrow.get(oci_config['created']).timestamp
+        else:
+            mtime = first_mtime(temp_dir)
+
         # bundle.conf must be written first, so we explicitly do that for zip and tar
 
         if args.use_shazar:
             with tempfile.NamedTemporaryFile() as zip_file_data:
                 with zipfile.ZipFile(zip_file_data, 'w') as zip_file:
-                    zip_file.writestr(bundle_conf_name, bundle_conf_data)
-                    dir_to_zip(temp_dir, zip_file, args.name)
+                    bundle_conf_zinfo = zipfile.ZipInfo(filename=bundle_conf_name, date_time=time.localtime(mtime)[:6])
+                    zip_file.writestr(bundle_conf_zinfo, bundle_conf_data)
+                    dir_to_zip(temp_dir, zip_file, args.name, mtime)
 
                 zip_file_data.flush()
                 zip_file_data.seek(0)
@@ -121,12 +133,14 @@ def bndl_create(args):
             with tarfile.open(fileobj=output, mode='w|') as tar:
                 info = tarfile.TarInfo(name=bundle_conf_name)
                 info.size = len(bundle_conf_data)
+                info.mtime = mtime
                 tar.addfile(tarinfo=info, fileobj=BytesIO(bundle_conf_data))
 
                 for (dir_path, dir_names, file_names) in os.walk(temp_dir):
                     for file_name in file_names:
                         path = os.path.join(dir_path, file_name)
                         name = os.path.join(args.name, os.path.relpath(path, start=temp_dir))
+                        os.utime(path, (mtime, mtime))
                         tar.add(path, arcname=name)
 
         output.flush()
diff --git a/conductr_cli/bndl_docker.py b/conductr_cli/bndl_docker.py
index def7abe..8435aca 100644
--- a/conductr_cli/bndl_docker.py
+++ b/conductr_cli/bndl_docker.py
@@ -218,7 +218,7 @@ def docker_unpack(destination, data, is_dir, maybe_name, maybe_tag):
                     # bundles are packaged into zips, so we don't want to compress, but OCI tooling
                     # as of 2017-03-14 is broken for plain tar files; they must be gzip
 
-                    with gzip.GzipFile(fileobj=dest_file_digest, mode='wb', compresslevel=0) as dest:
+                    with gzip.GzipFile(fileobj=dest_file_digest, mode='wb', compresslevel=0, mtime=0) as dest:
                         shutil.copyfileobj(fileobj, dest)
 
                     dest_file_hexdigest = dest_file_digest.digest_out.hexdigest()
diff --git a/conductr_cli/bndl_utils.py b/conductr_cli/bndl_utils.py
index 064080b..dba2f3c 100644
--- a/conductr_cli/bndl_utils.py
+++ b/conductr_cli/bndl_utils.py
@@ -137,6 +137,14 @@ def file_write_bytes(path, bs):
         file.write(bs)
 
 
+def first_mtime(path):
+    for (dir_path, dir_names, file_names) in os.walk(path):
+        for file_name in file_names:
+            return os.path.getmtime(os.path.join(dir_path, file_name))
+
+    return 0
+
+
 class DigestReaderWriter(object):
     def __init__(self, fileobj):
         self.digest_in = hashlib.sha256()
diff --git a/conductr_cli/conduct_dcos.py b/conductr_cli/conduct_dcos.py
index 65e4abc..a91dcef 100755
--- a/conductr_cli/conduct_dcos.py
+++ b/conductr_cli/conduct_dcos.py
@@ -2,22 +2,12 @@ from dcos import constants
 import logging
 import os
 import shutil
-import sys
 
 
 def setup(args):
     """`conduct setup-dcos` command"""
     log = logging.getLogger(__name__)
-
-    if sys.executable.endswith(os.path.sep + 'conduct'):
-        src = sys.executable
-    else:
-        src = shutil.which('conduct')
-
-        if src is None:
-            log.error('Unable to determine location of \'conduct\'; is it on the PATH?')
-            return False
-
+    src = shutil.which("conduct")
     dst = os.path.join(os.path.expanduser('~'),
                        constants.DCOS_DIR,
                        constants.DCOS_SUBCOMMAND_SUBDIR,
@@ -25,11 +15,10 @@ def setup(args):
                        constants.DCOS_SUBCOMMAND_ENV_SUBDIR,
                        'bin',
                        constants.DCOS_COMMAND_PREFIX + 'conduct')
-
-    if os.path.exists(dst) or os.path.islink(dst):
+    if os.path.exists(dst):
         os.remove(dst)
-
-    os.makedirs(os.path.dirname(dst), exist_ok=True)
+    else:
+        os.makedirs(os.path.dirname(dst), exist_ok=True)
     os.symlink(src, dst)
     log.screen('The DC/OS CLI is now configured.\n'
                'Prefix \'conduct\' with \'dcos\' when you want to contact ConductR on DC/OS e.g. \'dcos conduct info\'')
diff --git a/conductr_cli/shazar_main.py b/conductr_cli/shazar_main.py
index e9e02e8..e4e92ca 100755
--- a/conductr_cli/shazar_main.py
+++ b/conductr_cli/shazar_main.py
@@ -107,11 +107,15 @@ def shazar(args):
                 sys.stdout.write(dest + os.linesep)
 
 
-def dir_to_zip(dir, zip_file, source_base_name):
+def dir_to_zip(dir, zip_file, source_base_name, mtime=None):
     for (dir_path, dir_names, file_names) in os.walk(dir):
         for file_name in file_names:
             path = os.path.join(dir_path, file_name)
             name = os.path.join(source_base_name, os.path.relpath(path, start=dir))
+
+            if mtime is not None:
+                os.utime(path, (mtime, mtime))
+
             zip_file.write(path, name)
 
 


Test patch:
diff --git a/conductr_cli/test/test_bndl_create.py b/conductr_cli/test/test_bndl_create.py
index 33daf03..43a085c 100644
--- a/conductr_cli/test/test_bndl_create.py
+++ b/conductr_cli/test/test_bndl_create.py
@@ -4,7 +4,9 @@ from io import BytesIO
 from unittest.mock import patch, MagicMock
 import os
 import shutil
+import tarfile
 import tempfile
+import time
 import zipfile
 
 
@@ -60,33 +62,181 @@ class TestBndlCreate(CliTestCase):
     def test_no_ref(self):
         tmpdir = tempfile.mkdtemp()
 
-        with tempfile.NamedTemporaryFile() as output:
+        try:
+            with tempfile.NamedTemporaryFile() as output:
+                attributes = create_attributes_object({
+                    'name': 'test',
+                    'source': tmpdir,
+                    'format': 'oci-image',
+                    'tag': 'latest',
+                    'output': output.name
+                })
+
+            stdout_mock = MagicMock()
+            stderr_mock = MagicMock()
+            logging_setup.configure_logging(MagicMock(), stdout_mock, stderr_mock)
+
+            os.mkdir(os.path.join(tmpdir, 'refs'))
+            open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+
+            with \
+                    patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
+                    patch('sys.stdout.buffer.write', stdout_mock):
+                bndl_create.bndl_create(attributes)
+
+            self.assertEqual(
+                self.output(stderr_mock),
+                as_error('Error: bndl: Invalid OCI Image. Cannot find requested tag "latest" in OCI Image\n')
+            )
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def test_with_shazar(self):
+        stdout_mock = MagicMock()
+        tmpdir = tempfile.mkdtemp()
+        tmpfile = os.path.join(tmpdir, 'output')
+
+        try:
             attributes = create_attributes_object({
                 'name': 'test',
                 'source': tmpdir,
                 'format': 'oci-image',
                 'tag': 'latest',
-                'output': output.name
+                'output': tmpfile,
+                'component_description': '',
+                'use_shazar': True,
+                'use_default_endpoints': True,
+                'annotations': []
             })
 
+            os.mkdir(os.path.join(tmpdir, 'refs'))
+            open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'oci-layout'), (1234567890, 1234567890))
+            refs = open(os.path.join(tmpdir, 'refs/latest'), 'w')
+            refs.write('{}')
+            refs.close()
+            os.utime(os.path.join(tmpdir, 'refs/latest'), (1234567890, 1234567890))
+
+            with \
+                    patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
+                    patch('sys.stdout.buffer.write', stdout_mock):
+                self.assertEqual(bndl_create.bndl_create(attributes), 0)
+
+            self.assertTrue(zipfile.is_zipfile(tmpfile))
+
+            with zipfile.ZipFile(tmpfile) as zip:
+                infos = zip.infolist()
+
+                self.assertEqual(infos[0].date_time, time.localtime(1234567890)[:6])
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def test_without_shazar(self):
         stdout_mock = MagicMock()
-        stderr_mock = MagicMock()
-        logging_setup.configure_logging(MagicMock(), stdout_mock, stderr_mock)
+        tmpdir = tempfile.mkdtemp()
+        tmpfile = os.path.join(tmpdir, 'output')
 
-        os.mkdir(os.path.join(tmpdir, 'refs'))
-        open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+        try:
+            attributes = create_attributes_object({
+                'name': 'test',
+                'source': tmpdir,
+                'format': 'oci-image',
+                'tag': 'latest',
+                'output': tmpfile,
+                'component_description': '',
+                'use_shazar': False,
+                'use_default_endpoints': True,
+                'annotations': []
+            })
 
-        with \
-                patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
-                patch('sys.stdout.buffer.write', stdout_mock):
-            bndl_create.bndl_create(attributes)
+            os.mkdir(os.path.join(tmpdir, 'refs'))
+            open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'oci-layout'), (1234567890, 1234567890))
+            refs = open(os.path.join(tmpdir, 'refs/latest'), 'w')
+            refs.write('{}')
+            refs.close()
+            os.utime(os.path.join(tmpdir, 'refs/latest'), (1234567890, 1234567890))
 
-        self.assertEqual(
-            self.output(stderr_mock),
-            as_error('Error: bndl: Invalid OCI Image. Cannot find requested tag "latest" in OCI Image\n')
-        )
+            with \
+                    patch('conductr_cli.bndl_oci.oci_image_extract_manifest_config', lambda a, b: ({}, {})), \
+                    patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
+                    patch('sys.stdout.buffer.write', stdout_mock):
+                self.assertEqual(bndl_create.bndl_create(attributes), 0)
 
-    def test_with_shazar(self):
+            self.assertFalse(zipfile.is_zipfile(tmpfile))
+
+            with tarfile.TarFile.open(tmpfile) as tar:
+                for entry in tar:
+                    self.assertEqual(entry.mtime, 1234567890)
+                    break
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def test_deterministic_with_shazar(self):
+        stdout_mock = MagicMock()
+        tmpdir = tempfile.mkdtemp()
+        tmpdir2 = tempfile.mkdtemp()
+        tmpfile = os.path.join(tmpdir, 'output')
+        tmpfile2 = os.path.join(tmpdir2, 'output')
+
+        try:
+            attributes = create_attributes_object({
+                'name': 'test',
+                'source': tmpdir,
+                'format': 'oci-image',
+                'tag': 'latest',
+                'output': tmpfile,
+                'component_description': '',
+                'use_shazar': True,
+                'use_default_endpoints': True,
+                'annotations': []
+            })
+
+            os.mkdir(os.path.join(tmpdir, 'refs'))
+            open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'oci-layout'), (1234567890, 1234567890))
+            refs = open(os.path.join(tmpdir, 'refs/latest'), 'w')
+            refs.write('{}')
+            refs.close()
+            os.utime(os.path.join(tmpdir, 'refs/latest'), (1234567890, 1234567890))
+
+            attributes2 = create_attributes_object({
+                'name': 'test',
+                'source': tmpdir2,
+                'format': 'oci-image',
+                'tag': 'latest',
+                'output': tmpfile2,
+                'component_description': '',
+                'use_shazar': True,
+                'use_default_endpoints': True,
+                'annotations': []
+            })
+
+            os.mkdir(os.path.join(tmpdir2, 'refs'))
+            open(os.path.join(tmpdir2, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir2, 'oci-layout'), (1234567890, 1234567890))
+            refs2 = open(os.path.join(tmpdir2, 'refs/latest'), 'w')
+            refs2.write('{}')
+            refs2.close()
+            os.utime(os.path.join(tmpdir2, 'refs/latest'), (1234567890, 1234567890))
+
+            with \
+                    patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
+                    patch('sys.stdout.buffer.write', stdout_mock):
+                bndl_create.bndl_create(attributes)
+                bndl_create.bndl_create(attributes2)
+
+            with open(tmpfile, 'rb') as fileobj, open(tmpfile2, 'rb') as fileobj2:
+                self.assertEqual(fileobj.read(), fileobj2.read())
+
+        finally:
+            shutil.rmtree(tmpdir)
+            shutil.rmtree(tmpdir2)
+
+    def test_mtime_from_config(self):
+        config = {
+            'created': '2017-02-27T19:42:10.522384312Z'
+        }
         stdout_mock = MagicMock()
         tmpdir = tempfile.mkdtemp()
         tmpfile = os.path.join(tmpdir, 'output')
@@ -106,24 +256,32 @@ class TestBndlCreate(CliTestCase):
 
             os.mkdir(os.path.join(tmpdir, 'refs'))
             open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'oci-layout'), (1234567890, 1234567890))
             refs = open(os.path.join(tmpdir, 'refs/latest'), 'w')
             refs.write('{}')
             refs.close()
+            os.utime(os.path.join(tmpdir, 'refs/latest'), (1234567890, 1234567890))
 
             with \
+                    patch('conductr_cli.bndl_oci.oci_image_extract_manifest_config', lambda a, b: ({}, config)), \
                     patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
                     patch('sys.stdout.buffer.write', stdout_mock):
                 self.assertEqual(bndl_create.bndl_create(attributes), 0)
 
-            self.assertTrue(zipfile.is_zipfile(tmpfile))
+            with zipfile.ZipFile(tmpfile) as zip:
+                infos = zip.infolist()
+
+                self.assertEqual(infos[0].date_time, time.localtime(1488224530)[:6])
+
         finally:
             shutil.rmtree(tmpdir)
 
-    def test_without_shazar(self):
+    def test_deterministic_without_shazar(self):
         stdout_mock = MagicMock()
-        extract_config_mock = MagicMock()
         tmpdir = tempfile.mkdtemp()
+        tmpdir2 = tempfile.mkdtemp()
         tmpfile = os.path.join(tmpdir, 'output')
+        tmpfile2 = os.path.join(tmpdir2, 'output')
 
         try:
             attributes = create_attributes_object({
@@ -140,16 +298,41 @@ class TestBndlCreate(CliTestCase):
 
             os.mkdir(os.path.join(tmpdir, 'refs'))
             open(os.path.join(tmpdir, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'oci-layout'), (1234567890, 1234567890))
             refs = open(os.path.join(tmpdir, 'refs/latest'), 'w')
             refs.write('{}')
             refs.close()
+            os.utime(os.path.join(tmpdir, 'refs/latest'), (1234567890, 1234567890))
+
+            attributes2 = create_attributes_object({
+                'name': 'test',
+                'source': tmpdir2,
+                'format': 'oci-image',
+                'tag': 'latest',
+                'output': tmpfile2,
+                'component_description': '',
+                'use_shazar': False,
+                'use_default_endpoints': True,
+                'annotations': []
+            })
+
+            os.mkdir(os.path.join(tmpdir2, 'refs'))
+            open(os.path.join(tmpdir2, 'oci-layout'), 'w').close()
+            os.utime(os.path.join(tmpdir2, 'oci-layout'), (1234567890, 1234567890))
+            refs2 = open(os.path.join(tmpdir2, 'refs/latest'), 'w')
+            refs2.write('{}')
+            refs2.close()
+            os.utime(os.path.join(tmpdir2, 'refs/latest'), (1234567890, 1234567890))
 
             with \
-                    patch('conductr_cli.bndl_oci.oci_image_extract_manifest_config', extract_config_mock), \
                     patch('sys.stdin', MagicMock(**{'buffer': BytesIO(b'')})), \
                     patch('sys.stdout.buffer.write', stdout_mock):
-                self.assertEqual(bndl_create.bndl_create(attributes), 0)
+                bndl_create.bndl_create(attributes)
+                bndl_create.bndl_create(attributes2)
+
+            with open(tmpfile, 'rb') as fileobj, open(tmpfile2, 'rb') as fileobj2:
+                self.assertEqual(fileobj.read(), fileobj2.read())
 
-            self.assertFalse(zipfile.is_zipfile(tmpfile))
         finally:
             shutil.rmtree(tmpdir)
+            shutil.rmtree(tmpdir2)
diff --git a/conductr_cli/test/test_bndl_utils.py b/conductr_cli/test/test_bndl_utils.py
index 279d0ad..5851faf 100644
--- a/conductr_cli/test/test_bndl_utils.py
+++ b/conductr_cli/test/test_bndl_utils.py
@@ -97,6 +97,18 @@ class TestBndlUtils(CliTestCase):
         self.assertEqual(writer.size_in, 0)
         self.assertEqual(writer.size_out, 9)
 
+    def test_first_mtime(self):
+        tmpdir = tempfile.mkdtemp()
+
+        try:
+            open(os.path.join(tmpdir, 'one'), 'w').close()
+            os.mkdir(os.path.join(tmpdir, 'sub'))
+            open(os.path.join(tmpdir, 'sub', 'two'), 'w').close()
+            os.utime(os.path.join(tmpdir, 'one'), (1234, 1234))
+            self.assertEqual(bndl_utils.first_mtime(os.path.join(tmpdir)), 1234)
+        finally:
+            shutil.rmtree(tmpdir)
+
     def test_load_bundle_args_into_conf(self):
         base_args = create_attributes_object({
             'name': 'world',
diff --git a/conductr_cli/test/test_conduct_dcos.py b/conductr_cli/test/test_conduct_dcos.py
index 0b4ea94..db29460 100644
--- a/conductr_cli/test/test_conduct_dcos.py
+++ b/conductr_cli/test/test_conduct_dcos.py
@@ -7,7 +7,7 @@ import os
 
 class TestConductDcosCommand(CliTestCase):
 
-    def test_success_which(self):
+    def test_success(self):
         args = MagicMock()
         stdout = MagicMock()
         makedirs = MagicMock()
@@ -16,7 +16,6 @@ class TestConductDcosCommand(CliTestCase):
         logging_setup.configure_logging(args, stdout)
 
         with patch('shutil.which', lambda x: '/somefile'), \
-                patch('sys.executable', '/usr/bin/python3'), \
                 patch('os.path.exists', lambda x: True), \
                 patch('os.remove', lambda x: True), \
                 patch('os.makedirs', makedirs), \
@@ -24,10 +23,7 @@ class TestConductDcosCommand(CliTestCase):
             result = conduct_dcos.setup(args)
         self.assertTrue(result)
 
-        makedirs.assert_called_once_with(
-            '{}/.dcos/subcommands/conductr/env/bin'.format(os.path.expanduser('~')),
-            exist_ok=True
-        )
+        makedirs.assert_not_called()
 
         symlink.assert_called_with('/somefile',
                                    '{}/.dcos/subcommands/conductr/env/bin/dcos-conduct'.format(os.path.expanduser('~')))
@@ -38,27 +34,6 @@ class TestConductDcosCommand(CliTestCase):
                             |"""),
             self.output(stdout))
 
-    def test_success_executable(self):
-        args = MagicMock()
-        stdout = MagicMock()
-        makedirs = MagicMock()
-        symlink = MagicMock()
-
-        logging_setup.configure_logging(args, stdout)
-
-        with \
-                patch('shutil.which', lambda x: '/somefile'), \
-                patch('sys.executable', '/path/to/conduct'), \
-                patch('os.path.exists', lambda x: True), \
-                patch('os.remove', lambda x: True), \
-                patch('os.makedirs', makedirs), \
-                patch('os.symlink', symlink):
-            result = conduct_dcos.setup(args)
-        self.assertTrue(result)
-
-        symlink.assert_called_with('/path/to/conduct',
-                                   '{}/.dcos/subcommands/conductr/env/bin/dcos-conduct'.format(os.path.expanduser('~')))
-
     def test_create_dir(self):
         args = MagicMock()
         stdout = MagicMock()
@@ -68,10 +43,8 @@ class TestConductDcosCommand(CliTestCase):
 
         logging_setup.configure_logging(args, stdout)
 
-        with \
-                patch('shutil.which', lambda x: '/somefile'), \
+        with patch('shutil.which', lambda x: '/somefile'), \
                 patch('os.path.exists', lambda x: False), \
-                patch('os.path.islink', lambda x: False), \
                 patch('os.remove', remove), \
                 patch('os.makedirs', makedirs), \
                 patch('os.symlink', symlink):