dvc: use hardlinks instead of symlinks

Fixes #225
Fixes #273

Signed-off-by: Ruslan Kuprieiev <[email protected]>

dvc/command/import_file.py

@@ -132,8 +132,7 @@ class CmdImportFile(CmdBase):
                 Logger.debug('Skipping creating state file for failed import {}'.format(data_item.state.relative))
                 continue
 
-            Logger.debug('Creating symlink {} --> {}'.format(data_item.symlink_file, data_item.data.relative))
-            System.symlink(data_item.symlink_file, data_item.data.relative)
+            Logger.debug('Creating state file for {}'.format(data_item.data.relative))
 
             state_file = StateFile(StateFile.COMMAND_IMPORT_FILE,
                                data_item,

dvc/command/init.py

@@ -116,16 +116,11 @@ ProjectName =
         conf_file.write(self.CONFIG_TEMPLATE.format(data_dir_path.name))
         conf_file.close()
 
-        message = 'DVC init. data dir {}, cache dir {}, state dir {}, '.format(
-                        data_dir_path.name,
-                        cache_dir_path.name,
-                        state_dir_path.name
-        )
-        if self.commit_if_needed(message) == 1:
-            return 1
+        self.git.modify_gitignore([os.path.join(config_dir_path.name, cache_dir_path.name),
+                                   os.path.join(config_dir_path.name, os.path.basename(self.git.lock_file))])
 
-        self.modify_gitignore(config_dir_path.name, cache_dir_path.name)
-        return self.commit_if_needed('DVC init. Commit .gitignore file')
+        message = 'DVC init. cache dir {}, state dir {}, '.format(cache_dir_path.name, state_dir_path.name)
+        return self.commit_if_needed(message)
 
     def create_empty_file(self):
         empty_data_path = os.path.join(self.parsed_args.data_dir, self.EMPTY_FILE_NAME)
@@ -141,14 +136,3 @@ ProjectName =
                   output_files=[],
                   lock=False).save(is_update_target_metrics=False)
         pass
-
-    def modify_gitignore(self, config_dir_name, cache_dir_name):
-        gitignore_file = os.path.join(self.git.git_dir, '.gitignore')
-        if not os.path.exists(gitignore_file):
-            open(gitignore_file, 'a').close()
-            Logger.info('File .gitignore was created')
-        with open(gitignore_file, 'a') as fd:
-            fd.write('\n{}'.format(os.path.join(config_dir_name, cache_dir_name)))
-            fd.write('\n{}'.format(os.path.join(config_dir_name, os.path.basename(self.git.lock_file))))
-
-        Logger.info('Directory {} was added to .gitignore file'.format(cache_dir_name))

dvc/command/repro.py

@@ -119,9 +119,6 @@ class ReproChange(object):
         self._recursive = recursive
         self._force = force
 
-        if not System.islink(data_item.data.relative):
-            raise ReproError('data item {} is not symlink'.format(data_item.data.relative))
-
         try:
             self._state = StateFile.load(data_item, self.git)
         except Exception as ex:
@@ -246,8 +243,8 @@ class ReproChange(object):
                 result = True
                 Logger.debug(u'Repro data item {}: dependency {} was changed'.format(
                     data_item_dvc, data_item.data.dvc))
-            elif data_item.data.dvc in self._globally_changed_files:
-                msg = u'Repro data item {}: dependency {} was not changed but the data item global checksum was changed'
+            elif data_item.state.dvc in self._globally_changed_files:
+                msg = u'Repro data item {}: dependency {} was not changed but the state file was changed'
                 Logger.debug(msg.format(data_item_dvc, data_item.data.dvc))
                 result = True
             else:

dvc/command/run.py

@@ -116,7 +116,7 @@ class CmdRun(CmdBase):
 
         result = []
         for data_item in repo_change.changed_data_items:
