Merge branch 'baserock/richardmaw/S8694/edit-no-write-defaults-v3'

Reviewed-by: Lars Wirzenius
Reviewed-by: Jonathan Maw

4 files changed, 104 insertions, 166 deletions

diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py
index 70f46064..c94078f9 100644
--- a/morphlib/morphloader.py
+++ b/morphlib/morphloader.py
@@ -332,8 +332,22 @@ class MorphologyLoader(object):
             self._set_stratum_defaults(morphology)
         elif kind == 'chunk':
             self._set_chunk_defaults(morphology)
-        else:
-            assert kind == 'cluster'
+
+    def unset_defaults(self, morphology):
+        '''If a field is equal to its default, delete it.
+
+        The morphology is assumed to be valid.
+
+        '''
+
+        kind = morphology['kind']
+        defaults = self._static_defaults[kind]
+        for key in defaults:
+            if key in morphology and morphology[key] == defaults[key]:
+                del morphology[key]
+
+        if kind == 'stratum':
+            self._unset_stratum_defaults(morphology)
 
     def _set_system_defaults(self, morph):
         pass
@@ -345,6 +359,13 @@ class MorphologyLoader(object):
             if 'morph' not in spec:
                 spec['morph'] = spec['name']
 
+    def _unset_stratum_defaults(self, morph):
+        for spec in morph['chunks']:
+            if 'repo' in spec and spec['repo'] == spec['name']:
+                del spec['repo']
+            if 'morph' in spec and spec['morph'] == spec['name']:
+                del spec['morph']
+
     def _set_chunk_defaults(self, morph):
         if morph['max-jobs'] is not None:
             morph['max-jobs'] = int(morph['max-jobs'])
diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py
index 6e957510..ac0fef53 100644
--- a/morphlib/morphloader_tests.py
+++ b/morphlib/morphloader_tests.py
@@ -385,6 +385,20 @@ name: foo
                 'max-jobs': None,
             })
 
+    def test_unsets_defaults_for_chunks(self):
+        m = morphlib.morph3.Morphology({
+            'kind': 'chunk',
+            'name': 'foo',
+            'build-system': 'manual',
+        })
+        self.loader.unset_defaults(m)
+        self.assertEqual(
+            dict(m),
+            {
+                'kind': 'chunk',
+                'name': 'foo',
+            })
+
     def test_sets_defaults_for_strata(self):
         m = morphlib.morph3.Morphology({
             'kind': 'stratum',
@@ -421,6 +435,27 @@ name: foo
                 ],
             })
 
+    def test_unsets_defaults_for_strata(self):
+        test_dict = {
+            'kind': 'stratum',
+            'name': 'foo',
+            'chunks': [
+                {
+                    'name': 'bar',
+                    "ref": "bar",
+                    'build-mode': 'bootstrap',
+                    'build-depends': [],
+                },
+            ],
+        }
+        test_dict_with_build_depends = dict(test_dict)
+        test_dict_with_build_depends["build-depends"] = []
+        m = morphlib.morph3.Morphology(test_dict_with_build_depends)
+        self.loader.unset_defaults(m)
+        self.assertEqual(
+            dict(m),
+            test_dict)
+
     def test_sets_defaults_for_system(self):
         m = morphlib.morph3.Morphology({
             'kind': 'system',
@@ -442,6 +477,22 @@ name: foo
                 'disk-size': '1G',
             })
 
