From 28f7ccfc6c983f26e659cd851d5e4a8bd28af37d Mon Sep 17 00:00:00 2001
From: pared <pawelredzynski@gmail.com>
Date: Wed, 1 May 2019 23:34:52 -0700
Subject: [PATCH] remote: base: dump dir_info json with sorted keys

---
 dvc/remote/base.py        | 35 ++++++++++++++++++++---------------
 tests/func/test_remote.py | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 870a51a55..373c151a3 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -189,22 +189,9 @@ class RemoteBase(object):
 
     def get_dir_checksum(self, path_info):
         dir_info = self._collect_dir(path_info)
-        to_info = path_info.copy()
-        to_info["path"] = self.cache.ospath.join(
-            self.cache.prefix, tmp_fname("")
+        checksum, from_info, to_info = self._get_dir_info_checksum(
+            dir_info, path_info
         )
-
-        tmp = tempfile.NamedTemporaryFile(delete=False).name
-        with open(tmp, "w+") as fobj:
-            json.dump(dir_info, fobj)
-
-        from_info = {"scheme": "local", "path": tmp}
-        self.cache.upload([from_info], [to_info], no_progress_bar=True)
-
-        checksum = self.get_file_checksum(to_info) + self.CHECKSUM_DIR_SUFFIX
-
-        from_info = to_info.copy()
-        to_info["path"] = self.cache.checksum_to_path(checksum)
         if self.cache.changed_cache_file(checksum):
             self.cache.move(from_info, to_info)
 
@@ -213,6 +200,24 @@ class RemoteBase(object):
 
         return checksum
 
+    def _get_dir_info_checksum(self, dir_info, path_info):
+        to_info = path_info.copy()
+        to_info["path"] = self.cache.ospath.join(
+            self.cache.prefix, tmp_fname("")
+        )
+
+        tmp = tempfile.NamedTemporaryFile(delete=False).name
+        with open(tmp, "w+") as fobj:
+            json.dump(dir_info, fobj, sort_keys=True)
+
+        from_info = {"scheme": "local", "path": tmp}
+        self.cache.upload([from_info], [to_info], no_progress_bar=True)
+
+        checksum = self.get_file_checksum(to_info) + self.CHECKSUM_DIR_SUFFIX
+        from_info = to_info.copy()
+        to_info["path"] = self.cache.checksum_to_path(checksum)
+        return checksum, from_info, to_info
+
     def get_dir_cache(self, checksum):
         assert checksum
 
diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py
index 71f5ca0ad..84651be52 100644
--- a/tests/func/test_remote.py
+++ b/tests/func/test_remote.py
@@ -1,5 +1,6 @@
 import os
 import configobj
+from dvc.remote.base import RemoteBase
 from mock import patch
 
 from dvc.main import main
@@ -183,3 +184,40 @@ def test_large_dir_progress(repo_dir, dvc):
     with patch.object(progress, "update_target") as update_target:
         dvc.add("gen")
         assert update_target.called
+
+
+def test_dir_checksum_should_be_key_order_agnostic(dvc):
+    data_dir = os.path.join(dvc.root_dir, "data")
+    file1 = os.path.join(data_dir, "1")
+    file2 = os.path.join(data_dir, "2")
+
+    os.mkdir(data_dir)
+    with open(file1, "w") as fobj:
+        fobj.write("1")
+
+    with open(file2, "w") as fobj:
+        fobj.write("2")
+
+    path_info = {"scheme": "local", "path": data_dir}
+    with dvc.state:
+        with patch.object(
+            RemoteBase,
+            "_collect_dir",
+            return_value=[
+                {"relpath": "1", "md5": "1"},
+                {"relpath": "2", "md5": "2"},
+            ],
+        ):
+            checksum1 = dvc.cache.local.get_dir_checksum(path_info)
+
+        with patch.object(
+            RemoteBase,
+            "_collect_dir",
+            return_value=[
+                {"md5": "1", "relpath": "1"},
+                {"md5": "2", "relpath": "2"},
+            ],
+        ):
+            checksum2 = dvc.cache.local.get_dir_checksum(path_info)
+
+    assert checksum1 == checksum2