From 449517c5111273011081dced4fef5cb4932730cf Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 17:44:25 -0700 Subject: [PATCH 01/24] Windows symlink --- dvc/__init__.py | 28 ++++++++++++++++++++++++++++ tests/test_repro.py | 1 + 2 files changed, 29 insertions(+) diff --git a/dvc/__init__.py b/dvc/__init__.py index e69de29bb..d9cfe8679 100644 --- a/dvc/__init__.py +++ b/dvc/__init__.py @@ -0,0 +1,28 @@ +import os + +__CSL = None + + +def win_symlink(source, link_name): + '''symlink(source, link_name) - DVC override for Windows + Creates a symbolic link pointing to source named link_name''' + + flags = 0 + if source is not None and os.path.isdir(source): + flags = 1 + if __CSL(link_name, source, flags) == 0: + raise ctypes.WinError() + + +def setup(): + if os.name == 'nt': + global __CSL + if __CSL is None: + import ctypes + csl = ctypes.windll.kernel32.CreateSymbolicLinkW + csl.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) + csl.restype = ctypes.c_ubyte + __CSL = csl + + os.symlink = win_symlink + pass diff --git a/tests/test_repro.py b/tests/test_repro.py index bc05a9740..072955710 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -89,3 +89,4 @@ class ReproChangedDeepDependency(ReproChangedDependency): CmdRepro(self.settings).run() self.assertEqual(open(self.file_name_res).read(), 'Goodbye\nBobby') + self.assertEqual(1, 2) From a6ceb02f97f12830b68f992e4f22756683113e7b Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 18:22:22 -0700 Subject: [PATCH 02/24] WindowsSymlinkOverride class --- dvc/__init__.py | 40 +++++++++++++++++++++------------------- tests/test_repro.py | 1 - 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/dvc/__init__.py b/dvc/__init__.py index d9cfe8679..f0de0e442 100644 --- a/dvc/__init__.py +++ b/dvc/__init__.py @@ -1,28 +1,30 @@ import os - -__CSL = None +import ctypes -def win_symlink(source, link_name): - '''symlink(source, link_name) - DVC override for Windows - Creates a symbolic link pointing to source named link_name''' +class WindowsSymlinkOverride(object): + FUNC = None - flags = 0 - if source is not None and os.path.isdir(source): - flags = 1 - if __CSL(link_name, source, flags) == 0: - raise ctypes.WinError() + def __index__(self): + func = ctypes.windll.kernel32.CreateSymbolicLinkW + func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) + func.restype = ctypes.c_ubyte + + WindowsSymlinkOverride.FUNC = func + + @staticmethod + def symlink(source, link_name): + '''symlink(source, link_name) - DVC override for Windows + Creates a symbolic link pointing to source named link_name''' + + flags = 0 + if source is not None and os.path.isdir(source): + flags = 1 + if WindowsSymlinkOverride.FUNC(link_name, source, flags) == 0: + raise ctypes.WinError() def setup(): if os.name == 'nt': - global __CSL - if __CSL is None: - import ctypes - csl = ctypes.windll.kernel32.CreateSymbolicLinkW - csl.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) - csl.restype = ctypes.c_ubyte - __CSL = csl - - os.symlink = win_symlink + os.symlink = WindowsSymlinkOverride().symlink pass diff --git a/tests/test_repro.py b/tests/test_repro.py index 072955710..bc05a9740 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -89,4 +89,3 @@ class ReproChangedDeepDependency(ReproChangedDependency): CmdRepro(self.settings).run() self.assertEqual(open(self.file_name_res).read(), 'Goodbye\nBobby') - self.assertEqual(1, 2) From 14c47b037966fe8565ec5415f06541cc19609759 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 18:25:48 -0700 Subject: [PATCH 03/24] WindowsSymlinkOverride: small init bugfix --- dvc/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/__init__.py b/dvc/__init__.py index f0de0e442..0b03c7630 100644 --- a/dvc/__init__.py +++ b/dvc/__init__.py @@ -5,7 +5,7 @@ import ctypes class WindowsSymlinkOverride(object): FUNC = None - def __index__(self): + def __init__(self): func = ctypes.windll.kernel32.CreateSymbolicLinkW func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) func.restype = ctypes.c_ubyte From e9112658d558d825681f3fe4e166bd9d8c384057 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 18:45:58 -0700 Subject: [PATCH 04/24] Move WindowsSymlinkOverride logic to Runtime --- dvc/__init__.py | 29 ++--------------------------- dvc/runtime.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/dvc/__init__.py b/dvc/__init__.py index 0b03c7630..d509c8f0d 100644 --- a/dvc/__init__.py +++ b/dvc/__init__.py @@ -1,30 +1,5 @@ -import os -import ctypes - - -class WindowsSymlinkOverride(object): - FUNC = None - - def __init__(self): - func = ctypes.windll.kernel32.CreateSymbolicLinkW - func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) - func.restype = ctypes.c_ubyte - - WindowsSymlinkOverride.FUNC = func - - @staticmethod - def symlink(source, link_name): - '''symlink(source, link_name) - DVC override for Windows - Creates a symbolic link pointing to source named link_name''' - - flags = 0 - if source is not None and os.path.isdir(source): - flags = 1 - if WindowsSymlinkOverride.FUNC(link_name, source, flags) == 0: - raise ctypes.WinError() +from dvc.runtime import Runtime def setup(): - if os.name == 'nt': - os.symlink = WindowsSymlinkOverride().symlink - pass + Runtime.symlink_setup() diff --git a/dvc/runtime.py b/dvc/runtime.py index 3c04557a1..f30fe38a5 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -1,5 +1,6 @@ import os import sys +import ctypes from dvc.config import Config, ConfigI from dvc.exceptions import DvcException @@ -10,6 +11,31 @@ from dvc.settings import Settings class Runtime(object): CONFIG = 'dvc.conf' + SYMLINC_OVERRIDE = None + + @staticmethod + def symlink_setup(): + if os.name != 'nt': + return + + func = ctypes.windll.kernel32.CreateSymbolicLinkW + func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) + func.restype = ctypes.c_ubyte + + Runtime.SYMLINC_OVERRIDE = func + + os.symlink = Runtime.symlink + + @staticmethod + def symlink(source, link_name): + '''symlink(source, link_name) - DVC override for Windows + Creates a symbolic link pointing to source named link_name''' + + flags = 0 + if source is not None and os.path.isdir(source): + flags = 1 + if Runtime.SYMLINC_OVERRIDE(link_name, source, flags) == 0: + raise ctypes.WinError() @staticmethod def conf_file_path(git_dir): @@ -18,6 +44,7 @@ class Runtime(object): @staticmethod def run(cmd_class, parse_config=True): try: + Runtime.symlink_setup() runtime_git = GitWrapper() if parse_config: From 74a810174160a314fda304840df2c7273ad6a2fa Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 19:07:25 -0700 Subject: [PATCH 05/24] os.path.islink for Windows --- dvc/runtime.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/dvc/runtime.py b/dvc/runtime.py index f30fe38a5..e54439f4b 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -4,6 +4,7 @@ import ctypes from dvc.config import Config, ConfigI from dvc.exceptions import DvcException +from dvc.executor import Executor from dvc.git_wrapper import GitWrapper from dvc.logger import Logger from dvc.settings import Settings @@ -18,13 +19,26 @@ class Runtime(object): if os.name != 'nt': return + Runtime._setup_windows_symlink() + + os.symlink = Runtime.symlink + os.path.islink = Runtime.is_link + + @staticmethod + def _setup_windows_symlink(): func = ctypes.windll.kernel32.CreateSymbolicLinkW func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) func.restype = ctypes.c_ubyte - Runtime.SYMLINC_OVERRIDE = func - os.symlink = Runtime.symlink + @staticmethod + def is_link(path): + if not os.path.exists(path): + return False + + # Definitely ont the best way to check a symlink. + output = Executor.exec_cmd_only_success("dir", path) + return '' in output @staticmethod def symlink(source, link_name): From f42e4840e8802464d2b9ca7e890ddfba32c6de5f Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 21:26:39 -0700 Subject: [PATCH 06/24] Windows symlink: rmtree() and small fixes --- dvc/runtime.py | 7 +++++-- dvc/utils.py | 14 ++++++++++++++ tests/basic_env.py | 6 +++--- tests/test_cmd_run.py | 4 ++-- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/dvc/runtime.py b/dvc/runtime.py index e54439f4b..0b3f1abcd 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -36,8 +36,11 @@ class Runtime(object): if not os.path.exists(path): return False - # Definitely ont the best way to check a symlink. - output = Executor.exec_cmd_only_success("dir", path) + if not os.path.isfile(path): + return False + + # It is definitely not the best way to check a symlink. + output = Executor.exec_cmd_only_success(["dir", path]) return '' in output @staticmethod diff --git a/dvc/utils.py b/dvc/utils.py index 4528a937d..e9f4e4276 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -1,3 +1,6 @@ +import os +import stat +import shutil def cached_property(f): def get(self): @@ -12,3 +15,14 @@ def cached_property(f): return x return property(get) + + +def rmtree(dir): + '''Cross platform rmtree()''' + if os.name == 'nt': + if not os.access(dir, os.W_OK): + os.chmod(dir, stat.S_IWUSR) + else: + raise + + shutil.rmtree(dir, ignore_errors=True) diff --git a/tests/basic_env.py b/tests/basic_env.py index 9895a38a2..3bf6b1b3f 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -1,5 +1,4 @@ import os -import shutil from unittest import TestCase from dvc.config import ConfigI @@ -7,6 +6,7 @@ from dvc.git_wrapper import GitWrapperI from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory from dvc.settings import Settings +from dvc.utils import rmtree class BasicEnvironment(TestCase): @@ -16,7 +16,7 @@ class BasicEnvironment(TestCase): self._test_git_dir = os.path.join(self._test_dir, self._proj_dir) self._old_curr_dir_abs = os.path.realpath(os.curdir) - shutil.rmtree(self._test_dir, ignore_errors=True) + rmtree(self._test_dir) if curr_dir: self._curr_dir = os.path.realpath(curr_dir) @@ -39,7 +39,7 @@ class BasicEnvironment(TestCase): if self._old_curr_dir_abs: os.chdir(self._old_curr_dir_abs) if self._test_git_dir: - shutil.rmtree(self._test_dir, ignore_errors=True) + rmtree(self._test_dir) pass diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 748b6e745..0e79d8ac7 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -1,5 +1,4 @@ import os -import shutil import tempfile from unittest import TestCase @@ -10,6 +9,7 @@ from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory from dvc.git_wrapper import GitWrapper from dvc.settings import Settings +from dvc.utils import rmtree class RunBasicTest(TestCase): @@ -46,7 +46,7 @@ class RunBasicTest(TestCase): fd.close() def tearDown(self): - shutil.rmtree(self.test_dir, ignore_errors=True) + rmtree(self.test_dir) os.chdir(self._old_curr_dir_abs) From 03c6e90c3c0098a9b48719ca1b5e28729c7c455c Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 7 Apr 2017 21:37:35 -0700 Subject: [PATCH 07/24] Windows symlinks: allow to fail when check is_link --- dvc/runtime.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dvc/runtime.py b/dvc/runtime.py index 0b3f1abcd..526c54d0d 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -40,7 +40,10 @@ class Runtime(object): return False # It is definitely not the best way to check a symlink. - output = Executor.exec_cmd_only_success(["dir", path]) + code, output, _ = Executor.exec_cmd(["dir", path]) + if code != 0: + return False + return '' in output @staticmethod From 9b52063279f92f5acc1a8d6ed8292965c742b574 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sat, 8 Apr 2017 17:14:44 -0700 Subject: [PATCH 08/24] Windows symlink: extract rmfile() to utils --- dvc/command/remove.py | 7 ++++--- dvc/command/repro.py | 4 ++-- dvc/command/run.py | 4 ++-- dvc/logger.py | 2 +- dvc/utils.py | 24 +++++++++++++++++++++--- tests/basic_env.py | 2 +- tests/test_executor.py | 7 ++++--- tests/test_repository_change.py | 3 ++- 8 files changed, 37 insertions(+), 16 deletions(-) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 426d358f8..34aae7758 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -7,6 +7,7 @@ from dvc.command.base import CmdBase from dvc.exceptions import DvcException from dvc.logger import Logger from dvc.runtime import Runtime +from dvc.utils import rmfile class DataRemoveError(DvcException): @@ -97,7 +98,7 @@ class CmdDataRemove(CmdBase): self._remove_state_file(data_item) self._remove_cloud_cache(data_item) - os.remove(data_item.data.relative) + rmfile(data_item.data.relative) Logger.debug(u'[Cmd-Remove] Remove data item {}. Success.'.format(data_item.data.relative)) pass @@ -123,7 +124,7 @@ class CmdDataRemove(CmdBase): def _remove_dvc_path(self, dvc_path, name): Logger.debug(u'[Cmd-Remove] Remove {} {}.'.format(name, dvc_path.relative)) - os.remove(dvc_path.relative) + rmfile(dvc_path.relative) self.remove_dir_if_empty(dvc_path.relative) Logger.debug(u'[Cmd-Remove] Remove {}. Success.'.format(name)) @@ -150,7 +151,7 @@ class CmdDataRemove(CmdBase): else: self.remove_file(file) - os.rmdir(target) + rmfile(target) pass diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 44a085f06..c62065285 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -1,4 +1,3 @@ -import os import fasteners from dvc.command.run import CmdRun @@ -7,6 +6,7 @@ from dvc.exceptions import DvcException from dvc.path.data_item import NotInDataDirError from dvc.runtime import Runtime from dvc.state_file import StateFile +from dvc.utils import rmfile class ReproError(DvcException): @@ -134,7 +134,7 @@ class ReproChange(object): def reproduce_data_file(self): Logger.debug('Reproducing data item "{}". Removing the file...'.format( self._data_item.data.dvc)) - os.remove(self._data_item.data.relative) + rmfile(self._data_item.data.relative) Logger.debug('Reproducing data item "{}". Re-runs cmd: {}'.format( self._data_item.data.relative, ' '.join(self._repro_argv))) diff --git a/dvc/command/run.py b/dvc/command/run.py index bc8618b0e..55fea6c7a 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -9,7 +9,7 @@ from dvc.path.data_item import NotInDataDirError from dvc.repository_change import RepositoryChange from dvc.runtime import Runtime from dvc.state_file import StateFile -from dvc.utils import cached_property +from dvc.utils import cached_property, rmfile class RunError(DvcException): @@ -126,7 +126,7 @@ class CmdRun(CmdBase): def remove_new_files(repo_change): for data_item in repo_change.new_data_items: Logger.error('Removing created file: {}'.format(data_item.data.relative)) - os.remove(data_item.data.relative) + rmfile(data_item.data.relative) pass @staticmethod diff --git a/dvc/logger.py b/dvc/logger.py index 3b6a8f207..824291868 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -1,5 +1,5 @@ class Logger(object): - DEFAULT_LEVEL = 2 + DEFAULT_LEVEL = 1 LEVEL = DEFAULT_LEVEL diff --git a/dvc/utils.py b/dvc/utils.py index e9f4e4276..5c448b2a3 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -2,6 +2,10 @@ import os import stat import shutil +from dvc.exceptions import DvcException +from dvc.logger import Logger + + def cached_property(f): def get(self): try: @@ -20,9 +24,23 @@ def cached_property(f): def rmtree(dir): '''Cross platform rmtree()''' if os.name == 'nt': - if not os.access(dir, os.W_OK): + if os.path.exists(dir) and not os.access(dir, os.W_OK): os.chmod(dir, stat.S_IWUSR) else: - raise + raise DvcException('Windows rmtree() error') - shutil.rmtree(dir, ignore_errors=True) + before = os.path.exists(dir) + + if os.path.exists(dir) and os.path.islink(dir): + os.unlink(dir) + else: + # shutil.rmtree(dir, ignore_errors=True) + shutil.rmtree(dir) + # os.remove(dir) + + after = os.path.exists(dir) + Logger.error('$$$$$$$$$$$$$$$$$$ rmtree(%s) before=%s, after=%s' % (dir, before, after)) + + +def rmfile(file): + os.remove(file) diff --git a/tests/basic_env.py b/tests/basic_env.py index 3bf6b1b3f..4f2c082c1 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -71,7 +71,7 @@ class DirHierarchyEnvironment(BasicEnvironment): self.settings = Settings([], self._git, self._config) self.dir1 = os.path.join('dir1') - self.dir11 = os.path.join('dir1/dir11') + self.dir11 = os.path.join('dir1', 'dir11') self.dir2 = os.path.join('dir2') os.mkdir('cache') diff --git a/tests/test_executor.py b/tests/test_executor.py index 2754f9376..60f0250c6 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -4,6 +4,7 @@ import tempfile from unittest import TestCase from dvc.executor import Executor, ExecutorError +from dvc.utils import rmfile, rmtree class TestExecutor(TestCase): @@ -21,7 +22,7 @@ class TestExecutor(TestCase): fd2.close() def tearDown(self): - shutil.rmtree(self.test_dir) + rmtree(self.test_dir) def test_suppress_output(self): output = Executor.exec_cmd_only_success(['ls', self.test_dir]) @@ -43,8 +44,8 @@ class TestExecutor(TestCase): expected = {self.file1, self.file2, os.path.basename(stdout_file), os.path.basename(stderr_file)} self.assertEqual(output_set, expected) - os.remove(stdout_file) - os.remove(stderr_file) + rmfile(stdout_file) + rmfile(stderr_file) def test_wrong_command(self): with self.assertRaises(ExecutorError): diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py index 46209a4b6..7350c3f0f 100644 --- a/tests/test_repository_change.py +++ b/tests/test_repository_change.py @@ -9,6 +9,7 @@ from dvc.path.factory import PathFactory from dvc.git_wrapper import GitWrapperI from dvc.repository_change import RepositoryChange from dvc.settings import Settings +from dvc.utils import rmtree class BasicTestRepositoryChange(TestCase): @@ -55,7 +56,7 @@ class TestRepositoryChange(BasicTestRepositoryChange): stdout=self._devnull, stderr=None).wait() subprocess.Popen(['git', 'commit', '-m', '"Adding one file3"'], stdout=self._devnull, stderr=None).wait() - os.remove(file_to_remove) + rmtree(file_to_remove) change = RepositoryChange(['ls', '-la'], self.settings, From 4ce1e43041f7fd8b13bb5205fea93cfd6285e947 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sat, 8 Apr 2017 17:44:03 -0700 Subject: [PATCH 09/24] Revert "Windows symlink: extract rmfile() to utils" This reverts commit 9b52063279f92f5acc1a8d6ed8292965c742b574. --- dvc/command/remove.py | 7 +++---- dvc/command/repro.py | 4 ++-- dvc/command/run.py | 4 ++-- dvc/logger.py | 2 +- dvc/utils.py | 24 +++--------------------- tests/basic_env.py | 2 +- tests/test_executor.py | 7 +++---- tests/test_repository_change.py | 3 +-- 8 files changed, 16 insertions(+), 37 deletions(-) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 34aae7758..426d358f8 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -7,7 +7,6 @@ from dvc.command.base import CmdBase from dvc.exceptions import DvcException from dvc.logger import Logger from dvc.runtime import Runtime -from dvc.utils import rmfile class DataRemoveError(DvcException): @@ -98,7 +97,7 @@ class CmdDataRemove(CmdBase): self._remove_state_file(data_item) self._remove_cloud_cache(data_item) - rmfile(data_item.data.relative) + os.remove(data_item.data.relative) Logger.debug(u'[Cmd-Remove] Remove data item {}. Success.'.format(data_item.data.relative)) pass @@ -124,7 +123,7 @@ class CmdDataRemove(CmdBase): def _remove_dvc_path(self, dvc_path, name): Logger.debug(u'[Cmd-Remove] Remove {} {}.'.format(name, dvc_path.relative)) - rmfile(dvc_path.relative) + os.remove(dvc_path.relative) self.remove_dir_if_empty(dvc_path.relative) Logger.debug(u'[Cmd-Remove] Remove {}. Success.'.format(name)) @@ -151,7 +150,7 @@ class CmdDataRemove(CmdBase): else: self.remove_file(file) - rmfile(target) + os.rmdir(target) pass diff --git a/dvc/command/repro.py b/dvc/command/repro.py index c62065285..44a085f06 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -1,3 +1,4 @@ +import os import fasteners from dvc.command.run import CmdRun @@ -6,7 +7,6 @@ from dvc.exceptions import DvcException from dvc.path.data_item import NotInDataDirError from dvc.runtime import Runtime from dvc.state_file import StateFile -from dvc.utils import rmfile class ReproError(DvcException): @@ -134,7 +134,7 @@ class ReproChange(object): def reproduce_data_file(self): Logger.debug('Reproducing data item "{}". Removing the file...'.format( self._data_item.data.dvc)) - rmfile(self._data_item.data.relative) + os.remove(self._data_item.data.relative) Logger.debug('Reproducing data item "{}". Re-runs cmd: {}'.format( self._data_item.data.relative, ' '.join(self._repro_argv))) diff --git a/dvc/command/run.py b/dvc/command/run.py index 55fea6c7a..bc8618b0e 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -9,7 +9,7 @@ from dvc.path.data_item import NotInDataDirError from dvc.repository_change import RepositoryChange from dvc.runtime import Runtime from dvc.state_file import StateFile -from dvc.utils import cached_property, rmfile +from dvc.utils import cached_property class RunError(DvcException): @@ -126,7 +126,7 @@ class CmdRun(CmdBase): def remove_new_files(repo_change): for data_item in repo_change.new_data_items: Logger.error('Removing created file: {}'.format(data_item.data.relative)) - rmfile(data_item.data.relative) + os.remove(data_item.data.relative) pass @staticmethod diff --git a/dvc/logger.py b/dvc/logger.py index 824291868..3b6a8f207 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -1,5 +1,5 @@ class Logger(object): - DEFAULT_LEVEL = 1 + DEFAULT_LEVEL = 2 LEVEL = DEFAULT_LEVEL diff --git a/dvc/utils.py b/dvc/utils.py index 5c448b2a3..e9f4e4276 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -2,10 +2,6 @@ import os import stat import shutil -from dvc.exceptions import DvcException -from dvc.logger import Logger - - def cached_property(f): def get(self): try: @@ -24,23 +20,9 @@ def cached_property(f): def rmtree(dir): '''Cross platform rmtree()''' if os.name == 'nt': - if os.path.exists(dir) and not os.access(dir, os.W_OK): + if not os.access(dir, os.W_OK): os.chmod(dir, stat.S_IWUSR) else: - raise DvcException('Windows rmtree() error') + raise - before = os.path.exists(dir) - - if os.path.exists(dir) and os.path.islink(dir): - os.unlink(dir) - else: - # shutil.rmtree(dir, ignore_errors=True) - shutil.rmtree(dir) - # os.remove(dir) - - after = os.path.exists(dir) - Logger.error('$$$$$$$$$$$$$$$$$$ rmtree(%s) before=%s, after=%s' % (dir, before, after)) - - -def rmfile(file): - os.remove(file) + shutil.rmtree(dir, ignore_errors=True) diff --git a/tests/basic_env.py b/tests/basic_env.py index 4f2c082c1..3bf6b1b3f 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -71,7 +71,7 @@ class DirHierarchyEnvironment(BasicEnvironment): self.settings = Settings([], self._git, self._config) self.dir1 = os.path.join('dir1') - self.dir11 = os.path.join('dir1', 'dir11') + self.dir11 = os.path.join('dir1/dir11') self.dir2 = os.path.join('dir2') os.mkdir('cache') diff --git a/tests/test_executor.py b/tests/test_executor.py index 60f0250c6..2754f9376 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -4,7 +4,6 @@ import tempfile from unittest import TestCase from dvc.executor import Executor, ExecutorError -from dvc.utils import rmfile, rmtree class TestExecutor(TestCase): @@ -22,7 +21,7 @@ class TestExecutor(TestCase): fd2.close() def tearDown(self): - rmtree(self.test_dir) + shutil.rmtree(self.test_dir) def test_suppress_output(self): output = Executor.exec_cmd_only_success(['ls', self.test_dir]) @@ -44,8 +43,8 @@ class TestExecutor(TestCase): expected = {self.file1, self.file2, os.path.basename(stdout_file), os.path.basename(stderr_file)} self.assertEqual(output_set, expected) - rmfile(stdout_file) - rmfile(stderr_file) + os.remove(stdout_file) + os.remove(stderr_file) def test_wrong_command(self): with self.assertRaises(ExecutorError): diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py index 7350c3f0f..46209a4b6 100644 --- a/tests/test_repository_change.py +++ b/tests/test_repository_change.py @@ -9,7 +9,6 @@ from dvc.path.factory import PathFactory from dvc.git_wrapper import GitWrapperI from dvc.repository_change import RepositoryChange from dvc.settings import Settings -from dvc.utils import rmtree class BasicTestRepositoryChange(TestCase): @@ -56,7 +55,7 @@ class TestRepositoryChange(BasicTestRepositoryChange): stdout=self._devnull, stderr=None).wait() subprocess.Popen(['git', 'commit', '-m', '"Adding one file3"'], stdout=self._devnull, stderr=None).wait() - rmtree(file_to_remove) + os.remove(file_to_remove) change = RepositoryChange(['ls', '-la'], self.settings, From b706f60959834e120c29df2005e6e54617ed789d Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sat, 8 Apr 2017 17:57:16 -0700 Subject: [PATCH 10/24] Windows symlink: exec cmd as shell option; small os.path.join() test fix --- dvc/executor.py | 5 +++-- dvc/runtime.py | 2 +- dvc/utils.py | 4 +--- tests/basic_env.py | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dvc/executor.py b/dvc/executor.py index d48257a75..f81108c41 100644 --- a/dvc/executor.py +++ b/dvc/executor.py @@ -10,7 +10,7 @@ class ExecutorError(DvcException): class Executor: @staticmethod - def exec_cmd(cmd, stdout_file=None, stderr_file=None, cwd=None): + def exec_cmd(cmd, stdout_file=None, stderr_file=None, cwd=None, shell=False): stdout, stdout_fd = Executor.output_file(stdout_file) stderr, stderr_fd = Executor.output_file(stderr_file) @@ -18,7 +18,8 @@ class Executor: p = subprocess.Popen(cmd, cwd=cwd, stdout=stdout, - stderr=stderr) + stderr=stderr, + shell=False) p.wait() out, err = map(lambda s: s.decode().strip('\n\r') if s else '', p.communicate()) diff --git a/dvc/runtime.py b/dvc/runtime.py index 526c54d0d..acf66285f 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -40,7 +40,7 @@ class Runtime(object): return False # It is definitely not the best way to check a symlink. - code, output, _ = Executor.exec_cmd(["dir", path]) + code, output, _ = Executor.exec_cmd(["dir", path], shell=True) if code != 0: return False diff --git a/dvc/utils.py b/dvc/utils.py index e9f4e4276..8f73b897c 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -20,9 +20,7 @@ def cached_property(f): def rmtree(dir): '''Cross platform rmtree()''' if os.name == 'nt': - if not os.access(dir, os.W_OK): + if os.path.exists(dir) and not os.access(dir, os.W_OK): os.chmod(dir, stat.S_IWUSR) - else: - raise shutil.rmtree(dir, ignore_errors=True) diff --git a/tests/basic_env.py b/tests/basic_env.py index 3bf6b1b3f..4f2c082c1 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -71,7 +71,7 @@ class DirHierarchyEnvironment(BasicEnvironment): self.settings = Settings([], self._git, self._config) self.dir1 = os.path.join('dir1') - self.dir11 = os.path.join('dir1/dir11') + self.dir11 = os.path.join('dir1', 'dir11') self.dir2 = os.path.join('dir2') os.mkdir('cache') From 7a12395cd1b73f18aeaeebbc5fdfeed6f77affdd Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sun, 9 Apr 2017 01:06:27 -0700 Subject: [PATCH 11/24] Existing data file cache path fix; Executor shell fix. --- dvc/executor.py | 2 +- dvc/path/data_item.py | 12 +++++++----- dvc/path/factory.py | 3 +++ tests/basic_env.py | 16 ++++++++++------ tests/test_data_file_obj.py | 26 +++++++++++++------------- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/dvc/executor.py b/dvc/executor.py index f81108c41..31393709c 100644 --- a/dvc/executor.py +++ b/dvc/executor.py @@ -19,7 +19,7 @@ class Executor: cwd=cwd, stdout=stdout, stderr=stderr, - shell=False) + shell=shell) p.wait() out, err = map(lambda s: s.decode().strip('\n\r') if s else '', p.communicate()) diff --git a/dvc/path/data_item.py b/dvc/path/data_item.py index d43482eb7..9883f4e13 100644 --- a/dvc/path/data_item.py +++ b/dvc/path/data_item.py @@ -58,13 +58,15 @@ class DataItem(object): @cached_property def cache(self): + cache_dir = os.path.join(self._git.git_dir_abs, self._config.cache_dir) + if self._cache_file: - return Path(self._cache_file, self._git) + file_name = os.path.basename(self._cache_file) else: - cache_dir = os.path.join(self._git.git_dir_abs, self._config.cache_dir) - cache_file_suffix = self.CACHE_FILE_SEP + self._git.curr_commit - cache_file = os.path.join(cache_dir, self.data_dvc_short + cache_file_suffix) - return Path(cache_file, self._git) + file_name = self.data_dvc_short + self.CACHE_FILE_SEP + self._git.curr_commit + + cache_file = os.path.join(cache_dir, file_name) + return Path(cache_file, self._git) @cached_property def data_dir_abs(self): diff --git a/dvc/path/factory.py b/dvc/path/factory.py index a9615b7d9..ed8cc83d9 100644 --- a/dvc/path/factory.py +++ b/dvc/path/factory.py @@ -21,6 +21,9 @@ class PathFactory(object): return StatedDataItem(state, data_file, self._git, self._config) def existing_data_item(self, file): + if not os.path.exists(file): + raise DataItemError(u'Data file "%s" is not exist' % file) + if not os.path.islink(file): raise DataItemError(u'Data file "%s" must be a symbolic link' % file) resolved_symlink = os.path.realpath(file) diff --git a/tests/basic_env.py b/tests/basic_env.py index 4f2c082c1..e5f8cdf7f 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -1,4 +1,5 @@ import os +import tempfile from unittest import TestCase from dvc.config import ConfigI @@ -10,13 +11,14 @@ from dvc.utils import rmtree class BasicEnvironment(TestCase): - def init_environment(self, test_dir=os.path.join(os.path.sep, 'tmp', 'ntx_unit_test'), curr_dir=None): + def init_environment(self, test_dir=tempfile.mkdtemp(), curr_dir=None): self._test_dir = os.path.realpath(test_dir) self._proj_dir = 'proj' self._test_git_dir = os.path.join(self._test_dir, self._proj_dir) self._old_curr_dir_abs = os.path.realpath(os.curdir) - rmtree(self._test_dir) + if os.path.exists(self._test_dir): + rmtree(self._test_dir) if curr_dir: self._curr_dir = os.path.realpath(curr_dir) @@ -30,8 +32,12 @@ class BasicEnvironment(TestCase): os.makedirs(self._test_git_dir) data_dir = os.path.join(self._test_git_dir, 'data') - if not os.path.isdir(data_dir): - os.makedirs(data_dir) + cache_dir = os.path.join(self._test_git_dir, 'cache') + state_dir = os.path.join(self._test_git_dir, 'state') + + os.makedirs(data_dir) + os.makedirs(cache_dir) + os.makedirs(state_dir) os.chdir(self._curr_dir) @@ -74,8 +80,6 @@ class DirHierarchyEnvironment(BasicEnvironment): self.dir11 = os.path.join('dir1', 'dir11') self.dir2 = os.path.join('dir2') - os.mkdir('cache') - os.mkdir('state') self.create_dirs(self.dir1) self.create_dirs(self.dir11) self.create_dirs(self.dir2) diff --git a/tests/test_data_file_obj.py b/tests/test_data_file_obj.py index c8e083381..8d2a58136 100644 --- a/tests/test_data_file_obj.py +++ b/tests/test_data_file_obj.py @@ -43,40 +43,40 @@ class TestPathFactory(BasicEnvironment): config = ConfigI('data', 'cache', 'state') self.path_factory = PathFactory(git, config) - self.file = os.path.join('data', 'file.txt') + self.data_file = os.path.join('data', 'file.txt') + self.cache_file = os.path.join('cache', 'fsymlinc.txt') - fd = open(self.file, 'w+') + fd = open(self.cache_file, 'w+') fd.write('some text') fd.close() - self.file_symlink = os.path.join('data', 'fsymlinc.txt') os.chdir('data') - os.symlink('file.txt', 'fsymlinc.txt') + os.symlink(os.path.join('..', self.cache_file), 'file.txt') os.chdir('..') pass def test_data_file_factory(self): - data_path = self.path_factory.data_item(self.file) - self.assertEqual(data_path.data.dvc, self.file) + data_path = self.path_factory.data_item(self.data_file) + self.assertEqual(data_path.data.dvc, self.data_file) - indirect_file_path = os.path.join('..', self._proj_dir, self.file) + indirect_file_path = os.path.join('..', self._proj_dir, self.data_file) data_path_indirect = self.path_factory.data_item(indirect_file_path) - self.assertEqual(data_path_indirect.data.dvc, self.file) + self.assertEqual(data_path_indirect.data.dvc, self.data_file) pass def test_data_symlink_factory(self): - data_path = self.path_factory.existing_data_item(self.file_symlink) - self.assertEqual(data_path.data.dvc, self.file_symlink) + data_path = self.path_factory.existing_data_item(self.data_file) + self.assertEqual(data_path.cache.dvc, self.cache_file) pass def test_data_symlink_factory_cache(self): - data_path = self.path_factory.existing_data_item(self.file_symlink) - self.assertEqual(data_path.cache.dvc, self.file) + data_path = self.path_factory.existing_data_item(self.data_file) + self.assertEqual(data_path.data.dvc, self.data_file) pass def test_data_symlink_factory_exception(self): with self.assertRaises(DataItemError): - self.path_factory.existing_data_item(self.file) + self.path_factory.existing_data_item(self.cache_file) pass def test_data_path(self): From 741ad5435ad6f527466b240a408a78530603b85d Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Sun, 9 Apr 2017 14:22:17 -0700 Subject: [PATCH 12/24] Windows symlink: rewrite unit test for windows --- dvc/command/run.py | 14 ++++----- dvc/executor.py | 5 ++-- dvc/git_wrapper.py | 5 ++-- dvc/path/data_item.py | 36 ++++++++++++++++++----- dvc/repository_change.py | 13 +++++++-- tests/test_cmd_run.py | 19 ++++++------ tests/test_repository_change.py | 15 ++++++---- tests/test_repro.py | 51 ++++++++++++++++++++++----------- 8 files changed, 106 insertions(+), 52 deletions(-) diff --git a/dvc/command/run.py b/dvc/command/run.py index bc8618b0e..9c6647ff1 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -24,10 +24,8 @@ class CmdRun(CmdBase): def define_args(self, parser): self.set_skip_git_actions(parser) - parser.add_argument('--not-repro', - help='Not reproducible', - action='store_false', - default=False) + parser.add_argument('--not-repro', help='Not reproducible', + action='store_false', default=False) parser.add_argument('--stdout', help='output std output to a file') parser.add_argument('--stderr', help='output std error to a file') parser.add_argument('--input', '-i', action='append', @@ -36,6 +34,7 @@ class CmdRun(CmdBase): help='Declare output data items for reproducible cmd') parser.add_argument('--code', '-c', action='append', help='Code dependencies which produce the output') + parser.add_argument('--shell', help='Shell command', action='store_false', default=False) pass @property @@ -68,21 +67,22 @@ class CmdRun(CmdBase): self.run_command(self.command_args, self.data_items_from_args(self.command_args), self.parsed_args.stdout, - self.parsed_args.stderr) + self.parsed_args.stderr, + self.parsed_args.shell) return self.commit_if_needed('DVC run: {}'.format(' '.join(self.args))) finally: lock.release() return 1 - def run_command(self, cmd_args, data_items_from_args, stdout=None, stderr=None): + def run_command(self, cmd_args, data_items_from_args, stdout=None, stderr=None, shell=False): Logger.debug('Run command with args: {}. Data items from args: {}. stdout={}, stderr={}'.format( ' '.join(cmd_args), ', '.join([x.data.dvc for x in data_items_from_args]), stdout, stderr)) - repo_change = RepositoryChange(cmd_args, self.settings, stdout, stderr) + repo_change = RepositoryChange(cmd_args, self.settings, stdout, stderr, shell=shell) if not self.skip_git_actions and not self._validate_file_states(repo_change): self.remove_new_files(repo_change) diff --git a/dvc/executor.py b/dvc/executor.py index 31393709c..e2becc48d 100644 --- a/dvc/executor.py +++ b/dvc/executor.py @@ -47,8 +47,9 @@ class Executor: return output, output_fd @staticmethod - def exec_cmd_only_success(cmd, stdout_file=None, stderr_file=None, cwd=None): - code, out, err = Executor.exec_cmd(cmd, stdout_file=stdout_file, stderr_file=stderr_file, cwd=cwd) + def exec_cmd_only_success(cmd, stdout_file=None, stderr_file=None, cwd=None, shell=False): + code, out, err = Executor.exec_cmd(cmd, stdout_file=stdout_file, + stderr_file=stderr_file, cwd=cwd, shell=shell) if code != 0: raise ExecutorError('Git command error ({}):\n{}'.format(' '.join(cmd), err)) return out diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 02e9eec7e..0675df918 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -157,11 +157,12 @@ class GitWrapper(GitWrapperI): commit = Executor.exec_cmd_only_success(['git', 'log', '-1', '--pretty=format:"%h"', file]) commit = commit.strip('"') - Logger.debug('[Git] Identify changes. Success.') - changed_files = Executor.exec_cmd_only_success(['git', 'diff', '--name-only', 'HEAD', commit]) changed_files = changed_files.strip('"') + Logger.debug('[Git] Identify changes. Success. Changed files: {}'.format( + changed_files.replace('\n', ', '))) + code_files, code_dirs = self.separate_dependency_files_and_dirs(code_dependencies) code_files_set = set([path_factory.path(x).dvc for x in code_files]) for changed_file in changed_files.split('\n'): diff --git a/dvc/path/data_item.py b/dvc/path/data_item.py index 9883f4e13..fbf87b5d9 100644 --- a/dvc/path/data_item.py +++ b/dvc/path/data_item.py @@ -9,14 +9,28 @@ from dvc.utils import cached_property class DataItemError(DvcException): def __init__(self, msg): - DvcException.__init__(self, 'Data file error: {}'.format(msg)) + super(DataItemError, self).__init__('Data item error: {}'.format(msg)) class NotInDataDirError(DvcException): - def __init__(self, file, data_dir): - DvcException.__init__(self, - 'Data file location error: the file "{}" has to be in the data directory "{}"'. - format(file, data_dir)) + def __init__(self, msg): + super(NotInDataDirError, self).__init__(msg) + + @staticmethod + def build(file, data_dir): + msg = 'the file "{}" is not in the data directory "{}"'.format(file, data_dir) + return NotInDataDirError(msg) + + +class DataItemInStatusDirError(NotInDataDirError): + def __init__(self, msg): + super(DataItemInStatusDirError, self).__init__(msg) + + @staticmethod + def build(file, data_dir): + msg = 'the file "{}" is in state directory, not in the data directory "{}"'.format( + file, data_dir) + return DataItemInStatusDirError(msg) class DataItem(object): @@ -29,11 +43,15 @@ class DataItem(object): self._cache_file = cache_file self._data = Path(data_file, git) + + if self._data.abs.startswith(self.state_dir_abs): + raise DataItemInStatusDirError.build(data_file, self._config.data_dir) + if not self._data.abs.startswith(self.data_dir_abs): - raise NotInDataDirError(data_file, self._config.data_dir) + raise NotInDataDirError.build(data_file, self._config.data_dir) if not self._data.dvc.startswith(self._config.data_dir): - raise NotInDataDirError(data_file, self._config.data_dir) + raise NotInDataDirError.build(data_file, self._config.data_dir) pass def __hash__(self): @@ -72,6 +90,10 @@ class DataItem(object): def data_dir_abs(self): return os.path.join(self._git.git_dir_abs, self._config.data_dir) + @cached_property + def state_dir_abs(self): + return os.path.join(self._git.git_dir_abs, self._config.state_dir) + @property def _symlink_file(self): data_file_dir = os.path.dirname(self.data.relative) diff --git a/dvc/repository_change.py b/dvc/repository_change.py index a1428969c..cc1d089c6 100644 --- a/dvc/repository_change.py +++ b/dvc/repository_change.py @@ -4,7 +4,7 @@ from dvc.exceptions import DvcException from dvc.git_wrapper import GitWrapper from dvc.executor import Executor from dvc.logger import Logger -from dvc.path.data_item import NotInDataDirError +from dvc.path.data_item import NotInDataDirError, DataItemInStatusDirError from dvc.path.stated_data_item import StatedDataItem from dvc.utils import cached_property @@ -17,17 +17,18 @@ class RepositoryChangeError(DvcException): class RepositoryChange(object): """Pre-condition: git repository has no changes""" - def __init__(self, cmd_args, settings, stdout, stderr): + def __init__(self, cmd_args, settings, stdout, stderr, shell=False): self._settings = settings Logger.debug(u'[Repository change] Exec command: {}. stdout={}, stderr={}'.format( u' '.join(cmd_args), stdout, stderr)) - Executor.exec_cmd_only_success(cmd_args, stdout, stderr) + Executor.exec_cmd_only_success(cmd_args, stdout, stderr, shell=shell) self._stated_data_items = [] self._externally_created_files = [] + self._created_status_files = [] self._init_file_states() @cached_property @@ -57,6 +58,8 @@ class RepositoryChange(object): Logger.debug('[Repository change] Add status: {} {}'.format( item.status, item.data.dvc)) + except DataItemInStatusDirError: + self._created_status_files.append(file) except NotInDataDirError: self._externally_created_files.append(file) pass @@ -90,3 +93,7 @@ class RepositoryChange(object): @property def externally_created_files(self): return self._externally_created_files + + @property + def created_status_files(self): + return self._created_status_files diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 0e79d8ac7..602190ccf 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -55,7 +55,8 @@ class TestRunOutsideData(RunBasicTest): self.settings._args = [] cmd_run = CmdRun(self.settings) with self.assertRaises(RunError): - cmd_run.run_command(['touch', 'file1', 'file2'], []) + cmd_run.run_command(['echo', 'test'], [], shell=True, + stdout='file1', stderr='file2') pass @@ -65,18 +66,20 @@ class RunTwoFilesBase(RunBasicTest): self.settings._args = [] cmd_run = CmdRun(self.settings) - self.input_param_file = 'data/extra_input' + self.input_param_file = os.path.join('data', 'extra_input') cmd_run.parsed_args.input = [self.input_param_file] - self.extra_output_file = 'data/extra_output' + self.extra_output_file = os.path.join('data', 'extra_output') cmd_run.parsed_args.output = [self.extra_output_file] - self.file_name1 = 'data/file1' - self.file_name2 = 'data/file2' - self.state_objs = cmd_run.run_command(['touch', self.file_name1, self.file_name2], []) + self.file_name1 = os.path.join('data', 'file1') + self.file_name2 = os.path.join('data', 'file2') + self.state_objs = cmd_run.run_command(['echo', 'test'], [], shell=True, + stdout=self.file_name1, + stderr=self.file_name2) - self.state_file_name1 = 'state/file1' + DataItem.STATE_FILE_SUFFIX - self.state_file_name2 = 'state/file2' + DataItem.STATE_FILE_SUFFIX + self.state_file_name1 = os.path.join('state', 'file1' + DataItem.STATE_FILE_SUFFIX) + self.state_file_name2 = os.path.join('state', 'file2' + DataItem.STATE_FILE_SUFFIX) self.state_file1 = None self.state_file2 = None diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py index 46209a4b6..8627c73cf 100644 --- a/tests/test_repository_change.py +++ b/tests/test_repository_change.py @@ -57,10 +57,11 @@ class TestRepositoryChange(BasicTestRepositoryChange): stdout=self._devnull, stderr=None).wait() os.remove(file_to_remove) - change = RepositoryChange(['ls', '-la'], + change = RepositoryChange(['echo', 'test'], self.settings, file_created_by_run, - file_to_modify) + file_to_modify, + shell=True) expected_new = [file_created_by_run, file_created_before_run] new_file_abs = [x.data.dvc for x in change.new_data_items] @@ -85,10 +86,11 @@ class TestRepositoryChangeDeepInDirectory(BasicTestRepositoryChange): deep_file = os.path.join('data', 'dir32', 'file1.txt') self.create_file(deep_file) - change = RepositoryChange(['ls', '-la'], + change = RepositoryChange(['echo', 'text'], self.settings, deep_file, - None) + None, + shell=True) self.assertEqual([x.data.dvc for x in change.new_data_items], [deep_file]) @@ -98,10 +100,11 @@ class TestRepositoryChangeExternallyCreated(BasicTestRepositoryChange): not_in_data_dir_file = 'file1.txt' self.create_file(not_in_data_dir_file) - change = RepositoryChange(['ls', '-la'], + change = RepositoryChange(['echo', 'textsd'], self.settings, not_in_data_dir_file, - None) + None, + shell=True) self.assertEqual([os.path.realpath(not_in_data_dir_file)], change.externally_created_files) diff --git a/tests/test_repro.py b/tests/test_repro.py index bc05a9740..8cfd55f22 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -1,8 +1,9 @@ +import os + from dvc.command.remove import CmdDataRemove -from dvc.command.repro import ReproChange, CmdRepro -from dvc.command.run import CmdRun, RunError +from dvc.command.repro import CmdRepro +from dvc.command.run import CmdRun from dvc.executor import Executor -from dvc.settings import Settings from tests.test_cmd_run import RunBasicTest @@ -10,33 +11,45 @@ class ReproBasicEnv(RunBasicTest): def setUp(self): super(ReproBasicEnv, self).setUp() - self.file_name1 = 'data/file1' + self.file_name1 = os.path.join('data', 'file1') self.file1_code_file = 'file1.py' - self.create_file_and_commit(self.file1_code_file, 'An awesome code...') - self.settings._args = ['printf', 'Hello\nMary', '--not-repro', + self.create_file_and_commit(self.file1_code_file, 'print("Hello")' + os.linesep + 'print("Mary")') + self.settings._args = ['python', self.file1_code_file, '--not-repro', '--stdout', self.file_name1, '--code', self.file1_code_file] cmd_file1 = CmdRun(self.settings) self.assertEqual(cmd_file1.code_dependencies, [self.file1_code_file]) cmd_file1.run() - self.file_name11 = 'data/file11' - self.settings._args = ['head', '-n', '1', self.file_name1, '--stdout', self.file_name11] + self.file_name11 = os.path.join('data', 'file11') + self.file11_code_file = 'file11.py' + self.create_file_and_commit(self.file11_code_file, + 'import sys' + os.linesep + 'print(open(sys.argv[1]).readline().strip())') + + self.settings._args = ['python', self.file11_code_file, self.file_name1, + '--stdout', self.file_name11, '--code', self.file11_code_file] CmdRun(self.settings).run() - self.file_name2 = 'data/file2' - self.settings._args = ['printf', 'Bobby', '--not-repro', '--stdout', self.file_name2] + self.file_name2 = os.path.join('data', 'file2') + self.settings._args = ['echo', 'Bobby', '--not-repro', '--stdout', self.file_name2, '--shell'] CmdRun(self.settings).run() self.file_res_code_file = 'code_res.py' - self.create_file_and_commit(self.file_res_code_file, 'Another piece of code') - self.file_name_res = 'data/file_res' - self.settings._args = ['cat', self.file_name11, self.file_name2, '--stdout', self.file_name_res, + self.create_file_and_commit(self.file_res_code_file, + 'import sys' + os.linesep + + 'text1 = open(sys.argv[1]).read()' + os.linesep + + 'text2 = open(sys.argv[2]).read()' + os.linesep + + 'print(text1 + text2)') + self.file_name_res = os.path.join('data', 'file_res') + self.settings._args = ['python', self.file_res_code_file, + self.file_name11, + self.file_name2, + '--stdout', self.file_name_res, '--code', self.file_res_code_file] cmd_res = CmdRun(self.settings) self.assertEqual(cmd_res.code_dependencies, [self.file_res_code_file]) cmd_res.run() - self.assertEqual(open(self.file_name_res).read(), 'Hello\nBobby') + self.assertEqual(open(self.file_name_res).read().strip(), 'Hello' + os.linesep + 'Bobby') def create_file_and_commit(self, file_name, content='Any', message='Just a commit'): self.create_file(file_name, content) @@ -61,7 +74,7 @@ class ReproCodeDependencyTest(ReproBasicEnv): self.settings._args = [self.file_name_res] CmdRepro(self.settings).run() - self.assertEqual(open(self.file_name_res).read(), 'Hello\nBobby') + self.assertEqual(open(self.file_name_res).read().strip(), 'Hello\nBobby') class ReproChangedDependency(ReproBasicEnv): @@ -77,7 +90,11 @@ class ReproChangedDependency(ReproBasicEnv): self.settings._args = [self.file_name1, '--keep-in-cloud'] CmdDataRemove(self.settings).run() - self.settings._args = ['printf', 'Goodbye\nJack', '--stdout', self.file_name1] + file1_code_file = 'file1_2.py' + self.create_file_and_commit(file1_code_file, 'print("Goodbye")' + os.linesep + 'print("Jack")') + self.settings._args = ['python', file1_code_file, '--not-repro', + '--stdout', self.file_name1, '--code', file1_code_file] + CmdRun(self.settings).run() @@ -88,4 +105,4 @@ class ReproChangedDeepDependency(ReproChangedDependency): self.settings._args = [self.file_name_res] CmdRepro(self.settings).run() - self.assertEqual(open(self.file_name_res).read(), 'Goodbye\nBobby') + self.assertEqual(open(self.file_name_res).read().strip(), 'Goodbye\nBobby') From 027ef0871731a4c268b497143d32656269de952a Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 02:34:17 -0700 Subject: [PATCH 13/24] Windows symlink: Path with relative path issue --- dvc/git_wrapper.py | 27 ++++++++++++++++++++++++++- dvc/path/path.py | 10 ++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 0675df918..a230fd958 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -1,4 +1,6 @@ import os +if os.name == 'nt': + from ctypes import create_unicode_buffer from dvc.logger import Logger from dvc.config import Config @@ -43,6 +45,29 @@ class GitWrapperI(object): return GitWrapper.parse_porcelain_files(out) + @staticmethod + def git_path_to_system_path(path): + if os.name == 'nt': + return path.replace('/', '\\') + return path + + LONG_PATH_BUFFER_SIZE = 1024 + + @staticmethod + def get_cwd(): + """Convert short path to a full path. It is needed for Windows.""" + path = os.getcwd() + if os.name != 'nt': + return path + + buffer = create_unicode_buffer(GitWrapperI.LONG_PATH_BUFFER_SIZE) + get_long_path_name = windll.kernel32.GetLongPathNameW + result = get_long_path_name(unicode(path), buffer, GitWrapperI.LONG_PATH_BUFFER_SIZE) + if result == 0 or result > GitWrapperI.LONG_PATH_BUFFER_SIZE: + return path + return buffer.value + + @staticmethod def parse_porcelain_files(out): result = [] @@ -50,7 +75,7 @@ class GitWrapperI(object): lines = out.split('\n') for line in lines: status = line[:2] - file = line[3:] + file = GitWrapperI.git_path_to_system_path(line[3:]) result.append((status, file)) return result diff --git a/dvc/path/path.py b/dvc/path/path.py index 21e0c049a..3eb475f24 100644 --- a/dvc/path/path.py +++ b/dvc/path/path.py @@ -1,9 +1,15 @@ import os +from dvc.git_wrapper import GitWrapperI + class Path(object): - def __init__(self, relative_raw, git): - self._abs = os.path.abspath(relative_raw) + def __init__(self, path, git): + if not os.path.isabs(path): + pwd = GitWrapperI.get_cwd() + path = os.path.normpath(os.path.join(pwd, path)) + + self._abs = path self._dvc = os.path.relpath(self.abs, git.git_dir_abs) self._relative = os.path.relpath(self._abs, git.curr_dir_abs) self._filename = os.path.basename(self._abs) From d1d22643bcbcefa80cc512b171979416395e5b87 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 09:41:55 -0700 Subject: [PATCH 14/24] Forgotten lib windll --- dvc/git_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index a230fd958..271bc8ef6 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -1,6 +1,6 @@ import os if os.name == 'nt': - from ctypes import create_unicode_buffer + from ctypes import create_unicode_buffer, windll from dvc.logger import Logger from dvc.config import Config From cf2b7c097c11d894812b24072ad50fdcadb0f5ac Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 09:55:07 -0700 Subject: [PATCH 15/24] Windows symlink: full path get_cwd() for state file --- dvc/state_file.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dvc/state_file.py b/dvc/state_file.py index 6c64160a7..a04d0bb80 100644 --- a/dvc/state_file.py +++ b/dvc/state_file.py @@ -4,6 +4,7 @@ import json import time from dvc.exceptions import DvcException +from dvc.git_wrapper import GitWrapperI class StateFileError(DvcException): @@ -126,7 +127,7 @@ class StateFile(object): return result def get_dvc_path(self): - pwd = os.path.realpath(os.curdir) + pwd = GitWrapperI.get_cwd() if not pwd.startswith(self.git.git_dir_abs): raise StateFileError('the file cannot be created outside of a git repository') From e144bd3b7be5ddc5784ceb9a74cd1ff114bf86a4 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 12:06:34 -0700 Subject: [PATCH 16/24] Windows symlink: multiplatform ls command for unittests --- dvc/runtime.py | 6 ++++++ tests/test_executor.py | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/dvc/runtime.py b/dvc/runtime.py index acf66285f..545e69ec9 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -14,6 +14,12 @@ class Runtime(object): CONFIG = 'dvc.conf' SYMLINC_OVERRIDE = None + @staticmethod + def ls_command_name(): + if os.name == 'nt': + return 'dir' + return 'ls' + @staticmethod def symlink_setup(): if os.name != 'nt': diff --git a/tests/test_executor.py b/tests/test_executor.py index 2754f9376..0584aba74 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -3,6 +3,7 @@ import shutil import tempfile from unittest import TestCase +from dvc import Runtime from dvc.executor import Executor, ExecutorError @@ -24,7 +25,9 @@ class TestExecutor(TestCase): shutil.rmtree(self.test_dir) def test_suppress_output(self): - output = Executor.exec_cmd_only_success(['ls', self.test_dir]) + output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + shell=True, + cwd=self.test_dir) output_set = set(output.split()) self.assertEqual(output_set, {self.file1, self.file2}) @@ -35,12 +38,19 @@ class TestExecutor(TestCase): def test_custom_file_outputs(self): stdout_file = os.path.join(self.test_dir, 'stdout.txt') stderr_file = os.path.join(self.test_dir, 'stderr.txt') - output = Executor.exec_cmd_only_success(['ls', self.test_dir], stdout_file, stderr_file) + output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + stdout_file, + stderr_file, + shell=True, + cwd=self.test_dir) self.assertEqual(output, '') output_set = set(open(stdout_file).read().split()) - expected = {self.file1, self.file2, os.path.basename(stdout_file), os.path.basename(stderr_file)} + expected = {self.file1, + self.file2, + os.path.basename(stdout_file), + os.path.basename(stderr_file)} self.assertEqual(output_set, expected) os.remove(stdout_file) From f628c28d3d60097e0cf8c21257d0b785285c8bb5 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 12:08:19 -0700 Subject: [PATCH 17/24] Windows symlink: forgotten ls change --- tests/test_executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_executor.py b/tests/test_executor.py index 0584aba74..ed5e0aee4 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -32,7 +32,7 @@ class TestExecutor(TestCase): self.assertEqual(output_set, {self.file1, self.file2}) def test_output_to_std_console(self): - output = Executor.exec_cmd_only_success(['ls', self.test_dir], '-', '-') + output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], '-', '-') self.assertEqual(output, '') def test_custom_file_outputs(self): From f539fa8c600d54dcd0f5f85fa8f9a3c96c2dcfb5 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 12:20:06 -0700 Subject: [PATCH 18/24] Windows symlink: wrap tempfile.mkdtemp() to long path func --- dvc/git_wrapper.py | 6 ++++-- tests/basic_env.py | 4 +++- tests/test_cmd_run.py | 4 ++-- tests/test_executor.py | 3 ++- tests/test_repository_change.py | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 271bc8ef6..0aa85a692 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -54,9 +54,8 @@ class GitWrapperI(object): LONG_PATH_BUFFER_SIZE = 1024 @staticmethod - def get_cwd(): + def get_long_path(path): """Convert short path to a full path. It is needed for Windows.""" - path = os.getcwd() if os.name != 'nt': return path @@ -67,6 +66,9 @@ class GitWrapperI(object): return path return buffer.value + @staticmethod + def get_cwd(): + return GitWrapperI.get_long_path(os.getcwd()) @staticmethod def parse_porcelain_files(out): diff --git a/tests/basic_env.py b/tests/basic_env.py index e5f8cdf7f..d542432b8 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -11,7 +11,9 @@ from dvc.utils import rmtree class BasicEnvironment(TestCase): - def init_environment(self, test_dir=tempfile.mkdtemp(), curr_dir=None): + def init_environment(self, + test_dir=GitWrapperI.get_long_path(tempfile.mkdtemp()), + curr_dir=None): self._test_dir = os.path.realpath(test_dir) self._proj_dir = 'proj' self._test_git_dir = os.path.join(self._test_dir, self._proj_dir) diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 602190ccf..0b2e2b27f 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -7,14 +7,14 @@ from dvc.config import ConfigI from dvc.executor import Executor from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory -from dvc.git_wrapper import GitWrapper +from dvc.git_wrapper import GitWrapper, GitWrapperI from dvc.settings import Settings from dvc.utils import rmtree class RunBasicTest(TestCase): def setUp(self): - self.test_dir = tempfile.mkdtemp() + self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) self._old_curr_dir_abs = os.path.realpath(os.curdir) self.tearDown() diff --git a/tests/test_executor.py b/tests/test_executor.py index ed5e0aee4..b48f5b087 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -5,11 +5,12 @@ from unittest import TestCase from dvc import Runtime from dvc.executor import Executor, ExecutorError +from dvc.git_wrapper import GitWrapperI class TestExecutor(TestCase): def setUp(self): - self.test_dir = tempfile.mkdtemp() + self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) self.file1 = 'file1.txt' fd1 = open(os.path.join(self.test_dir, self.file1), 'w+') diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py index 8627c73cf..da0b41404 100644 --- a/tests/test_repository_change.py +++ b/tests/test_repository_change.py @@ -13,7 +13,7 @@ from dvc.settings import Settings class BasicTestRepositoryChange(TestCase): def setUp(self): - self.test_dir = tempfile.mkdtemp() + self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) self._old_curr_dir_abs = os.path.realpath(os.curdir) self.tearDown() From 376f139f81a25726bcb6793a4e85f9bbed89e065 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 12:47:03 -0700 Subject: [PATCH 19/24] Windows symlink: replace echo to python code and fix --shell command line propagation --- dvc/command/run.py | 7 ++++--- dvc/repository_change.py | 5 +++-- tests/test_executor.py | 5 ++++- tests/test_repro.py | 6 +++++- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/dvc/command/run.py b/dvc/command/run.py index 9c6647ff1..d6ffb29b6 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -34,7 +34,7 @@ class CmdRun(CmdBase): help='Declare output data items for reproducible cmd') parser.add_argument('--code', '-c', action='append', help='Code dependencies which produce the output') - parser.add_argument('--shell', help='Shell command', action='store_false', default=False) + parser.add_argument('--shell', help='Shell command', action='store_true', default=False) pass @property @@ -76,11 +76,12 @@ class CmdRun(CmdBase): return 1 def run_command(self, cmd_args, data_items_from_args, stdout=None, stderr=None, shell=False): - Logger.debug('Run command with args: {}. Data items from args: {}. stdout={}, stderr={}'.format( + Logger.debug('Run command with args: {}. Data items from args: {}. stdout={}, stderr={}, shell={}'.format( ' '.join(cmd_args), ', '.join([x.data.dvc for x in data_items_from_args]), stdout, - stderr)) + stderr, + shell)) repo_change = RepositoryChange(cmd_args, self.settings, stdout, stderr, shell=shell) diff --git a/dvc/repository_change.py b/dvc/repository_change.py index cc1d089c6..fa8879619 100644 --- a/dvc/repository_change.py +++ b/dvc/repository_change.py @@ -20,10 +20,11 @@ class RepositoryChange(object): def __init__(self, cmd_args, settings, stdout, stderr, shell=False): self._settings = settings - Logger.debug(u'[Repository change] Exec command: {}. stdout={}, stderr={}'.format( + Logger.debug(u'[Repository change] Exec command: {}. stdout={}, stderr={}, shell={}'.format( u' '.join(cmd_args), stdout, - stderr)) + stderr, + shell)) Executor.exec_cmd_only_success(cmd_args, stdout, stderr, shell=shell) self._stated_data_items = [] diff --git a/tests/test_executor.py b/tests/test_executor.py index b48f5b087..ef8ae2c99 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -33,7 +33,10 @@ class TestExecutor(TestCase): self.assertEqual(output_set, {self.file1, self.file2}) def test_output_to_std_console(self): - output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], '-', '-') + output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + '-', + '-', + shell=True) self.assertEqual(output, '') def test_custom_file_outputs(self): diff --git a/tests/test_repro.py b/tests/test_repro.py index 8cfd55f22..64cf3dbb5 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -30,7 +30,11 @@ class ReproBasicEnv(RunBasicTest): CmdRun(self.settings).run() self.file_name2 = os.path.join('data', 'file2') - self.settings._args = ['echo', 'Bobby', '--not-repro', '--stdout', self.file_name2, '--shell'] + self.file2_code_file = 'file2.py' + self.create_file_and_commit(self.file2_code_file, + 'print("Bobby")') + self.settings._args = ['python', self.file2_code_file, + '--not-repro', '--stdout', self.file_name2] CmdRun(self.settings).run() self.file_res_code_file = 'code_res.py' From 92553358e651a721616e85fbc3bf33bdfa4718ea Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 14:32:26 -0700 Subject: [PATCH 20/24] Windows symlink: more small fixes --- tests/test_dvc_path.py | 2 +- tests/test_repro.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_dvc_path.py b/tests/test_dvc_path.py index 760eaa0b8..ca69ebeda 100644 --- a/tests/test_dvc_path.py +++ b/tests/test_dvc_path.py @@ -25,7 +25,7 @@ class TestDvcPathTest(TestCase): self.assertEqual(path.dvc, dvc_file_name) self.assertEqual(path.filename, os.path.basename(dvc_file_name)) self.assertEqual(path.relative, relative_file_name) - self.assertEquals(path.abs[0], os.path.sep) + self.assertTrue(os.path.isabs(path.abs[0])) self.assertTrue(path.abs.endswith(dvc_file_name)) def basic_test(self): diff --git a/tests/test_repro.py b/tests/test_repro.py index 64cf3dbb5..6bd78eaf4 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -53,7 +53,8 @@ class ReproBasicEnv(RunBasicTest): self.assertEqual(cmd_res.code_dependencies, [self.file_res_code_file]) cmd_res.run() - self.assertEqual(open(self.file_name_res).read().strip(), 'Hello' + os.linesep + 'Bobby') + lines = list(filter(None, map(str.strip, open(self.file_name_res).readlines()))) + self.assertEqual(lines, ['Hello', 'Bobby']) def create_file_and_commit(self, file_name, content='Any', message='Just a commit'): self.create_file(file_name, content) From 0d0d4853ac7956250fb600b65335b1168099c771 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 16:44:48 -0700 Subject: [PATCH 21/24] Windows symlink: os.path.realpath resolves symlink for windows --- dvc/runtime.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dvc/runtime.py b/dvc/runtime.py index 545e69ec9..5dec34043 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -1,4 +1,5 @@ import os +import re import sys import ctypes @@ -29,6 +30,7 @@ class Runtime(object): os.symlink = Runtime.symlink os.path.islink = Runtime.is_link + os.path.realpath = Runtime.realpath @staticmethod def _setup_windows_symlink(): @@ -52,6 +54,26 @@ class Runtime(object): return '' in output + @staticmethod + def realpath(path): + if os.name != 'nt': + return os.path.realpath(path) + + if not os.path.islink(path): + return os.path.realpath(path) + + # It is definitely not the best way to check a symlink. + code, output, _ = Executor.exec_cmd(["dir", path], shell=True) + if code != 0 or not '' in output: + return os.path.realpath(path) + + groups = re.compile(r'\[\S+\]$').findall(output.strip()) + if len(groups) < 1: + return os.path.realpath(path) + + resolved_link = groups[0][1:-1] + return resolved_link + @staticmethod def symlink(source, link_name): '''symlink(source, link_name) - DVC override for Windows From 8b024eba53b3046457943b2a3b4fdc70a7787068 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 16:49:32 -0700 Subject: [PATCH 22/24] Windows symlink: os.path.realpath - get rid of recursion --- dvc/runtime.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dvc/runtime.py b/dvc/runtime.py index 5dec34043..662282af7 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -14,6 +14,7 @@ from dvc.settings import Settings class Runtime(object): CONFIG = 'dvc.conf' SYMLINC_OVERRIDE = None + REALPATH_NATIVE = None @staticmethod def ls_command_name(): @@ -30,6 +31,8 @@ class Runtime(object): os.symlink = Runtime.symlink os.path.islink = Runtime.is_link + + Runtime.REALPATH_NATIVE = os.path.realpath os.path.realpath = Runtime.realpath @staticmethod @@ -57,10 +60,10 @@ class Runtime(object): @staticmethod def realpath(path): if os.name != 'nt': - return os.path.realpath(path) + return Runtime.REALPATH_NATIVE(path) if not os.path.islink(path): - return os.path.realpath(path) + return Runtime.REALPATH_NATIVE(path) # It is definitely not the best way to check a symlink. code, output, _ = Executor.exec_cmd(["dir", path], shell=True) @@ -69,7 +72,7 @@ class Runtime(object): groups = re.compile(r'\[\S+\]$').findall(output.strip()) if len(groups) < 1: - return os.path.realpath(path) + return Runtime.REALPATH_NATIVE(path) resolved_link = groups[0][1:-1] return resolved_link From 4578b6aeb447f25c3b119cfa8a7193c422a0543d Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 21:04:40 -0700 Subject: [PATCH 23/24] Windows symlink: extract all system depended code to System static class --- dvc/__init__.py | 5 -- dvc/command/base.py | 5 +- dvc/command/data_sync.py | 5 +- dvc/git_wrapper.py | 28 ++------- dvc/path/data_item.py | 6 +- dvc/path/factory.py | 7 ++- dvc/path/path.py | 4 +- dvc/runtime.py | 75 +----------------------- dvc/state_file.py | 4 +- dvc/system.py | 101 ++++++++++++++++++++++++++++++++ tests/basic_env.py | 11 ++-- tests/test_cmd_run.py | 7 ++- tests/test_data_file_obj.py | 3 +- tests/test_dvc_path.py | 5 +- tests/test_executor.py | 11 ++-- tests/test_repository_change.py | 7 ++- 16 files changed, 147 insertions(+), 137 deletions(-) create mode 100644 dvc/system.py diff --git a/dvc/__init__.py b/dvc/__init__.py index d509c8f0d..e69de29bb 100644 --- a/dvc/__init__.py +++ b/dvc/__init__.py @@ -1,5 +0,0 @@ -from dvc.runtime import Runtime - - -def setup(): - Runtime.symlink_setup() diff --git a/dvc/command/base.py b/dvc/command/base.py index b79de01b1..2db660051 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -3,8 +3,7 @@ import os from dvc.config import ConfigError from dvc.logger import Logger -from dvc.path.factory import PathFactory -from dvc.utils import cached_property +from dvc.system import System class CmdBase(object): @@ -89,7 +88,7 @@ class CmdBase(object): pass def get_cache_file_s3_name(self, cache_file): - cache_prefix_file_name = os.path.relpath(os.path.realpath(cache_file), os.path.realpath(self.git.git_dir)) + cache_prefix_file_name = os.path.relpath(System.realpath(cache_file), System.realpath(self.git.git_dir)) file_name = os.path.relpath(cache_prefix_file_name, self.config.cache_dir) dvc_file_path_trim = file_name.replace(os.sep, '/').strip('/') return self.config.aws_storage_prefix + '/' + dvc_file_path_trim diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index abdaaf923..5509f1a62 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -7,6 +7,7 @@ from dvc.command.base import CmdBase from dvc.logger import Logger from dvc.exceptions import DvcException from dvc.runtime import Runtime +from dvc.system import System class DataSyncError(DvcException): @@ -52,7 +53,7 @@ class CmdDataSync(CmdBase): pass def run(self): - if os.path.islink(self.parsed_args.target): + if System.islink(self.parsed_args.target): data_item = self.path_factory.existing_data_item(self.parsed_args.target) return self.sync_symlink(data_item) @@ -66,7 +67,7 @@ class CmdDataSync(CmdBase): fname = os.path.join(dir, f) if os.path.isdir(fname): self.sync_dir(fname) - elif os.path.islink(fname): + elif System.islink(fname): self.sync_symlink(self.path_factory.existing_data_item(fname)) else: raise DataSyncError('Unsupported file type "{}"'.format(fname)) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 0aa85a692..85b72d516 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -1,10 +1,9 @@ import os -if os.name == 'nt': - from ctypes import create_unicode_buffer, windll from dvc.logger import Logger from dvc.config import Config from dvc.executor import Executor, ExecutorError +from dvc.system import System class GitWrapperI(object): @@ -22,7 +21,7 @@ class GitWrapperI(object): @property def git_dir_abs(self): - return os.path.realpath(self.git_dir) + return System.realpath(self.git_dir) @property def curr_dir_abs(self): @@ -51,25 +50,6 @@ class GitWrapperI(object): return path.replace('/', '\\') return path - LONG_PATH_BUFFER_SIZE = 1024 - - @staticmethod - def get_long_path(path): - """Convert short path to a full path. It is needed for Windows.""" - if os.name != 'nt': - return path - - buffer = create_unicode_buffer(GitWrapperI.LONG_PATH_BUFFER_SIZE) - get_long_path_name = windll.kernel32.GetLongPathNameW - result = get_long_path_name(unicode(path), buffer, GitWrapperI.LONG_PATH_BUFFER_SIZE) - if result == 0 or result > GitWrapperI.LONG_PATH_BUFFER_SIZE: - return path - return buffer.value - - @staticmethod - def get_cwd(): - return GitWrapperI.get_long_path(os.getcwd()) - @staticmethod def parse_porcelain_files(out): result = [] @@ -161,11 +141,11 @@ class GitWrapper(GitWrapperI): @staticmethod def abs_paths_to_relative(files): - cur_dir = os.path.realpath(os.curdir) + cur_dir = System.realpath(os.curdir) result = [] for file in files: - result.append(os.path.relpath(os.path.realpath(file), cur_dir)) + result.append(os.path.relpath(System.realpath(file), cur_dir)) return result diff --git a/dvc/path/data_item.py b/dvc/path/data_item.py index fbf87b5d9..92c4d048e 100644 --- a/dvc/path/data_item.py +++ b/dvc/path/data_item.py @@ -1,9 +1,9 @@ import os - import shutil from dvc.path.path import Path from dvc.exceptions import DvcException +from dvc.system import System from dvc.utils import cached_property @@ -100,7 +100,7 @@ class DataItem(object): return os.path.relpath(self.cache.relative, data_file_dir) def create_symlink(self): - os.symlink(self._symlink_file, self.data.relative) + System.symlink(self._symlink_file, self.data.relative) def move_data_to_cache(self): cache_dir = os.path.dirname(self.cache.relative) @@ -108,4 +108,4 @@ class DataItem(object): os.makedirs(cache_dir) shutil.move(self.data.relative, self.cache.relative) - os.symlink(self._symlink_file, self.data.relative) + System.symlink(self._symlink_file, self.data.relative) diff --git a/dvc/path/factory.py b/dvc/path/factory.py index ed8cc83d9..9e70c498b 100644 --- a/dvc/path/factory.py +++ b/dvc/path/factory.py @@ -3,13 +3,14 @@ import os from dvc.path.data_item import DataItem, DataItemError, NotInDataDirError from dvc.path.path import Path from dvc.path.stated_data_item import StatedDataItem +from dvc.system import System class PathFactory(object): def __init__(self, git, config): self._git = git self._config = config - self._curr_dir_abs = os.path.realpath(os.curdir) + self._curr_dir_abs = System.realpath(os.curdir) def path(self, relative_raw): return Path(relative_raw, self._git) @@ -24,9 +25,9 @@ class PathFactory(object): if not os.path.exists(file): raise DataItemError(u'Data file "%s" is not exist' % file) - if not os.path.islink(file): + if not System.islink(file): raise DataItemError(u'Data file "%s" must be a symbolic link' % file) - resolved_symlink = os.path.realpath(file) + resolved_symlink = System.realpath(file) return DataItem(file, self._git, self._config, resolved_symlink) def to_data_items(self, files): diff --git a/dvc/path/path.py b/dvc/path/path.py index 3eb475f24..e129502f3 100644 --- a/dvc/path/path.py +++ b/dvc/path/path.py @@ -1,12 +1,12 @@ import os -from dvc.git_wrapper import GitWrapperI +from dvc.system import System class Path(object): def __init__(self, path, git): if not os.path.isabs(path): - pwd = GitWrapperI.get_cwd() + pwd = System.get_cwd() path = os.path.normpath(os.path.join(pwd, path)) self._abs = path diff --git a/dvc/runtime.py b/dvc/runtime.py index 662282af7..5dd942728 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -1,96 +1,25 @@ import os -import re import sys -import ctypes from dvc.config import Config, ConfigI from dvc.exceptions import DvcException -from dvc.executor import Executor from dvc.git_wrapper import GitWrapper from dvc.logger import Logger from dvc.settings import Settings +from dvc.system import System class Runtime(object): CONFIG = 'dvc.conf' - SYMLINC_OVERRIDE = None - REALPATH_NATIVE = None - - @staticmethod - def ls_command_name(): - if os.name == 'nt': - return 'dir' - return 'ls' @staticmethod def symlink_setup(): if os.name != 'nt': return - Runtime._setup_windows_symlink() - - os.symlink = Runtime.symlink - os.path.islink = Runtime.is_link - - Runtime.REALPATH_NATIVE = os.path.realpath - os.path.realpath = Runtime.realpath - - @staticmethod - def _setup_windows_symlink(): - func = ctypes.windll.kernel32.CreateSymbolicLinkW - func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) - func.restype = ctypes.c_ubyte - Runtime.SYMLINC_OVERRIDE = func - - @staticmethod - def is_link(path): - if not os.path.exists(path): - return False - - if not os.path.isfile(path): - return False - - # It is definitely not the best way to check a symlink. - code, output, _ = Executor.exec_cmd(["dir", path], shell=True) - if code != 0: - return False - - return '' in output - - @staticmethod - def realpath(path): - if os.name != 'nt': - return Runtime.REALPATH_NATIVE(path) - - if not os.path.islink(path): - return Runtime.REALPATH_NATIVE(path) - - # It is definitely not the best way to check a symlink. - code, output, _ = Executor.exec_cmd(["dir", path], shell=True) - if code != 0 or not '' in output: - return os.path.realpath(path) - - groups = re.compile(r'\[\S+\]$').findall(output.strip()) - if len(groups) < 1: - return Runtime.REALPATH_NATIVE(path) - - resolved_link = groups[0][1:-1] - return resolved_link - - @staticmethod - def symlink(source, link_name): - '''symlink(source, link_name) - DVC override for Windows - Creates a symbolic link pointing to source named link_name''' - - flags = 0 - if source is not None and os.path.isdir(source): - flags = 1 - if Runtime.SYMLINC_OVERRIDE(link_name, source, flags) == 0: - raise ctypes.WinError() - @staticmethod def conf_file_path(git_dir): - return os.path.realpath(os.path.join(git_dir, Runtime.CONFIG)) + return System.realpath(os.path.join(git_dir, Runtime.CONFIG)) @staticmethod def run(cmd_class, parse_config=True): diff --git a/dvc/state_file.py b/dvc/state_file.py index a04d0bb80..d6ab87233 100644 --- a/dvc/state_file.py +++ b/dvc/state_file.py @@ -4,7 +4,7 @@ import json import time from dvc.exceptions import DvcException -from dvc.git_wrapper import GitWrapperI +from dvc.system import System class StateFileError(DvcException): @@ -127,7 +127,7 @@ class StateFile(object): return result def get_dvc_path(self): - pwd = GitWrapperI.get_cwd() + pwd = System.get_cwd() if not pwd.startswith(self.git.git_dir_abs): raise StateFileError('the file cannot be created outside of a git repository') diff --git a/dvc/system.py b/dvc/system.py new file mode 100644 index 000000000..c63e7ddc7 --- /dev/null +++ b/dvc/system.py @@ -0,0 +1,101 @@ +import ctypes +import os +import re + +if os.name == 'nt': + from ctypes import create_unicode_buffer, windll + +from dvc.executor import Executor + + +class System(object): + SYMLINK_OUTPUT = '' + LONG_PATH_BUFFER_SIZE = 1024 + + @staticmethod + def is_unix(): + return os.name != 'nt' + + @staticmethod + def symlink(source, link_name): + if System.is_unix(): + return os.symlink(source, link_name) + + flags = 0 + if source is not None and os.path.isdir(source): + flags = 1 + + func = ctypes.windll.kernel32.CreateSymbolicLinkW + func.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32) + func.restype = ctypes.c_ubyte + + if func(link_name, source, flags) == 0: + raise ctypes.WinError() + + @staticmethod + def islink(path): + # It is definitely not the best way to check a symlink. + + if System.is_unix(): + return os.path.islink(path) + + if not os.path.exists(path): + return False + + if not os.path.isfile(path): + return False + + return System._get_symlink_string() is not None + + @staticmethod + def _get_symlink_string(path): + code, output, _ = Executor.exec_cmd(["dir", path], shell=True) + if code != 0: + return False + + lines = output.split('\n') + for line in lines: + if System.SYMLINK_OUTPUT in line: + return line + return None + + @staticmethod + def realpath(path): + # It is definitely not the best way to check a symlink. + + if System.is_unix(): + return os.path.realpath(path) + + output = System._get_symlink_string(path) + if output is None: + return os.path.realpath(path) + + groups = re.compile(r'\[\S+\]$').findall(output.strip()) + if len(groups) < 1: + return os.path.realpath(path) + + resolved_link = groups[0][1:-1] + return resolved_link + + @staticmethod + def ls_command_name(): + if System.is_unix(): + return 'ls' + return 'dir' + + @staticmethod + def get_long_path(path): + """Convert short path to a full path. It is needed for Windows.""" + if System.is_unix(): + return path + + buffer = create_unicode_buffer(System.LONG_PATH_BUFFER_SIZE) + get_long_path_name = windll.kernel32.GetLongPathNameW + result = get_long_path_name(unicode(path), buffer, System.LONG_PATH_BUFFER_SIZE) + if result == 0 or result > System.LONG_PATH_BUFFER_SIZE: + return path + return buffer.value + + @staticmethod + def get_cwd(): + return System.get_long_path(os.getcwd()) diff --git a/tests/basic_env.py b/tests/basic_env.py index d542432b8..ca25df40b 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -7,23 +7,24 @@ from dvc.git_wrapper import GitWrapperI from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory from dvc.settings import Settings +from dvc.system import System from dvc.utils import rmtree class BasicEnvironment(TestCase): def init_environment(self, - test_dir=GitWrapperI.get_long_path(tempfile.mkdtemp()), + test_dir=System.get_long_path(tempfile.mkdtemp()), curr_dir=None): - self._test_dir = os.path.realpath(test_dir) + self._test_dir = System.realpath(test_dir) self._proj_dir = 'proj' self._test_git_dir = os.path.join(self._test_dir, self._proj_dir) - self._old_curr_dir_abs = os.path.realpath(os.curdir) + self._old_curr_dir_abs = System.realpath(os.curdir) if os.path.exists(self._test_dir): rmtree(self._test_dir) if curr_dir: - self._curr_dir = os.path.realpath(curr_dir) + self._curr_dir = System.realpath(curr_dir) else: self._curr_dir = self._test_git_dir @@ -112,7 +113,7 @@ class DirHierarchyEnvironment(BasicEnvironment): self.create_content_file(cache_result, content) relevant_dir = self.relevant_dir(data_file) - os.symlink(os.path.join(relevant_dir, cache_result), file_result) + System.symlink(os.path.join(relevant_dir, cache_result), file_result) else: cache_result = None self.create_content_file(file_result, content) diff --git a/tests/test_cmd_run.py b/tests/test_cmd_run.py index 0b2e2b27f..97a58572f 100644 --- a/tests/test_cmd_run.py +++ b/tests/test_cmd_run.py @@ -7,15 +7,16 @@ from dvc.config import ConfigI from dvc.executor import Executor from dvc.path.data_item import DataItem from dvc.path.factory import PathFactory -from dvc.git_wrapper import GitWrapper, GitWrapperI +from dvc.git_wrapper import GitWrapper from dvc.settings import Settings +from dvc.system import System from dvc.utils import rmtree class RunBasicTest(TestCase): def setUp(self): - self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) - self._old_curr_dir_abs = os.path.realpath(os.curdir) + self.test_dir = System.get_long_path(tempfile.mkdtemp()) + self._old_curr_dir_abs = System.realpath(os.curdir) self.tearDown() os.mkdir(self.test_dir) diff --git a/tests/test_data_file_obj.py b/tests/test_data_file_obj.py index 8d2a58136..82d24c2cf 100644 --- a/tests/test_data_file_obj.py +++ b/tests/test_data_file_obj.py @@ -4,6 +4,7 @@ from dvc.config import ConfigI from dvc.git_wrapper import GitWrapperI from dvc.path.data_item import NotInDataDirError, DataItemError, DataItem from dvc.path.factory import PathFactory +from dvc.system import System from tests.basic_env import BasicEnvironment @@ -51,7 +52,7 @@ class TestPathFactory(BasicEnvironment): fd.close() os.chdir('data') - os.symlink(os.path.join('..', self.cache_file), 'file.txt') + System.symlink(os.path.join('..', self.cache_file), 'file.txt') os.chdir('..') pass diff --git a/tests/test_dvc_path.py b/tests/test_dvc_path.py index ca69ebeda..bfc49e572 100644 --- a/tests/test_dvc_path.py +++ b/tests/test_dvc_path.py @@ -4,12 +4,13 @@ from unittest import TestCase from dvc.path.path import Path from dvc.git_wrapper import GitWrapperI +from dvc.system import System class TestDvcPathTest(TestCase): def setUp(self): - self.curr_dir = os.path.realpath('.') - self.test_dir = os.path.realpath('/tmp/ntx_unit_test/dvc_path') + self.curr_dir = System.realpath('.') + self.test_dir = System.realpath('/tmp/ntx_unit_test/dvc_path') self.tearDown() os.makedirs(os.path.join(self.test_dir, 'data')) os.makedirs(os.path.join(self.test_dir, 'code', 'lib')) diff --git a/tests/test_executor.py b/tests/test_executor.py index ef8ae2c99..00d8bbc5d 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -3,14 +3,13 @@ import shutil import tempfile from unittest import TestCase -from dvc import Runtime from dvc.executor import Executor, ExecutorError -from dvc.git_wrapper import GitWrapperI +from dvc.system import System class TestExecutor(TestCase): def setUp(self): - self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) + self.test_dir = System.get_long_path(tempfile.mkdtemp()) self.file1 = 'file1.txt' fd1 = open(os.path.join(self.test_dir, self.file1), 'w+') @@ -26,14 +25,14 @@ class TestExecutor(TestCase): shutil.rmtree(self.test_dir) def test_suppress_output(self): - output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], shell=True, cwd=self.test_dir) output_set = set(output.split()) self.assertEqual(output_set, {self.file1, self.file2}) def test_output_to_std_console(self): - output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], '-', '-', shell=True) @@ -42,7 +41,7 @@ class TestExecutor(TestCase): def test_custom_file_outputs(self): stdout_file = os.path.join(self.test_dir, 'stdout.txt') stderr_file = os.path.join(self.test_dir, 'stderr.txt') - output = Executor.exec_cmd_only_success([Runtime.ls_command_name(), self.test_dir], + output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], stdout_file, stderr_file, shell=True, diff --git a/tests/test_repository_change.py b/tests/test_repository_change.py index da0b41404..2425464cb 100644 --- a/tests/test_repository_change.py +++ b/tests/test_repository_change.py @@ -9,12 +9,13 @@ from dvc.path.factory import PathFactory from dvc.git_wrapper import GitWrapperI from dvc.repository_change import RepositoryChange from dvc.settings import Settings +from dvc.system import System class BasicTestRepositoryChange(TestCase): def setUp(self): - self.test_dir = GitWrapperI.get_long_path(tempfile.mkdtemp()) - self._old_curr_dir_abs = os.path.realpath(os.curdir) + self.test_dir = System.get_long_path(tempfile.mkdtemp()) + self._old_curr_dir_abs = System.realpath(os.curdir) self.tearDown() os.mkdir(self.test_dir) @@ -106,5 +107,5 @@ class TestRepositoryChangeExternallyCreated(BasicTestRepositoryChange): None, shell=True) - self.assertEqual([os.path.realpath(not_in_data_dir_file)], + self.assertEqual([System.realpath(not_in_data_dir_file)], change.externally_created_files) From 48c86267e73002a9296c392ef6d49ce137a42054 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Apr 2017 21:49:10 -0700 Subject: [PATCH 24/24] Windows symlink: fix executor tests and small bugs --- dvc/system.py | 10 ++------- tests/test_data_file_obj.py | 3 ++- tests/test_dvc_path.py | 2 +- tests/test_executor.py | 43 +++++++++++-------------------------- 4 files changed, 18 insertions(+), 40 deletions(-) diff --git a/dvc/system.py b/dvc/system.py index c63e7ddc7..b67f840f3 100644 --- a/dvc/system.py +++ b/dvc/system.py @@ -45,13 +45,13 @@ class System(object): if not os.path.isfile(path): return False - return System._get_symlink_string() is not None + return System._get_symlink_string(path) is not None @staticmethod def _get_symlink_string(path): code, output, _ = Executor.exec_cmd(["dir", path], shell=True) if code != 0: - return False + return None lines = output.split('\n') for line in lines: @@ -77,12 +77,6 @@ class System(object): resolved_link = groups[0][1:-1] return resolved_link - @staticmethod - def ls_command_name(): - if System.is_unix(): - return 'ls' - return 'dir' - @staticmethod def get_long_path(path): """Convert short path to a full path. It is needed for Windows.""" diff --git a/tests/test_data_file_obj.py b/tests/test_data_file_obj.py index 82d24c2cf..32ef904ff 100644 --- a/tests/test_data_file_obj.py +++ b/tests/test_data_file_obj.py @@ -133,7 +133,8 @@ class TestDataPathInDataDir(BasicEnvironment): self.assertEqual(self.data_path.state.abs, target) def test_symlink(self): - self.assertEqual(self.data_path._symlink_file, '../../../ca/dir1/d2/file.txt_eeeff8f') + expected = os.path.join('..', '..', '..', 'ca', 'dir1', 'd2', 'file.txt_eeeff8f') + self.assertEqual(self.data_path._symlink_file, expected) def test_data_dir(self): data_path = self.path_factory.data_item(self.data_dir) diff --git a/tests/test_dvc_path.py b/tests/test_dvc_path.py index bfc49e572..5f2cd7149 100644 --- a/tests/test_dvc_path.py +++ b/tests/test_dvc_path.py @@ -26,7 +26,7 @@ class TestDvcPathTest(TestCase): self.assertEqual(path.dvc, dvc_file_name) self.assertEqual(path.filename, os.path.basename(dvc_file_name)) self.assertEqual(path.relative, relative_file_name) - self.assertTrue(os.path.isabs(path.abs[0])) + self.assertTrue(os.path.isabs(path.abs)) self.assertTrue(path.abs.endswith(dvc_file_name)) def basic_test(self): diff --git a/tests/test_executor.py b/tests/test_executor.py index 00d8bbc5d..2d6f7dabb 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -11,50 +11,33 @@ class TestExecutor(TestCase): def setUp(self): self.test_dir = System.get_long_path(tempfile.mkdtemp()) - self.file1 = 'file1.txt' - fd1 = open(os.path.join(self.test_dir, self.file1), 'w+') - fd1.write('this is file 1') - fd1.close() - - self.file2 = 'f.csv' - fd2 = open(os.path.join(self.test_dir, self.file2), 'w+') - fd2.write('this is file 222') - fd2.close() + self.code_file = 'myfile.py' + fd = open(self.code_file, 'w') + fd.write('import sys\nsys.stdout.write("111")\nsys.stderr.write("222")\n') + fd.close() def tearDown(self): shutil.rmtree(self.test_dir) def test_suppress_output(self): - output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], - shell=True, - cwd=self.test_dir) - output_set = set(output.split()) - self.assertEqual(output_set, {self.file1, self.file2}) + output = Executor.exec_cmd_only_success(['python', self.code_file]) + self.assertEqual(output, '111') def test_output_to_std_console(self): - output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], - '-', - '-', - shell=True) + # This output "111" in stdout and "222" are not suppressible. + # You will see the outputs when the unit-test runs. + output = Executor.exec_cmd_only_success(['python', self.code_file], '-', '-') self.assertEqual(output, '') def test_custom_file_outputs(self): stdout_file = os.path.join(self.test_dir, 'stdout.txt') stderr_file = os.path.join(self.test_dir, 'stderr.txt') - output = Executor.exec_cmd_only_success([System.ls_command_name(), self.test_dir], - stdout_file, - stderr_file, - shell=True, - cwd=self.test_dir) + + output = Executor.exec_cmd_only_success(['python', self.code_file], stdout_file, stderr_file) self.assertEqual(output, '') - - output_set = set(open(stdout_file).read().split()) - expected = {self.file1, - self.file2, - os.path.basename(stdout_file), - os.path.basename(stderr_file)} - self.assertEqual(output_set, expected) + self.assertEqual(open(stdout_file).read(), "111") + self.assertEqual(open(stderr_file).read(), "222") os.remove(stdout_file) os.remove(stderr_file)