Repository: iterative/dvc
Pull request #944 (2018-07-24 12:37:26)

Bug fixes


Patch:
diff --git a/dvc/dependency/local.py b/dvc/dependency/local.py
index 278e6d9f5..df1294fad 100644
--- a/dvc/dependency/local.py
+++ b/dvc/dependency/local.py
@@ -1,7 +1,5 @@
 import os
 import schema
-import posixpath
-import ntpath
 
 try:
     from urlparse import urlparse
@@ -31,7 +29,7 @@ class DependencyLOCAL(DependencyBase):
             path = os.path.join(remote.prefix, urlparse(path).path.lstrip('/'))
 
         if not os.path.isabs(path):
-            path = self.ospath(path)
+            path = self.remote.ospath(path)
             path = os.path.join(stage.cwd, path)
         self.path = os.path.abspath(os.path.normpath(path))
 
@@ -67,26 +65,9 @@ class DependencyLOCAL(DependencyBase):
 
         self.info = self.remote.save_info(self.path_info)
 
-    def ospath(self, path):
-        if os.name == 'nt':
-            return self.ntpath(path)
-        return self.unixpath(path)
-
-    @staticmethod
-    def unixpath(path):
-        assert not ntpath.isabs(path)
-        assert not posixpath.isabs(path)
-        return path.replace('\\', '/')
-
-    @staticmethod
-    def ntpath(path):
-        assert not ntpath.isabs(path)
-        assert not posixpath.isabs(path)
-        return path.replace('/', '\\')
-
     def dumpd(self):
         if self.path.startswith(self.stage.project.root_dir):
-            path = self.unixpath(os.path.relpath(self.path, self.stage.cwd))
+            path = self.remote.unixpath(os.path.relpath(self.path, self.stage.cwd))
         else:
             path = self.path
 
diff --git a/dvc/project.py b/dvc/project.py
index dbddfee7f..641ae6309 100644
--- a/dvc/project.py
+++ b/dvc/project.py
@@ -131,7 +131,8 @@ class Project(object):
 
     def add(self, fname):
         stage = Stage.loads(project=self,
-                            outs=[fname])
+                            outs=[fname],
+                            add=True)
 
         stage.save()
         stage.dump()
diff --git a/dvc/remote/local.py b/dvc/remote/local.py
index 5bde700fd..75d18d8dc 100644
--- a/dvc/remote/local.py
+++ b/dvc/remote/local.py
@@ -1,14 +1,16 @@
 import os
 import uuid
 import json
+import ntpath
 import shutil
 import filecmp
+import posixpath
+from operator import itemgetter
 
 from dvc.system import System
 from dvc.remote.base import RemoteBase, STATUS_MAP
-from dvc.state import State
 from dvc.logger import Logger
-from dvc.utils import remove, move, copyfile, file_md5, to_chunks
+from dvc.utils import remove, move, copyfile, file_md5, dict_md5, to_chunks
 from dvc.config import Config
 from dvc.exceptions import DvcException
 from dvc.progress import progress
@@ -18,8 +20,9 @@ from concurrent.futures import ThreadPoolExecutor, ProcessPoolExecutor
 class RemoteLOCAL(RemoteBase):
     scheme = ''
     REGEX = r'^(?P<path>(/+|.:\\+).*)$'
