diff --git a/dvc/remote/local.py b/dvc/remote/local.py index da1cdf724..e30477852 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -385,6 +385,12 @@ class RemoteLOCAL(RemoteBase): self.link(cache, path) self.state.update_link(path) + # we need to update path and cache, since in case of reflink, + # or copy cache type moving original file results in updates on + # next executed command, which causes md5 recalculation + self.state.update(path, md5) + self.state.update(cache, md5) + return {self.PARAM_CHECKSUM: md5} def _save_dir(self, path_info): diff --git a/dvc/state.py b/dvc/state.py index bf2ad8068..64d377e62 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -30,6 +30,10 @@ class StateVersionTooNewError(DvcException): ) +def _file_metadata_changed(actual_mtime, mtime, actual_size, size): + return actual_mtime != mtime or actual_size != size + + class State(object): # pylint: disable=too-many-instance-attributes """Class for the state database. @@ -316,83 +320,146 @@ class State(object): # pylint: disable=too-many-instance-attributes logger.debug("Path {} inode {}".format(path, System.inode(path))) return System.inode(path) - def _do_update(self, path): + def _do_update(self, path, known_checksum=None): """ Make sure the stored info for the given path is up to date. """ if not os.path.exists(path): - return (None, None) + return None, None actual_mtime, actual_size = self._mtime_and_size(path) actual_inode = self._inode(path) + existing_records = self._get_state_records_for_inode(actual_inode) + should_insert_new_record = not existing_records + + if should_insert_new_record: + md5, info = self._insert_new_state_record( + path, actual_inode, actual_mtime, actual_size, known_checksum + ) + else: + md5, info = self._update_existing_state_record( + path, + actual_inode, + actual_mtime, + actual_size, + existing_records, + known_checksum, + ) + + return md5, info + + def _update_existing_state_record( + self, + path, + actual_inode, + actual_mtime, + actual_size, + existing_records, + known_checksum=None, + ): + + md5, mtime, size = self._get_existing_record_data( + actual_inode, actual_mtime, actual_size, existing_records + ) + if _file_metadata_changed(actual_mtime, mtime, actual_size, size): + md5, info = self._update_state_for_path_changed( + path, actual_inode, actual_mtime, actual_size, known_checksum + ) + else: + info = None + self._update_state_record_timestamp_for_inode(actual_inode) + return md5, info + + def _get_existing_record_data( + self, actual_inode, actual_mtime, actual_size, existing_records + ): + assert len(existing_records) == 1 + assert len(existing_records[0]) == 5 + inode, mtime, size, md5, _ = existing_records[0] + inode = self._from_sqlite(inode) + assert inode == actual_inode + logger.debug( + "Inode '{}', mtime '{}', actual mtime '{}', size '{}', " + "actual size '{}'.".format( + inode, mtime, actual_mtime, size, actual_size + ) + ) + return md5, mtime, size + + def _update_state_record_timestamp_for_inode(self, actual_inode): + cmd = 'UPDATE {} SET timestamp = "{}" WHERE inode = {}' + self._execute( + cmd.format( + self.STATE_TABLE, + int(nanotime.timestamp(time.time())), + self._to_sqlite(actual_inode), + ) + ) + + def _update_state_for_path_changed( + self, + path, + actual_inode, + actual_mtime, + actual_size, + known_checksum=None, + ): + if known_checksum: + md5, info = known_checksum, None + else: + md5, info = self._collect(path) + cmd = ( + "UPDATE {} SET " + 'mtime = "{}", size = "{}", ' + 'md5 = "{}", timestamp = "{}" ' + "WHERE inode = {}" + ) + self._execute( + cmd.format( + self.STATE_TABLE, + actual_mtime, + actual_size, + md5, + int(nanotime.timestamp(time.time())), + self._to_sqlite(actual_inode), + ) + ) + return md5, info + + def _insert_new_state_record( + self, path, actual_inode, actual_mtime, actual_size, known_checksum + ): + if known_checksum: + md5, info = known_checksum, None + else: + md5, info = self._collect(path) + cmd = ( + "INSERT INTO {}(inode, mtime, size, md5, timestamp) " + 'VALUES ({}, "{}", "{}", "{}", "{}")' + ) + self._execute( + cmd.format( + self.STATE_TABLE, + self._to_sqlite(actual_inode), + actual_mtime, + actual_size, + md5, + int(nanotime.timestamp(time.time())), + ) + ) + self.inserts += 1 + return md5, info + + def _get_state_records_for_inode(self, actual_inode): cmd = "SELECT * from {} WHERE inode={}".format( self.STATE_TABLE, self._to_sqlite(actual_inode) ) - self._execute(cmd) ret = self._fetchall() - if not ret: - md5, info = self._collect(path) - cmd = ( - "INSERT INTO {}(inode, mtime, size, md5, timestamp) " - 'VALUES ({}, "{}", "{}", "{}", "{}")' - ) - self._execute( - cmd.format( - self.STATE_TABLE, - self._to_sqlite(actual_inode), - actual_mtime, - actual_size, - md5, - int(nanotime.timestamp(time.time())), - ) - ) - self.inserts += 1 - else: - assert len(ret) == 1 - assert len(ret[0]) == 5 - inode, mtime, size, md5, _ = ret[0] - inode = self._from_sqlite(inode) - assert inode == actual_inode - logger.debug( - "Inode '{}', mtime '{}', actual mtime '{}', size '{}', " - "actual size '{}'.".format( - inode, mtime, actual_mtime, size, actual_size - ) - ) - if actual_mtime != mtime or actual_size != size: - md5, info = self._collect(path) - cmd = ( - "UPDATE {} SET " - 'mtime = "{}", size = "{}", ' - 'md5 = "{}", timestamp = "{}" ' - "WHERE inode = {}" - ) - self._execute( - cmd.format( - self.STATE_TABLE, - actual_mtime, - actual_size, - md5, - int(nanotime.timestamp(time.time())), - self._to_sqlite(actual_inode), - ) - ) - else: - info = None - cmd = 'UPDATE {} SET timestamp = "{}" WHERE inode = {}' - self._execute( - cmd.format( - self.STATE_TABLE, - int(nanotime.timestamp(time.time())), - self._to_sqlite(actual_inode), - ) - ) + return ret - return (md5, info) - - def update(self, path): + def update(self, path, known_checksum=None): """Gets the checksum for the specified path. Checksum will be retrieved from the state database if available, otherwise it will be computed and cached in the state database for the further use. @@ -403,7 +470,7 @@ class State(object): # pylint: disable=too-many-instance-attributes Returns: str: checksum for the specified path. """ - return self._do_update(path)[0] + return self._do_update(path, known_checksum)[0] def update_info(self, path): """Gets the checksum and the directory info (if applicable) for the diff --git a/tests/test_add.py b/tests/test_add.py index a4ca03c32..ef343cc75 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -1,12 +1,17 @@ from __future__ import unicode_literals import os + +import dvc import yaml import time import shutil import filecmp +from mock import patch + from dvc.main import main +from dvc.state import State from dvc.utils import file_md5 from dvc.stage import Stage from dvc.exceptions import DvcException @@ -14,6 +19,7 @@ from dvc.output.base import OutputAlreadyTrackedError from dvc.repo import Repo as DvcRepo from tests.basic_env import TestDvc +from tests.utils import spy class TestAdd(TestDvc): @@ -214,3 +220,24 @@ class TestDoubleAddUnchanged(TestDvc): ret = main(["add", self.DATA_DIR]) self.assertEqual(ret, 0) + + +class TestShouldUpdateStateEntryForFileAfterAdd(TestDvc): + def test(self): + file_md5_counter = spy(dvc.state.file_md5) + with patch.object(dvc.state, "file_md5", file_md5_counter): + + ret = main(["config", "cache.type", "copy"]) + self.assertEqual(ret, 0) + + ret = main(["add", self.FOO]) + self.assertEqual(ret, 0) + self.assertEqual(file_md5_counter.mock.call_count, 1) + + ret = main(["status"]) + self.assertEqual(ret, 0) + self.assertEqual(file_md5_counter.mock.call_count, 1) + + ret = main(["run", "-d", self.FOO, "cat foo"]) + self.assertEqual(ret, 0) + self.assertEqual(file_md5_counter.mock.call_count, 1)