Better file conflict message (canonical#766)

When parts conflict the message is plain and blunt, we
should do some hand holding.

LP: #1617390

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>

integration_tests/test_stage.py

@@ -1,6 +1,6 @@
 # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
 #
-# Copyright (C) 2015 Canonical Ltd
+# Copyright (C) 2015-2016 Canonical Ltd
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
@@ -16,7 +16,7 @@
 
 import subprocess
 
-from testtools.matchers import EndsWith
+from testtools.matchers import Contains
 
 import integration_tests
 
@@ -30,7 +30,19 @@ def test_conflicts(self):
             self.run_snapcraft, 'stage', project_dir)
 
         self.assertEqual(1, exception.returncode)
-        expected = (
+        expected_conflicts = (
             "Parts 'p1' and 'p2' have the following file paths in common "
-            "which have different contents:\nbin/test\n")
-        self.assertThat(exception.output, EndsWith(expected))
+            "which have different contents:\n    bin/test\n")
+        self.assertThat(exception.output, Contains(expected_conflicts))
+
+        expected_help = (
+            'Snapcraft offers some capabilities to solve this by use '
+            'of the following keywords:\n'
+            '    - `filesets`\n'
+            '    - `stage`\n'
+            '    - `snap`\n'
+            '    - `organize`\n\n'
+            'Learn more about these part keywords by running '
+            '`snapcraft help plugins`'
+        )
+        self.assertThat(exception.output, Contains(expected_help))

snapcraft/__init__.py

@@ -213,7 +213,7 @@
         - -usr/lib/libtest.so   # Excludng libtest.so
         - $manpages             # Including the 'manpages' fileset
 
-  - prime: YAML file and fileset list
+  - snap: YAML file and fileset list
 
     A list of files from a part install directory to copy into `prime/`.
     This section takes exactly the same form as the 'stage' section  but the

snapcraft/internal/errors.py

@@ -0,0 +1,62 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2016 Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+class SnapcraftError(Exception):
+    """Base class for all snapcraft exceptions.
+
+    :cvar fmt: A format string that daughter classes override
+
+    """
+    fmt = 'Daughter classes should redefine this'
+
+    def __init__(self, **kwargs):
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def __str__(self):
+        return self.fmt.format([], **self.__dict__)
+
+
+class PluginError(Exception):
+    pass
+
+
+class MissingState(Exception):
+    pass
+
+
+class SnapcraftPartConflictError(SnapcraftError):
+
+    fmt = (
+        'Parts {other_part_name!r} and {part_name!r} have the following file '
+        'paths in common which have different contents:\n'
+        '{file_paths}\n\n'
+        'Snapcraft offers some capabilities to solve this by use of the '
+        'following keywords:\n'
+        '    - `filesets`\n'
+        '    - `stage`\n'
+        '    - `snap`\n'
+        '    - `organize`\n\n'
+        'Learn more about these part keywords by running '
+        '`snapcraft help plugins`'
+    )
+
+    def __init__(self, *, part_name, other_part_name, conflict_files):
+        spaced_conflict_files = ('    {}'.format(i) for i in conflict_files)
+        super().__init__(part_name=part_name,
+                         other_part_name=other_part_name,
+                         file_paths='\n'.join(sorted(spaced_conflict_files)))

snapcraft/internal/pluginhandler.py

@@ -30,6 +30,11 @@
 
 import snapcraft
 from snapcraft import file_utils
+from snapcraft.internal.errors import (
+    PluginError,
+    MissingState,
+    SnapcraftPartConflictError,
+)
 from snapcraft.internal import (
     common,
     libraries,
@@ -40,14 +45,6 @@
 logger = logging.getLogger(__name__)
 
 
-class PluginError(Exception):
-    pass
-
-
-class MissingState(Exception):
-    pass
-
-
 class PluginHandler:
 
     @property
@@ -845,11 +842,10 @@ def check_for_collisions(parts):
                     conflict_files.append(f)
 
             if conflict_files:
-                raise EnvironmentError(
-                    'Parts {!r} and {!r} have the following file paths in '
-                    'common which have different contents:\n{}'.format(
-                        other_part_name, part.name,
-                        '\n'.join(sorted(conflict_files))))
+                raise SnapcraftPartConflictError(
+                    other_part_name=other_part_name,
+                    part_name=part.name,
+                    conflict_files=conflict_files)
 
         # And add our files to the list
         parts_files[part.name] = {'files': part_files,

snapcraft/storeapi/errors.py

@@ -14,25 +14,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-
 from simplejson.scanner import JSONDecodeError
 
-
-# TODO move to snapcraft.errors --elopio - 2016-06-20
-class SnapcraftError(Exception):
-    """Base class for all storeapi exceptions.
-
-    :cvar fmt: A format string that daughter classes override
-
-    """
-    fmt = 'Daughter classes should redefine this'
-
-    def __init__(self, **kwargs):
-        for key, value in kwargs.items():
-            setattr(self, key, value)
-
-    def __str__(self):
-        return self.fmt.format([], **self.__dict__)
+from snapcraft.internal.errors import SnapcraftError
 
 
 class InvalidCredentialsError(SnapcraftError):

snapcraft/tests/test_pluginhandler.py

@@ -31,6 +31,7 @@
 import fixtures
 
 import snapcraft
+from snapcraft.internal.errors import SnapcraftPartConflictError
 from snapcraft.internal import (
     common,
     lifecycle,
@@ -1949,24 +1950,24 @@ def test_no_collisions(self):
         pluginhandler.check_for_collisions([self.part1, self.part2])
 
     def test_collisions_between_two_parts(self):
-        with self.assertRaises(EnvironmentError) as raised:
+        with self.assertRaises(SnapcraftPartConflictError) as raised:
             pluginhandler.check_for_collisions(
                 [self.part1, self.part2, self.part3])
 
-        self.assertEqual(
-            raised.exception.__str__(),
+        self.assertIn(
             "Parts 'part2' and 'part3' have the following file paths in "
-            "common which have different contents:\n1\na/2")
+            "common which have different contents:\n    1\n    a/2",
+            raised.exception.__str__())
 
     def test_collisions_between_two_parts_pc_files(self):
-        with self.assertRaises(EnvironmentError) as raised:
+        with self.assertRaises(SnapcraftPartConflictError) as raised:
             pluginhandler.check_for_collisions(
                 [self.part1, self.part4])
 
-        self.assertEqual(
-            raised.exception.__str__(),
+        self.assertIn(
             "Parts 'part1' and 'part4' have the following file paths in "
-            "common which have different contents:\nfile.pc")
+            "common which have different contents:\n    file.pc",
+            raised.exception.__str__())
 
 
 class StagePackagesTestCase(tests.TestCase):