-    PARAM_MD5 = State.PARAM_MD5
-    PARAM_RELPATH = State.PARAM_RELPATH
+    PARAM_MD5 = 'md5'
+    PARAM_RELPATH = 'relpath'
+    MD5_DIR_SUFFIX = '.dir'
 
     CACHE_TYPES = ['reflink', 'hardlink', 'symlink', 'copy']
     CACHE_TYPE_MAP = {
@@ -104,6 +107,50 @@ class RemoteLOCAL(RemoteBase):
 
         raise DvcException('No possible cache types left to try out.')
 
+    @classmethod
+    def ospath(cls, path):
+        if os.name == 'nt':
+            return cls.ntpath(path)
+        return cls.unixpath(path)
+
+    @staticmethod
+    def unixpath(path):
+        assert not ntpath.isabs(path)
+        assert not posixpath.isabs(path)
+        return path.replace('\\', '/')
+
+    @staticmethod
+    def ntpath(path):
+        assert not ntpath.isabs(path)
+        assert not posixpath.isabs(path)
+        return path.replace('/', '\\')
+
+    def collect_dir_cache(self, dname):
+        dir_info = []
+
+        for root, dirs, files in os.walk(dname):
+            for fname in files:
+                path = os.path.join(root, fname)
+                relpath = self.unixpath(os.path.relpath(path, dname))
+
+                # FIXME: we could've used `md5 = state.update(path, dump=False)`
+                # here, but it is around twice as slow(on ssd, don't know about
+                # hdd) for a directory with small files. What we could do here
+                # is introduce some kind of a limit for file size, after which
+                # we would actually register it in our state file.
+                md5 = file_md5(path)[0]
+                dir_info.append({self.PARAM_RELPATH: relpath,
+                                 self.PARAM_MD5: md5})
+
+        # NOTE: sorting the list by path to ensure reproducibility
+        dir_info = sorted(dir_info, key=itemgetter(self.PARAM_RELPATH))
+
+        md5 = dict_md5(dir_info) + self.MD5_DIR_SUFFIX
+        if self.changed(md5):
+            self.dump_dir_cache(md5, dir_info)
+
+        return (md5, dir_info)
+
     def load_dir_cache(self, md5):
         path = self.get(md5)
 
@@ -122,6 +169,9 @@ class RemoteLOCAL(RemoteBase):
             Logger.error(msg.format(os.path.relpath(path)))
             return []
 
+        for info in d:
+            info['relpath'] = self.ospath(info['relpath'])
+
         return d
 
     def dump_dir_cache(self, md5, dir_info):
@@ -141,9 +191,9 @@ class RemoteLOCAL(RemoteBase):
             json.dump(dir_info, fd, sort_keys=True)
         move(tmp, path)
 
-    @staticmethod
-    def is_dir_cache(cache):
-        return cache.endswith(State.MD5_DIR_SUFFIX)
+    @classmethod
+    def is_dir_cache(cls, cache):
+        return cache.endswith(cls.MD5_DIR_SUFFIX)
 
     def checkout(self, path_info, checksum_info):
         path = path_info['path']
@@ -215,8 +265,8 @@ class RemoteLOCAL(RemoteBase):
         md5, dir_info = self.state.update_info(path)
 
         for entry in dir_info:
-            relpath = entry[State.PARAM_RELPATH]
-            m = entry[State.PARAM_MD5]
+            relpath = entry[self.PARAM_RELPATH]
+            m = entry[self.PARAM_MD5]
             p = os.path.join(path, relpath)
             c = self.get(m)
 
diff --git a/dvc/stage.py b/dvc/stage.py
index 8c247273d..a4a207a59 100644
--- a/dvc/stage.py
+++ b/dvc/stage.py
@@ -185,7 +185,7 @@ class Stage(object):
         return stage
 
     @classmethod
-    def _stage_fname_cwd(cls, fname, cwd, outs):
+    def _stage_fname_cwd(cls, fname, cwd, outs, add):
         if fname and cwd:
             return (fname, cwd)
 
@@ -199,7 +199,7 @@ class Stage(object):
             path = posixpath
 
         fname = fname if fname else path.basename(out.path) + cls.STAGE_FILE_SUFFIX
-        cwd = cwd if cwd else path.dirname(out.path)
+        cwd = path.dirname(out.path) if not cwd or add else cwd
 
         return (fname, cwd)
 
@@ -212,7 +212,8 @@ class Stage(object):
               metrics_no_cache=[],
               fname=None,
               cwd=os.curdir,
-              locked=False):
+              locked=False,
+              add=False):
         stage = Stage(project=project,
                       cwd=cwd,
                       cmd=cmd,
@@ -223,7 +224,7 @@ class Stage(object):
         stage.outs += output.loads_from(stage, metrics_no_cache, use_cache=False, metric=True)
         stage.deps = dependency.loads_from(stage, deps)
 
-        fname, cwd = Stage._stage_fname_cwd(fname, cwd, stage.outs)
+        fname, cwd = Stage._stage_fname_cwd(fname, cwd, stage.outs, add=add)
 
         cwd = os.path.abspath(cwd)
         path = os.path.join(cwd, fname)
diff --git a/dvc/state.py b/dvc/state.py
index 0cfcd2cd2..689740efe 100644
--- a/dvc/state.py
+++ b/dvc/state.py
@@ -2,11 +2,10 @@ import os
 import json
 import nanotime
 import threading
-from operator import itemgetter
 
 from dvc.lock import Lock
 from dvc.system import System
-from dvc.utils import file_md5, dict_md5, remove
+from dvc.utils import file_md5, remove
 from dvc.exceptions import DvcException
 from dvc.signal_handler import SignalHandler
 from dvc.logger import Logger
@@ -45,10 +44,6 @@ class State(object):
     STATE_FILE = 'state'
     STATE_LOCK_FILE = 'state.lock'
 
-    PARAM_RELPATH = 'relpath'
-    PARAM_MD5 = 'md5'
-    MD5_DIR_SUFFIX = '.dir'
-
     def __init__(self, project):
         self.project = project
         self.dvc_dir = project.dvc_dir
@@ -65,34 +60,9 @@ class State(object):
     def init(project):
         return State(project)
 
-    def _collect_dir(self, dname):
-        dir_info = []
-
-        for root, dirs, files in os.walk(dname):
-            for fname in files:
-                path = os.path.join(root, fname)
-                relpath = os.path.relpath(path, dname)
-
-                # FIXME: we could've used `md5 = self.update(path, dump=False)` here,
-                # but it is around twice as slow(on ssd, don't know about hdd) for a
-                # directory with small files. What we could do here is introduce some
-                # kind of a limit for file size, after which we would actually register
-                # it in our state file.
-                md5 = file_md5(path)[0]
-                dir_info.append({self.PARAM_RELPATH: relpath, self.PARAM_MD5: md5})
-
-        # NOTE: sorting the list by path to ensure reproducibility
-        dir_info = sorted(dir_info, key=itemgetter(self.PARAM_RELPATH))
-
-        md5 = dict_md5(dir_info) + self.MD5_DIR_SUFFIX
-        if self.project.cache.local.changed(md5):
-            self.project.cache.local.dump_dir_cache(md5, dir_info)
-
-        return (md5, dir_info)
-
     def _collect(self, path):
         if os.path.isdir(path):
-            return self._collect_dir(path)
+            return self.project.cache.local.collect_dir_cache(path)
         else:
             return (file_md5(path)[0], None)
 


Test patch:
diff --git a/tests/test_add.py b/tests/test_add.py
index 59894b40a..1b1d72fdc 100644
--- a/tests/test_add.py
+++ b/tests/test_add.py
@@ -39,7 +39,17 @@ class TestAddDirectory(TestDvc):
         dname = 'directory'
         os.mkdir(dname)
         self.create(os.path.join(dname, 'file'), 'file')
-        self.dvc.add(dname)
+        stage = self.dvc.add(dname)
+        self.assertNotEqual(stage, None)
+        self.assertEqual(len(stage.deps), 0)
+        self.assertEqual(len(stage.outs), 1)
+
+        md5 = stage.outs[0].info['md5']
+
+        dir_info = self.dvc.cache.local.load_dir_cache(md5)
+        for info in dir_info:
+            self.assertTrue('\\' not in info['relpath'])
+
 
 class TestAddDirectoryWithForwardSlash(TestDvc):
     def test(self):
@@ -78,6 +88,15 @@ class TestAddModifiedDir(TestDvc):
         self.dvc.add(self.DATA_DIR)
 
 
+class TestAddFileInDir(TestDvc):
+    def test(self):
+        stage = self.dvc.add(self.DATA_SUB)
+        self.assertNotEqual(stage, None)
+        self.assertEqual(len(stage.deps), 0)
+        self.assertEqual(len(stage.outs), 1)
+        self.assertEqual(stage.relpath, self.DATA_SUB + '.dvc')
+
+
 class TestCmdAdd(TestDvc):
     def test(self):
         ret = main(['add',