diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 9452b7d41..c5b5e3241 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -21,7 +21,7 @@ from readthedocs.projects.constants import (PRIVACY_CHOICES, REPO_TYPE_GIT, from .constants import (BUILD_STATE, BUILD_TYPES, VERSION_TYPES, LATEST, NON_REPOSITORY_VERSIONS, STABLE, - BUILD_STATE_FINISHED, BRANCH) + BUILD_STATE_FINISHED, BRANCH, TAG) from .version_slug import VersionSlugField @@ -99,12 +99,46 @@ class Version(models.Model): @property def commit_name(self): - """Return the branch name, the tag name or the revision identifier.""" - if self.type == BRANCH: + """ + Return the branch name, the tag name or the revision identifier. + + The result could be used as ref in a git repo, e.g. for linking to + GitHub or Bitbucket. + """ + + # LATEST is special as it is usually a branch but does not contain the + # name in verbose_name. + if self.slug == LATEST: + if self.project.default_branch: + return self.project.default_branch + else: + return self.project.vcs_repo().fallback_branch + + if self.slug == STABLE: + if self.type == BRANCH: + # Special case, as we do not store the original branch name + # that the stable version works on. We can only interpolate the + # name from the commit identifier, but it's hacky. + # TODO: Refactor ``Version`` to store more actual info about + # the underlying commits. + if self.identifier.startswith('origin/'): + return self.identifier[len('origin/'):] return self.identifier - if self.verbose_name in NON_REPOSITORY_VERSIONS: - return self.identifier - return self.verbose_name + + # By now we must have handled all special versions. + assert self.slug not in NON_REPOSITORY_VERSIONS + + if self.type in (BRANCH, TAG): + # If this version is a branch or a tag, the verbose_name will + # contain the actual name. We cannot use identifier as this might + # include the "origin/..." part in the case of a branch. A tag + # would contain the hash in identifier, which is not as pretty as + # the actual tag name. + return self.verbose_name + + # If we came that far it's not a special version nor a branch or tag. + # Therefore just return the identifier to make a safe guess. + return self.identifier def get_absolute_url(self): if not self.built and not self.uploaded: @@ -124,16 +158,6 @@ class Version(models.Model): self.project.sync_supported_versions() return obj - @property - def remote_slug(self): - if self.slug == LATEST: - if self.project.default_branch: - return self.project.default_branch - else: - return self.project.vcs_repo().fallback_branch - else: - return self.slug - @property def identifier_friendly(self): '''Return display friendly identifier''' @@ -211,24 +235,6 @@ class Version(models.Model): except OSError: log.error('Build path cleanup failed', exc_info=True) - def get_vcs_slug(self): - slug = None - if self.slug == LATEST: - if self.project.default_branch: - slug = self.project.default_branch - else: - slug = self.project.vcs_repo().fallback_branch - elif self.slug == STABLE: - return self.identifier - else: - slug = self.slug - # https://github.com/rtfd/readthedocs.org/issues/561 - # version identifiers with / characters in branch name need to un-slugify - # the branch name for remote links to work - if slug.replace('-', '/') in self.identifier: - slug = slug.replace('-', '/') - return slug - def get_github_url(self, docroot, filename, source_suffix='.rst', action='view'): repo_url = self.project.repo if 'github' not in repo_url: @@ -259,7 +265,7 @@ class Version(models.Model): return GITHUB_URL.format( user=user, repo=repo, - version=self.remote_slug, + version=self.commit_name, docroot=docroot, path=filename, source_suffix=source_suffix, @@ -285,7 +291,7 @@ class Version(models.Model): return BITBUCKET_URL.format( user=user, repo=repo, - version=self.remote_slug, + version=self.commit_name, docroot=docroot, path=filename, source_suffix=source_suffix, diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index ddd5ef5da..e2d76108e 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -73,7 +73,7 @@ class BaseSphinx(BaseBuilder): raise ProjectImportError('Conf file not found'), None, trace outfile.write("\n") conf_py_path = self.version.get_conf_py_path() - remote_version = self.version.get_vcs_slug() + remote_version = self.version.commit_name github_user, github_repo = version_utils.get_github_username_repo( url=self.project.repo) diff --git a/readthedocs/rtd_tests/tests/test_version_commit_name.py b/readthedocs/rtd_tests/tests/test_version_commit_name.py new file mode 100644 index 000000000..0be61131d --- /dev/null +++ b/readthedocs/rtd_tests/tests/test_version_commit_name.py @@ -0,0 +1,50 @@ +from django.test import TestCase +from django_dynamic_fixture import get +from django_dynamic_fixture import new + +from readthedocs.builds.constants import BRANCH +from readthedocs.builds.constants import LATEST +from readthedocs.builds.constants import STABLE +from readthedocs.builds.constants import TAG +from readthedocs.builds.models import Version +from readthedocs.projects.constants import REPO_TYPE_GIT +from readthedocs.projects.constants import REPO_TYPE_HG +from readthedocs.projects.models import Project + + +class VersionCommitNameTests(TestCase): + def test_branch_name(self): + version = new(Version, identifier='release-2.5.x', + slug='release-2.5.x', verbose_name='release-2.5.x', + type=BRANCH) + self.assertEqual(version.commit_name, 'release-2.5.x') + + def test_tag_name(self): + version = new(Version, identifier='10f1b29a2bd2', slug='release-2.5.0', + verbose_name='release-2.5.0', type=TAG) + self.assertEqual(version.commit_name, 'release-2.5.0') + + def test_branch_with_name_stable(self): + version = new(Version, identifier='origin/stable', slug=STABLE, + verbose_name='stable', type=BRANCH) + self.assertEqual(version.commit_name, 'stable') + + def test_stable_version_tag(self): + version = new(Version, + identifier='3d92b728b7d7b842259ac2020c2fa389f13aff0d', + slug=STABLE, verbose_name=STABLE, type=TAG) + self.assertEqual(version.commit_name, + '3d92b728b7d7b842259ac2020c2fa389f13aff0d') + + def test_hg_latest_branch(self): + hg_project = get(Project, repo_type=REPO_TYPE_HG) + version = new(Version, identifier='default', slug=LATEST, + verbose_name=LATEST, type=BRANCH, project=hg_project) + self.assertEqual(version.commit_name, 'default') + + def test_git_latest_branch(self): + git_project = get(Project, repo_type=REPO_TYPE_GIT) + version = new(Version, project=git_project, + identifier='origin/master', slug=LATEST, + verbose_name=LATEST, type=BRANCH) + self.assertEqual(version.commit_name, 'master')