Existing data file cache path fix; Executor shell fix.

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())
 

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):

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)

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)

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):