Merge lp:~jelmer/bzr-builddeb/build-type-enum into lp:bzr-builddeb

[Jelmer Vernooij]

This changes bzr-builddeb to use a "build_type" enum internally, rather than a tuple with three booleans ("split", "merge", "native").

This simplifies the code a bit, it makes the automatic detection of native packages better (looks at the debian revision if there was a previous upload for this distroseries) and should make it easier to fix bug 586617.

[Andrew Bennetts (spiv)]

208 +BUILD_TYPE_NORMAL = 0
209 +BUILD_TYPE_NATIVE = 1
210 +BUILD_TYPE_MERGE = 2
211 +BUILD_TYPE_SPLIT = 3

In my experience it's nicer to use string constants for enums (e.g. 'NORMAL') rather than integers. This makes debugging much more pleasant.

[lp:~jelmer/bzr-builddeb/build-type-enum]

489. By Jelmer Vernooij on 2011-01-12

Use string constants for enum, per spiv's suggestion.

[Jelmer Vernooij (jelmer)]

On Wed, 2011-01-12 at 20:00 +0000, Andrew Bennetts wrote:
> In my experience it's nicer to use string constants for enums (e.g. 'NORMAL') rather than integers. This makes debugging much more pleasant.
Thanks, that's a useful suggestion. Fixed!

Cheers,

Jelmer

=== modified file '__init__.py'
--- __init__.py	2011-01-10 22:44:18 +0000
+++ __init__.py	2011-01-12 21:33:12 +0000
@@ -135,6 +135,7 @@
 
 
 def debian_tag_name(branch, revid):
+    from bzrlib.plugins.builddeb.config import BUILD_TYPE_MERGE
     from bzrlib.plugins.builddeb.errors import MissingChangelogError
     from bzrlib.plugins.builddeb.import_dsc import (DistributionBranch,
         DistributionBranchSet)
@@ -142,7 +143,7 @@
     t = branch.repository.revision_tree(revid)
     config = debuild_config(t, False)
     try:
-        (changelog, larstiq) = find_changelog(t, config.merge)
+        (changelog, larstiq) = find_changelog(t, config.build_type == BUILD_TYPE_MERGE)
     except MissingChangelogError:
         # Not a debian package
         return None

=== modified file 'cmds.py'
--- cmds.py	2010-11-17 16:53:31 +0000
+++ cmds.py	2011-01-12 21:33:12 +0000
@@ -61,7 +61,16 @@
 from bzrlib.plugins.builddeb.builder import (
                      DebBuild,
                      )