-            Logger.debug('Move output file "{}" to cache dir "{}" and create a symlink'.format(
+            Logger.debug('Move output file "{}" to cache dir "{}" and create a hardlink'.format(
                 data_item.data.relative, data_item.cache_dir))
             data_item.move_data_to_cache()
 
@@ -168,16 +168,13 @@ class CmdRun(CmdBase):
         not_data_items = []
 
         for arg in argv:
-            try:
-                if os.path.isfile(arg):
-                    data_item = self.settings.path_factory.data_item(arg)
-                    data_items.append(data_item)
-            except NotInGitDirError as ex:
-                msg = 'File {} from argv is outside of git directory and cannot be traced: {}'
-                Logger.warn(msg.format(arg, ex))
-            except NotInDataDirError:
+            if self.settings.path_factory.is_data_item(arg):
+                data_items.append(self.settings.path_factory.data_item(arg))
+            elif os.path.isfile(arg):
                 not_data_items.append(arg)
-                pass
+            else:
+                msg = 'File {} from argv is outside of git directory and cannot be traced'
+                Logger.warn(msg.format(arg))
 
         return data_items, not_data_items
 

dvc/command/traverse.py

@@ -72,7 +72,7 @@ class Traverse(CmdBase):
         try:
             data_item = self.settings.path_factory.existing_data_item(target)
         except DataItemError:
-            Logger.warn(u'[TraverseFileTree] Data file {} is not valid symbolic link'.format(target))
+            Logger.warn(u'[TraverseFileTree] Data file {} does not have existing state file'.format(target))
             data_item = self.settings.path_factory.data_item(target)
 
         return data_item

dvc/data_cloud.py

@@ -219,11 +219,13 @@ class DataCloudLOCAL(DataCloudBase):
         tmp_file = self.tmp_file(item.data.dvc)
         try:
             copyfile(i, tmp_file)
-            item.import_cache(tmp_file)
         except Exception as exc:
             Logger.error('Failed to copy "{}": {}'.format(i, exc))
             return None
 
+        os.rename(tmp_file, item.data.dvc)
+        item.move_data_to_cache()
+
         return item
 
     def pull(self, item):
@@ -363,7 +365,8 @@ class DataCloudHTTP(DataCloudBase):
         if not self._verify_md5(req, tmp_file):
             return None
 
-        item.import_cache(tmp_file)
+        os.rename(tmp_file, item.data.dvc)
+        item.move_data_to_cache()
 
         return item
 
@@ -476,11 +479,13 @@ class DataCloudAWS(DataCloudBase):
                                          num_retries=10)
         try:
             key.get_contents_to_filename(tmp_file, cb=create_cb(name), res_download_handler=res_h)
-            data_item.import_cache(tmp_file)
         except Exception as exc:
             Logger.error('Failed to download "{}": {}'.format(key_name, exc))
             return None
 
+        os.rename(tmp_file, fname)
+        data_item.move_data_to_cache()
+
         progress.finish_target(name)
         Logger.debug('Downloading completed')
 
@@ -726,6 +731,9 @@ class DataCloudGCP(DataCloudBase):
             Logger.error('Failed to download "{}": {}'.format(key, exc))
             return None
 
+        os.rename(tmp_file, fname)
+        data_item.move_data_to_cache()
+
         progress.finish_target(name)
 
         Logger.debug('Downloading completed')
@@ -877,7 +885,7 @@ class DataCloud(object):
         """
         Collect target as a file or directory.
         """
-        if System.islink(target):
+        if self._settings.path_factory.is_data_item(target):
             item = self._settings.path_factory.data_item(target)
             return [item]
         elif os.path.isdir(target):

dvc/git_wrapper.py

@@ -96,6 +96,17 @@ class GitWrapperI(object):
     def commit_all_changes_and_log_status(self, message):
         pass
 
+    @staticmethod
+    def modify_gitignore(flist):
+        for fname in flist:
+            # We're modifying .gitignore in the same dir as the file itself, so
+            # that if dir consists only of data files it will be still tracked by
+            # git and we will not have any problems with --porcelain output for
+            # specific changed files.
+            gitignore = os.path.join(os.path.dirname(fname), '.gitignore')
+            with open(gitignore, 'a') as fd:
+                fd.write('\n' + os.path.basename(fname))
+
 
 class GitWrapper(GitWrapperI):
     FLOATS_FROM_STRING = re.compile(r'[-+]?(?:(?:\d*\.\d+)|(?:\d+\.?))(?:[Ee][+-]?\d+)?')

dvc/path/data_item.py

@@ -125,15 +125,10 @@ class DataItem(object):
     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)
-        return os.path.relpath(self.cache.relative, data_file_dir)
-
     def move_data_to_cache(self):
-        self.import_cache(self.data.relative)
-        System.symlink(self.symlink_file, self.data.relative)
-
-    def import_cache(self, fname):
-        self._cache_file = os.path.join(self.cache_dir, file_md5(fname)[0])
-        os.rename(fname, self._cache_file)
+        md5 = file_md5(self.data.relative)[0]
+        self._cache_file = os.path.join(self.cache_dir, md5)
+        self._git.modify_gitignore([self.data.relative])
+        if os.path.isfile(self.cache.relative):
+            return
+        System.hardlink(self.data.relative, self.cache.relative)

dvc/path/factory.py

@@ -25,11 +25,16 @@ class PathFactory(object):
         path = Path.from_dvc_path(dvc_path, self._git)
         return self.existing_data_item(path.relative)
 
-    def existing_data_item(self, file):
-        if not System.islink(file):
-            raise DataItemError(u'Data file "%s" must be a symbolic link' % file)
-        resolved_symlink = System.realpath(file)
-        return DataItem(file, self._git, self._config, resolved_symlink)
+    def is_data_item(self, fname):
+        data = os.path.relpath(os.path.realpath(fname), self._config.data_dir)
+        state = os.path.join(self._config.state_dir, data + DataItem.STATE_FILE_SUFFIX)
+        return os.path.isfile(state)
+
+    def existing_data_item(self, fname):
+        if not self.is_data_item(fname):
+            raise DataItemError(u'No state file found for data file {}'.format(fname))
+
+        return DataItem(fname, self._git, self._config)
 
     def to_data_items(self, files):
         result = []

dvc/system.py

@@ -10,7 +10,6 @@ from dvc.executor import Executor
 
 
 class System(object):
-    SYMLINK_OUTPUT = '<SYMLINK>'
     LONG_PATH_BUFFER_SIZE = 1024
 
     @staticmethod
@@ -18,47 +17,11 @@ class System(object):
         return os.name != 'nt'
 
     @staticmethod
-    def symlink(source, link_name):
+    def hardlink(source, link_name):
         if System.is_unix():
-            return os.symlink(source, link_name)
+            return os.link(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(path) is not None
-
-    @staticmethod
-    def _get_symlink_string(path):
-        code, output, _ = Executor.exec_cmd(["dir", path], shell=True)
-        if code != 0:
-            return None
-
-        lines = output.split('\n')
-        for line in lines:
-            if System.SYMLINK_OUTPUT in line:
-                return line
-        return None
+        raise Exception('Not Implemented Yet')
 
     @staticmethod
     def realpath(path):