Only consider tags that look like versions.

pbr has been allowing any tag (e.g. 'fred') to be a version - but fred
is clearly not a version! Since git describe cannot validate versions
for us today, we need to perform the describe calculations ourselves.

In order for this to work on Python3, a missing method __hash__ needed
to be added to SemanticVersion.

Change-Id: I09fa54231e77ae7671aa6f84eb91fd655e49ab25
Closes-Bug: #1356784
This commit is contained in:
Robert Collins 2014-08-15 12:41:41 +12:00
parent 8460101cc3
commit dc62764a24
4 changed files with 73 additions and 19 deletions

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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