dch: make automatic adding of new sections more robust

This code that determined if we found a snapshot header was obuscated by
the code that determines the commits to add. Split those and better
document their purpose. Also always return the commit to start from so
we don't need to repeat the logic in the uper levels.

2 files changed, 111 insertions, 19 deletions

diff --git a/gbp/scripts/dch.py b/gbp/scripts/dch.py
index 9b73b24..eb4de5a 100644
--- a/gbp/scripts/dch.py
+++ b/gbp/scripts/dch.py
@@ -1,6 +1,6 @@
 # vim: set fileencoding=utf-8 :
 #
-# (C) 2007, 2008, 2009, 2010 Guido Guenther <agx@sigxcpu.org>
+# (C) 2007, 2008, 2009, 2010, 2013 Guido Guenther <agx@sigxcpu.org>
 #    This program is free software; you can redistribute it and/or modify
 #    it under the terms of the GNU General Public License as published by
 #    the Free Software Foundation; either version 2 of the License, or
@@ -192,28 +192,46 @@ def parse_commit(repo, commitid, opts, last_commit=False):
     return entry, (author, email)
 
 
-def guess_snapshot_commit(cp, repo, options):
+def guess_documented_commit(cp, repo, tagformat):
     """
-    Guess the last commit documented in the changelog from the snapshot banner
-    or the last point the changelog was touched.
+    Guess the last commit documented in the changelog from the snapshot banner,
+    the last tagged version or the last point the changelog was touched.
+
+    @param cp: the changelog
+    @param repo: the git repository
+    @param tagformat: the format for Debian tags
+    @returns: the commit that was last documented in the changelog
+    @rtype: C{str}
+    @raises GbpError: In case we fail to find a commit to start at
     """
+    # Check for snapshot banner
     sr = re.search(snapshot_re, cp['Changes'])
     if sr:
         return sr.group('commit')
-    # If the current topmost changelog entry has already been tagged rely on
-    # the version information only. The upper level relies then on the version
-    # info anyway:
-    if repo.find_version(options.debian_tag, cp.version):
-        return None
-    # If we didn't find a snapshot header we look at the point the changelog
-    # was last touched.
+
+    # Check if the latest version in the changelog is already tagged. If
+    # so this is the last documented commit.
+    commit = repo.find_version(tagformat, cp.version)
+    if commit:
+        gbp.log.info("Found tag for topmost changelog version '%s'" % commit)
+        return commit
+
+    # Check when the changelog was last touched
     last = repo.get_commits(paths="debian/changelog", num=1)
     if last:
         gbp.log.info("Changelog last touched at '%s'" % last[0])
         return last[0]
+
+    # Changelog not touched yet
     return None
 
 
+def has_snapshot_banner(cp):
+    """Whether the changelog has a snapshot banner"""
+    sr = re.search(snapshot_re, cp['Changes'])
+    return True if sr else False
+
+
 def get_customizations(customization_file):
     if customization_file:
         execfile(customization_file,
@@ -269,7 +287,7 @@ def main(argv):
     ret = 0
     changelog = 'debian/changelog'
     until = 'HEAD'
-    found_snapshot_header = False
+    found_snapshot_banner = False
     version_change = {}
     branch = None
 
@@ -386,13 +404,14 @@ def main(argv):
         else:
             since = ''
             if options.auto:
-                since = guess_snapshot_commit(cp, repo, options)
+                since = guess_documented_commit(cp, repo, options.debian_tag)
                 if since:
-                    gbp.log.info("Continuing from commit '%s'" % since)
-                    found_snapshot_header = True
+                    msg = "Continuing from commit '%s'" % since
                 else:
-                    gbp.log.info("Couldn't find snapshot header, using version info")
-            if not since:
+                    msg = "Starting from first commit"
+                gbp.log.info(msg)
+                found_snapshot_banner = has_snapshot_banner(cp)
+            else: # Fallback: continue from last tag
                 since = repo.find_version(options.debian_tag, cp['Version'])
                 if not since:
                     raise GbpError("Version %s not found" % cp['Version'])
@@ -420,10 +439,10 @@ def main(argv):
                 version_change['version'] = options.new_version
             # the user wants to force a new version
             add_section = True
-        elif cp['Distribution'] != "UNRELEASED" and not found_snapshot_header and commits:
+        elif cp['Distribution'] != "UNRELEASED" and not found_snapshot_banner and commits:
             # the last version was a release and we have pending commits
             add_section = True
-        elif options.snapshot and not found_snapshot_header:
+        elif options.snapshot and not found_snapshot_banner:
             # the user want to switch to snapshot mode
             add_section = True
         else:
diff --git a/tests/17_test_dch_guess_documented_commit.py b/tests/17_test_dch_guess_documented_commit.py
new file mode 100644
index 0000000..89eba2d
--- /dev/null
+++ b/tests/17_test_dch_guess_documented_commit.py
@@ -0,0 +1,73 @@
+# vim: set fileencoding=utf-8 :
+
+"""Test L{Changelog}'s guess_version_from_upstream"""
+
+from . import context
+
+import testutils
+
+from gbp.scripts import dch
+
+class TestGuessDocumentedCommit(testutils.DebianGitTestRepo):
+    def setUp(self):
+        self.version = '1.0-1'
+        self.tagformat = 'debian/%(version)s'
+
+        testutils.DebianGitTestRepo.setUp(self)
+
+    def test_01_from_snapshot_banner(self):
+        """
+        Guess the commit to start from from the snapshot banner
+        """
+        cp = testutils.MockedChangeLog(self.version,
+                             "*** SNAPSHOT build @12345 ***")
+        guessed_commit = dch.guess_documented_commit(cp, None, None)
+        self.assertEqual(guessed_commit, '12345')
+
+    def test_02_from_tag(self):
+        """
+        Guess the commit to start from from the tag matching
+        the topmost version in the changelog
+        """
+        cp = testutils.MockedChangeLog(self.version)
+
+        self.add_file('doesnot', 'matter')
+        tag = self.repo.version_to_tag(self.tagformat,
+                                       self.version)
+        self.repo.create_tag(name=tag,
+                             msg="Debian release %s" % self.version,
+                             sign=False)
+        commit = self.repo.rev_parse('%s^0' % tag)
+        guessed_commit = dch.guess_documented_commit(cp,
+                                                     self.repo,
+                                                     self.tagformat)
+        self.assertEqual(guessed_commit, commit)
+
+    def test_03_from_changelog_commit(self):
+        """
+        Guess the commit to start from from the commit that
+        last touched the changelog
+        """
+        cp = testutils.MockedChangeLog(self.version)
+
+        self.add_file('debian/changelog', 'foo')
+        commit = self.repo.head
+        self.add_file('doesnot', 'matter')
+        guessed_commit = dch.guess_documented_commit(cp,
+                                                     self.repo,
+                                                     self.tagformat)
+        self.assertEqual(guessed_commit, commit)
+
+    def test_04_not_touched(self):
+        """
+        None of the above matched so we want to start from
+        the beginning of history
+        """
+        cp = testutils.MockedChangeLog(self.version)
+
+        self.add_file('doesnot', 'matter')
+        self.add_file('doesnot', 'mattereither')
+        guessed_commit = dch.guess_documented_commit(cp,
+                                                     self.repo,
+                                                     self.tagformat)
+        self.assertIsNone(guessed_commit)