Merge pull request #2122 from mnowster/2050-relative-volume-paths-windows

2050 ensure windows volume paths are compatible with engine

compose/config/config.py

@@ -526,12 +526,13 @@ def path_mappings_from_dict(d):
     return [join_path_mapping(v) for v in d.items()]
 
 
-def split_path_mapping(string):
+def split_path_mapping(volume_path):
-    if ':' in string:
+    drive, volume_config = os.path.splitdrive(volume_path)
-        (host, container) = string.split(':', 1)
+    if ':' in volume_config:
-        return (container, host)
+        (host, container) = volume_config.split(':', 1)
+        return (container, drive + host)
     else:
-        return (string, None)
+        return (volume_path, None)
 
 
 def join_path_mapping(pair):

compose/const.py

@@ -2,11 +2,11 @@ import os
 import sys
 
 DEFAULT_TIMEOUT = 10
+HTTP_TIMEOUT = int(os.environ.get('COMPOSE_HTTP_TIMEOUT', os.environ.get('DOCKER_CLIENT_TIMEOUT', 60)))
+IS_WINDOWS_PLATFORM = (sys.platform == "win32")
 LABEL_CONTAINER_NUMBER = 'com.docker.compose.container-number'
 LABEL_ONE_OFF = 'com.docker.compose.oneoff'
 LABEL_PROJECT = 'com.docker.compose.project'
 LABEL_SERVICE = 'com.docker.compose.service'
 LABEL_VERSION = 'com.docker.compose.version'
 LABEL_CONFIG_HASH = 'com.docker.compose.config-hash'
-HTTP_TIMEOUT = int(os.environ.get('COMPOSE_HTTP_TIMEOUT', os.environ.get('DOCKER_CLIENT_TIMEOUT', 60)))
-IS_WINDOWS_PLATFORM = (sys.platform == 'win32')

compose/service.py

@@ -20,6 +20,7 @@ from .config import DOCKER_CONFIG_KEYS
 from .config import merge_environment
 from .config.validation import VALID_NAME_CHARS
 from .const import DEFAULT_TIMEOUT
+from .const import IS_WINDOWS_PLATFORM
 from .const import LABEL_CONFIG_HASH
 from .const import LABEL_CONTAINER_NUMBER
 from .const import LABEL_ONE_OFF
@@ -936,20 +937,54 @@ def build_volume_binding(volume_spec):
     return volume_spec.internal, "{}:{}:{}".format(*volume_spec)
 
 
+def normalize_paths_for_engine(external_path, internal_path):
+    """
+    Windows paths, c:\my\path\shiny, need to be changed to be compatible with
+    the Engine. Volume paths are expected to be linux style /c/my/path/shiny/
+    """
+    if IS_WINDOWS_PLATFORM:
+        if external_path:
+            drive, tail = os.path.splitdrive(external_path)
+
+            if drive:
+                reformatted_drive = "/{}".format(drive.replace(":", ""))
+                external_path = reformatted_drive + tail
+
+            external_path = "/".join(external_path.split("\\"))
+
+        return external_path, "/".join(internal_path.split("\\"))
+    else:
+        return external_path, internal_path
+
+
 def parse_volume_spec(volume_config):
-    parts = volume_config.split(':')
+    """
+    Parse a volume_config path and split it into external:internal[:mode]
+    parts to be returned as a valid VolumeSpec.
+    """
+    if IS_WINDOWS_PLATFORM:
+        # relative paths in windows expand to include the drive, eg C:\
+        # so we join the first 2 parts back together to count as one
+        drive, tail = os.path.splitdrive(volume_config)
+        parts = tail.split(":")
+
+        if drive:
+            parts[0] = drive + parts[0]
+    else:
+        parts = volume_config.split(':')
+
     if len(parts) > 3:
         raise ConfigError("Volume %s has incorrect format, should be "
                           "external:internal[:mode]" % volume_config)
 
     if len(parts) == 1:
-        external = None
+        external, internal = normalize_paths_for_engine(None, os.path.normpath(parts[0]))
-        internal = os.path.normpath(parts[0])
     else:
