Merge pull request #398 from kyrofa/feature/1537786/clean_build_step

Support cleaning build step.

integration_tests/snaps/independent-parts/part1/Makefile

@@ -0,0 +1,5 @@
+all:
+
+install:
+	@mkdir -p $(DESTDIR)/bin
+	@cp file1 $(DESTDIR)/bin/

integration_tests/snaps/independent-parts/part2/Makefile

@@ -0,0 +1,5 @@
+all:
+
+install:
+	@mkdir -p $(DESTDIR)/bin
+	@cp file2 $(DESTDIR)/bin/

integration_tests/snaps/independent-parts/snapcraft.yaml

@@ -5,11 +5,9 @@ description: Two independent parts
 
 parts:
   part1:
-    plugin: copy
-    files:
-      file1: bin/file1
+    plugin: make
+    source: part1
 
   part2:
-    plugin: copy
-    files:
-      file2: bin/file2
+    plugin: make
+    source: part2

integration_tests/test_clean_build_step.py

@@ -0,0 +1,166 @@
+# -*- 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/>.
+
+import os
+
+from testtools.matchers import (
+    DirExists,
+    FileExists,
+    Not
+)
+
+import integration_tests
+
+
+class CleanBuildStepBuiltTestCase(integration_tests.TestCase):
+
+    def setUp(self):
+        super().setUp()
+
+        self.project_dir = 'independent-parts'
+        self.run_snapcraft('build', self.project_dir)
+        self.partsdir = os.path.join(self.project_dir, 'parts')
+        self.parts = {}
+        for part in ['part1', 'part2']:
+            partdir = os.path.join(self.partsdir, part)
+            self.parts[part] = {
+                'partdir': partdir,
+                'sourcedir': os.path.join(partdir, 'src'),
+                'builddir': os.path.join(partdir, 'build'),
+                'installdir': os.path.join(partdir, 'install'),
+                'bindir': os.path.join(partdir, 'install', 'bin'),
+            }
+
+    def assert_files_exist(self):
+        for d in ['builddir', 'bindir']:
+            self.assertThat(os.path.join(self.parts['part1'][d], 'file1'),
+                            FileExists())
+            self.assertThat(os.path.join(self.parts['part2'][d], 'file2'),
+                            FileExists())
+
+    def test_clean_build_step(self):
+        self.assert_files_exist()
+
+        self.run_snapcraft(['clean', '--step=build'], self.project_dir)
+
+        for part_name, part in self.parts.items():
+            self.assertThat(part['builddir'], Not(DirExists()))
+            self.assertThat(part['installdir'], Not(DirExists()))
+            self.assertThat(part['sourcedir'], DirExists())
+
+        # Now try to build again
+        self.run_snapcraft('build', self.project_dir)
+        self.assert_files_exist()
+
+    def test_clean_build_step_single_part(self):
+        self.assert_files_exist()
+
+        self.run_snapcraft(['clean', 'part1', '--step=build'],
+                           self.project_dir)
+        self.assertThat(self.parts['part1']['builddir'], Not(DirExists()))
+        self.assertThat(self.parts['part1']['installdir'], Not(DirExists()))
+        self.assertThat(self.parts['part1']['sourcedir'], DirExists())
+
+        self.assertThat(
+            os.path.join(self.parts['part2']['builddir'], 'file2'),
+            FileExists())
+        self.assertThat(
+            os.path.join(self.parts['part2']['bindir'], 'file2'),
+            FileExists())
+
+        # Now try to build again
+        self.run_snapcraft('build', self.project_dir)
+        self.assert_files_exist()
+
+
+class CleanBuildStepStrippedTestCase(integration_tests.TestCase):
+
+    def setUp(self):
+        super().setUp()
+
+        self.project_dir = 'independent-parts'
+        self.run_snapcraft('strip', self.project_dir)
+
+        self.snapdir = os.path.join(self.project_dir, 'snap')
+        self.snap_bindir = os.path.join(self.snapdir, 'bin')
+        self.stagedir = os.path.join(self.project_dir, 'stage')
+        self.stage_bindir = os.path.join(self.stagedir, 'bin')
+        self.partsdir = os.path.join(self.project_dir, 'parts')
+        self.parts = {}
+        for part in ['part1', 'part2']:
+            partdir = os.path.join(self.partsdir, part)
+            self.parts[part] = {
+                'partdir': partdir,
+                'sourcedir': os.path.join(partdir, 'src'),
+                'builddir': os.path.join(partdir, 'build'),
+                'installdir': os.path.join(partdir, 'install'),
+                'bindir': os.path.join(partdir, 'install', 'bin'),
+            }
+
+    def assert_files_exist(self):
+        for d in ['builddir', 'bindir']:
+            self.assertThat(os.path.join(self.parts['part1'][d], 'file1'),
+                            FileExists())
+            self.assertThat(os.path.join(self.parts['part2'][d], 'file2'),
+                            FileExists())
+
+        self.assertThat(os.path.join(self.snap_bindir, 'file1'), FileExists())
+        self.assertThat(os.path.join(self.snap_bindir, 'file2'), FileExists())
+        self.assertThat(os.path.join(self.stage_bindir, 'file1'), FileExists())
+        self.assertThat(os.path.join(self.stage_bindir, 'file2'), FileExists())
+
+    def test_clean_build_step(self):
+        self.assert_files_exist()
+
+        self.run_snapcraft(['clean', '--step=build'], self.project_dir)
+        self.assertThat(self.stagedir, Not(DirExists()))
+        self.assertThat(self.snapdir, Not(DirExists()))
+
+        for part_name, part in self.parts.items():
+            self.assertThat(part['builddir'], Not(DirExists()))
+            self.assertThat(part['installdir'], Not(DirExists()))
+            self.assertThat(part['sourcedir'], DirExists())
+
+        # Now try to strip again
+        self.run_snapcraft('strip', self.project_dir)
+        self.assert_files_exist()
+
+    def test_clean_build_step_single_part(self):
+        self.assert_files_exist()
+
+        self.run_snapcraft(['clean', 'part1', '--step=build'],
+                           self.project_dir)
+        self.assertThat(os.path.join(self.stage_bindir, 'file1'),
+                        Not(FileExists()))
+        self.assertThat(os.path.join(self.stage_bindir, 'file2'), FileExists())
+        self.assertThat(os.path.join(self.snap_bindir, 'file1'),
+                        Not(FileExists()))
+        self.assertThat(os.path.join(self.snap_bindir, 'file2'), FileExists())
+
+        self.assertThat(self.parts['part1']['builddir'], Not(DirExists()))
+        self.assertThat(self.parts['part1']['installdir'], Not(DirExists()))
+        self.assertThat(self.parts['part1']['sourcedir'], DirExists())
+
+        self.assertThat(
+            os.path.join(self.parts['part2']['builddir'], 'file2'),
+            FileExists())
+        self.assertThat(
+            os.path.join(self.parts['part2']['bindir'], 'file2'),
+            FileExists())
+
+        # Now try to strip again
+        self.run_snapcraft('strip', self.project_dir)
+        self.assert_files_exist()

