From a8b7e33010e7146ee73654697a113a000c010aa7 Mon Sep 17 00:00:00 2001
From: charlesbaynham <bayners123@gmail.com>
Date: Fri, 24 Apr 2020 00:47:21 +0100
Subject: [PATCH] Convert all paths in config to posix-style (#3665)

* Convert all paths in config to posix-style

* Use PathInfo for consistancy, add unit test and change functional tests

* Exclude Windows test on Linux

* Don't patch Config for all the tests!

* Don't do the linux test on Windows either

* De-hackify the unit test, make the functional test more explicit

* tests: adjust tests

Co-authored-by: Charles Baynham <charles.baynham@npl.co.uk>
Co-authored-by: Ruslan Kuprieiev <ruslan@iterative.ai>
---
 dvc/config.py             | 38 ++++++++++++++++++++++++++------------
 tests/func/test_cache.py  |  7 +++----
 tests/func/test_remote.py |  4 +++-
 tests/unit/test_config.py | 17 +++++++++++++++++
 4 files changed, 49 insertions(+), 17 deletions(-)
 create mode 100644 tests/unit/test_config.py

diff --git a/dvc/config.py b/dvc/config.py
index 1fc63acbb..224e1ac28 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -1,16 +1,27 @@
 """DVC config objects."""
-from contextlib import contextmanager
 import logging
 import os
 import re
+from contextlib import contextmanager
+from functools import partial
 from urllib.parse import urlparse
 
-from funcy import cached_property, re_find, walk_values, compact
 import configobj
-from voluptuous import Schema, Optional, Invalid, ALLOW_EXTRA
-from voluptuous import All, Any, Lower, Range, Coerce
+from funcy import cached_property, compact, re_find, walk_values
+from voluptuous import (
+    ALLOW_EXTRA,
+    All,
+    Any,
+    Coerce,
+    Invalid,
+    Lower,
+    Optional,
+    Range,
+    Schema,
+)
 
 from dvc.exceptions import DvcException, NotDvcRepoError
+from dvc.path_info import PathInfo
 from dvc.utils import relpath
 
 logger = logging.getLogger(__name__)
@@ -311,17 +322,20 @@ class Config(dict):
 
         return Config._map_dirs(conf, resolve)
 
+    @staticmethod
+    def _to_relpath(conf_dir, path):
+        if re.match(r"\w+://", path):
+            return path
+
+        if isinstance(path, RelPath) or not os.path.isabs(path):
+            path = relpath(path, conf_dir)
+
+        return PathInfo(path).as_posix()
+
     @staticmethod
     def _save_paths(conf, filename):
         conf_dir = os.path.dirname(filename)
-
-        def rel(path):
-            if re.match(r"\w+://", path):
-                return path
-
-            if isinstance(path, RelPath) or not os.path.isabs(path):
-                return relpath(path, conf_dir)
-            return path
+        rel = partial(Config._to_relpath, conf_dir)
 
         return Config._map_dirs(conf, rel)
 
diff --git a/tests/func/test_cache.py b/tests/func/test_cache.py
index 8d692938d..841e06f1c 100644
--- a/tests/func/test_cache.py
+++ b/tests/func/test_cache.py
@@ -8,8 +8,7 @@ from dvc.cache import Cache
 from dvc.main import main
 from dvc.remote.base import DirCacheError
 from dvc.utils import relpath
-from tests.basic_env import TestDir
-from tests.basic_env import TestDvc
+from tests.basic_env import TestDir, TestDvc
 
 
 class TestCache(TestDvc):
@@ -155,7 +154,7 @@ class TestCmdCacheDir(TestDvc):
         self.assertEqual(ret, 0)
 
         config = configobj.ConfigObj(self.dvc.config.files["repo"])
-        self.assertEqual(config["cache"]["dir"], dname)
+        self.assertEqual(config["cache"]["dir"], dname.replace("\\", "/"))
 
     def test_relative_path(self):
         tmpdir = self.mkdtemp()
@@ -167,7 +166,7 @@ class TestCmdCacheDir(TestDvc):
         # dir path written to config should be just one level above.
         rel = os.path.join("..", dname)
         config = configobj.ConfigObj(self.dvc.config.files["repo"])
-        self.assertEqual(config["cache"]["dir"], rel)
+        self.assertEqual(config["cache"]["dir"], rel.replace("\\", "/"))
 
         ret = main(["add", self.FOO])
         self.assertEqual(ret, 0)
diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py
index 4369203dd..0d3f9e9f5 100644
--- a/tests/func/test_remote.py
+++ b/tests/func/test_remote.py
@@ -47,7 +47,9 @@ class TestRemote(TestDvc):
         # dir path written to config should be just one level above.
         rel = os.path.join("..", dname)
         config = configobj.ConfigObj(self.dvc.config.files["repo"])
-        self.assertEqual(config['remote "mylocal"']["url"], rel)
+        self.assertEqual(
+            config['remote "mylocal"']["url"], rel.replace("\\", "/")
+        )
 
     def test_overwrite(self):
         remote_name = "a"
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
new file mode 100644
index 000000000..e0899cbf3
--- /dev/null
+++ b/tests/unit/test_config.py
@@ -0,0 +1,17 @@
+import os
+import pytest
+
+from dvc.config import Config
+
+
+@pytest.mark.parametrize(
+    "path, expected",
+    [
+        ("cache", "../cache"),
+        (os.path.join("..", "cache"), "../../cache"),
+        (os.getcwd(), os.getcwd().replace("\\", "/")),
+        ("ssh://some/path", "ssh://some/path"),
+    ],
+)
+def test_to_relpath(path, expected):
+    assert Config._to_relpath(os.path.join(".", "config"), path) == expected