Skip to content

Commit

Permalink
Merge pull request #2795 from algomaster99/test-arg-types-remove
Browse files Browse the repository at this point in the history
Ensure `remove` accepts str and Path-like objects
  • Loading branch information
efiop authored Nov 16, 2019
2 parents 276621b + d9cd9ec commit 38c2100
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 37 deletions.
2 changes: 1 addition & 1 deletion dvc/external_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from dvc.config import NoRemoteError
from dvc.exceptions import RemoteNotSpecifiedInExternalRepoError
from dvc.utils import remove
from dvc.utils.fs import remove


REPO_CACHE = {}
Expand Down
2 changes: 1 addition & 1 deletion dvc/remote/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
from dvc.utils import file_md5
from dvc.utils import makedirs
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
from dvc.utils.fs import remove

logger = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/destroy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from dvc.utils import remove
from dvc.utils.fs import remove


def destroy(self):
Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/get.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
from dvc.path_info import PathInfo
from dvc.stage import Stage
from dvc.state import StateNoop
from dvc.utils import remove
from dvc.utils import resolve_output
from dvc.utils.fs import remove

logger = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from dvc.scm import SCM
from dvc.utils import boxify
from dvc.utils import relpath
from dvc.utils import remove
from dvc.utils.fs import remove

logger = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion dvc/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
from dvc.exceptions import DvcException
from dvc.utils import current_timestamp
from dvc.utils import relpath
from dvc.utils import remove
from dvc.utils import to_chunks
from dvc.utils.compat import fspath_py35
from dvc.utils.compat import is_py2
from dvc.utils.compat import urlencode
from dvc.utils.compat import urlunparse
from dvc.utils.fs import get_inode
from dvc.utils.fs import get_mtime_and_size
from dvc.utils.fs import remove


SQLITE_MAX_VARIABLES_NUMBER = 999
Expand Down
30 changes: 0 additions & 30 deletions dvc/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
"""Helpers for other modules."""
from __future__ import unicode_literals

import errno
import hashlib
import json
import logging
import math
import os
import re
import shutil
import stat
import sys
import time

Expand Down Expand Up @@ -159,33 +156,6 @@ def makedirs(path, exist_ok=False, mode=None):
os.umask(umask)


def _chmod(func, p, excinfo):
perm = os.lstat(p).st_mode
perm |= stat.S_IWRITE

try:
os.chmod(p, perm)
except OSError as exc:
# broken symlink or file is not owned by us
if exc.errno not in [errno.ENOENT, errno.EPERM]:
raise

func(p)


def remove(path):
logger.debug("Removing '{}'".format(relpath(path)))

try:
if os.path.isdir(path):
shutil.rmtree(path, onerror=_chmod)
else:
_chmod(os.unlink, path, None)
except OSError as exc:
if exc.errno != errno.ENOENT:
raise


def _split(list_to_split, chunk_size):
return [
list_to_split[i : i + chunk_size]
Expand Down
31 changes: 31 additions & 0 deletions dvc/utils/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
import os
import shutil
import stat

import nanotime
from shortuuid import uuid
Expand All @@ -13,6 +14,7 @@
from dvc.utils import dict_md5
from dvc.utils import fspath
from dvc.utils import fspath_py35
from dvc.utils import relpath
from dvc.utils import walk_files
from dvc.utils.compat import str

Expand Down Expand Up @@ -103,3 +105,32 @@ def move(src, dst, mode=None):
os.chmod(tmp, mode)

shutil.move(tmp, dst)


def _chmod(func, p, excinfo):
perm = os.lstat(p).st_mode
perm |= stat.S_IWRITE

try:
os.chmod(p, perm)
except OSError as exc:
# broken symlink or file is not owned by us
if exc.errno not in [errno.ENOENT, errno.EPERM]:
raise

func(p)


def remove(path):
path = fspath_py35(path)

logger.debug("Removing '{}'".format(relpath(path)))

try:
if os.path.isdir(path):
shutil.rmtree(path, onerror=_chmod)
else:
_chmod(os.unlink, path, None)
except OSError as exc:
if exc.errno != errno.ENOENT:
raise
2 changes: 1 addition & 1 deletion tests/basic_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
from git.exc import GitCommandNotFound

from dvc.repo import Repo as DvcRepo
from dvc.utils import remove
from dvc.utils.compat import open
from dvc.utils.compat import str
from dvc.utils.fs import remove


logger = logging.getLogger("dvc")
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/utils/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from dvc.utils.fs import get_inode
from dvc.utils.fs import get_mtime_and_size
from dvc.utils.fs import move
from dvc.utils.fs import remove
from tests.basic_env import TestDir
from tests.utils import spy

Expand Down Expand Up @@ -152,3 +153,14 @@ def test_move(repo_dir):
move(src_info, dest_info)
assert not os.path.isfile(src_info.fspath)
assert len(os.listdir(dest_info.fspath)) == 1


def test_remove(repo_dir):
path = repo_dir.FOO
path_info = PathInfo(repo_dir.BAR)

remove(path)
assert not os.path.isfile(path)

remove(path_info)
assert not os.path.isfile(path_info.fspath)

0 comments on commit 38c2100

Please sign in to comment.