integration_tests/test_clean_stage_step.py

@@ -35,23 +35,23 @@ def setUp(self):
         self.stagedir = os.path.join(self.project_dir, 'stage')
         self.bindir = os.path.join(self.stagedir, 'bin')
 
-    def verify_files_exist(self):
+    def assert_files_exist(self):
         self.assertThat(os.path.join(self.bindir, 'file1'), FileExists())
         self.assertThat(os.path.join(self.bindir, 'file2'), FileExists())
 
     def test_clean_stage_step(self):
-        self.verify_files_exist()
+        self.assert_files_exist()
 
         self.run_snapcraft(['clean', '--step=stage'], self.project_dir)
         self.assertThat(self.stagedir, Not(DirExists()))
         self.assertThat(os.path.join(self.project_dir, 'parts'), DirExists())
 
         # Now try to stage again
         self.run_snapcraft('stage', self.project_dir)
-        self.verify_files_exist()
+        self.assert_files_exist()
 
     def test_clean_stage_step_single_part(self):
-        self.verify_files_exist()
+        self.assert_files_exist()
 
         self.run_snapcraft(['clean', 'part1', '--step=stage'],
                            self.project_dir)
@@ -61,7 +61,7 @@ def test_clean_stage_step_single_part(self):
 
         # Now try to stage again
         self.run_snapcraft('stage', self.project_dir)
-        self.verify_files_exist()
+        self.assert_files_exist()
 
 
 class CleanStageStepStrippedTestCase(integration_tests.TestCase):
@@ -77,14 +77,14 @@ def setUp(self):
         self.stagedir = os.path.join(self.project_dir, 'stage')
         self.stage_bindir = os.path.join(self.stagedir, 'bin')
 
-    def verify_files_exist(self):
+    def assert_files_exist(self):
         self.assertThat(os.path.join(self.snap_bindir, 'file1'), FileExists())
         self.assertThat(os.path.join(self.snap_bindir, 'file2'), FileExists())
         self.assertThat(os.path.join(self.stage_bindir, 'file1'), FileExists())
         self.assertThat(os.path.join(self.stage_bindir, 'file2'), FileExists())
 
     def test_clean_stage_step(self):
-        self.verify_files_exist()
+        self.assert_files_exist()
 
         self.run_snapcraft(['clean', '--step=stage'], self.project_dir)
         self.assertThat(self.stagedir, Not(DirExists()))
@@ -93,10 +93,10 @@ def test_clean_stage_step(self):
 
         # Now try to strip again
         self.run_snapcraft('strip', self.project_dir)
-        self.verify_files_exist()
+        self.assert_files_exist()
 
     def test_clean_stage_step_single_part(self):
