From 110da0c366b133a9981e89f221fe6dcd1ca166f2 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Thu, 22 Aug 2019 01:00:16 -0500
Subject: [PATCH 01/44] ssh: adapt link logic and support symlinks

---
 dvc/remote/ssh/__init__.py   | 87 ++++++++++++++++++++++++++++++++++++
 dvc/remote/ssh/connection.py | 12 +++++
 2 files changed, 99 insertions(+)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 98ee172bc..f19e1df80 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -4,11 +4,13 @@ import errno
 import itertools
 import io
 import os
+import posixpath
 import getpass
 import logging
 import threading
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import contextmanager, closing
+from copy import copy
 
 try:
     import paramiko
@@ -23,6 +25,7 @@ from dvc.remote.base import RemoteBASE
 from dvc.scheme import Schemes
 from dvc.remote.pool import get_connection
 from dvc.progress import Tqdm
+from dvc.exceptions import DvcException
 
 from .connection import SSHConnection
 
@@ -47,9 +50,21 @@ class RemoteSSH(RemoteBASE):
     # We use conservative setting of 4 instead to not exhaust max sessions.
     CHECKSUM_JOBS = 4
 
+    # XXX: Friendly reminder to add "reflink" to the default types
+    # after is implemented
+    DEFAULT_CACHE_TYPES = ["symlink", "copy"]
+
     def __init__(self, repo, config):
         super(RemoteSSH, self).__init__(repo, config)
 
+        types = config.get(Config.SECTION_CACHE_TYPE, None)
+        if types:
+            if isinstance(types, str):
+                types = [t.strip() for t in types.split(",")]
+            self.cache_types = types
+        else:
+            self.cache_types = copy(self.DEFAULT_CACHE_TYPES)
+
         url = config.get(Config.SECTION_REMOTE_URL)
         if url:
             parsed = urlparse(url)
@@ -278,3 +293,75 @@ class RemoteSSH(RemoteBASE):
                 ret = list(itertools.compress(checksums, in_remote))
                 pbar.update_desc("", 0)  # clear path name description
                 return ret
+
+    def link(self, from_info, to_info):
+        with self.ssh(from_info) as ssh:
+            self._link(from_info, to_info, self.cache_types, ssh)
+
+    def _link(self, from_info, to_info, link_types, ssh):
+        from_path = from_info.path
+        to_path = to_info.path
+
+        assert ssh.isfile(from_path)
+
+        dname = posixpath.dirname(to_path)
+
+        # XXX: We can remove the `exists` call, since `makedirs`
+        # will ignore it
+        if not ssh.exists(dname):
+            ssh.makedirs(dname)
+
+        # NOTE: just create an empty file for an empty cache
+        if ssh.getsize(from_path) == 0:
+            ssh.open(to_path, "w+").close()
+
+            msg = "Created empty file: {} -> {}".format(from_path, to_path)
+            logger.debug(msg)
+            return
+
+        self._try_links(from_info, to_info, link_types, ssh)
+
+    @classmethod
+    def _get_link_method(cls, link_type, ssh):
+        CACHE_TYPE_MAP = {"copy": ssh.cp, "symlink": ssh.symlink}
+
+        try:
+            return CACHE_TYPE_MAP[link_type]
+        except KeyError:
+            raise DvcException(
+                "Cache type: '{}' not supported!".format(link_type)
+            )
+
+    def _do_link(self, from_info, to_info, link_method, ssh):
+        # XXX: We are testing if file exists rather than if file is a link
+        if ssh.exists(to_info.path):
+            raise DvcException("Link '{}' already exists!".format(to_info))
+        else:
+            link_method(from_info.path, to_info.path)
+
+        if self.protected:
+            self.protect(to_info)
+
+        msg = "Created {}'{}': {} -> {}".format(
+            "protected " if self.protected else "",
+            self.cache_types[0],
+            from_info,
+            to_info,
+        )
+        logger.debug(msg)
+
+    def _try_links(self, from_info, to_info, link_types, ssh):
+        i = len(link_types)
+        while i > 0:
+            link_method = self._get_link_method(link_types[0], ssh)
+            try:
+                self._do_link(from_info, to_info, link_method, ssh)
+                return
+
+            except DvcException as exc:
+                msg = "Cache type '{}' is not supported: {}"
+                logger.debug(msg.format(link_types[0], str(exc)))
+                del link_types[0]
+                i -= 1
+
+        raise DvcException("no possible cache types left to try out.")
diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index fbb0c2b7b..423ce6604 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -64,6 +64,12 @@ class SSHConnection:
 
         return 0
 
+    def getsize(self, path):
+        with ignore_file_not_found():
+            return self.sftp.stat(path).st_size
+
+        return 0
+
     def exists(self, path, sftp=None):
         return bool(self.st_mode(path))
 
@@ -282,3 +288,9 @@ class SSHConnection:
                         raise
                     break
         return self._sftp_channels
+
+    def open(self, *args, **kwargs):
+        return self.sftp.open(*args, **kwargs)
+
+    def symlink(self, *args, **kwargs):
+        return self.sftp.symlink(*args, **kwargs)

From f4563bdd85cd598284de5bd896d2f0864f51cc53 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 23 Aug 2019 21:20:50 -0500
Subject: [PATCH 02/44] ssh: support reflinks and hardlinks

---
 dvc/remote/ssh/__init__.py   | 11 +++++++----
 dvc/remote/ssh/connection.py |  8 ++++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index f19e1df80..3ff2ae546 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -50,9 +50,7 @@ class RemoteSSH(RemoteBASE):
     # We use conservative setting of 4 instead to not exhaust max sessions.
     CHECKSUM_JOBS = 4
 
-    # XXX: Friendly reminder to add "reflink" to the default types
-    # after is implemented
-    DEFAULT_CACHE_TYPES = ["symlink", "copy"]
+    DEFAULT_CACHE_TYPES = ["reflink, copy"]
 
     def __init__(self, repo, config):
         super(RemoteSSH, self).__init__(repo, config)
@@ -323,7 +321,12 @@ class RemoteSSH(RemoteBASE):
 
     @classmethod
     def _get_link_method(cls, link_type, ssh):
-        CACHE_TYPE_MAP = {"copy": ssh.cp, "symlink": ssh.symlink}
+        CACHE_TYPE_MAP = {
+            "copy": ssh.cp,
+            "symlink": ssh.symlink,
+            "reflink": ssh.reflink,
+            "hardlink": ssh.hardlink,
+        }
 
         try:
             return CACHE_TYPE_MAP[link_type]
diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index 423ce6604..6e49132b6 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -294,3 +294,11 @@ class SSHConnection:
 
     def symlink(self, *args, **kwargs):
         return self.sftp.symlink(*args, **kwargs)
+
+    def reflink(self, src, dest):
+        self.makedirs(posixpath.dirname(dest))
+        self.execute("cp --reflink {} {}".format(src, dest))
+
+    def hardlink(self, src, dest):
+        self.makedirs(posixpath.dirname(dest))
+        self.execute("ln {} {}".format(src, dest))

From 521647e47e907629bf09964a04a70511624c0fb9 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 23 Aug 2019 21:54:53 -0500
Subject: [PATCH 03/44] ssh: implement slow_link_detection

---
 dvc/remote/ssh/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 3ff2ae546..6562b19af 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -22,6 +22,7 @@ from dvc.config import Config
 from dvc.utils import to_chunks
 from dvc.utils.compat import urlparse, StringIO
 from dvc.remote.base import RemoteBASE
+from dvc.remote.local.slow_link_detection import slow_link_guard
 from dvc.scheme import Schemes
 from dvc.remote.pool import get_connection
 from dvc.progress import Tqdm
@@ -353,6 +354,7 @@ class RemoteSSH(RemoteBASE):
         )
         logger.debug(msg)
 
+    @slow_link_guard
     def _try_links(self, from_info, to_info, link_types, ssh):
         i = len(link_types)
         while i > 0:

From 02cef7b9a36c5eebc7af0a930ea2b907ffda83a0 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 23 Aug 2019 22:18:48 -0500
Subject: [PATCH 04/44] ssh: makedirs before symlink

---
 dvc/remote/ssh/connection.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index 6e49132b6..bfb058187 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -292,8 +292,9 @@ class SSHConnection:
     def open(self, *args, **kwargs):
         return self.sftp.open(*args, **kwargs)
 
-    def symlink(self, *args, **kwargs):
-        return self.sftp.symlink(*args, **kwargs)
+    def symlink(self, src, dest):
+        self.makedirs(posixpath.dirname(dest))
+        self.sftp.symlink(src, dest)
 
     def reflink(self, src, dest):
         self.makedirs(posixpath.dirname(dest))

From d13b5a14f87e13aa55a3a8c71479cac31082536d Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 23 Aug 2019 22:45:54 -0500
Subject: [PATCH 05/44] local: use PathInfo for link operations

---
 dvc/remote/local/__init__.py | 54 ++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 8381e3511..147594a69 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -51,12 +51,6 @@ class RemoteLOCAL(RemoteBASE):
     UNPACKED_DIR_SUFFIX = ".unpacked"
 
     DEFAULT_CACHE_TYPES = ["reflink", "copy"]
-    CACHE_TYPE_MAP = {
-        "copy": shutil.copyfile,
-        "symlink": System.symlink,
-        "hardlink": System.hardlink,
-        "reflink": System.reflink,
-    }
 
     SHARED_MODE_MAP = {None: (0o644, 0o755), "group": (0o664, 0o775)}
 
@@ -136,29 +130,32 @@ class RemoteLOCAL(RemoteBASE):
         self._link(from_info, to_info, self.cache_types)
 
     def _link(self, from_info, to_info, link_types):
-        from_path = from_info.fspath
-        to_path = to_info.fspath
+        assert self.isfile(from_info)
 
-        assert os.path.isfile(from_path)
-
-        dname = os.path.dirname(to_path)
-        if not os.path.exists(dname):
-            os.makedirs(dname)
+        self.makedirs(to_info.parent)
 
         # NOTE: just create an empty file for an empty cache
-        if os.path.getsize(from_path) == 0:
-            open(to_path, "w+").close()
+        if self.getsize(from_info) == 0:
+            self.open(to_info, "w+").close()
 
-            msg = "Created empty file: {} -> {}".format(from_path, to_path)
-            logger.debug(msg)
+            logger.debug(
+                "Created empty file: {src} -> {dest}"
+                .format(src=str(from_path), dest=str(to_path))
+            )
             return
 
         self._try_links(from_info, to_info, link_types)
 
-    @classmethod
-    def _get_link_method(cls, link_type):
+    def _get_link_method(self, link_type):
+        CACHE_TYPE_MAP = {
+            "copy": self.copy,
+            "symlink": self.symlink,
+            "hardlink": self.hardlink,
+            "reflink": self.reflink,
+        }
+
         try:
-            return cls.CACHE_TYPE_MAP[link_type]
+            return CACHE_TYPE_MAP[link_type]
         except KeyError:
             raise DvcException(
                 "Cache type: '{}' not supported!".format(link_type)
@@ -168,7 +165,7 @@ class RemoteLOCAL(RemoteBASE):
         if self.exists(to_info):
             raise DvcException("Link '{}' already exists!".format(to_info))
         else:
-            link_method(from_info.fspath, to_info.fspath)
+            link_method(from_info, to_info)
 
         if self.protected:
             self.protect(to_info)
@@ -225,6 +222,9 @@ class RemoteLOCAL(RemoteBASE):
     def isdir(self, path_info):
         return os.path.isdir(fspath_py35(path_info))
 
+    def getsize(self, path_info):
+        return os.path.getsize(fspath_py35(path_info))
+
     def walk(self, path_info):
         return dvc_walk(path_info, self.repo.dvcignore)
 
@@ -251,6 +251,18 @@ class RemoteLOCAL(RemoteBASE):
 
         move(from_info, to_info, mode=mode)
 
+    def copy(self, from_info, to_info):
+        shutil.copyfile(from_info.fspath, to_info.fspath)
+
+    def symlink(self, from_info, to_info):
+        System.symlink(from_info.fspath, to_info.fspath)
+
+    def hardlink(self, from_info, to_info):
+        System.hardlink(from_info.fspath, to_info.fspath)
+
+    def reflink(self, from_info, to_info):
+        System.reflink(from_info.fspath, to_info.fspath)
+
     def cache_exists(self, checksums, jobs=None):
         return [
             checksum

From 1be63dd93e0593bb245c88d23ae0b0ec0568e229 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 24 Aug 2019 00:16:37 -0500
Subject: [PATCH 06/44] ssh: implement interface for fs related operations

---
 dvc/remote/ssh/__init__.py | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 6562b19af..e03154fd3 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -176,6 +176,18 @@ class RemoteSSH(RemoteBASE):
         with self.ssh(path_info) as ssh:
             return ssh.isdir(path_info.path)
 
+    def isfile(self, path_info):
+        with self.ssh(path_info) as ssh:
+            return ssh.isfile(path_info.path)
+
+    def getsize(self, path_info):
+        with self.ssh(path_info) as ssh:
+            return ssh.getsize(path_info.path)
+
+    def open(self, path_info, mode="r", encoding=None):
+        with self.ssh(path_info) as ssh:
+            return ssh.open(path_info.path, mode=mode, encoding=encoding)
+
     def copy(self, from_info, to_info):
         if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
             raise NotImplementedError
@@ -183,6 +195,27 @@ class RemoteSSH(RemoteBASE):
         with self.ssh(from_info) as ssh:
             ssh.cp(from_info.path, to_info.path)
 
+    def symlink(self, from_info, to_info):
+        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+            raise NotImplementedError
+
+        with self.ssh(from_info) as ssh:
+            ssh.symlink(from_info.path, to_info.path)
+
+    def hardlink(self, from_info, to_info):
+        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+            raise NotImplementedError
+
+        with self.ssh(from_info) as ssh:
+            ssh.hardlink(from_info.path, to_info.path)
+
+    def reflink(self, from_info, to_info):
+        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+            raise NotImplementedError
+
+        with self.ssh(from_info) as ssh:
+            ssh.reflink(from_info.path, to_info.path)
+
     def remove(self, path_info):
         if path_info.scheme != self.scheme:
             raise NotImplementedError

From 84872d231ac0ec9c737a1f6729c8797a2e7bb828 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 24 Aug 2019 00:52:27 -0500
Subject: [PATCH 07/44] docs: explain potential of hitting link limit

---
 dvc/remote/local/__init__.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 147594a69..54475f449 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -134,7 +134,19 @@ class RemoteLOCAL(RemoteBASE):
 
         self.makedirs(to_info.parent)
 
-        # NOTE: just create an empty file for an empty cache
+        # If there are a lot of empty files (which happens a lot in datasets),
+        # and the cache type is `hardlink`, we might reach link limits and
+        # will get something like: `too many links error`
+        #
+        # This is because all those empty files will have the same checksum
+        # (i.e. 68b329da9893e34099c7d8ad5cb9c940), therfore, they will be
+        # linked to the same file in the cache.
+        #
+        # From https://en.wikipedia.org/wiki/Hard_link
+        #   * ext4 limits the number of hard links on a file to 65,000
+        #   * Windows with NTFS has a limit of 1024 hard links on a file
+        #
+        # That's why we simply create an empty file rather than a link.
         if self.getsize(from_info) == 0:
             self.open(to_info, "w+").close()
 

From 745b2e13a04756bf1a30d4adca3d57ee884319ba Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 24 Aug 2019 01:47:58 -0500
Subject: [PATCH 08/44] [wip] remote: abstract link logic to base

---
 dvc/remote/base.py           | 102 ++++++++++++++++++++++++++++++++++-
 dvc/remote/local/__init__.py |   8 ---
 2 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 11e42d066..9a04d81a0 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -79,6 +79,7 @@ class RemoteBASE(object):
     PARAM_RELPATH = "relpath"
     CHECKSUM_DIR_SUFFIX = ".dir"
     CHECKSUM_JOBS = max(1, min(4, cpu_count() // 2))
+    DEFAULT_CACHE_TYPES = ["copy"]
 
     state = StateNoop()
 
@@ -119,6 +120,14 @@ class RemoteBASE(object):
         self.no_traverse = config.get(Config.SECTION_REMOTE_NO_TRAVERSE, True)
         self._dir_info = {}
 
+        types = config.get(Config.SECTION_CACHE_TYPE, None)
+        if types:
+            if isinstance(types, str):
+                types = [t.strip() for t in types.split(",")]
+            self.cache_types = types
+        else:
+            self.cache_types = copy(self.DEFAULT_CACHE_TYPES)
+
     def __repr__(self):
         return "{class_name}: '{path_info}'".format(
             class_name=type(self).__name__,
@@ -352,7 +361,89 @@ class RemoteBASE(object):
         return False
 
     def link(self, from_info, to_info):
-        self.copy(from_info, to_info)
+        self._link(from_info, to_info, self.cache_types)
+
+    def _link(self, from_info, to_info, link_types):
+        # XXX: This is a fallback until the following methods are
+        # implemented on all remotes:
+        #   * `isfile`
+        #   * `makedirs`
+        #   * `getsize`
+        #
+        # Another thing we can do is to delegate the edge case handling
+        # to hardlink, ommit the `isfile` check, and let the link
+        # implementation to create a directory before copying
+        if link_types == ["copy"]:
+            return self.copy(from_info, to_info)
+
+        assert self.isfile(from_info)
+
+        self.makedirs(to_info.parent)
+
+        # If there are a lot of empty files (which happens a lot in datasets),
+        # and the cache type is `hardlink`, we might reach link limits and
+        # will get something like: `too many links error`
+        #
+        # This is because all those empty files will have the same checksum
+        # (i.e. 68b329da9893e34099c7d8ad5cb9c940), therfore, they will be
+        # linked to the same file in the cache.
+        #
+        # From https://en.wikipedia.org/wiki/Hard_link
+        #   * ext4 limits the number of hard links on a file to 65,000
+        #   * Windows with NTFS has a limit of 1024 hard links on a file
+        #
+        # That's why we simply create an empty file rather than a link.
+        if self.getsize(from_info) == 0:
+            self.open(to_info, "w+").close()
+
+            logger.debug(
+                "Created empty file: {src} -> {dest}"
+                .format(src=str(from_path), dest=str(to_path))
+            )
+            return
+
+        self._try_links(from_info, to_info, link_types)
+
+    @slow_link_guard
+    def _try_links(self, from_info, to_info, link_types):
+        link_methods = {
+            "copy": self.copy,
+            "symlink": self.symlink,
+            "hardlink": self.hardlink,
+            "reflink": self.reflink,
+        }
+
+        i = len(link_types)
+        while i > 0:
+            link_method = link_methods[link_types[0]]
+            try:
+                self._do_link(from_info, to_info, link_method)
+                return
+
+            except DvcException as exc:
+                msg = "Cache type '{}' is not supported: {}"
+                logger.debug(msg.format(link_types[0], str(exc)))
+                del link_types[0]
+                i -= 1
+
+        raise DvcException("no possible cache types left to try out.")
+
+    def _do_link(self, from_info, to_info, link_method):
+        if self.exists(to_info):
+            raise DvcException("Link '{}' already exists!".format(to_info))
+        else:
+            link_method(from_info, to_info)
+
+        if self.protected:
+            self.protect(to_info)
+
+        msg = "Created {}'{}': {} -> {}".format(
+            "protected " if self.protected else "",
+            self.cache_types[0],
+            from_info,
+            to_info,
+        )
+        logger.debug(msg)
 
     def _save_file(self, path_info, checksum, save_link=True):
         assert checksum
@@ -512,6 +603,15 @@ class RemoteBASE(object):
     def copy(self, from_info, to_info):
         raise RemoteActionNotImplemented("copy", self.scheme)
 
+    def symlink(self, from_info, to_info):
+        raise RemoteActionNotImplemented("symlink", self.scheme)
+
+    def hardlink(self, from_info, to_info):
+        raise RemoteActionNotImplemented("hardlink", self.scheme)
+
+    def reflink(self, from_info, to_info):
+        raise RemoteActionNotImplemented("reflink", self.scheme)
+
     def exists(self, path_info):
         raise NotImplementedError
 
diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 54475f449..5c60d650b 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -65,14 +65,6 @@ class RemoteLOCAL(RemoteBASE):
             # cache files are set to be read-only for everyone
             self._file_mode = stat.S_IREAD | stat.S_IRGRP | stat.S_IROTH
 
-        types = config.get(Config.SECTION_CACHE_TYPE, None)
-        if types:
-            if isinstance(types, str):
-                types = [t.strip() for t in types.split(",")]
-            self.cache_types = types
-        else:
-            self.cache_types = copy(self.DEFAULT_CACHE_TYPES)
-
         # A clunky way to detect cache dir
         storagepath = config.get(Config.SECTION_LOCAL_STORAGEPATH, None)
         cache_dir = config.get(Config.SECTION_REMOTE_URL, storagepath)

From 9c9079753bd61ed5fb1fa4ddeb76f315210d4f7c Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 24 Aug 2019 11:44:50 -0500
Subject: [PATCH 09/44] remote: use getattr instead of dict to get link methods

---
 dvc/remote/base.py | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 9a04d81a0..b40f20206 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -406,16 +406,9 @@ class RemoteBASE(object):
 
     @slow_link_guard
     def _try_links(self, from_info, to_info, link_types):
-        link_methods = {
-            "copy": self.copy,
-            "symlink": self.symlink,
-            "hardlink": self.hardlink,
-            "reflink": self.reflink,
-        }
-
         i = len(link_types)
         while i > 0:
-            link_method = link_methods[link_types[0]]
+            link_method = getattr(self, link_types[0])
             try:
                 self._do_link(from_info, to_info, link_method)
                 return

From 83a9a6362b63f49e6ae6c1e0891425a56be8da13 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 24 Aug 2019 11:45:43 -0500
Subject: [PATCH 10/44] remote: remove unecessary + on open mode

---
 dvc/remote/base.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index b40f20206..2eaa3a4c7 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -394,7 +394,7 @@ class RemoteBASE(object):
         #
         # That's why we simply create an empty file rather than a link.
         if self.getsize(from_info) == 0:
-            self.open(to_info, "w+").close()
+            self.open(to_info, "w").close()
 
             logger.debug(
                 "Created empty file: {src} -> {dest}"

From 40c61b3aaa5a29fa69842fa3a8fe2e73734a4cae Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 28 Aug 2019 22:56:42 -0500
Subject: [PATCH 11/44] remote: abstract link logic to base

---
 dvc/remote/base.py                            |  51 +++-----
 dvc/remote/hdfs.py                            |   2 -
 dvc/remote/local/__init__.py                  | 108 ++++------------
 dvc/remote/{local => }/slow_link_detection.py |  10 +-
 dvc/remote/ssh/__init__.py                    | 118 +++---------------
 dvc/remote/ssh/connection.py                  |   6 +-
 dvc/repo/__init__.py                          |   2 +
 dvc/utils/http.py                             |   2 -
 tests/unit/remote/test_slow_link_detection.py |  15 +--
 9 files changed, 67 insertions(+), 247 deletions(-)
 rename dvc/remote/{local => }/slow_link_detection.py (85%)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 2eaa3a4c7..72b174c3d 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -12,6 +12,8 @@ from operator import itemgetter
 from multiprocessing import cpu_count
 from functools import partial
 from concurrent.futures import ThreadPoolExecutor
+from copy import copy
+from dvc.remote.slow_link_detection import slow_link_guard
 
 import dvc.prompt as prompt
 from dvc.config import Config
@@ -364,44 +366,10 @@ class RemoteBASE(object):
         self._link(from_info, to_info, self.cache_types)
 
     def _link(self, from_info, to_info, link_types):
-        # XXX: This is a fallback until the following methods are
-        # implemented on all remotes:
-        #   * `isfile`
-        #   * `makedirs`
-        #   * `getsize`
-        #
-        # Another thing we can do is to delegate the edge case handling
-        # to hardlink, ommit the `isfile` check, and let the link
-        # implementation to create a directory before copying
-        if link_types == ["copy"]:
-            return self.copy(from_info, to_info)
-
         assert self.isfile(from_info)
 
         self.makedirs(to_info.parent)
 
-        # If there are a lot of empty files (which happens a lot in datasets),
-        # and the cache type is `hardlink`, we might reach link limits and
-        # will get something like: `too many links error`
-        #
-        # This is because all those empty files will have the same checksum
-        # (i.e. 68b329da9893e34099c7d8ad5cb9c940), therfore, they will be
-        # linked to the same file in the cache.
-        #
-        # From https://en.wikipedia.org/wiki/Hard_link
-        #   * ext4 limits the number of hard links on a file to 65,000
-        #   * Windows with NTFS has a limit of 1024 hard links on a file
-        #
-        # That's why we simply create an empty file rather than a link.
-        if self.getsize(from_info) == 0:
-            self.open(to_info, "w").close()
-
-            logger.debug(
-                "Created empty file: {src} -> {dest}"
-                .format(src=str(from_path), dest=str(to_path))
-            )
-            return
-
         self._try_links(from_info, to_info, link_types)
 
     @slow_link_guard
@@ -422,6 +390,7 @@ class RemoteBASE(object):
         raise DvcException("no possible cache types left to try out.")
 
     def _do_link(self, from_info, to_info, link_method):
+        # XXX: We are testing if file exists rather than if file is a link
         if self.exists(to_info):
             raise DvcException("Link '{}' already exists!".format(to_info))
         else:
@@ -475,9 +444,15 @@ class RemoteBASE(object):
         return False
 
     def isfile(self, path_info):
-        raise NotImplementedError
+        """Optional: Overwrite only if the remote has a way to distinguish
+        between a directory and a file.
+        """
+        return True
 
     def isdir(self, path_info):
+        """Optional: Overwrite only if the remote has a way to distinguish
+        between a directory and a file.
+        """
         return False
 
     def walk(self, path_info):
@@ -775,7 +750,10 @@ class RemoteBASE(object):
             progress_callback(str(path_info))
 
     def makedirs(self, path_info):
-        raise NotImplementedError
+        """Optional: Implement only if the remote needs to create
+        directories before copying/linking/moving data
+        """
+        pass
 
     def _checkout_dir(
         self, path_info, checksum, force, progress_callback=None
@@ -799,6 +777,7 @@ class RemoteBASE(object):
             if self.changed(entry_info, entry_checksum_info):
                 if self.exists(entry_info):
                     self.safe_remove(entry_info, force=force)
+                self.makedirs(entry_info.parent)
                 self.link(entry_cache_info, entry_info)
                 self.state.save(entry_info, entry_checksum)
             if progress_callback:
diff --git a/dvc/remote/hdfs.py b/dvc/remote/hdfs.py
index efdbfbba4..ec37ace63 100644
--- a/dvc/remote/hdfs.py
+++ b/dvc/remote/hdfs.py
@@ -143,8 +143,6 @@ class RemoteHDFS(RemoteBASE):
 
     @contextmanager
     def open(self, path_info, mode="r", encoding=None):
-        assert mode in {"r", "rt", "rb"}
-
         try:
             with self.hdfs(path_info) as hdfs, closing(
                 hdfs.open(path_info.path, mode="rb")
diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 5c60d650b..ebec6b85e 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -1,9 +1,6 @@
 from __future__ import unicode_literals
 
-from copy import copy
-
 from dvc.scheme import Schemes
-from dvc.remote.local.slow_link_detection import slow_link_guard
 from dvc.utils.compat import str, fspath_py35, open
 
 import os
@@ -118,87 +115,6 @@ class RemoteLOCAL(RemoteBASE):
     def makedirs(self, path_info):
         makedirs(path_info, exist_ok=True, mode=self._dir_mode)
 
-    def link(self, from_info, to_info):
-        self._link(from_info, to_info, self.cache_types)
-
-    def _link(self, from_info, to_info, link_types):
-        assert self.isfile(from_info)
-
-        self.makedirs(to_info.parent)
-
-        # If there are a lot of empty files (which happens a lot in datasets),
-        # and the cache type is `hardlink`, we might reach link limits and
-        # will get something like: `too many links error`
-        #
-        # This is because all those empty files will have the same checksum
-        # (i.e. 68b329da9893e34099c7d8ad5cb9c940), therfore, they will be
-        # linked to the same file in the cache.
-        #
-        # From https://en.wikipedia.org/wiki/Hard_link
-        #   * ext4 limits the number of hard links on a file to 65,000
-        #   * Windows with NTFS has a limit of 1024 hard links on a file
-        #
-        # That's why we simply create an empty file rather than a link.
-        if self.getsize(from_info) == 0:
-            self.open(to_info, "w+").close()
-
-            logger.debug(
-                "Created empty file: {src} -> {dest}"
-                .format(src=str(from_path), dest=str(to_path))
-            )
-            return
-
-        self._try_links(from_info, to_info, link_types)
-
-    def _get_link_method(self, link_type):
-        CACHE_TYPE_MAP = {
-            "copy": self.copy,
-            "symlink": self.symlink,
-            "hardlink": self.hardlink,
-            "reflink": self.reflink,
-        }
-
-        try:
-            return CACHE_TYPE_MAP[link_type]
-        except KeyError:
-            raise DvcException(
-                "Cache type: '{}' not supported!".format(link_type)
-            )
-
-    def _do_link(self, from_info, to_info, link_method):
-        if self.exists(to_info):
-            raise DvcException("Link '{}' already exists!".format(to_info))
-        else:
-            link_method(from_info, to_info)
-
-        if self.protected:
-            self.protect(to_info)
-
-        msg = "Created {}'{}': {} -> {}".format(
-            "protected " if self.protected else "",
-            self.cache_types[0],
-            from_info,
-            to_info,
-        )
-        logger.debug(msg)
-
-    @slow_link_guard
-    def _try_links(self, from_info, to_info, link_types):
-        i = len(link_types)
-        while i > 0:
-            link_method = self._get_link_method(link_types[0])
-            try:
-                self._do_link(from_info, to_info, link_method)
-                return
-
-            except DvcException as exc:
-                msg = "Cache type '{}' is not supported: {}"
-                logger.debug(msg.format(link_types[0], str(exc)))
-                del link_types[0]
-                i -= 1
-
-        raise DvcException("no possible cache types left to try out.")
-
     def already_cached(self, path_info):
         assert path_info.scheme in ["", "local"]
 
@@ -262,6 +178,29 @@ class RemoteLOCAL(RemoteBASE):
         System.symlink(from_info.fspath, to_info.fspath)
 
     def hardlink(self, from_info, to_info):
+        # If there are a lot of empty files (which happens a lot in datasets),
+        # and the cache type is `hardlink`, we might reach link limits and
+        # will get something like: `too many links error`
+        #
+        # This is because all those empty files will have the same checksum
+        # (i.e. 68b329da9893e34099c7d8ad5cb9c940), therfore, they will be
+        # linked to the same file in the cache.
+        #
+        # From https://en.wikipedia.org/wiki/Hard_link
+        #   * ext4 limits the number of hard links on a file to 65,000
+        #   * Windows with NTFS has a limit of 1024 hard links on a file
+        #
+        # That's why we simply create an empty file rather than a link.
+        if self.getsize(from_info) == 0:
+            self.open(to_info, "w").close()
+
+            logger.debug(
+                "Created empty file: {src} -> {dest}".format(
+                    src=str(from_info), dest=str(to_info)
+                )
+            )
+            return
+
         System.hardlink(from_info.fspath, to_info.fspath)
 
     def reflink(self, from_info, to_info):
@@ -296,7 +235,6 @@ class RemoteLOCAL(RemoteBASE):
         )
 
     def open(self, path_info, mode="r", encoding=None):
-        assert mode in {"r", "rt", "rb"}
         return open(fspath_py35(path_info), mode=mode, encoding=encoding)
 
     def _group(self, checksum_infos, show_checksums=False):
diff --git a/dvc/remote/local/slow_link_detection.py b/dvc/remote/slow_link_detection.py
similarity index 85%
rename from dvc/remote/local/slow_link_detection.py
rename to dvc/remote/slow_link_detection.py
index 7bd3bb143..2e51413ba 100644
--- a/dvc/remote/local/slow_link_detection.py
+++ b/dvc/remote/slow_link_detection.py
@@ -46,17 +46,15 @@ class SlowLinkDetectorDecorator(object):
 
 
 def slow_link_guard(method):
-    def call(remote_local, *args, **kwargs):
-        cache_config = remote_local.repo.config.config.get(
-            Config.SECTION_CACHE, {}
-        )
+    def call(remote, *args, **kwargs):
+        cache_config = remote.repo.config.config.get(Config.SECTION_CACHE, {})
         should_warn = cache_config.get(
             Config.SECTION_CACHE_SLOW_LINK_WARNING, True
         ) and not cache_config.get(Config.SECTION_CACHE_TYPE, None)
 
         if should_warn:
             decorated = SlowLinkDetectorDecorator(method)
-            return decorated(remote_local, *args, **kwargs)
-        return method(remote_local, *args, **kwargs)
+            return decorated(remote, *args, **kwargs)
+        return method(remote, *args, **kwargs)
 
     return call
diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index e03154fd3..12d95e381 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -4,13 +4,11 @@ import errno
 import itertools
 import io
 import os
-import posixpath
 import getpass
 import logging
 import threading
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import contextmanager, closing
-from copy import copy
 
 try:
     import paramiko
@@ -22,11 +20,9 @@ from dvc.config import Config
 from dvc.utils import to_chunks
 from dvc.utils.compat import urlparse, StringIO
 from dvc.remote.base import RemoteBASE
-from dvc.remote.local.slow_link_detection import slow_link_guard
 from dvc.scheme import Schemes
 from dvc.remote.pool import get_connection
 from dvc.progress import Tqdm
-from dvc.exceptions import DvcException
 
 from .connection import SSHConnection
 
@@ -51,19 +47,10 @@ class RemoteSSH(RemoteBASE):
     # We use conservative setting of 4 instead to not exhaust max sessions.
     CHECKSUM_JOBS = 4
 
-    DEFAULT_CACHE_TYPES = ["reflink, copy"]
+    DEFAULT_CACHE_TYPES = ["copy"]
 
     def __init__(self, repo, config):
         super(RemoteSSH, self).__init__(repo, config)
-
-        types = config.get(Config.SECTION_CACHE_TYPE, None)
-        if types:
-            if isinstance(types, str):
-                types = [t.strip() for t in types.split(",")]
-            self.cache_types = types
-        else:
-            self.cache_types = copy(self.DEFAULT_CACHE_TYPES)
-
         url = config.get(Config.SECTION_REMOTE_URL)
         if url:
             parsed = urlparse(url)
@@ -184,28 +171,35 @@ class RemoteSSH(RemoteBASE):
         with self.ssh(path_info) as ssh:
             return ssh.getsize(path_info.path)
 
-    def open(self, path_info, mode="r", encoding=None):
-        with self.ssh(path_info) as ssh:
-            return ssh.open(path_info.path, mode=mode, encoding=encoding)
-
     def copy(self, from_info, to_info):
-        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+        if not from_info.scheme == to_info.scheme == self.scheme:
             raise NotImplementedError
 
         with self.ssh(from_info) as ssh:
-            ssh.cp(from_info.path, to_info.path)
+            ssh.copy(from_info.path, to_info.path)
 
     def symlink(self, from_info, to_info):
-        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+        if not from_info.scheme == to_info.scheme == self.scheme:
             raise NotImplementedError
 
         with self.ssh(from_info) as ssh:
             ssh.symlink(from_info.path, to_info.path)
 
     def hardlink(self, from_info, to_info):
-        if from_info.scheme != self.scheme or to_info.scheme != self.scheme:
+        if not from_info.scheme == to_info.scheme == self.scheme:
             raise NotImplementedError
 
+        # See dvc/remote/local/__init__.py - hardlink()
+        if self.getsize(from_info) == 0:
+            self.open(to_info, "w").close()
+
+            logger.debug(
+                "Created empty file: {src} -> {dest}".format(
+                    src=str(from_info), dest=str(to_info)
+                )
+            )
+            return
+
         with self.ssh(from_info) as ssh:
             ssh.hardlink(from_info.path, to_info.path)
 
@@ -252,8 +246,6 @@ class RemoteSSH(RemoteBASE):
 
     @contextmanager
     def open(self, path_info, mode="r", encoding=None):
-        assert mode in {"r", "rt", "rb"}
-
         with self.ssh(path_info) as ssh, closing(
             ssh.sftp.file(path_info.path, mode="r")
         ) as fd:
@@ -325,81 +317,3 @@ class RemoteSSH(RemoteBASE):
                 ret = list(itertools.compress(checksums, in_remote))
                 pbar.update_desc("", 0)  # clear path name description
                 return ret
-
-    def link(self, from_info, to_info):
-        with self.ssh(from_info) as ssh:
-            self._link(from_info, to_info, self.cache_types, ssh)
-
-    def _link(self, from_info, to_info, link_types, ssh):
-        from_path = from_info.path
-        to_path = to_info.path
-
-        assert ssh.isfile(from_path)
-
-        dname = posixpath.dirname(to_path)
-
-        # XXX: We can remove the `exists` call, since `makedirs`
-        # will ignore it
-        if not ssh.exists(dname):
-            ssh.makedirs(dname)
-
-        # NOTE: just create an empty file for an empty cache
-        if ssh.getsize(from_path) == 0:
-            ssh.open(to_path, "w+").close()
-
-            msg = "Created empty file: {} -> {}".format(from_path, to_path)
-            logger.debug(msg)
-            return
-
-        self._try_links(from_info, to_info, link_types, ssh)
-
-    @classmethod
-    def _get_link_method(cls, link_type, ssh):
-        CACHE_TYPE_MAP = {
-            "copy": ssh.cp,
-            "symlink": ssh.symlink,
-            "reflink": ssh.reflink,
-            "hardlink": ssh.hardlink,
-        }
-
-        try:
-            return CACHE_TYPE_MAP[link_type]
-        except KeyError:
-            raise DvcException(
-                "Cache type: '{}' not supported!".format(link_type)
-            )
-
-    def _do_link(self, from_info, to_info, link_method, ssh):
-        # XXX: We are testing if file exists rather than if file is a link
-        if ssh.exists(to_info.path):
-            raise DvcException("Link '{}' already exists!".format(to_info))
-        else:
-            link_method(from_info.path, to_info.path)
-
-        if self.protected:
-            self.protect(to_info)
-
-        msg = "Created {}'{}': {} -> {}".format(
-            "protected " if self.protected else "",
-            self.cache_types[0],
-            from_info,
-            to_info,
-        )
-        logger.debug(msg)
-
-    @slow_link_guard
-    def _try_links(self, from_info, to_info, link_types, ssh):
-        i = len(link_types)
-        while i > 0:
-            link_method = self._get_link_method(link_types[0], ssh)
-            try:
-                self._do_link(from_info, to_info, link_method, ssh)
-                return
-
-            except DvcException as exc:
-                msg = "Cache type '{}' is not supported: {}"
-                logger.debug(msg.format(link_types[0], str(exc)))
-                del link_types[0]
-                i -= 1
-
-        raise DvcException("no possible cache types left to try out.")
diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index bfb058187..0ef7b4bb1 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -273,8 +273,7 @@ class SSHConnection:
         assert len(md5) == 32
         return md5
 
-    def cp(self, src, dest):
-        self.makedirs(posixpath.dirname(dest))
+    def copy(self, src, dest):
         self.execute("cp {} {}".format(src, dest))
 
     def open_max_sftp_channels(self):
@@ -293,13 +292,10 @@ class SSHConnection:
         return self.sftp.open(*args, **kwargs)
 
     def symlink(self, src, dest):
-        self.makedirs(posixpath.dirname(dest))
         self.sftp.symlink(src, dest)
 
     def reflink(self, src, dest):
-        self.makedirs(posixpath.dirname(dest))
         self.execute("cp --reflink {} {}".format(src, dest))
 
     def hardlink(self, src, dest):
-        self.makedirs(posixpath.dirname(dest))
         self.execute("ln {} {}".format(src, dest))
diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py
index e1139d90a..4b83eda67 100644
--- a/dvc/repo/__init__.py
+++ b/dvc/repo/__init__.py
@@ -459,6 +459,8 @@ class Repo(object):
     @contextmanager
     def open(self, path, remote=None, mode="r", encoding=None):
         """Opens a specified resource as a file descriptor"""
+        assert mode in {"r", "rt", "rb"}
+
         try:
             with self._open(path, remote, mode, encoding) as fd:
                 yield fd
diff --git a/dvc/utils/http.py b/dvc/utils/http.py
index b2ecf4bb3..7d8c6f55a 100644
--- a/dvc/utils/http.py
+++ b/dvc/utils/http.py
@@ -13,8 +13,6 @@ def open_url(url, mode="r", encoding=None):
     Resumes on connection error.
     Url could be a string or a callable returning a string.
     """
-    assert mode in {"r", "rt", "rb"}
-
     with iter_url(url) as (response, it):
         bytes_stream = IterStream(it)
 
diff --git a/tests/unit/remote/test_slow_link_detection.py b/tests/unit/remote/test_slow_link_detection.py
index 3b981e69e..791c8a945 100644
--- a/tests/unit/remote/test_slow_link_detection.py
+++ b/tests/unit/remote/test_slow_link_detection.py
@@ -3,7 +3,7 @@ import logging
 import colorama
 import pytest
 from dvc.config import Config
-from dvc.remote.local.slow_link_detection import (
+from dvc.remote.slow_link_detection import (
     slow_link_guard,
     SlowLinkDetectorDecorator,
 )
@@ -11,18 +11,15 @@ from mock import Mock, patch
 
 
 @patch(
-    "dvc.remote.local.slow_link_detection.SlowLinkDetectorDecorator",
-    autospec=True,
+    "dvc.remote.slow_link_detection.SlowLinkDetectorDecorator", autospec=True
 )
 class TestSlowLinkGuard(object):
     @pytest.fixture(autouse=True)
     def setUp(self):
         self.method = Mock()
-        self.remote_local_mock = Mock()
+        self.remote_mock = Mock()
         self.config_mock = Mock()
-        self.remote_local_mock.repo.config.config.get.return_value = (
-            self.config_mock
-        )
+        self.remote_mock.repo.config.config.get.return_value = self.config_mock
 
     def _cache_config(self, slow_link_warning, cache_type):
         def config_side_effect(section_name, _):
@@ -38,7 +35,7 @@ class TestSlowLinkGuard(object):
     ):
         self._cache_config(slow_link_warning=True, cache_type=None)
 
-        slow_link_guard(self.method)(self.remote_local_mock)
+        slow_link_guard(self.method)(self.remote_mock)
 
         assert 1 == SlowLinkDetectorDecoratorClassMock.call_count
         assert 0 == self.method.call_count
@@ -54,7 +51,7 @@ class TestSlowLinkGuard(object):
             slow_link_warning=slow_link_warning, cache_type=cache_type
         )
 
-        slow_link_guard(self.method)(self.remote_local_mock)
+        slow_link_guard(self.method)(self.remote_mock)
 
         assert 0 == SlowLinkDetectorDecoratorClassMock.call_count
         assert 1 == self.method.call_count

From 0361b1b9c02e62bea786d51d828b32a22ffe1644 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 01:56:00 -0500
Subject: [PATCH 12/44] tests: ssh connection link methods

---
 tests/unit/remote/ssh/test_connection.py | 40 ++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index bdd1a98b4..42371bc8f 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -2,6 +2,27 @@ from __future__ import unicode_literals
 
 import os
 import posixpath
+import platform
+import pytest
+import filecmp
+
+from dvc.system import System
+
+
+@pytest.fixture
+def chdir_tmp(tmp_path):
+    cwd = os.getcwd()
+    try:
+        os.chdir(tmp_path)
+        yield
+    finally:
+        os.chdir(cwd)
+
+@pytest.fixture
+def foo():
+    with open("foo", "w") as fobj:
+        fobj.write("foo")
+
 
 here = os.path.abspath(os.path.dirname(__file__))
 
@@ -64,3 +85,22 @@ def test_walk(tmp_path, ssh):
             paths.add(posixpath.join(root, entry))
 
     assert paths == expected
+
+@pytest.mark.skipif(platform.system() != "Darwin",
+                    reason="Only works on OSX (because of APFS)")
+def test_reflink(chdir_tmp, foo, ssh):
+    ssh.reflink("foo", "bar")
+    assert System.is_reflink(bar_path)
+
+@pytest.mark.skip(reason="MockSSH doesn't support symlinks")
+def test_symlink(chdir_tmp, foo, ssh):
+    ssh.symlink("foo", "bar")
+    assert System.is_symlink("bar")
+
+def test_hardlink(chdir_tmp, foo, ssh):
+    ssh.hardlink("foo", "bar")
+    assert System.is_hardlink("bar")
+
+def test_copy(chdir_tmp, foo, ssh):
+    ssh.copy("foo", "bar")
+    assert filecmp.cmp("foo", "bar")

From 0fc83be9ff019342edadb3cdfc6f2c7e6a69b7c9 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 03:23:35 -0500
Subject: [PATCH 13/44] tests: ssh hardlink optimization

---
 dvc/path_info.py                         |  2 +-
 dvc/remote/ssh/__init__.py               |  4 +++-
 tests/conftest.py                        | 10 ++++++++++
 tests/unit/remote/ssh/test_connection.py |  9 ---------
 tests/unit/remote/ssh/test_ssh.py        | 23 +++++++++++++++++++++++
 5 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/dvc/path_info.py b/dvc/path_info.py
index c7d0051d8..596c5071b 100644
--- a/dvc/path_info.py
+++ b/dvc/path_info.py
@@ -7,7 +7,7 @@ import posixpath
 from funcy import cached_property
 
 from dvc.utils.compat import str, builtin_str, basestring, is_py2
-from dvc.utils.compat import pathlib, urlparse
+from dvc.utils.compat import pathlib, urlparse, urljoin
 
 
 # On Python 2.7/Windows sys.getfilesystemencoding() is set to mbcs,
diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 2f8483abc..d0082d0e6 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -193,7 +193,9 @@ class RemoteSSH(RemoteBASE):
 
         # See dvc/remote/local/__init__.py - hardlink()
         if self.getsize(from_info) == 0:
-            self.open(to_info, "w").close()
+
+            with self.ssh(to_info) as ssh:
+                ssh.sftp.open(to_info.path, "w").close()
 
             logger.debug(
                 "Created empty file: {src} -> {dest}".format(
diff --git a/tests/conftest.py b/tests/conftest.py
index 3356a4a0b..051d2b7a9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -177,3 +177,13 @@ def _close_pools():
 
     yield
     close_pools()
+
+
+@pytest.fixture
+def chdir_tmp(tmp_path):
+    cwd = os.getcwd()
+    try:
+        os.chdir(tmp_path)
+        yield
+    finally:
+        os.chdir(cwd)
diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index 42371bc8f..ae79d4d92 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -9,15 +9,6 @@ import filecmp
 from dvc.system import System
 
 
-@pytest.fixture
-def chdir_tmp(tmp_path):
-    cwd = os.getcwd()
-    try:
-        os.chdir(tmp_path)
-        yield
-    finally:
-        os.chdir(cwd)
-
 @pytest.fixture
 def foo():
     with open("foo", "w") as fobj:
diff --git a/tests/unit/remote/ssh/test_ssh.py b/tests/unit/remote/ssh/test_ssh.py
index 0dee1ac3e..009e12a53 100644
--- a/tests/unit/remote/ssh/test_ssh.py
+++ b/tests/unit/remote/ssh/test_ssh.py
@@ -1,6 +1,7 @@
 import getpass
 import os
 import sys
+import posixpath
 
 from unittest import TestCase
 
@@ -8,6 +9,8 @@ from mock import patch, mock_open
 import pytest
 
 from dvc.remote.ssh import RemoteSSH
+from dvc.path_info import URLInfo
+from dvc.system import System
 
 
 class TestRemoteSSH(TestCase):
@@ -182,3 +185,23 @@ def test_ssh_gss_auth(mock_file, mock_exists, config, expected_gss_auth):
     mock_exists.assert_called_with(RemoteSSH.ssh_config_filename())
     mock_file.assert_called_with(RemoteSSH.ssh_config_filename())
     assert remote.gss_auth == expected_gss_auth
+
+def test_hardlink_optimization(chdir_tmp, ssh_server):
+    config = {
+        "url": "ssh://localhost",
+        "hostname": ssh_server.test_creds["host"],
+        "port": ssh_server.test_creds["port"],
+        "user": ssh_server.test_creds["username"],
+        "keyfile": ssh_server.test_creds["key_filename"],
+    }
+    remote = RemoteSSH(None, config)
+
+    open("empty", "w").close()
+
+    def urlpath(path):
+        netloc = remote.path_info.netloc
+        path = posixpath.abspath(path)
+        return URLInfo("ssh://{}/{}".format(netloc, path))
+
+    remote.hardlink(urlpath("empty"), urlpath("bar"))
+    assert not System.is_hardlink("bar")

From 8d41833c7c4265d77c20ff38be44da14694d3c04 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 03:32:23 -0500
Subject: [PATCH 14/44] path_info: remove urljoin leftover

---
 dvc/path_info.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dvc/path_info.py b/dvc/path_info.py
index 596c5071b..c7d0051d8 100644
--- a/dvc/path_info.py
+++ b/dvc/path_info.py
@@ -7,7 +7,7 @@ import posixpath
 from funcy import cached_property
 
 from dvc.utils.compat import str, builtin_str, basestring, is_py2
-from dvc.utils.compat import pathlib, urlparse, urljoin
+from dvc.utils.compat import pathlib, urlparse
 
 
 # On Python 2.7/Windows sys.getfilesystemencoding() is set to mbcs,

From 70010bbb3105d25eefa7c4c86d690cc7b6fa3289 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 12:59:34 -0500
Subject: [PATCH 15/44] remote: remove unecessary makedirs on checkout

---
 dvc/remote/base.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 46b050e82..88a2c0bf0 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -790,7 +790,6 @@ class RemoteBASE(object):
             if self.changed(entry_info, entry_checksum_info):
                 if self.exists(entry_info):
                     self.safe_remove(entry_info, force=force)
-                self.makedirs(entry_info.parent)
                 self.link(entry_cache_info, entry_info)
                 self.state.save(entry_info, entry_checksum)
             if progress_callback:

From 7b4584f86cfc36862db9c92eaa0d28a178e9e31c Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 13:02:17 -0500
Subject: [PATCH 16/44] ssh: stat -> lstat

---
 dvc/remote/ssh/connection.py             | 4 ++--
 tests/unit/remote/ssh/test_connection.py | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index 16f74c2d5..039652e61 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -60,13 +60,13 @@ class SSHConnection:
 
     def st_mode(self, path):
         with ignore_file_not_found():
-            return self.sftp.stat(path).st_mode
+            return self.sftp.lstat(path).st_mode
 
         return 0
 
     def getsize(self, path):
         with ignore_file_not_found():
-            return self.sftp.stat(path).st_size
+            return self.sftp.lstat(path).st_size
 
         return 0
 
diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index ae79d4d92..cadba03f2 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -81,7 +81,9 @@ def test_walk(tmp_path, ssh):
                     reason="Only works on OSX (because of APFS)")
 def test_reflink(chdir_tmp, foo, ssh):
     ssh.reflink("foo", "bar")
-    assert System.is_reflink(bar_path)
+    assert filecmp.cmp("foo", "bar")
+    assert not System.is_symlink("bar")
+    assert not System.is_hardlink("bar")
 
 @pytest.mark.skip(reason="MockSSH doesn't support symlinks")
 def test_symlink(chdir_tmp, foo, ssh):

From da65f8fe9aa1091c8a150b1d6e92652686336869 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 31 Aug 2019 13:34:22 -0500
Subject: [PATCH 17/44] tests: functional ssh symlink

---
 tests/func/test_data_cloud.py            | 22 ++++++++++++++++++++++
 tests/unit/remote/ssh/test_connection.py | 11 +++++------
 tests/unit/remote/ssh/test_ssh.py        |  1 +
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py
index 780755c73..6a301afc7 100644
--- a/tests/func/test_data_cloud.py
+++ b/tests/func/test_data_cloud.py
@@ -9,6 +9,7 @@ import platform
 import copy
 import logging
 import pytest
+import posixpath
 
 from mock import patch
 
@@ -27,8 +28,10 @@ from dvc.data_cloud import (
     RemoteHTTP,
 )
 from dvc.remote.base import STATUS_OK, STATUS_NEW, STATUS_DELETED
+from dvc.path_info import URLInfo
 from dvc.utils import file_md5
 from dvc.utils.stage import load_stage_file, dump_stage_file
+from dvc.system import System
 
 from tests.basic_env import TestDvc
 from tests.utils import spy
@@ -449,6 +452,25 @@ class TestRemoteSSH(TestDataCloudBase):
     def _get_cloud_class(self):
         return RemoteSSH
 
+    def test_symlinks(self):
+        self._setup_cloud()
+
+        remote = self.cloud.get_remote()
+
+        os.remove("bar")
+
+        def urlinfo(path):
+            cwd = os.getcwd()
+            path = posixpath.abspath(os.path.join(cwd, path))
+            netloc = remote.path_info.netloc
+            return URLInfo("ssh://{}{}".format(netloc, path))
+
+        from_info = urlinfo("foo")
+        to_info = urlinfo("bar")
+
+        remote.symlink(from_info, to_info)
+        assert System.is_symlink("bar")
+
 
 @pytest.mark.usefixtures("ssh_server")
 class TestRemoteSSHMocked(TestDataCloudBase):
diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index cadba03f2..f28cf3ecd 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -77,23 +77,22 @@ def test_walk(tmp_path, ssh):
 
     assert paths == expected
 
-@pytest.mark.skipif(platform.system() != "Darwin",
-                    reason="Only works on OSX (because of APFS)")
+
+@pytest.mark.skipif(
+    platform.system() != "Darwin", reason="Only works on OSX (because of APFS)"
+)
 def test_reflink(chdir_tmp, foo, ssh):
     ssh.reflink("foo", "bar")
     assert filecmp.cmp("foo", "bar")
     assert not System.is_symlink("bar")
     assert not System.is_hardlink("bar")
 
-@pytest.mark.skip(reason="MockSSH doesn't support symlinks")
-def test_symlink(chdir_tmp, foo, ssh):
-    ssh.symlink("foo", "bar")
-    assert System.is_symlink("bar")
 
 def test_hardlink(chdir_tmp, foo, ssh):
     ssh.hardlink("foo", "bar")
     assert System.is_hardlink("bar")
 
+
 def test_copy(chdir_tmp, foo, ssh):
     ssh.copy("foo", "bar")
     assert filecmp.cmp("foo", "bar")
diff --git a/tests/unit/remote/ssh/test_ssh.py b/tests/unit/remote/ssh/test_ssh.py
index 009e12a53..1668680c9 100644
--- a/tests/unit/remote/ssh/test_ssh.py
+++ b/tests/unit/remote/ssh/test_ssh.py
@@ -186,6 +186,7 @@ def test_ssh_gss_auth(mock_file, mock_exists, config, expected_gss_auth):
     mock_file.assert_called_with(RemoteSSH.ssh_config_filename())
     assert remote.gss_auth == expected_gss_auth
 
+
 def test_hardlink_optimization(chdir_tmp, ssh_server):
     config = {
         "url": "ssh://localhost",

From bbfaf3a22b80c714a4dacc06ff4924b92b2a9166 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Mon, 2 Sep 2019 23:17:44 -0500
Subject: [PATCH 18/44] setup: use mock-ssh-server version supporting lstat

---
 setup.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 83db8b3fb..05243f036 100644
--- a/setup.py
+++ b/setup.py
@@ -116,7 +116,8 @@ tests_requirements = [
     "flake8-docstrings",
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
-    "mock-ssh-server>=0.5.0",
+    # Using fork of mock-ssh-server that supports `lstat`
+    "mock-ssh-server @ git+https://github.com/mroutis/mock-ssh-server@master#egg=mock-ssh-server",
     "moto>=1.3.14.dev55",
     "rangehttpserver==1.2.0",
 ]

From 69638527cb5ec18d956140f6c9a1690d3357b430 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Tue, 3 Sep 2019 00:01:14 -0500
Subject: [PATCH 19/44] setup: mroutis/mock-ssh-server ->
 iterative/mock-ssh-server

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 05243f036..1f6ec9821 100644
--- a/setup.py
+++ b/setup.py
@@ -117,7 +117,7 @@ tests_requirements = [
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
     # Using fork of mock-ssh-server that supports `lstat`
-    "mock-ssh-server @ git+https://github.com/mroutis/mock-ssh-server@master#egg=mock-ssh-server",
+    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server",
     "moto>=1.3.14.dev55",
     "rangehttpserver==1.2.0",
 ]

From 88574f300e8efb788e68261d62f8180dc01e0ff8 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Tue, 3 Sep 2019 00:01:38 -0500
Subject: [PATCH 20/44] remote: remove unecessary else clause

---
 dvc/remote/base.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 88a2c0bf0..860c84193 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -390,11 +390,10 @@ class RemoteBASE(object):
         raise DvcException("no possible cache types left to try out.")
 
     def _do_link(self, from_info, to_info, link_method):
-        # XXX: We are testing if file exists rather than if file is a link
         if self.exists(to_info):
             raise DvcException("Link '{}' already exists!".format(to_info))
-        else:
-            link_method(from_info, to_info)
+
+        link_method(from_info, to_info)
 
         if self.protected:
             self.protect(to_info)

From 3c69dc9fd56307ca239b001f11d8e91ed1819f2f Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Tue, 3 Sep 2019 00:04:44 -0500
Subject: [PATCH 21/44] tests: unit ssh symlink

---
 tests/unit/remote/ssh/test_connection.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index f28cf3ecd..c458a6641 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -88,6 +88,11 @@ def test_reflink(chdir_tmp, foo, ssh):
     assert not System.is_hardlink("bar")
 
 
+def test_symlink(chdir_tmp, foo, ssh):
+    ssh.symlink("foo", "bar")
+    assert System.is_symlink("bar")
+
+
 def test_hardlink(chdir_tmp, foo, ssh):
     ssh.hardlink("foo", "bar")
     assert System.is_hardlink("bar")

From cdee18a7a6a141c9b99065d7b49de24823abba6b Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Tue, 3 Sep 2019 00:27:10 -0500
Subject: [PATCH 22/44] fix formatting

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 1f6ec9821..ab3a4f926 100644
--- a/setup.py
+++ b/setup.py
@@ -117,7 +117,7 @@ tests_requirements = [
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
     # Using fork of mock-ssh-server that supports `lstat`
-    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server",
+    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server", # noqa: E501
     "moto>=1.3.14.dev55",
     "rangehttpserver==1.2.0",
 ]

From 1f0a57d4af1d8bfc49e0804a426bf41d74c20369 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 09:48:46 -0500
Subject: [PATCH 23/44] remote: remove unnecessary variable in `_try_links`

---
 dvc/remote/base.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 860c84193..f6a43e5af 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -374,8 +374,7 @@ class RemoteBASE(object):
 
     @slow_link_guard
     def _try_links(self, from_info, to_info, link_types):
-        i = len(link_types)
-        while i > 0:
+        while link_types:
             link_method = getattr(self, link_types[0])
             try:
                 self._do_link(from_info, to_info, link_method)
@@ -385,7 +384,6 @@ class RemoteBASE(object):
                 msg = "Cache type '{}' is not supported: {}"
                 logger.debug(msg.format(link_types[0], str(exc)))
                 del link_types[0]
-                i -= 1
 
         raise DvcException("no possible cache types left to try out.")
 

From 1ae9078fee01a6d5057d59d47f72c61af7f12157 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 12:35:54 -0500
Subject: [PATCH 24/44] revert remove assertions from open

---
 dvc/remote/hdfs.py   | 2 ++
 dvc/repo/__init__.py | 2 --
 dvc/utils/http.py    | 2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/dvc/remote/hdfs.py b/dvc/remote/hdfs.py
index ec37ace63..efdbfbba4 100644
--- a/dvc/remote/hdfs.py
+++ b/dvc/remote/hdfs.py
@@ -143,6 +143,8 @@ class RemoteHDFS(RemoteBASE):
 
     @contextmanager
     def open(self, path_info, mode="r", encoding=None):
+        assert mode in {"r", "rt", "rb"}
+
         try:
             with self.hdfs(path_info) as hdfs, closing(
                 hdfs.open(path_info.path, mode="rb")
diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py
index af9d7a92f..2f743a6ff 100644
--- a/dvc/repo/__init__.py
+++ b/dvc/repo/__init__.py
@@ -440,8 +440,6 @@ class Repo(object):
     @contextmanager
     def open(self, path, remote=None, mode="r", encoding=None):
         """Opens a specified resource as a file descriptor"""
-        assert mode in {"r", "rt", "rb"}
-
         try:
             with self._open(path, remote, mode, encoding) as fd:
                 yield fd
diff --git a/dvc/utils/http.py b/dvc/utils/http.py
index 7d8c6f55a..b2ecf4bb3 100644
--- a/dvc/utils/http.py
+++ b/dvc/utils/http.py
@@ -13,6 +13,8 @@ def open_url(url, mode="r", encoding=None):
     Resumes on connection error.
     Url could be a string or a callable returning a string.
     """
+    assert mode in {"r", "rt", "rb"}
+
     with iter_url(url) as (response, it):
         bytes_stream = IterStream(it)
 

From 9295bbd16059ffddba4abe60610bddfc60682397 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 15:13:26 -0500
Subject: [PATCH 25/44] ssh: support opening files for writing

---
 dvc/remote/ssh/__init__.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index d0082d0e6..4cc76d972 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -251,12 +251,14 @@ class RemoteSSH(RemoteBASE):
     @contextmanager
     def open(self, path_info, mode="r", encoding=None):
         with self.ssh(path_info) as ssh, closing(
-            ssh.sftp.file(path_info.path, mode="r")
+            ssh.sftp.open(path_info.path, mode)
         ) as fd:
-            if mode == "rb":
+            if "b" in mode:
                 yield fd
             else:
-                yield io.TextIOWrapper(fd, encoding=encoding)
+                yield io.TextIOWrapper(
+                    fd, encoding=encoding, write_through=True
+                )
 
     def list_cache_paths(self):
         with self.ssh(self.path_info) as ssh:

From 2fe6fc1ba65c7f107c2dd238e9e05dd7e82fe131 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 19:32:55 -0500
Subject: [PATCH 26/44] refactor: use a module for slow link detection

---
 dvc/remote/slow_link_detection.py             |  75 +++++-----
 tests/unit/remote/test_slow_link_detection.py | 128 +++++-------------
 2 files changed, 65 insertions(+), 138 deletions(-)

diff --git a/dvc/remote/slow_link_detection.py b/dvc/remote/slow_link_detection.py
index 2e51413ba..bd47612ef 100644
--- a/dvc/remote/slow_link_detection.py
+++ b/dvc/remote/slow_link_detection.py
@@ -1,60 +1,49 @@
 import logging
 import time
+import sys
+from functools import wraps
 
 import colorama
 from dvc.config import Config
 
+
 logger = logging.getLogger(__name__)
+this = sys.modules[__name__]
+
+this.timeout_seconds = 10.0
+this.already_displayed = False
+this.message = (
+    "You can cut execution time considerably by using a different "
+    "`cache.type` configuration.\n"
+    "See {blue}https://dvc.org/doc/commands-reference/config#cache{reset} "
+    "for more information.\n"
+    "To disable this message, run:\n"
+    "'dvc config cache.slow_link_warning false'"
+    .format(blue=colorama.Fore.BLUE, reset=colorama.Fore.RESET)
+)
 
 
-class SlowLinkDetectorDecorator(object):
-    LINKING_TIMEOUT_SECONDS = 10.0
-    was_displayed = False
+def slow_link_guard(f):
+    @wraps(f)
+    def wrapper(remote, *args, **kwargs):
+        if this.already_displayed:
+            return f(remote, *args, **kwargs)
 
-    @classmethod
-    def should_display(cls):
-        if not cls.was_displayed:
-            cls.was_displayed = True
-            return True
-        return False
+        config = remote.repo.config.config.get(Config.SECTION_CACHE, {})
+        cache_type = config.get(Config.SECTION_CACHE_TYPE)
+        should_warn = config.get(Config.SECTION_CACHE_SLOW_LINK_WARNING, True)
 
-    def __init__(self, method):
-        self.method = method
+        if not should_warn or cache_type:
+            return f(remote, *args, **kwargs)
 
-    def __call__(self, *args, **kwargs):
         start = time.time()
-        result = self.method(*args, **kwargs)
-        execution_time_seconds = time.time() - start
+        result = f(remote, *args, **kwargs)
+        delta = time.time() - start
 
-        if (
-            execution_time_seconds >= self.LINKING_TIMEOUT_SECONDS
-            and self.should_display()
-        ):
-            msg = (
-                "You can cut execution time considerably by using a different"
-                " `cache.type` configuration.\nSee "
-                "{blue}https://dvc.org/doc/commands-reference/config#cache{"
-                "reset} for more information.\n"
-                "To disable this message, run:\n"
-                "'dvc config cache.slow_link_warning false'".format(
-                    blue=colorama.Fore.BLUE, reset=colorama.Fore.RESET
-                )
-            )
-            logger.warning(msg)
+        if delta >= this.timeout_seconds:
+            logger.warning(this.message)
+            this.already_displayed = True
 
         return result
 
-
-def slow_link_guard(method):
-    def call(remote, *args, **kwargs):
-        cache_config = remote.repo.config.config.get(Config.SECTION_CACHE, {})
-        should_warn = cache_config.get(
-            Config.SECTION_CACHE_SLOW_LINK_WARNING, True
-        ) and not cache_config.get(Config.SECTION_CACHE_TYPE, None)
-
-        if should_warn:
-            decorated = SlowLinkDetectorDecorator(method)
-            return decorated(remote, *args, **kwargs)
-        return method(remote, *args, **kwargs)
-
-    return call
+    return wrapper
diff --git a/tests/unit/remote/test_slow_link_detection.py b/tests/unit/remote/test_slow_link_detection.py
index 791c8a945..d94a79e7b 100644
--- a/tests/unit/remote/test_slow_link_detection.py
+++ b/tests/unit/remote/test_slow_link_detection.py
@@ -1,111 +1,49 @@
-import logging
-
-import colorama
 import pytest
+import mock
+
+import dvc.remote.slow_link_detection
+from dvc.remote.slow_link_detection import slow_link_guard
 from dvc.config import Config
-from dvc.remote.slow_link_detection import (
-    slow_link_guard,
-    SlowLinkDetectorDecorator,
-)
-from mock import Mock, patch
 
 
-@patch(
-    "dvc.remote.slow_link_detection.SlowLinkDetectorDecorator", autospec=True
-)
-class TestSlowLinkGuard(object):
-    @pytest.fixture(autouse=True)
-    def setUp(self):
-        self.method = Mock()
-        self.remote_mock = Mock()
-        self.config_mock = Mock()
-        self.remote_mock.repo.config.config.get.return_value = self.config_mock
-
-    def _cache_config(self, slow_link_warning, cache_type):
-        def config_side_effect(section_name, _):
-            if section_name == Config.SECTION_CACHE_SLOW_LINK_WARNING:
-                return slow_link_warning
-            elif section_name == Config.SECTION_CACHE_TYPE:
-                return cache_type
-
-        self.config_mock.get.side_effect = config_side_effect
-
-    def test_should_decorate_on_slow_link_warning_and_no_cache_type(
-        self, SlowLinkDetectorDecoratorClassMock
-    ):
-        self._cache_config(slow_link_warning=True, cache_type=None)
-
-        slow_link_guard(self.method)(self.remote_mock)
-
-        assert 1 == SlowLinkDetectorDecoratorClassMock.call_count
-        assert 0 == self.method.call_count
-
-    @pytest.mark.parametrize(
-        "slow_link_warning, cache_type",
-        [(True, "any_cache_type"), (False, None)],
+@pytest.fixture(autouse=True)
+def timeout_immediately(monkeypatch):
+    monkeypatch.setattr(
+        dvc.remote.slow_link_detection, 'timeout_seconds', 0.0
     )
-    def test_should_not_decorate(
-        self, SlowLinkDetectorDecoratorClassMock, slow_link_warning, cache_type
-    ):
-        self._cache_config(
-            slow_link_warning=slow_link_warning, cache_type=cache_type
-        )
-
-        slow_link_guard(self.method)(self.remote_mock)
-
-        assert 0 == SlowLinkDetectorDecoratorClassMock.call_count
-        assert 1 == self.method.call_count
 
 
-class TestSlowLinkDetectorDecorator(object):
-    @pytest.fixture(autouse=True)
-    def setUp(self):
-        self.args_mock = Mock()
-        self.method_result_mock = Mock()
+@pytest.fixture
+def make_remote():
+    def _make_remote(cache_type=None, should_warn=True):
+        remote = mock.Mock()
+        remote.repo.config.config.get.return_value = {
+            Config.SECTION_CACHE_TYPE: cache_type,
+            Config.SECTION_CACHE_SLOW_LINK_WARNING: should_warn,
+        }
+        return remote
 
-        def method_effect(arg):
-            assert self.args_mock == arg
-            return self.method_result_mock
+    return _make_remote
 
-        self.method_mock = Mock(side_effect=method_effect)
 
-    @patch.object(SlowLinkDetectorDecorator, "LINKING_TIMEOUT_SECONDS", 0.0)
-    def test_should_log_only_once(self, caplog):
+def test_show_warning_once(caplog, make_remote):
+    remote = make_remote()
+    slow_link_guard(lambda x: None)(remote)
+    slow_link_guard(lambda x: None)(remote)
 
-        with caplog.at_level(logging.WARNING, logger="dvc"):
-            test_instance = SlowLinkDetectorDecorator(self.method_mock)
-            result = test_instance(self.args_mock)
-            assert self.method_result_mock == result
+    assert caplog.records[0].message == dvc.remote.slow_link_detection.message
+    assert len(caplog.records) == 1
 
-            # slow link guard keeps creating new SlowLinkDetectorDecorators
-            # each time it is called, so we need to make sure that we are
-            # preventing every new instance of this class from displaying the
-            # message
-            test_instance2 = SlowLinkDetectorDecorator(self.method_mock)
-            result = test_instance2(self.args_mock)
-            assert self.method_result_mock == result
 
-        msg = (
-            "You can cut execution time considerably by using a different"
-            " `cache.type` configuration.\nSee "
-            "{blue}https://dvc.org/doc/commands-reference/config#cache{"
-            "reset} for more information.\n"
-            "To disable this message, run:\n"
-            "'dvc config cache.slow_link_warning false'".format(
-                blue=colorama.Fore.BLUE, reset=colorama.Fore.RESET
-            )
-        )
-        assert 1 == len(caplog.messages)
-        assert msg == caplog.messages[0]
+def test_dont_warn_when_cache_type_is_set(caplog, make_remote):
+    remote = make_remote(cache_type="copy")
+    slow_link_guard(lambda x: None)(remote)
 
-    @patch.object(
-        SlowLinkDetectorDecorator, "LINKING_TIMEOUT_SECONDS", 99999.0
-    )
-    def test_should_not_log(self, caplog):
-        test_instance = SlowLinkDetectorDecorator(self.method_mock)
+    assert len(caplog.records) == 0
 
-        with caplog.at_level(logging.WARNING, logger="dvc"):
-            result = test_instance(self.args_mock)
 
-        assert self.method_result_mock == result
-        assert 0 == len(caplog.messages)
+def test_dont_warn_when_warning_is_disabled(caplog, make_remote):
+    remote = make_remote(should_warn=False)
+    slow_link_guard(lambda x: None)(remote)
+
+    assert len(caplog.records) == 0

From 079d589b593391e6d9f497bd51431589210248f9 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 19:38:16 -0500
Subject: [PATCH 27/44] local: use path_info for link operations

---
 dvc/remote/local/__init__.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 3b9bfb98e..12a844778 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -175,7 +175,7 @@ class RemoteLOCAL(RemoteBASE):
         shutil.copyfile(from_info.fspath, to_info.fspath)
 
     def symlink(self, from_info, to_info):
-        System.symlink(from_info.fspath, to_info.fspath)
+        System.symlink(from_info, to_info)
 
     def hardlink(self, from_info, to_info):
         # If there are a lot of empty files (which happens a lot in datasets),
@@ -201,10 +201,10 @@ class RemoteLOCAL(RemoteBASE):
             )
             return
 
-        System.hardlink(from_info.fspath, to_info.fspath)
+        System.hardlink(from_info, to_info)
 
     def reflink(self, from_info, to_info):
-        System.reflink(from_info.fspath, to_info.fspath)
+        System.reflink(from_info, to_info)
 
     def cache_exists(self, checksums, jobs=None, name=None):
         return [

From 0c231b68d73410d020f9f7867b541be1ab03bb86 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Wed, 4 Sep 2019 22:07:02 -0500
Subject: [PATCH 28/44] fix formatting

---
 dvc/remote/slow_link_detection.py             | 5 +++--
 tests/unit/remote/test_slow_link_detection.py | 4 +---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/dvc/remote/slow_link_detection.py b/dvc/remote/slow_link_detection.py
index bd47612ef..0dd2285d6 100644
--- a/dvc/remote/slow_link_detection.py
+++ b/dvc/remote/slow_link_detection.py
@@ -18,8 +18,9 @@ this.message = (
     "See {blue}https://dvc.org/doc/commands-reference/config#cache{reset} "
     "for more information.\n"
     "To disable this message, run:\n"
-    "'dvc config cache.slow_link_warning false'"
-    .format(blue=colorama.Fore.BLUE, reset=colorama.Fore.RESET)
+    "'dvc config cache.slow_link_warning false'".format(
+        blue=colorama.Fore.BLUE, reset=colorama.Fore.RESET
+    )
 )
 
 
diff --git a/tests/unit/remote/test_slow_link_detection.py b/tests/unit/remote/test_slow_link_detection.py
index d94a79e7b..141b4ff95 100644
--- a/tests/unit/remote/test_slow_link_detection.py
+++ b/tests/unit/remote/test_slow_link_detection.py
@@ -8,9 +8,7 @@ from dvc.config import Config
 
 @pytest.fixture(autouse=True)
 def timeout_immediately(monkeypatch):
-    monkeypatch.setattr(
-        dvc.remote.slow_link_detection, 'timeout_seconds', 0.0
-    )
+    monkeypatch.setattr(dvc.remote.slow_link_detection, "timeout_seconds", 0.0)
 
 
 @pytest.fixture

From d7da4fcd37925fada72a6424a157ca9d57c17c86 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Thu, 5 Sep 2019 10:52:15 -0500
Subject: [PATCH 29/44] fix formatting

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index ab3a4f926..899a07adb 100644
--- a/setup.py
+++ b/setup.py
@@ -117,7 +117,7 @@ tests_requirements = [
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
     # Using fork of mock-ssh-server that supports `lstat`
-    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server", # noqa: E501
+    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server",  # noqa: E501
     "moto>=1.3.14.dev55",
     "rangehttpserver==1.2.0",
 ]

From 739236eab014a5e85c46466d2a94b3ba237ab438 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 08:48:22 -0500
Subject: [PATCH 30/44] ssh: accept only wb mode for open

---
 dvc/remote/ssh/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 4cc76d972..27dd81c0a 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -250,6 +250,8 @@ class RemoteSSH(RemoteBASE):
 
     @contextmanager
     def open(self, path_info, mode="r", encoding=None):
+        assert mode in {"r", "rt", "rb", "wb"}
+
         with self.ssh(path_info) as ssh, closing(
             ssh.sftp.open(path_info.path, mode)
         ) as fd:

From 38d4cbf5c15abe504bfacbdeb4078df3538287b9 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 09:42:56 -0500
Subject: [PATCH 31/44] fix broken merge

---
 dvc/remote/base.py           |   1 +
 dvc/remote/local/__init__.py | 137 -----------------------------------
 2 files changed, 1 insertion(+), 137 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 56ff49a6b..73d0710ac 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -379,6 +379,7 @@ class RemoteBASE(object):
             link_method = getattr(self, link_types[0])
             try:
                 self._do_link(from_info, to_info, link_method)
+                self.cache_type_confirmed = True
                 return
 
             except DvcException as exc:
diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 59b201470..3a4192721 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -115,143 +115,6 @@ class RemoteLOCAL(RemoteBASE):
     def makedirs(self, path_info):
         makedirs(path_info, exist_ok=True, mode=self._dir_mode)
 
-<<<<<<< HEAD
-||||||| merged common ancestors
-    def link(self, from_info, to_info):
-        self._link(from_info, to_info, self.cache_types)
-
-    def _link(self, from_info, to_info, link_types):
-        from_path = from_info.fspath
-        to_path = to_info.fspath
-
-        assert os.path.isfile(from_path)
-
-        dname = os.path.dirname(to_path)
-        if not os.path.exists(dname):
-            os.makedirs(dname)
-
-        # NOTE: just create an empty file for an empty cache
-        if os.path.getsize(from_path) == 0:
-            open(to_path, "w+").close()
-
-            msg = "Created empty file: {} -> {}".format(from_path, to_path)
-            logger.debug(msg)
-            return
-
-        self._try_links(from_info, to_info, link_types)
-
-    @classmethod
-    def _get_link_method(cls, link_type):
-        try:
-            return cls.CACHE_TYPE_MAP[link_type]
-        except KeyError:
-            raise DvcException(
-                "Cache type: '{}' not supported!".format(link_type)
-            )
-
-    def _do_link(self, from_info, to_info, link_method):
-        if self.exists(to_info):
-            raise DvcException("Link '{}' already exists!".format(to_info))
-        else:
-            link_method(from_info.fspath, to_info.fspath)
-
-        if self.protected:
-            self.protect(to_info)
-
-        msg = "Created {}'{}': {} -> {}".format(
-            "protected " if self.protected else "",
-            self.cache_types[0],
-            from_info,
-            to_info,
-        )
-        logger.debug(msg)
-
-    @slow_link_guard
-    def _try_links(self, from_info, to_info, link_types):
-        i = len(link_types)
-        while i > 0:
-            link_method = self._get_link_method(link_types[0])
-            try:
-                self._do_link(from_info, to_info, link_method)
-                return
-
-            except DvcException as exc:
-                msg = "Cache type '{}' is not supported: {}"
-                logger.debug(msg.format(link_types[0], str(exc)))
-                del link_types[0]
-                i -= 1
-
-        raise DvcException("no possible cache types left to try out.")
-
-=======
-    def link(self, from_info, to_info):
-        self._link(from_info, to_info, self.cache_types)
-
-    def _link(self, from_info, to_info, link_types):
-        from_path = from_info.fspath
-        to_path = to_info.fspath
-
-        assert os.path.isfile(from_path)
-
-        dname = os.path.dirname(to_path)
-        if not os.path.exists(dname):
-            os.makedirs(dname)
-
-        # NOTE: just create an empty file for an empty cache
-        if os.path.getsize(from_path) == 0:
-            open(to_path, "w+").close()
-
-            msg = "Created empty file: {} -> {}".format(from_path, to_path)
-            logger.debug(msg)
-            return
-
-        self._try_links(from_info, to_info, link_types)
-
-    @classmethod
-    def _get_link_method(cls, link_type):
-        try:
-            return cls.CACHE_TYPE_MAP[link_type]
-        except KeyError:
-            raise DvcException(
-                "Cache type: '{}' not supported!".format(link_type)
-            )
-
-    def _do_link(self, from_info, to_info, link_method):
-        if self.exists(to_info):
-            raise DvcException("Link '{}' already exists!".format(to_info))
-        else:
-            link_method(from_info.fspath, to_info.fspath)
-
-        if self.protected:
-            self.protect(to_info)
-
-        msg = "Created {}'{}': {} -> {}".format(
-            "protected " if self.protected else "",
-            self.cache_types[0],
-            from_info,
-            to_info,
-        )
-        logger.debug(msg)
-
-    @slow_link_guard
-    def _try_links(self, from_info, to_info, link_types):
-        i = len(link_types)
-        while i > 0:
-            link_method = self._get_link_method(link_types[0])
-            try:
-                self._do_link(from_info, to_info, link_method)
-                self.cache_type_confirmed = True
-                return
-
-            except DvcException as exc:
-                msg = "Cache type '{}' is not supported: {}"
-                logger.debug(msg.format(link_types[0], str(exc)))
-                del link_types[0]
-                i -= 1
-
-        raise DvcException("no possible cache types left to try out.")
-
->>>>>>> master
     def already_cached(self, path_info):
         assert path_info.scheme in ["", "local"]
 

From 6eb26de856b81e6e2732f51f3578200953a8440b Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 11:31:51 -0500
Subject: [PATCH 32/44] fix tests

---
 dvc/remote/local/__init__.py |  3 +--
 dvc/system.py                |  6 ++++++
 tests/func/test_add.py       | 19 ++++++++++---------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 3a4192721..299a6ad4b 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -7,7 +7,6 @@ import os
 import stat
 import errno
 from shortuuid import uuid
-import shutil
 import logging
 from functools import partial
 
@@ -172,7 +171,7 @@ class RemoteLOCAL(RemoteBASE):
         move(from_info, to_info, mode=mode)
 
     def copy(self, from_info, to_info):
-        shutil.copyfile(from_info.fspath, to_info.fspath)
+        System.copy(from_info, to_info)
 
     def symlink(self, from_info, to_info):
         System.symlink(from_info, to_info)
diff --git a/dvc/system.py b/dvc/system.py
index b445ec01a..c7c7cf8e4 100644
--- a/dvc/system.py
+++ b/dvc/system.py
@@ -4,6 +4,7 @@ from dvc.utils.compat import str, open, fspath
 
 import os
 import errno
+import shutil
 
 
 class System(object):
@@ -11,6 +12,11 @@ class System(object):
     def is_unix():
         return os.name != "nt"
 
+    @staticmethod
+    def copy(src, dest):
+        src, dest = fspath(src), fspath(dest)
+        return shutil.copyfile(src, dest)
+
     @staticmethod
     def hardlink(source, link_name):
         import ctypes
diff --git a/tests/func/test_add.py b/tests/func/test_add.py
index c11e0a3b6..c804fa203 100644
--- a/tests/func/test_add.py
+++ b/tests/func/test_add.py
@@ -563,12 +563,12 @@ def test_should_not_checkout_when_adding_cached_copy(repo_dir, dvc_repo):
 
     shutil.copy(repo_dir.BAR, repo_dir.FOO)
 
-    copy_spy = spy(shutil.copyfile)
+    copy_spy = spy(dvc_repo.cache.local.copy)
 
-    RemoteLOCAL.CACHE_TYPE_MAP["copy"] = copy_spy
-    dvc_repo.add(repo_dir.FOO)
+    with patch.object(dvc_repo.cache.local, "copy", copy_spy):
+        dvc_repo.add(repo_dir.FOO)
 
-    assert copy_spy.mock.call_count == 0
+        assert copy_spy.mock.call_count == 0
 
 
 @pytest.mark.parametrize(
@@ -583,13 +583,13 @@ def test_should_not_checkout_when_adding_cached_copy(repo_dir, dvc_repo):
 def test_should_relink_on_repeated_add(
     link, new_link, link_test_func, repo_dir, dvc_repo
 ):
-    dvc_repo.cache.local.cache_types = [link]
+    dvc_repo.config.set("cache", "type", link)
 
     dvc_repo.add(repo_dir.FOO)
     dvc_repo.add(repo_dir.BAR)
 
     os.remove(repo_dir.FOO)
-    RemoteLOCAL.CACHE_TYPE_MAP[link](repo_dir.BAR, repo_dir.FOO)
+    getattr(dvc_repo.cache.local, link)(repo_dir.BAR, repo_dir.FOO)
 
     dvc_repo.cache.local.cache_types = [new_link]
 
@@ -613,10 +613,11 @@ def test_should_relink_single_file_in_dir(link, link_func, dvc_repo, repo_dir):
     dvc_repo.unprotect(repo_dir.DATA_SUB)
 
     link_spy = spy(link_func)
-    RemoteLOCAL.CACHE_TYPE_MAP[link] = link_spy
-    dvc_repo.add(repo_dir.DATA_DIR)
 
-    assert link_spy.mock.call_count == 1
+    with patch.object(dvc_repo.cache.local, link, link_spy):
+        dvc_repo.add(repo_dir.DATA_DIR)
+
+        assert link_spy.mock.call_count == 1
 
 
 @pytest.mark.parametrize("link", ["hardlink", "symlink", "copy"])

From f97c33178f75e5e94006dca2273af3e46a348c71 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 12:05:53 -0500
Subject: [PATCH 33/44] tests: use copy as cache type for HDFS

---
 tests/func/test_repro.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py
index f1a006ede..8540fe449 100644
--- a/tests/func/test_repro.py
+++ b/tests/func/test_repro.py
@@ -808,6 +808,10 @@ class TestReproExternalBase(TestDvc):
     def cache_scheme(self):
         return self.scheme
 
+    @property
+    def cache_type(self):
+        return "hardlink"
+
     @property
     def scheme(self):
         return None
@@ -873,7 +877,7 @@ class TestReproExternalBase(TestDvc):
 
         ret = main(["remote", "add", remote_name, remote])
         self.assertEqual(ret, 0)
-        ret = main(["remote", "modify", remote_name, "type", "hardlink"])
+        ret = main(["remote", "modify", remote_name, "type", self.cache_type])
         self.assertEqual(ret, 0)
 
         self.dvc = DvcRepo(".")
@@ -993,6 +997,10 @@ class TestReproExternalHDFS(TestReproExternalBase):
     def scheme(self):
         return "hdfs"
 
+    @property
+    def cache_type(self):
+        return "copy"
+
     @property
     def bucket(self):
         return "{}@127.0.0.1".format(getpass.getuser())

From 23f2650b2498023bbba553fb6f10f546f5909638 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 12:07:15 -0500
Subject: [PATCH 34/44] ssh: remove write_through in TextIOWrapper

---
 dvc/remote/ssh/__init__.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py
index 27dd81c0a..233f133ee 100644
--- a/dvc/remote/ssh/__init__.py
+++ b/dvc/remote/ssh/__init__.py
@@ -258,9 +258,7 @@ class RemoteSSH(RemoteBASE):
             if "b" in mode:
                 yield fd
             else:
-                yield io.TextIOWrapper(
-                    fd, encoding=encoding, write_through=True
-                )
+                yield io.TextIOWrapper(fd, encoding=encoding)
 
     def list_cache_paths(self):
         with self.ssh(self.path_info) as ssh:

From 73133e8c7ac20ed55e317529c2ff2a9a02ae4029 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 12:35:38 -0500
Subject: [PATCH 35/44] tests: [re] fix hardlink stuff with HDFS

---
 tests/func/test_repro.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py
index 8540fe449..970c3448d 100644
--- a/tests/func/test_repro.py
+++ b/tests/func/test_repro.py
@@ -866,7 +866,7 @@ class TestReproExternalBase(TestDvc):
         self.assertEqual(ret, 0)
         ret = main(["remote", "add", "myrepo", cache])
         self.assertEqual(ret, 0)
-        ret = main(["remote", "modify", "myrepo", "type", "hardlink"])
+        ret = main(["remote", "modify", "myrepo", "type", self.cache_type])
         self.assertEqual(ret, 0)
 
         remote_name = "myremote"

From dd1d5bdbb0a6eff23f113334daea57b8410cc616 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 12:57:06 -0500
Subject: [PATCH 36/44] tests: PosixPath to string in chdir_tmp fixture

---
 tests/conftest.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index 62355b02c..d5851b6a7 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -182,7 +182,7 @@ def _close_pools():
 def chdir_tmp(tmp_path):
     cwd = os.getcwd()
     try:
-        os.chdir(tmp_path)
+        os.chdir(str(tmp_path))
         yield
     finally:
         os.chdir(cwd)

From b66996e0a104faeacf96918521930e2ea2e1206d Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 13:24:25 -0500
Subject: [PATCH 37/44] ssh: support reflinks in Darwin

---
 dvc/remote/ssh/connection.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py
index 039652e61..4e1e52a43 100644
--- a/dvc/remote/ssh/connection.py
+++ b/dvc/remote/ssh/connection.py
@@ -295,7 +295,15 @@ class SSHConnection:
         self.sftp.symlink(src, dest)
 
     def reflink(self, src, dest):
-        self.execute("cp --reflink {} {}".format(src, dest))
+        if self.uname == "Linux":
+            return self.execute("cp --reflink {} {}".format(src, dest))
+
+        if self.uname == "Darwin":
+            return self.execute("cp -c {} {}".format(src, dest))
+
+        raise DvcException(
+            "'{}' is not supported as a SSH remote".format(self.uname)
+        )
 
     def hardlink(self, src, dest):
         self.execute("ln {} {}".format(src, dest))

From 737f1e141d82d1cad37ff5f8c26cad3fb4d0e494 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 13:24:54 -0500
Subject: [PATCH 38/44] tests: use repo_dir instead of custom fixtures

---
 tests/conftest.py                        | 10 -------
 tests/unit/remote/ssh/test_connection.py | 34 ++++++++++--------------
 tests/unit/remote/ssh/test_ssh.py        |  6 ++---
 3 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index d5851b6a7..e9855056d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -178,16 +178,6 @@ def _close_pools():
     close_pools()
 
 
-@pytest.fixture
-def chdir_tmp(tmp_path):
-    cwd = os.getcwd()
-    try:
-        os.chdir(str(tmp_path))
-        yield
-    finally:
-        os.chdir(cwd)
-
-
 @pytest.fixture
 def git_erepo():
     repo = TestGitFixture()
diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index c458a6641..f90308e54 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -9,12 +9,6 @@ import filecmp
 from dvc.system import System
 
 
-@pytest.fixture
-def foo():
-    with open("foo", "w") as fobj:
-        fobj.write("foo")
-
-
 here = os.path.abspath(os.path.dirname(__file__))
 
 
@@ -81,23 +75,23 @@ def test_walk(tmp_path, ssh):
 @pytest.mark.skipif(
     platform.system() != "Darwin", reason="Only works on OSX (because of APFS)"
 )
-def test_reflink(chdir_tmp, foo, ssh):
-    ssh.reflink("foo", "bar")
-    assert filecmp.cmp("foo", "bar")
-    assert not System.is_symlink("bar")
-    assert not System.is_hardlink("bar")
+def test_reflink(repo_dir, ssh):
+    ssh.reflink("foo", "link")
+    assert filecmp.cmp("foo", "link")
+    assert not System.is_symlink("link")
+    assert not System.is_hardlink("link")
 
 
-def test_symlink(chdir_tmp, foo, ssh):
-    ssh.symlink("foo", "bar")
-    assert System.is_symlink("bar")
+def test_symlink(repo_dir, ssh):
+    ssh.symlink("foo", "link")
+    assert System.is_symlink("link")
 
 
-def test_hardlink(chdir_tmp, foo, ssh):
-    ssh.hardlink("foo", "bar")
-    assert System.is_hardlink("bar")
+def test_hardlink(repo_dir, ssh):
+    ssh.hardlink("foo", "link")
+    assert System.is_hardlink("link")
 
 
-def test_copy(chdir_tmp, foo, ssh):
-    ssh.copy("foo", "bar")
-    assert filecmp.cmp("foo", "bar")
+def test_copy(repo_dir, ssh):
+    ssh.copy("foo", "link")
+    assert filecmp.cmp("foo", "link")
diff --git a/tests/unit/remote/ssh/test_ssh.py b/tests/unit/remote/ssh/test_ssh.py
index 1668680c9..43a2c0efb 100644
--- a/tests/unit/remote/ssh/test_ssh.py
+++ b/tests/unit/remote/ssh/test_ssh.py
@@ -187,7 +187,7 @@ def test_ssh_gss_auth(mock_file, mock_exists, config, expected_gss_auth):
     assert remote.gss_auth == expected_gss_auth
 
 
-def test_hardlink_optimization(chdir_tmp, ssh_server):
+def test_hardlink_optimization(repo_dir, ssh_server):
     config = {
         "url": "ssh://localhost",
         "hostname": ssh_server.test_creds["host"],
@@ -204,5 +204,5 @@ def test_hardlink_optimization(chdir_tmp, ssh_server):
         path = posixpath.abspath(path)
         return URLInfo("ssh://{}/{}".format(netloc, path))
 
-    remote.hardlink(urlpath("empty"), urlpath("bar"))
-    assert not System.is_hardlink("bar")
+    remote.hardlink(urlpath("empty"), urlpath("link"))
+    assert not System.is_hardlink("link")

From 05cbe0c922e3ee057d6910fa52e5b730a09ea8b4 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 14:16:43 -0500
Subject: [PATCH 39/44] tests: support windows

---
 tests/func/test_data_cloud.py     | 22 ---------------------
 tests/unit/remote/ssh/test_ssh.py | 32 ++++++++++++++++++-------------
 2 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py
index 6a301afc7..780755c73 100644
--- a/tests/func/test_data_cloud.py
+++ b/tests/func/test_data_cloud.py
@@ -9,7 +9,6 @@ import platform
 import copy
 import logging
 import pytest
-import posixpath
 
 from mock import patch
 
@@ -28,10 +27,8 @@ from dvc.data_cloud import (
     RemoteHTTP,
 )
 from dvc.remote.base import STATUS_OK, STATUS_NEW, STATUS_DELETED
-from dvc.path_info import URLInfo
 from dvc.utils import file_md5
 from dvc.utils.stage import load_stage_file, dump_stage_file
-from dvc.system import System
 
 from tests.basic_env import TestDvc
 from tests.utils import spy
@@ -452,25 +449,6 @@ class TestRemoteSSH(TestDataCloudBase):
     def _get_cloud_class(self):
         return RemoteSSH
 
-    def test_symlinks(self):
-        self._setup_cloud()
-
-        remote = self.cloud.get_remote()
-
-        os.remove("bar")
-
-        def urlinfo(path):
-            cwd = os.getcwd()
-            path = posixpath.abspath(os.path.join(cwd, path))
-            netloc = remote.path_info.netloc
-            return URLInfo("ssh://{}{}".format(netloc, path))
-
-        from_info = urlinfo("foo")
-        to_info = urlinfo("bar")
-
-        remote.symlink(from_info, to_info)
-        assert System.is_symlink("bar")
-
 
 @pytest.mark.usefixtures("ssh_server")
 class TestRemoteSSHMocked(TestDataCloudBase):
diff --git a/tests/unit/remote/ssh/test_ssh.py b/tests/unit/remote/ssh/test_ssh.py
index 43a2c0efb..6e6e101aa 100644
--- a/tests/unit/remote/ssh/test_ssh.py
+++ b/tests/unit/remote/ssh/test_ssh.py
@@ -1,15 +1,15 @@
 import getpass
 import os
 import sys
-import posixpath
 
 from unittest import TestCase
 
 from mock import patch, mock_open
 import pytest
 
+from tests.func.test_data_cloud import get_ssh_url_mocked
+
 from dvc.remote.ssh import RemoteSSH
-from dvc.path_info import URLInfo
 from dvc.system import System
 
 
@@ -188,21 +188,27 @@ def test_ssh_gss_auth(mock_file, mock_exists, config, expected_gss_auth):
 
 
 def test_hardlink_optimization(repo_dir, ssh_server):
+    port = ssh_server.test_creds["port"]
+    user = ssh_server.test_creds["username"]
+
     config = {
-        "url": "ssh://localhost",
-        "hostname": ssh_server.test_creds["host"],
-        "port": ssh_server.test_creds["port"],
-        "user": ssh_server.test_creds["username"],
+        "url": get_ssh_url_mocked(user, port),
+        "port": port,
+        "user": user,
         "keyfile": ssh_server.test_creds["key_filename"],
     }
     remote = RemoteSSH(None, config)
 
-    open("empty", "w").close()
+    from_info = remote.path_info / "empty"
+    to_info = remote.path_info / "link"
 
-    def urlpath(path):
-        netloc = remote.path_info.netloc
-        path = posixpath.abspath(path)
-        return URLInfo("ssh://{}/{}".format(netloc, path))
+    with remote.open(from_info, "wb"):
+        pass
 
-    remote.hardlink(urlpath("empty"), urlpath("link"))
-    assert not System.is_hardlink("link")
+    if os.name == "nt":
+        link_path = "c:" + to_info.path.replace("/", "\\")
+    else:
+        link_path = to_info.path
+
+    remote.hardlink(from_info, to_info)
+    assert not System.is_hardlink(link_path)

From b4507444d582bb95191798e538929fcdb147601f Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 6 Sep 2019 19:26:56 -0500
Subject: [PATCH 40/44] version: adjust get_fs_type to discover all partitions

---
 dvc/command/version.py                   |  8 ++++++--
 setup.py                                 |  1 +
 tests/unit/remote/ssh/test_connection.py | 10 +++++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/dvc/command/version.py b/dvc/command/version.py
index ad041b4dd..3e2b780f0 100644
--- a/dvc/command/version.py
+++ b/dvc/command/version.py
@@ -5,6 +5,7 @@ import platform
 import argparse
 import logging
 import uuid
+import itertools
 
 try:
     import psutil
@@ -69,9 +70,12 @@ class CmdVersion(CmdBaseNoRepo):
     def get_fs_type(path):
         partition = {
             pathlib.Path(part.mountpoint): (part.fstype, part.device)
-            for part in psutil.disk_partitions()
+            for part in psutil.disk_partitions(all=True)
         }
-        for parent in pathlib.Path(path).parents:
+
+        path = pathlib.Path(path)
+
+        for parent in itertools.chain([path], path.parents):
             if parent in partition:
                 return partition[parent]
         return ("unkown", "none")
diff --git a/setup.py b/setup.py
index 899a07adb..19408761d 100644
--- a/setup.py
+++ b/setup.py
@@ -113,6 +113,7 @@ tests_requirements = [
     "Pygments",  # required by collective.checkdocs,
     "collective.checkdocs",
     "flake8",
+    "psutil",
     "flake8-docstrings",
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index f90308e54..7c02c8012 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -5,8 +5,10 @@ import posixpath
 import platform
 import pytest
 import filecmp
+import tempfile
 
 from dvc.system import System
+from dvc.command.version import CmdVersion
 
 
 here = os.path.abspath(os.path.dirname(__file__))
@@ -73,7 +75,9 @@ def test_walk(tmp_path, ssh):
 
 
 @pytest.mark.skipif(
-    platform.system() != "Darwin", reason="Only works on OSX (because of APFS)"
+    CmdVersion.get_fs_type(tempfile.gettempdir())[0]
+    not in ["xfs", "apfs", "btrfs"],
+    reason="Reflinks only work in specified file systems",
 )
 def test_reflink(repo_dir, ssh):
     ssh.reflink("foo", "link")
@@ -82,6 +86,10 @@ def test_reflink(repo_dir, ssh):
     assert not System.is_hardlink("link")
 
 
+@pytest.mark.skipif(
+    platform.system() != "Windows",
+    reason="sftp symlink is not supported on Windows",
+)
 def test_symlink(repo_dir, ssh):
     ssh.symlink("foo", "link")
     assert System.is_symlink("link")

From 5103dfd4aa1dbcfc8ee3cc941a06f06419d58b12 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Sat, 7 Sep 2019 21:38:50 -0500
Subject: [PATCH 41/44] tests: fix skipif on ssh/test_connection.py

Co-Authored-By: Ruslan Kuprieiev <kupruser@gmail.com>
---
 tests/unit/remote/ssh/test_connection.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index 7c02c8012..dd89864fb 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -87,7 +87,7 @@ def test_reflink(repo_dir, ssh):
 
 
 @pytest.mark.skipif(
-    platform.system() != "Windows",
+    platform.system() == "Windows",
     reason="sftp symlink is not supported on Windows",
 )
 def test_symlink(repo_dir, ssh):

From e0fd808ab0c1b9f02b2b8574f5f54a30dd5788ed Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Mon, 9 Sep 2019 11:15:49 -0500
Subject: [PATCH 42/44] setup: iterative/mock-ssh-server -> pypi
 mock-ssh-server

---
 setup.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/setup.py b/setup.py
index 19408761d..d718bc8c9 100644
--- a/setup.py
+++ b/setup.py
@@ -117,8 +117,7 @@ tests_requirements = [
     "flake8-docstrings",
     "pydocstyle<4.0",
     "jaraco.windows==3.9.2",
-    # Using fork of mock-ssh-server that supports `lstat`
-    "mock-ssh-server @ git+https://github.com/iterative/mock-ssh-server@master#egg=mock-ssh-server",  # noqa: E501
+    "mock-ssh-server>=0.6.0",
     "moto>=1.3.14.dev55",
     "rangehttpserver==1.2.0",
 ]

From 2a847ef25c44635fe3c6ea7485028296afdb7a7b Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Mon, 9 Sep 2019 11:20:29 -0500
Subject: [PATCH 43/44] tests: ignore ssh's hardlink and copy tests on windows

---
 tests/unit/remote/ssh/test_connection.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py
index dd89864fb..f0c12ec21 100644
--- a/tests/unit/remote/ssh/test_connection.py
+++ b/tests/unit/remote/ssh/test_connection.py
@@ -95,11 +95,19 @@ def test_symlink(repo_dir, ssh):
     assert System.is_symlink("link")
 
 
+@pytest.mark.skipif(
+    platform.system() == "Windows",
+    reason="hardlink is temporarily not supported on Windows",
+)
 def test_hardlink(repo_dir, ssh):
     ssh.hardlink("foo", "link")
     assert System.is_hardlink("link")
 
 
+@pytest.mark.skipif(
+    platform.system() == "Windows",
+    reason="copy is temporarily not supported on Windows",
+)
 def test_copy(repo_dir, ssh):
     ssh.copy("foo", "link")
     assert filecmp.cmp("foo", "link")

From 3c5bcbd4636141cc55ba3313f668328d317956e1 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Mon, 9 Sep 2019 12:14:10 -0500
Subject: [PATCH 44/44] local: mark static methods

---
 dvc/remote/local/__init__.py | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py
index 299a6ad4b..1c1256efd 100644
--- a/dvc/remote/local/__init__.py
+++ b/dvc/remote/local/__init__.py
@@ -107,7 +107,8 @@ class RemoteLOCAL(RemoteBASE):
 
         return self.checksum_to_path_info(md5).url
 
-    def exists(self, path_info):
+    @staticmethod
+    def exists(path_info):
         assert path_info.scheme == "local"
         return os.path.lexists(fspath_py35(path_info))
 
@@ -135,13 +136,16 @@ class RemoteLOCAL(RemoteBASE):
 
         return False
 
-    def isfile(self, path_info):
+    @staticmethod
+    def isfile(path_info):
         return os.path.isfile(fspath_py35(path_info))
 
-    def isdir(self, path_info):
+    @staticmethod
+    def isdir(path_info):
         return os.path.isdir(fspath_py35(path_info))
 
-    def getsize(self, path_info):
+    @staticmethod
+    def getsize(path_info):
         return os.path.getsize(fspath_py35(path_info))
 
     def walk(self, path_info):
@@ -170,10 +174,12 @@ class RemoteLOCAL(RemoteBASE):
 
         move(from_info, to_info, mode=mode)
 
-    def copy(self, from_info, to_info):
+    @staticmethod
+    def copy(from_info, to_info):
         System.copy(from_info, to_info)
 
-    def symlink(self, from_info, to_info):
+    @staticmethod
+    def symlink(from_info, to_info):
         System.symlink(from_info, to_info)
 
     def hardlink(self, from_info, to_info):
@@ -202,7 +208,8 @@ class RemoteLOCAL(RemoteBASE):
 
         System.hardlink(from_info, to_info)
 
-    def reflink(self, from_info, to_info):
+    @staticmethod
+    def reflink(from_info, to_info):
         System.reflink(from_info, to_info)
 
     def cache_exists(self, checksums, jobs=None, name=None):
@@ -238,7 +245,8 @@ class RemoteLOCAL(RemoteBASE):
             name=name,
         )
 
-    def open(self, path_info, mode="r", encoding=None):
+    @staticmethod
+    def open(path_info, mode="r", encoding=None):
         return open(fspath_py35(path_info), mode=mode, encoding=encoding)
 
     def _group(self, checksum_infos, show_checksums=False):