Merge "Only consider tags that look like versions."

4 files changed, 73 insertions, 19 deletions

diff --git a/pbr/packaging.py b/pbr/packaging.py
index 2b71d2f..65a1f65 100644
--- a/pbr/packaging.py
+++ b/pbr/packaging.py
@@ -870,15 +870,18 @@ def _get_revno_and_last_tag(git_dir):
     tags then we fall back to counting commits since the beginning
     of time.
     """
-    describe = _run_git_command(['describe', '--always'], git_dir)
-    if "-" in describe:
-        tag, count, hash = describe.split("-")
-        return tag, int(count)
-
-    # no tags found
-    revlist = _run_git_command(
-        ['rev-list', '--abbrev-commit', 'HEAD'], git_dir)
-    return "", len(revlist.splitlines())
+    changelog = _iter_log_oneline(git_dir=git_dir)
+    row_count = 0
+    for row_count, (ignored, tag_set, ignored) in enumerate(changelog):
+        version_tags = set()
+        for tag in list(tag_set):
+            try:
+                version_tags.add(version.SemanticVersion.from_pip_string(tag))
+            except Exception:
+                pass
+        if version_tags:
+            return max(version_tags).release_string(), row_count
+    return "", row_count
 
 
 def _get_version_from_git_target(git_dir, target_version):
@@ -900,7 +903,11 @@ def _get_version_from_git_target(git_dir, target_version):
         ['log', '-n1', '--pretty=format:%h'], git_dir)
     tag, distance = _get_revno_and_last_tag(git_dir)
     last_semver = version.SemanticVersion.from_pip_string(tag or '0')
-    new_version = last_semver.increment(**_get_increment_kwargs(git_dir, tag))
+    if distance == 0:
+        new_version = last_semver
+    else:
+        new_version = last_semver.increment(
+            **_get_increment_kwargs(git_dir, tag))
     if target_version is not None and new_version > target_version:
         raise ValueError(
             "git history requires a target version of %(new)s, but target "
@@ -927,9 +934,10 @@ def _get_version_from_git(pre_version=None):
     git_dir = _get_git_directory()
     if git_dir and _git_is_installed():
         try:
-            return _run_git_command(
+            tagged = _run_git_command(
                 ['describe', '--exact-match'], git_dir,
                 throw_on_error=True).replace('-', '.')
+            target_version = version.SemanticVersion.from_pip_string(tagged)
         except Exception:
             if pre_version:
                 # not released yet - use pre_version as the target
diff --git a/pbr/tests/base.py b/pbr/tests/base.py
index 8a846a7..ab4d1fb 100644
--- a/pbr/tests/base.py
+++ b/pbr/tests/base.py
@@ -110,6 +110,16 @@ class BaseTestCase(testtools.TestCase, testresources.ResourcedTestCase):
         self.addCleanup(os.chdir, os.getcwd())
         os.chdir(self.package_dir)
         self.addCleanup(self._discard_testpackage)
+        # Tests can opt into non-PBR_VERSION by setting preversioned=False as
+        # an attribute.
+        if not getattr(self, 'preversioned', True):
+            self.useFixture(fixtures.EnvironmentVariable('PBR_VERSION'))
+            setup_cfg_path = os.path.join(self.package_dir, 'setup.cfg')
+            with open(setup_cfg_path, 'rt') as cfg:
+                content = cfg.read()
+            content = content.replace(u'version = 0.1.dev', u'')
+            with open(setup_cfg_path, 'wt') as cfg:
+                cfg.write(content)
 
     def _discard_testpackage(self):
         # Remove pbr.testpackage from sys.modules so that it can be freshly
diff --git a/pbr/tests/test_packaging.py b/pbr/tests/test_packaging.py
index 2610e05..902e0fc 100644
--- a/pbr/tests/test_packaging.py
+++ b/pbr/tests/test_packaging.py
@@ -130,14 +130,6 @@ class TestPackagingInGitRepoWithCommit(base.BaseTestCase):
         super(TestPackagingInGitRepoWithCommit, self).setUp()
         repo = self.useFixture(TestRepo(self.package_dir))
         repo.commit()
-        if not self.preversioned:
-            self.useFixture(fixtures.EnvironmentVariable('PBR_VERSION'))
-            setup_cfg_path = os.path.join(self.package_dir, 'setup.cfg')
-            with open(setup_cfg_path, 'rt') as cfg:
-                content = cfg.read()
-            content = content.replace(u'version = 0.1.dev', u'')
-            with open(setup_cfg_path, 'wt') as cfg:
-                cfg.write(content)
         self.run_setup('sdist', allow_fail=False)
 
     def test_authors(self):
@@ -221,6 +213,11 @@ class TestNestedRequirements(base.BaseTestCase):
 
 class TestVersions(base.BaseTestCase):
 
+    scenarios = [
+        ('preversioned', dict(preversioned=True)),
+        ('postversioned', dict(preversioned=False)),
+    ]
+
     def setUp(self):
         super(TestVersions, self).setUp()
         self.repo = self.useFixture(TestRepo(self.package_dir))
@@ -323,6 +320,42 @@ class TestVersions(base.BaseTestCase):
         self.repo.tag('1.2.3')
         _check_combinations('1.2.3')
 
+    def test_invalid_tag_ignored(self):
+        # Fix for bug 1356784 - we treated any tag as a version, not just those
+        # that are valid versions.
+        self.repo.commit()
+        self.repo.tag('1')
+        self.repo.commit()
+        # when the tree is tagged and its wrong:
+        self.repo.tag('badver')
+        version = packaging._get_version_from_git()
+        self.assertThat(version, matchers.StartsWith('1.0.1.dev1.g'))
+        # When the tree isn't tagged, we also fall through.
+        self.repo.commit()
+        version = packaging._get_version_from_git()
+        self.assertThat(version, matchers.StartsWith('1.0.1.dev2.g'))
+        # We don't fall through x.y versions
+        self.repo.commit()
+        self.repo.tag('1.2')
+        self.repo.commit()
+        self.repo.tag('badver2')
+        version = packaging._get_version_from_git()
+        self.assertThat(version, matchers.StartsWith('1.2.1.dev1.g'))
+        # Or x.y.z versions
+        self.repo.commit()
+        self.repo.tag('1.2.3')
+        self.repo.commit()
+        self.repo.tag('badver3')
+        version = packaging._get_version_from_git()
+        self.assertThat(version, matchers.StartsWith('1.2.4.dev1.g'))
+        # Or alpha/beta/pre versions
+        self.repo.commit()
+        self.repo.tag('1.2.4.0a1')
+        self.repo.commit()
+        self.repo.tag('badver4')
+        version = packaging._get_version_from_git()
+        self.assertThat(version, matchers.StartsWith('1.2.4.dev1.g'))
+
 
 def load_tests(loader, in_tests, pattern):
     return testscenarios.load_tests_apply_scenarios(loader, in_tests, pattern)
diff --git a/pbr/version.py b/pbr/version.py
index dc88500..9ef51fb 100644
--- a/pbr/version.py
+++ b/pbr/version.py
@@ -76,6 +76,9 @@ class SemanticVersion(object):
             return False
         return self.__dict__ == other.__dict__
 
+    def __hash__(self):
+        return sum(map(hash, self.__dict__.values()))
+
     def __lt__(self, other):
         """Compare self and other, another Semantic Version."""
         # NB(lifeless) this could perhaps be rewritten as