-        self.verify_files_exist()
+        self.assert_files_exist()
 
         self.run_snapcraft(['clean', 'part1', '--step=stage'],
                            self.project_dir)
@@ -110,4 +110,4 @@ def test_clean_stage_step_single_part(self):
 
         # Now try to strip again
         self.run_snapcraft('strip', self.project_dir)
-        self.verify_files_exist()
+        self.assert_files_exist()

snapcraft/__init__.py

@@ -214,6 +214,7 @@ def build(self):
         Override this method if you need to process the source code to make it
         runnable.
         """
+
         if os.path.exists(self.build_basedir):
             shutil.rmtree(self.build_basedir)
 
@@ -222,6 +223,20 @@ def build(self):
             ignore=lambda d, s: common.SNAPCRAFT_FILES
             if d is self.sourcedir else [])
 
+    def clean_build(self):
+        """Clean the artifacts that resulted from building this part.
+
+        The base implementation simply removes the build_basedir and
+        installdir. Override this method if your build process was more
+        involved and needs more cleaning.
+        """
+
+        if os.path.exists(self.build_basedir):
+            shutil.rmtree(self.build_basedir)
+
+        if os.path.exists(self.installdir):
+            shutil.rmtree(self.installdir)
+
     def snap_fileset(self):
         """Return a list of files to include or exclude in the resulting snap
 

snapcraft/pluginhandler.py

@@ -85,6 +85,26 @@ def __eq__(self, other):
         return False
 
 
+class PullState(yaml.YAMLObject):
+    yaml_tag = u'!PullState'
+
+    def __init__(self, stage_package_files, stage_package_directories):
+        self.stage_package_files = stage_package_files
+        self.stage_package_directories = stage_package_directories
+
+    def __repr__(self):
+        return ('{}(stage-package-files: {}, '
+                'stage-package-directories: {})').format(
+            self.__class__, self.stage_package_files,
+            self.stage_packages_directories)
+
+    def __eq__(self, other):
+        if type(other) is type(self):
+            return self.__dict__ == other.__dict__
+
+        return False
+
+
 class PluginHandler:
 
     @property
@@ -100,6 +120,7 @@ def __init__(self, plugin_name, part_name, properties):
         self.code = None
         self.config = {}
         self._name = part_name
+        self._ubuntu = None
         self.deps = []
 
         self.stagedir = os.path.join(os.getcwd(), 'stage')
@@ -229,20 +250,27 @@ def _setup_stage_packages(self):
         ubuntu.get(self.code.stage_packages)
         ubuntu.unpack(self.installdir)
 
-        snap_files, snap_dirs = self.migratable_fileset_for('stage')
-        _migrate_files(snap_files, snap_dirs, self.code.installdir,
+        package_files, package_dirs = self.migratable_fileset_for('stage')
+        _migrate_files(package_files, package_dirs, self.code.installdir,
                        self.stagedir, missing_ok=True)
 
+        return (package_files, package_dirs)
+
     def pull(self, force=False):
         if not self.should_step_run('pull', force):
             self.notify_stage('Skipping pull', ' (already ran)')
             return
         self.makedirs()
         self.notify_stage('Pulling')
+        package_files = set()
+        package_directories = set()
         if self.code.stage_packages:
-            self._setup_stage_packages()
+            package_files, package_directories = self._setup_stage_packages()
         self.code.pull()
-        self.mark_done('pull')
+
+        # Record the files and directories unpacked from the stage packages
+        state = PullState(package_files, package_directories)
+        self.mark_done('pull', state)
 
     def clean_pull(self):
         raise NotImplementedError(
@@ -255,13 +283,32 @@ def build(self, force=False):
             return
         self.makedirs()
         self.notify_stage('Building')
+        if self.code.stage_packages:
+            # Stage packages were already fetched and unpacked in pull(), but
+            # they need to be unpacked into the stage directory again in case
+            # it's been cleaned.
+            state = self.get_state('pull')
+            try:
+                _migrate_files(
+                    state.stage_package_files,
+                    state.stage_package_directories, self.code.installdir,
+                    self.stagedir, missing_ok=True)
+            except AttributeError:
+                raise MissingState(
+                    "Failed to build: Missing necessary pull state. "
+                    "Please run pull again.")
         self.code.build()
         self.mark_done('build')
 
     def clean_build(self):
-        raise NotImplementedError(
-            "Cleaning up step 'build' for part {!r} is not yet "
-            "supported".format(self.name))
+        state_file = self._step_state_file('build')
+        if not os.path.isfile(state_file):
+            self.notify_stage('Skipping cleaning build for', '(already clean)')
+            return
+
+        self.notify_stage('Cleaning build for')
+        self.code.clean_build()
+        self.mark_cleaned('build')
 
     def migratable_fileset_for(self, step):
         plugin_fileset = self.code.snap_fileset()