+    def test_unsets_defaults_for_system(self):
+        m = morphlib.morph3.Morphology({
+            'kind': 'system',
+            'name': 'foo',
+            'arch': 'x86_64',
+            'strata': [],
+        })
+        self.loader.unset_defaults(m)
+        self.assertEqual(
+            dict(m),
+            {
+                'kind': 'system',
+                'name': 'foo',
+                'arch': 'x86_64',
+            })
+
     def test_sets_stratum_chunks_repo_and_morph_from_name(self):
         m = morphlib.morph3.Morphology(
             {
@@ -461,6 +512,26 @@ name: foo
         self.assertEqual(m['chunks'][0]['repo'], 'le-chunk')
         self.assertEqual(m['chunks'][0]['morph'], 'le-chunk')
 
+    def test_collapses_stratum_chunks_repo_and_morph_from_name(self):
+        m = morphlib.morph3.Morphology(
+            {
+                "name": "foo",
+                "kind": "stratum",
+                "chunks": [
+                    {
+                        "name": "le-chunk",
+                        "repo": "le-chunk",
+                        "morph": "le-chunk",
+                        "ref": "ref",
+                        "build-depends": [],
+                    }
+                ]
+            })
+
+        self.loader.unset_defaults(m)
+        self.assertTrue('repo' not in m['chunks'][0])
+        self.assertTrue('morph' not in m['chunks'][0])
+
     def test_convertes_max_jobs_to_an_integer(self):
         m = morphlib.morph3.Morphology(
             {
diff --git a/morphlib/plugins/branch_and_merge_new_plugin.py b/morphlib/plugins/branch_and_merge_new_plugin.py
index 9dd9c915..7c326e48 100644
--- a/morphlib/plugins/branch_and_merge_new_plugin.py
+++ b/morphlib/plugins/branch_and_merge_new_plugin.py
@@ -42,7 +42,7 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
         self.app.add_subcommand(
             'branch', self.branch, arg_synopsis='REPO NEW [OLD]')
         self.app.add_subcommand(
-            'new-edit', self.edit, arg_synopsis='SYSTEM STRATUM [CHUNK]')
+            'edit', self.edit, arg_synopsis='SYSTEM STRATUM [CHUNK]')
         self.app.add_subcommand(
             'show-system-branch', self.show_system_branch, arg_synopsis='')
         self.app.add_subcommand(
@@ -235,6 +235,7 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
                 logging.debug(
                     'Saving morphology: %s %s %s' %
                         (morph.repo_url, morph.ref, morph.filename))
+                loader.unset_defaults(morph)
                 loader.save_to_file(
                     sb.get_filename(morph.repo_url, morph.filename), morph)
                 morph.dirty = False
@@ -395,6 +396,11 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
 
         '''
 
+        if len(args) not in (2, 3):
+            raise cliapp.AppException('morph edit needs the names of a system,'
+                                      ' a stratum and optionally a chunk'
+                                      ' as parameters')
+
         system_name = args[0]
         stratum_name = args[1]
         chunk_name = args[2] if len(args) == 3 else None
diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py
index 53b94859..85e74501 100644
--- a/morphlib/plugins/branch_and_merge_plugin.py
+++ b/morphlib/plugins/branch_and_merge_plugin.py
@@ -57,15 +57,10 @@ class BranchAndMergePlugin(cliapp.Plugin):
 
     def enable(self):
         # User-facing commands
-        self.app.add_subcommand('old-init', self.init, arg_synopsis='[DIR]')
-        self.app.add_subcommand('old-branch', self.branch,
-                                arg_synopsis='REPO NEW [OLD]')
-        self.app.add_subcommand('old-checkout', self.checkout,
-                                arg_synopsis='REPO BRANCH')
         self.app.add_subcommand('merge', self.merge,
                                 arg_synopsis='BRANCH')
-        self.app.add_subcommand('edit', self.edit,
-                                arg_synopsis='SYSTEM STRATUM [CHUNK]')
+#        self.app.add_subcommand('edit', self.edit,
+#                                arg_synopsis='SYSTEM STRATUM [CHUNK]')
         self.app.add_subcommand('petrify', self.petrify)
         self.app.add_subcommand('unpetrify', self.unpetrify)
         self.app.add_subcommand(
@@ -87,15 +82,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
         self.app.add_subcommand('foreach', self.foreach,
                                 arg_synopsis='-- COMMAND [ARGS...]')
 
-        # Plumbing commands (FIXME: should be hidden from --help by default)
-        self.app.add_subcommand('old-workspace', self.workspace,
-                                arg_synopsis='')
-        self.app.add_subcommand(
-            'old-show-system-branch', self.show_system_branch,
-            arg_synopsis='')
-        self.app.add_subcommand('old-show-branch-root', self.show_branch_root,
-                                arg_synopsis='')
-
     def disable(self):
         pass
 
@@ -367,7 +353,7 @@ class BranchAndMergePlugin(cliapp.Plugin):
             'cluster': [
                 'name',
                 'systems',
-            ]
+            ],
         }
         
         also_known = {
@@ -555,50 +541,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
 
         return system_key, metadata_cache_id_lookup
 
-    def init(self, args):
-        '''Initialize a workspace directory.
-
-        Command line argument:
-
-        * `DIR` is the directory to use as a workspace, and defaults to
-          the current directory.
-
-        This creates a workspace, either in the current working directory,
-        or if `DIR` is given, in that directory. If the directory doesn't
-        exist, it is created. If it does exist, it must be empty.
-
-        You need to run `morph init` to initialise a workspace, or none
-        of the other system branching tools will work: they all assume
-        an existing workspace. Note that a workspace only exists on your
-        machine, not on the git server.
-
-        Example:
-
-            morph init /src/workspace
-            cd /src/workspace
-
-        '''
-
-        if not args:
-            args = ['.']
-        elif len(args) > 1:
-            raise cliapp.AppException('init must get at most one argument')
-
-        dirname = args[0]
-
-        # verify the workspace is empty (and thus, can be used) or
-        # create it if it doesn't exist yet
-        if os.path.exists(dirname):
-            if os.listdir(dirname) != []:
-                raise cliapp.AppException('can only initialize empty '
-                                          'directory as a workspace: %s' %
-                                          dirname)
-        else:
-            os.makedirs(dirname)
-
-        os.mkdir(os.path.join(dirname, '.morph'))
-        self.app.status(msg='Initialized morph workspace', chatty=True)
-
     def _create_branch(self, workspace, branch_name, repo, original_ref):
         '''Create a branch called branch_name based off original_ref.
            
@@ -638,81 +580,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
             self.remove_branch_dir_safe(workspace, branch_name)
             raise
 
-    @warns_git_identity
-    def branch(self, args):
-        '''Create a new system branch.
-
-        Command line arguments:
-
-        * `REPO` is a repository URL.
-        * `NEW` is the name of the new system branch.
-        * `OLD` is the point from which to branch, and defaults to `master`.
-
-        This creates a new system branch. It needs to be run in an
-        existing workspace (see `morph workspace`). It creates a new
-        git branch in the clone of the repository in the workspace. The
-        system branch will not be visible on the git server until you
-        push your changes to the repository.
-
-        Example:
-
-            cd /src/workspace
-            morph branch baserock:baserock:morphs jrandom/new-feature
-
-        '''
-
-        if len(args) not in [2, 3]:
-            raise cliapp.AppException('morph branch needs name of branch '
-                                      'as parameter')
-
-        repo = args[0]
-        new_branch = args[1]
-        commit = 'master' if len(args) == 2 else args[2]
-
-        self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app)
-        if self.get_cached_repo(repo).ref_exists(new_branch):
-            raise cliapp.AppException('branch %s already exists in '
-                                      'repository %s' % (new_branch, repo))
-
-        # Create the system branch directory.
-        workspace = self.deduce_workspace()
-        self._create_branch(workspace, new_branch, repo, commit)
-
-    @warns_git_identity
-    def checkout(self, args):
-        '''Check out an existing system branch.
-
-        Command line arguments:
-
-        * `REPO` is the URL to the repository to the root repository of
-          a system branch.
-        * `BRANCH` is the name of the system branch.
-
-        This will check out an existing system branch to an existing
-        workspace.  You must create the workspace first. This only checks
-        out the root repository, not the repositories for individual
-        components. You need to use `morph edit` to check out those.
-
-        Example:
-
-            cd /src/workspace
-            morph checkout baserock:baserock/morphs master
-
-        '''
-
-        if len(args) != 2:
-            raise cliapp.AppException('morph checkout needs a repo and the '
-                                      'name of a branch as parameters')
-
-        repo = args[0]
-        system_branch = args[1]
-
-        self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app)
-
-        # Create the system branch directory.
-        workspace = self.deduce_workspace()
-        self._create_branch(workspace, system_branch, repo, system_branch)
-
     def checkout_repository(self, branch_dir, repo, ref, parent_ref=None):
         '''Make a chunk or stratum repository available for a system branch
 
@@ -2052,30 +1919,3 @@ class BranchAndMergePlugin(cliapp.Plugin):
                 raise cliapp.AppException(
                     'Command failed at repo %s: %s' % (repo, ' '.join(args)))
 
-    def workspace(self, args):
-        '''Show the toplevel directory of the current workspace.'''
-
-        self.app.output.write('%s\n' % self.deduce_workspace())
-
-    def show_system_branch(self, args):
-        '''Show the name of the current system branch.'''
-
-        branch, dirname = self.deduce_system_branch()
-        self.app.output.write('%s\n' % branch)
-
-    def show_branch_root(self, args):
-        '''Show the name of the repository holding the system morphologies.
-
-        This would, for example, write out something like:
-
-            /src/ws/master/baserock:baserock/morphs
-
-        when the master branch of the `baserock:baserock/morphs`
-        repository is checked out.
-
-        '''
-
-        workspace = self.deduce_workspace()
-        system_branch, branch_dir = self.deduce_system_branch()
-        branch_root = self.get_branch_config(branch_dir, 'branch.root')
-        self.app.output.write('%s\n' % branch_root)