Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure move accepts str and Path-like objects #2769

Merged
merged 2 commits into from
Nov 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dvc/remote/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
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__)
Expand Down
2 changes: 1 addition & 1 deletion dvc/remote/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
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
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__)

Expand Down
26 changes: 0 additions & 26 deletions dvc/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions dvc/utils/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import errno
import logging
import os
import shutil

import nanotime
from shortuuid import uuid

from dvc.exceptions import DvcException
from dvc.system import System
Expand Down Expand Up @@ -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)
20 changes: 20 additions & 0 deletions tests/unit/utils/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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

Expand Down Expand Up @@ -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