From 5e97b806d51e72f282046231af417b4d647cf64f Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@docker.com>
Date: Mon, 9 Nov 2015 14:24:44 -0500
Subject: [PATCH] Fix a bug in ExtendsResolver where the service name of the
 extended service was wrong.

This bug can be seen by the change to the test case. When the extended service
uses a different name, the error was reported incorrectly.

By fixing this bug we can simplify self.signature and self.detect_cycles to
always use self.service_name.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
---
 compose/config/config.py            | 20 +++++++++++---------
 tests/fixtures/extends/circle-1.yml |  2 +-
 tests/fixtures/extends/circle-2.yml |  2 +-
 tests/unit/config/config_test.py    | 23 ++++++++++++-----------
 4 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index 7846ea7b4..1ddb2abe0 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -247,11 +247,17 @@ class ServiceExtendsResolver(object):
         self.service_name = service_name
         self.service_dict['name'] = service_name
 
-    def detect_cycle(self, name):
-        if self.signature(name) in self.already_seen:
-            raise CircularReference(self.already_seen + [self.signature(name)])
+    @property
+    def signature(self):
+        return self.filename, self.service_name
+
+    def detect_cycle(self):
+        if self.signature in self.already_seen:
+            raise CircularReference(self.already_seen + [self.signature])
 
     def run(self):
+        self.detect_cycle()
+
         service_dict = dict(self.service_dict)
         env = resolve_environment(self.working_dir, self.service_dict)
         if env:
@@ -284,12 +290,11 @@ class ServiceExtendsResolver(object):
         resolver = ServiceExtendsResolver(
             os.path.dirname(extended_config_path),
             extended_config_path,
-            self.service_name,
+            service_name,
             service_config,
-            already_seen=self.already_seen + [self.signature(self.service_name)],
+            already_seen=self.already_seen + [self.signature],
         )
 
-        resolver.detect_cycle(service_name)
         other_service_dict = process_service(resolver.working_dir, resolver.run())
         validate_extended_service_dict(
             other_service_dict,
@@ -309,9 +314,6 @@ class ServiceExtendsResolver(object):
             return expand_path(self.working_dir, extends_options['file'])
         return self.filename
 
-    def signature(self, name):
-        return self.filename, name
-
 
 def resolve_environment(working_dir, service_dict):
     """Unpack any environment variables from an env_file, if set.
diff --git a/tests/fixtures/extends/circle-1.yml b/tests/fixtures/extends/circle-1.yml
index a034e9619..d88ea61d0 100644
--- a/tests/fixtures/extends/circle-1.yml
+++ b/tests/fixtures/extends/circle-1.yml
@@ -5,7 +5,7 @@ bar:
 web:
   extends:
     file: circle-2.yml
-    service: web
+    service: other
 baz:
   image: busybox
 quux:
diff --git a/tests/fixtures/extends/circle-2.yml b/tests/fixtures/extends/circle-2.yml
index fa6ddefcc..de05bc8da 100644
--- a/tests/fixtures/extends/circle-2.yml
+++ b/tests/fixtures/extends/circle-2.yml
@@ -2,7 +2,7 @@ foo:
   image: busybox
 bar:
   image: busybox
-web:
+other:
   extends:
     file: circle-1.yml
     service: web
diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py
index 2835a4317..717831681 100644
--- a/tests/unit/config/config_test.py
+++ b/tests/unit/config/config_test.py
@@ -1080,18 +1080,19 @@ class ExtendsTest(unittest.TestCase):
         ]))
 
     def test_circular(self):
-        try:
+        with pytest.raises(config.CircularReference) as exc:
             load_from_filename('tests/fixtures/extends/circle-1.yml')
-            raise Exception("Expected config.CircularReference to be raised")
-        except config.CircularReference as e:
-            self.assertEqual(
-                [(os.path.basename(filename), service_name) for (filename, service_name) in e.trail],
-                [
-                    ('circle-1.yml', 'web'),
-                    ('circle-2.yml', 'web'),
-                    ('circle-1.yml', 'web'),
-                ],
-            )
+
+        path = [
+            (os.path.basename(filename), service_name)
+            for (filename, service_name) in exc.value.trail
+        ]
+        expected = [
+            ('circle-1.yml', 'web'),
+            ('circle-2.yml', 'other'),
+            ('circle-1.yml', 'web'),
+        ]
+        self.assertEqual(path, expected)
 
     def test_extends_validation_empty_dictionary(self):
         with self.assertRaisesRegexp(ConfigurationError, 'service'):