-        external = os.path.normpath(parts[0])
+        external, internal = normalize_paths_for_engine(os.path.normpath(parts[0]), os.path.normpath(parts[1]))
-        internal = os.path.normpath(parts[1])
 
-    mode = parts[2] if len(parts) == 3 else 'rw'
+    mode = 'rw'
+    if len(parts) == 3:
+        mode = parts[2]
 
     return VolumeSpec(external, internal, mode)
 

tests/unit/config/config_test.py

@@ -420,7 +420,6 @@ class VolumeConfigTest(unittest.TestCase):
         d = make_service_dict('foo', {'build': '.', 'volumes': ['/data']}, working_dir='.')
         self.assertEqual(d['volumes'], ['/data'])
 
-    @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash')
     @mock.patch.dict(os.environ)
     def test_volume_binding_with_environment_variable(self):
         os.environ['VOLUME_PATH'] = '/host/path'
@@ -433,7 +432,7 @@ class VolumeConfigTest(unittest.TestCase):
         )[0]
         self.assertEqual(d['volumes'], ['/host/path:/container/path'])
 
-    @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash')
+    @pytest.mark.skipif(IS_WINDOWS_PLATFORM, reason='posix paths')
     @mock.patch.dict(os.environ)
     def test_volume_binding_with_home(self):
         os.environ['HOME'] = '/home/user'
@@ -464,6 +463,7 @@ class VolumeConfigTest(unittest.TestCase):
         self.assertEqual(d['volumes'], ['/home/me/otherproject:/data'])
 
     @pytest.mark.skipif(not IS_WINDOWS_PLATFORM, reason='windows paths')
+    @pytest.mark.skipif(IS_WINDOWS_PLATFORM, reason='waiting for this to be resolved: https://github.com/docker/compose/issues/2128')
     def test_relative_path_does_expand_windows(self):
         d = make_service_dict('foo', {'build': '.', 'volumes': ['./data:/data']}, working_dir='C:\\Users\\me\\myproject')
         self.assertEqual(d['volumes'], ['C:\\Users\\me\\myproject\\data:/data'])
@@ -1124,6 +1124,21 @@ class ExpandPathTest(unittest.TestCase):
         self.assertEqual(result, user_path + 'otherdir/somefile')
 
 
+class VolumePathTest(unittest.TestCase):
+
+    @pytest.mark.xfail((not IS_WINDOWS_PLATFORM), reason='does not have a drive')
+    def test_split_path_mapping_with_windows_path(self):
+        windows_volume_path = "c:\\Users\\msamblanet\\Documents\\anvil\\connect\\config:/opt/connect/config:ro"
+        expected_mapping = (
+            "/opt/connect/config:ro",
+            "c:\\Users\\msamblanet\\Documents\\anvil\\connect\\config"
+        )
+
+        mapping = config.split_path_mapping(windows_volume_path)
+
+        self.assertEqual(mapping, expected_mapping)
+
+
 @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash')
 class BuildPathTest(unittest.TestCase):
     def setUp(self):

tests/unit/service_test.py

@@ -441,7 +441,6 @@ def mock_get_image(images):
         raise NoSuchImageError()
 
 
-@pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash')
 class ServiceVolumesTest(unittest.TestCase):
 
     def setUp(self):
@@ -466,6 +465,21 @@ class ServiceVolumesTest(unittest.TestCase):
         with self.assertRaises(ConfigError):
             parse_volume_spec('one:two:three:four')
 
+    @pytest.mark.xfail((not IS_WINDOWS_PLATFORM), reason='does not have a drive')
+    def test_parse_volume_windows_absolute_path(self):
+        windows_absolute_path = "c:\\Users\\me\\Documents\\shiny\\config:\\opt\\shiny\\config:ro"
+
+        spec = parse_volume_spec(windows_absolute_path)
+
+        self.assertEqual(
+            spec,
+            (
+                "/c/Users/me/Documents/shiny/config",
+                "/opt/shiny/config",
+                "ro"
+            )
+        )
+
     def test_build_volume_binding(self):
         binding = build_volume_binding(parse_volume_spec('/outside:/inside'))
         self.assertEqual(binding, ('/inside', '/outside:/inside:rw'))