-from bzrlib.plugins.builddeb.errors import BuildFailedError
+from bzrlib.plugins.builddeb.config import (
+    BUILD_TYPE_MERGE,
+    BUILD_TYPE_NATIVE,
+    BUILD_TYPE_NORMAL,
+    BUILD_TYPE_SPLIT,
+    )
+from bzrlib.plugins.builddeb.errors import (
+    BuildFailedError,
+    NoPreviousUpload,
+    )
 from bzrlib.plugins.builddeb.hooks import run_hook
 from bzrlib.plugins.builddeb.import_dsc import (
         DistributionBranch,
@@ -222,25 +231,14 @@
             working_tree = False
         return tree, working_tree
 
-    def _build_type(self, config, merge, native, split):
-        if not merge:
-            merge = config.merge
+    def _build_type(self, merge, native, split):
         if merge:
-            note("Running in merge mode")
-            native = False
-            split = False
-        else:
-            if not native:
-                native = config.native
-            if native:
-                note("Running in native mode")
-                split = False
-            else:
-                if not split:
-                    split = config.split
-                if split:
-                    note("Running in split mode")
-        return merge, native, split
+            return BUILD_TYPE_MERGE
+        if native:
+            return BUILD_TYPE_NATIVE
+        if split:
+            return BUILD_TYPE_SPLIT
+        return None
 
     def _get_build_command(self, config, builder, quick, build_options):
         if builder is None:
@@ -327,10 +325,10 @@
     def run(self, branch_or_build_options_list=None, verbose=False,
             working_tree=False,
             export_only=False, dont_purge=False, use_existing=False,
-            result_dir=None, builder=None, merge=False, build_dir=None,
+            result_dir=None, builder=None, merge=None, build_dir=None,
             export_upstream=None, export_upstream_revision=None,
             orig_dir=None, split=None,
-            quick=False, reuse=False, native=False,
+            quick=False, reuse=False, native=None,
             source=False, revision=None, result=None, package_merge=None):
         if result is not None:
             warning("--result is deprected, use --result-dir instead")
@@ -351,14 +349,32 @@
                 note("Reusing existing build dir")
                 dont_purge = True
                 use_existing = True
-            merge, native, split = self._build_type(config, merge, native, split)
-            if (not merge and not native and not split and
-                not tree_contains_upstream_source(tree)):
-                # Default to merge mode if there's only a debian/ directory
-                merge = True
-            (changelog, larstiq) = find_changelog(tree, merge)
+            build_type = self._build_type(merge, native, split)
+            if build_type is None:
+                build_type = config.build_type
+            contains_upstream_source = tree_contains_upstream_source(tree)
+            (changelog, larstiq) = find_changelog(tree, not contains_upstream_source)
+            try:
+                prev_version = find_previous_upload(tree, not contains_upstream_source)
+            except NoPreviousUpload:
+                prev_version = None
+            if build_type is None:
+                if prev_version and not prev_version.debian_revision:
+                    # If the package doesn't have a debian revision, assume it's native.
+                    build_type = BUILD_TYPE_NATIVE
+                elif not contains_upstream_source:
+                    # Default to merge mode if there's only a debian/ directory
+                    build_type = BUILD_TYPE_MERGE
+                else:
+                    build_type = BUILD_TYPE_NORMAL
+
+            note("Building package in %s mode" % {
+                BUILD_TYPE_NATIVE: "native",
+                BUILD_TYPE_MERGE: "merge",
+                BUILD_TYPE_SPLIT: "split",
+                BUILD_TYPE_NORMAL: "normal"}[build_type])
+
             if package_merge:
-                prev_version = find_previous_upload(tree, merge)
                 build_options.append("-v%s" % str(prev_version))
                 if (prev_version.upstream_version
                         != changelog.version.upstream_version
@@ -370,7 +386,7 @@
                     is_local, result_dir or result, build_dir, orig_dir)
 
             upstream_sources = [
-                PristineTarSource(tree, branch), 
+                PristineTarSource(tree, branch),
                 AptSource(),
                 ]
             if merge:
@@ -689,10 +705,10 @@
                         "working tree. You must commit before using this "
                         "command.")
             config = debuild_config(tree, tree)
-            if config.merge:
+            if config.build_type == BUILD_TYPE_MERGE:
                 raise BzrCommandError("Merge upstream in merge mode is not "
                         "yet supported.")
-            if config.native:
+            if config.build_type == BUILD_TYPE_NATIVE:
                 raise BzrCommandError("Merge upstream in native mode is not "
                         "yet supported.")
 
@@ -803,7 +819,7 @@
                                       "source package to install, or use the "
                                       "--file option.")
             config = debuild_config(tree, tree)
-            if config.merge:
+            if config.build_type == BUILD_TYPE_MERGE:
                 raise BzrCommandError("import-dsc in merge mode is not "
                         "yet supported.")
             orig_dir = config.orig_dir or default_orig_dir
@@ -959,8 +975,7 @@
     def run(self, command_list=None):
         t = WorkingTree.open_containing('.')[0]
         config = debuild_config(t, t)
-
-        if not config.merge:
+        if config.build_type != BUILD_TYPE_MERGE:
             raise BzrCommandError("This command only works for merge mode "
                                   "packages. See /usr/share/doc/bzr-builddeb"
                                   "/user_manual/merge.html for more information.")
@@ -1040,7 +1055,7 @@
 
     takes_options = [merge_opt, force]
 
-    def run(self, merge=False, force=None):
+    def run(self, merge=None, force=None):
         t = WorkingTree.open_containing('.')[0]
         t.lock_write()
         try:
@@ -1049,8 +1064,8 @@
                       "working tree. You must commit before using this "
                       "command")
             config = debuild_config(t, t)
