Merge pull request #2769 from algomaster99/test-arg-types-move

Ensure `move` accepts str and Path-like objects

dvc/remote/base.py

@@ -22,13 +22,13 @@ from dvc.progress import Tqdm
 from dvc.remote.slow_link_detection import slow_link_guard
 from dvc.state import StateNoop
 from dvc.utils import makedirs
-from dvc.utils import move
 from dvc.utils import relpath
 from dvc.utils import tmp_fname
 from dvc.utils.compat import basestring
 from dvc.utils.compat import FileNotFoundError
 from dvc.utils.compat import str
 from dvc.utils.compat import urlparse
+from dvc.utils.fs import move
 from dvc.utils.http import open_url
 
 logger = logging.getLogger(__name__)

dvc/remote/local.py

@@ -25,7 +25,6 @@ from dvc.system import System
 from dvc.utils import copyfile
 from dvc.utils import file_md5
 from dvc.utils import makedirs
-from dvc.utils import move
 from dvc.utils import relpath
 from dvc.utils import remove
 from dvc.utils import tmp_fname
@@ -33,6 +32,7 @@ from dvc.utils import walk_files
 from dvc.utils.compat import fspath_py35
 from dvc.utils.compat import open
 from dvc.utils.compat import str
+from dvc.utils.fs import move
 
 logger = logging.getLogger(__name__)
 

dvc/utils/__init__.py

@@ -159,32 +159,6 @@ def makedirs(path, exist_ok=False, mode=None):
         os.umask(umask)
 
 
-def move(src, dst, mode=None):
-    """Atomically move src to dst and chmod it with mode.
-
-    Moving is performed in two stages to make the whole operation atomic in
-    case src and dst are on different filesystems and actual physical copying
-    of data is happening.
-    """
-
-    src = fspath_py35(src)
-    dst = fspath_py35(dst)
-
-    dst = os.path.abspath(dst)
-    tmp = "{}.{}".format(dst, str(uuid()))
-
-    if os.path.islink(src):
-        shutil.copy(os.readlink(src), tmp)
-        os.unlink(src)
-    else:
-        shutil.move(src, tmp)
-
-    if mode is not None:
-        os.chmod(tmp, mode)
-
-    shutil.move(tmp, dst)
-
-
 def _chmod(func, p, excinfo):
     perm = os.lstat(p).st_mode
     perm |= stat.S_IWRITE

dvc/utils/fs.py

@@ -3,8 +3,10 @@ from __future__ import unicode_literals
 import errno
 import logging
 import os
+import shutil
 
 import nanotime
+from shortuuid import uuid
 
 from dvc.exceptions import DvcException
 from dvc.system import System
@@ -75,3 +77,29 @@ def contains_symlink_up_to(path, base_path):
     if os.path.dirname(path) == path:
         return False
     return contains_symlink_up_to(os.path.dirname(path), base_path)
+
+
+def move(src, dst, mode=None):
+    """Atomically move src to dst and chmod it with mode.
+
+    Moving is performed in two stages to make the whole operation atomic in
+    case src and dst are on different filesystems and actual physical copying
+    of data is happening.
+    """
+
+    src = fspath_py35(src)
+    dst = fspath_py35(dst)
+
+    dst = os.path.abspath(dst)
+    tmp = "{}.{}".format(dst, str(uuid()))
+
+    if os.path.islink(src):
+        shutil.copy(os.readlink(src), tmp)
+        os.unlink(src)
+    else:
+        shutil.move(src, tmp)
+
+    if mode is not None:
+        os.chmod(tmp, mode)
+
+    shutil.move(tmp, dst)

tests/unit/utils/test_fs.py

@@ -15,6 +15,7 @@ from dvc.utils.fs import BasePathNotInCheckedPathException
 from dvc.utils.fs import contains_symlink_up_to
 from dvc.utils.fs import get_inode
 from dvc.utils.fs import get_mtime_and_size
+from dvc.utils.fs import move
 from tests.basic_env import TestDir
 from tests.utils import spy
 
@@ -132,3 +133,22 @@ def test_path_object_and_str_are_valid_types_get_mtime_and_size(
     object_time, object_size = get_mtime_and_size(PathInfo(path), dvcignore)
     assert time == object_time
     assert size == object_size
+
+
+def test_move(repo_dir):
+    src = repo_dir.FOO
+    src_info = PathInfo(repo_dir.BAR)
+    dest = os.path.join("some", "directory")
+    dest_info = PathInfo(os.path.join("some", "path-like", "directory"))
+
+    os.makedirs(dest)
+    assert len(os.listdir(dest)) == 0
+    move(src, dest)
+    assert not os.path.isfile(src)
+    assert len(os.listdir(dest)) == 1
+
+    os.makedirs(dest_info.fspath)
+    assert len(os.listdir(dest_info.fspath)) == 0
+    move(src_info, dest_info)
+    assert not os.path.isfile(src_info.fspath)
+    assert len(os.listdir(dest_info.fspath)) == 1