Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions apps/downloads/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,29 @@ def get_absolute_url(self):
return self.release_page.get_absolute_url()
return reverse("download:download_release_detail", kwargs={"release_slug": self.slug})

@property
def corrected_release_notes_url(self):
"""Return the release notes URL, converting dead hg.python.org links to GitHub.

Old Mercurial-hosted URLs (hg.python.org) are no longer reachable.
This property remaps them to their equivalent paths on GitHub so that
the "Release notes" links on the downloads page still work for legacy
releases (3.3.6 and earlier).

Example::

http://hg.python.org/cpython/file/v3.3.6/Misc/NEWS
→ https://github.com/python/cpython/blob/v3.3.6/Misc/NEWS
"""
url = self.release_notes_url
if not url:
return url
match = re.match(r"https?://hg\.python\.org/cpython/file/([^/]+)/(.+)", url)
if match:
tag, path = match.group(1), match.group(2)
return f"https://github.com/python/cpython/blob/{tag}/{path}"
Comment on lines +134 to +137
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re.match can be confusing because it anchors the start of the string, but not the end.

Can we use re.fullmatch instead? Or re.search with explicit ^?

https://docs.python.org/3/library/re.html#search-vs-match


Or better yet, ditch the regex:

Suggested change
match = re.match(r"https?://hg\.python\.org/cpython/file/([^/]+)/(.+)", url)
if match:
tag, path = match.group(1), match.group(2)
return f"https://github.com/python/cpython/blob/{tag}/{path}"
for prefix in (
"http://hg.python.org/cpython/file/",
"https://hg.python.org/cpython/file/",
):
if url.startswith(prefix):
return "https://github.com/python/cpython/blob/" + url[len(prefix):]
return url

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually:

We should handle both.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, the str.startswith approach is much cleaner. Will switch to that — also makes it easier to extend for the raw-file case you spotted below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, missed that one. Will handle both /file/ and /raw-file/ in the updated version.

return url

def download_file_for_os(self, os_slug):
"""Given an OS slug return the appropriate download file."""
try:
Expand Down Expand Up @@ -430,3 +453,4 @@ class Meta:
violation_error_message="All file URLs must begin with 'https://www.python.org/'",
),
]

3 changes: 2 additions & 1 deletion apps/downloads/templates/downloads/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ <h2 class="widget-title">Looking for a specific release?</h2>
<span class="release-number"><a href="{{ r.get_absolute_url }}">{{ r.name }}</a></span>
<span class="release-date">{{ r.release_date|date }}</span>
<span class="release-download"><a href="{{ r.get_absolute_url }}"><span aria-hidden="true" class="icon-download"></span> Download</a></span>
<span class="release-enhancements"><a href="{{ r.release_notes_url }}">Release notes</a></span>
<span class="release-enhancements">{% if r.corrected_release_notes_url %}<a href="{{ r.corrected_release_notes_url }}">Release notes</a>{% else %}Release notes{% endif %}</span>
</li>
{% endfor %}
</ol>
Expand Down Expand Up @@ -127,3 +127,4 @@ <h2 class="widget-title">Looking for a specific release?</h2>
{% box 'download-pgp' %}
</div>
{% endblock content %}

50 changes: 50 additions & 0 deletions apps/downloads/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,53 @@ def test_release_file_urls_not_python_dot_org(self):
name="Windows installer draft",
**kwargs,
)

class ReleaseNotesURLTests(BaseDownloadTests):
"""Tests for Release.corrected_release_notes_url property."""

def test_hg_url_converted_to_github(self):
"""An hg.python.org URL is remapped to its GitHub equivalent."""
release = Release.objects.create(
version=Release.PYTHON3,
name="Python 3.3.6",
is_published=True,
release_notes_url="http://hg.python.org/cpython/file/v3.3.6/Misc/NEWS",
)
self.assertEqual(
release.corrected_release_notes_url,
"https://github.com/python/cpython/blob/v3.3.6/Misc/NEWS",
)

def test_https_hg_url_also_converted(self):
"""An https hg.python.org URL is also remapped to GitHub."""
release = Release.objects.create(
version=Release.PYTHON3,
name="Python 3.2.6",
is_published=True,
release_notes_url="https://hg.python.org/cpython/file/v3.2.6/Misc/NEWS",
)
self.assertEqual(
release.corrected_release_notes_url,
"https://github.com/python/cpython/blob/v3.2.6/Misc/NEWS",
)

def test_modern_url_returned_unchanged(self):
"""A non-hg URL (e.g. already on GitHub) is returned unchanged."""
url = "https://github.com/python/cpython/blob/v3.12.0/Misc/NEWS.d"
release = Release.objects.create(
version=Release.PYTHON3,
name="Python 3.12.0",
is_published=True,
release_notes_url=url,
)
self.assertEqual(release.corrected_release_notes_url, url)

def test_empty_url_returns_empty(self):
"""An empty release_notes_url returns an empty string."""
release = Release.objects.create(
version=Release.PYTHON3,
name="Python 3.11.0",
is_published=True,
release_notes_url="",
)
self.assertEqual(release.corrected_release_notes_url, "")
Loading