Fix retrieval of commit data and most recent tag.

git._iter_log_online() returns None in some cases for instance
when SKIP_WRITE_GIT_CHANGELOG is set to true. This is fixed by
returning an empty list in _iter_log_online.

Also the check if a changelog based on git should be written
was moved from git._iter_log_online to git.write_git_changelog
which makes more sense since some functions were calling
git._iter_log_online to get the changelog for other reasons than
writing it.

Additionally a unittest was added to check that setting the
environment variable SKIP_WRITE_GIT_CHANGELOG to true does not
break anything when retrieving the git version.

Change-Id: Ib12df23ab25b290dd394f9cb1456b8d5da57306a
Closes-Bug: 1467440

3 files changed, 23 insertions, 11 deletions

diff --git a/pbr/git.py b/pbr/git.py
index b4ae300..fd1686c 100644
--- a/pbr/git.py
+++ b/pbr/git.py
@@ -162,7 +162,7 @@ def _iter_changelog(changelog):
         first_line = False
 
 
-def _iter_log_oneline(git_dir=None, option_dict=None):
+def _iter_log_oneline(git_dir=None):
     """Iterate over --oneline log entries if possible.
 
     This parses the output into a structured form but does not apply
@@ -172,16 +172,10 @@ def _iter_log_oneline(git_dir=None, option_dict=None):
     :return: An iterator of (hash, tags_set, 1st_line) tuples, or None if
         changelog generation is disabled / not available.
     """
-    if not option_dict:
-        option_dict = {}
-    should_skip = options.get_boolean_option(option_dict, 'skip_changelog',
-                                             'SKIP_WRITE_GIT_CHANGELOG')
-    if should_skip:
-        return
     if git_dir is None:
         git_dir = _get_git_directory()
     if not git_dir:
-        return
+        return []
     return _iter_log_inner(git_dir)
 
 
@@ -220,11 +214,17 @@ def _iter_log_inner(git_dir):
 
 
 def write_git_changelog(git_dir=None, dest_dir=os.path.curdir,
-                        option_dict=dict(), changelog=None):
+                        option_dict=None, changelog=None):
     """Write a changelog based on the git changelog."""
     start = time.time()
+    if not option_dict:
+        option_dict = {}
+    should_skip = options.get_boolean_option(option_dict, 'skip_changelog',
+                                             'SKIP_WRITE_GIT_CHANGELOG')
+    if should_skip:
+        return
     if not changelog:
-        changelog = _iter_log_oneline(git_dir=git_dir, option_dict=option_dict)
+        changelog = _iter_log_oneline(git_dir=git_dir)
         if changelog:
             changelog = _iter_changelog(changelog)
     if not changelog:
diff --git a/pbr/packaging.py b/pbr/packaging.py
index e444165..c8bb61b 100644
--- a/pbr/packaging.py
+++ b/pbr/packaging.py
@@ -414,7 +414,7 @@ class LocalEggInfo(egg_info.egg_info):
 
 def _from_git(distribution):
     option_dict = distribution.get_option_dict('pbr')
-    changelog = git._iter_log_oneline(option_dict=option_dict)
+    changelog = git._iter_log_oneline()
     if changelog:
         changelog = git._iter_changelog(changelog)
     git.write_git_changelog(option_dict=option_dict, changelog=changelog)
diff --git a/pbr/tests/test_packaging.py b/pbr/tests/test_packaging.py
index 24373ab..4a188d0 100644
--- a/pbr/tests/test_packaging.py
+++ b/pbr/tests/test_packaging.py
@@ -444,6 +444,18 @@ class TestVersions(base.BaseTestCase):
         version = packaging._get_version_from_git()
         self.assertEqual('1.3.0.0a1', version)
 
+    def test_skip_write_git_changelog(self):
+        # Fix for bug 1467440
+        self.repo.commit()
+        self.repo.tag('1.2.3')
+        os.environ['SKIP_WRITE_GIT_CHANGELOG'] = '1'
+        version = packaging._get_version_from_git('1.2.3')
+        self.assertEqual('1.2.3', version)
+
+    def tearDown(self):
+        super(TestVersions, self).tearDown()
+        os.environ.pop('SKIP_WRITE_GIT_CHANGELOG', None)
+
 
 class TestRequirementParsing(base.BaseTestCase):