-            if not merge:
-                merge = config.merge
+            if merge is None:
+                merge = (config.build_type == BUILD_TYPE_MERGE)
             (changelog, larstiq) = find_changelog(t, merge)
             if changelog.distributions == 'UNRELEASED':
                 if not force:
@@ -1104,7 +1119,8 @@
         this_config = debuild_config(tree, tree)
         that_config = debuild_config(source_branch.basis_tree(),
                 source_branch.basis_tree())
-        if not (this_config.native or that_config.native):
+        if not (this_config.build_type == BUILD_TYPE_NATIVE or
+                that_config.build_type == BUILD_TYPE_NATIVE):
             fix_ancestry_as_needed(tree, source_branch)
 
         # Merge source packaging branch in to the target packaging branch.

=== modified file 'config.py'
--- config.py	2010-02-10 13:59:07 +0000
+++ config.py	2011-01-12 21:33:12 +0000
@@ -26,6 +26,12 @@
   from configobj import ParseError
 
 
+BUILD_TYPE_NORMAL = "normal"
+BUILD_TYPE_NATIVE = "native"
+BUILD_TYPE_MERGE = "merge"
+BUILD_TYPE_SPLIT = "split"
+
+
 class SvnBuildPackageMappedConfig(object):
   """Config object that provides a bzr-builddeb configuration 
   based on a svn-buildpackage configuration.
@@ -257,6 +263,17 @@
 
   merge = _bool_property('merge', "Run in merge mode")
 
+  @property
+  def build_type(self):
+    if self.merge:
+      return BUILD_TYPE_MERGE
+    elif self.native:
+      return BUILD_TYPE_NATIVE
+    elif self.split:
+      return BUILD_TYPE_SPLIT
+    else:
+      return BUILD_TYPE_NORMAL
+
   quick_builder = _opt_property('quick-builder',
                           "A quick command to build with", True)
 

=== modified file 'tests/test_config.py'
--- tests/test_config.py	2009-03-04 13:25:53 +0000
+++ tests/test_config.py	2011-01-12 21:33:12 +0000
@@ -21,7 +21,11 @@
 from bzrlib.branch import Branch
 from bzrlib.tests import TestCaseWithTransport
 
-from bzrlib.plugins.builddeb.config import DebBuildConfig
+from bzrlib.plugins.builddeb.config import (
+    BUILD_TYPE_MERGE,
+    BUILD_TYPE_NORMAL,
+    DebBuildConfig,
+    )
 
 
 class DebBuildConfigTests(TestCaseWithTransport):
@@ -75,6 +79,7 @@
 
   def test_no_entry(self):
     self.assertEqual(self.config.merge, False)
+    self.assertEqual(self.config.build_type, BUILD_TYPE_NORMAL)
 
   def test_parse_error(self):
     f = open('invalid.conf', 'wb')
@@ -107,6 +112,7 @@
 
       cfg = DebBuildConfig([], tree=Branch.open(repos_url).basis_tree())
       self.assertEquals(True, cfg.merge)
+      self.assertEquals(BUILD_TYPE_MERGE, cfg.build_type)
       self.assertEquals("someorigdir", cfg.orig_dir)
 
     def test_from_svn_layout_file(self):

=== modified file 'util.py'
--- util.py	2010-10-30 15:17:44 +0000
+++ util.py	2011-01-12 21:33:12 +0000
@@ -140,7 +140,7 @@
     try:
         if not t.has_filename(changelog_file):
             if merge:
-                #Assume LarstiQ's layout (.bzr in debian/)
+                # Assume LarstiQ's layout (.bzr in debian/)
                 changelog_file = 'changelog'
                 larstiq = True
                 if not t.has_filename(changelog_file):
@@ -606,6 +606,9 @@
     :return: Boolean indicating whether or not the tree contains the upstream
         source
     """
-    present_files = set(tree.inventory.root.children.keys())
+    root = tree.inventory.root
+    if root is None:
+        return False # Empty tree
+    present_files = set(root.children.keys())
     packaging_files = frozenset(["debian", ".bzr-builddeb"])
     return (len(present_files - packaging_files) > 0)