Repository: jupyterhub/binderhub (python)

Issue:
Long-term fix for deleted default branches
<!-- Thank you for contributing. These HTML comments will not render in the issue, but you can delete them once you've read them if you prefer! -->

### Proposed change
<!-- Use this section to describe the feature you'd like to be added. -->

Redirect builds where  `unresolved_ref=master` and that ref doesn't resolve to the same spec with `unresolved_ref=HEAD`.

As discussed in #1145 (cc @bryevdv), there are likely a lot of old links that can't easily be updated after a default branch rename. We've fixed this for _new_ default URLs in #1186, but not in a way that keeps old links working.

### Alternative options
<!-- Use this section to describe alternative options and why you've decided on the proposed feature above. -->

Let old links break.

### Who would use this feature?
<!-- Describe who would benefit from using this feature. -->

Anyone with old Binder links in documentation, across the web who has renamed their default branch in the past few years, prior to the adoption of 'HEAD' as the default ref. We know folks are keeping an unused 'master' branch up-to-date just to avoid breaking links. This would allow them to stop doing that.

### (Optional): Suggest a solution
<!-- Describe what you think needs to be done. Doing that is an excellent first step to get the feature implemented. -->

Two possible solutions:

1. (better, IMO, but more complex): I think the 'right' behavior would be for master builds to redirect the browser to HEAD, rather that keeping the resolution of dead links behind the scenes. That's a little tricky, since the resolution occurs in the event stream, not in the regular page handler, and event streams can't redirect in the middle. So we'd actually need to do the redirect in javascript on the page in response to a new event type.

2. Retrying with unresolved_ref=HEAD within the event stream handler would be a lot simpler to implement, but would keep broken links canonically working instead of redirecting. But since it's not a 'true' redirect (page load succeeds before the redirect is discovered), there's little difference, I think.

Pull request:
keep old default branch links working
If 'master' or 'main' fail to resolve, try again with 'HEAD' after a delay.

this keeps outdated links to renamed default branches working for the big master->main migration

closes #1570

Patch:
diff --git a/binderhub/builder.py b/binderhub/builder.py
index 6aaad7ae..de17837b 100644
--- a/binderhub/builder.py
+++ b/binderhub/builder.py
@@ -2,6 +2,7 @@
 Handlers for working with version control services (i.e. GitHub) for builds.
 """
 
+import asyncio
 import hashlib
 import json
 import re
@@ -303,16 +304,43 @@ class BuildHandler(BaseHandler):
                     'GitHub recently changed default branches from "master" to "main".'
                 )
 
-                if provider.unresolved_ref == "master":
-                    error_message.append('Did you mean the "main" branch?')
-                elif provider.unresolved_ref == "main":
-                    error_message.append('Did you mean the "master" branch?')
+                if provider.unresolved_ref in {"master", "main"}:
+                    error_message.append(
+                        "Tip: HEAD will always resolve to a repository's default branch."
+                    )
+
+                    # keep old links working for default branch names
+                    # by substituting 'master' or 'main' with 'HEAD'
+                    pre_ref_spec, _ = spec.rsplit("/", 1)
+                    spec = f"{pre_ref_spec}/HEAD"
+                    unresolved_ref = provider.unresolved_ref
+                    try:
+                        provider = self.get_provider(provider_prefix, spec=spec)
+                        ref = await provider.get_resolved_ref()
+                    except Exception as e:
+                        # if this fails, leave ref as None, which will fail below
+                        self.log.error(f"Error redirecting {key} to HEAD: {e}")
+                    else:
+                        # delayed redirect for deleted default branches
+                        await self.emit(
+                            {
+                                "phase": "waiting",
+                                "message": (
+                                    " ".join(error_message) + "\n"
+                                    f"Trying again with HEAD instead of {unresolved_ref}. Please update your links.\n"
+                                ),
+                            }
+                        )
+                        # artificial delay for what should be broken links
+                        await asyncio.sleep(10)
 
             else:
                 error_message.append("Is your repo public?")
 
-            await self.fail(" ".join(error_message))
-            return
+            if ref is None:
+                # ref can become non-None if redirected to HEAD
+                await self.fail(" ".join(error_message))
+                return
 
         self.ref_url = await provider.get_resolved_ref_url()
         resolved_spec = await provider.get_resolved_spec()
diff --git a/binderhub/repoproviders.py b/binderhub/repoproviders.py
index 4169f34e..38fb05de 100644
--- a/binderhub/repoproviders.py
+++ b/binderhub/repoproviders.py
@@ -38,8 +38,8 @@ def tokenize_spec(spec):
     spec_parts = spec.split("/", 2)  # allow ref to contain "/"
     if len(spec_parts) != 3:
         msg = f'Spec is not of the form "user/repo/ref", provided: "{spec}".'
-        if len(spec_parts) == 2 and spec_parts[-1] not in ("main", "master"):
-            msg += f' Did you mean "{spec}/main"?'
+        if len(spec_parts) == 2 and spec_parts[-1] not in {"main", "master", "HEAD"}:
+            msg += f' Did you mean "{spec}/HEAD"?'
         raise ValueError(msg)
 
     return spec_parts
@@ -564,7 +564,7 @@ class GitLabRepoProvider(RepoProvider):
     <url-escaped-namespace>/<unresolved_ref>
 
     eg:
-    group%2Fproject%2Frepo/master
+    group%2Fproject%2Frepo/main
     """
 
     name = Unicode("GitLab")
@@ -956,10 +956,9 @@ class GistRepoProvider(GitHubRepoProvider):
 
     The ref is optional, valid values are
         - a full sha1 of a ref in the history
-        - master
-        - HEAD for the default branch
+        - HEAD for the latest ref (also allow 'master', 'main' as aliases for HEAD)
 
-    If master or no ref is specified the latest revision will be used.
+    If HEAD or no ref is specified the latest revision will be used.
     """
 
     name = Unicode("Gist")
@@ -1020,7 +1019,7 @@ class GistRepoProvider(GitHubRepoProvider):
             )
 
         all_versions = [e["version"] for e in ref_info["history"]]
-        if self.unresolved_ref in {"", "HEAD", "master"}:
+        if self.unresolved_ref in {"", "HEAD", "master", "main"}:
             self.resolved_ref = all_versions[0]
         else:
             if self.unresolved_ref not in all_versions:


Test patch:
diff --git a/binderhub/tests/test_build.py b/binderhub/tests/test_build.py
index 1a14d077..ba13ab83 100644
--- a/binderhub/tests/test_build.py
+++ b/binderhub/tests/test_build.py
@@ -41,6 +41,8 @@ from .utils import async_requests
         # gh/ GitHub repo provider
         "gh/binderhub-ci-repos/cached-minimal-dockerfile/HEAD",
         "gh/binderhub-ci-repos/cached-minimal-dockerfile/596b52f10efb0c9befc0c4ae850cc5175297d71c",
+        # test redirect master->HEAD
+        "gh/binderhub-ci-repos/cached-minimal-dockerfile/master",
         # gl/ GitLab repo provider
         "gl/binderhub-ci-repos%2Fcached-minimal-dockerfile/HEAD",
         "gl/binderhub-ci-repos%2Fcached-minimal-dockerfile/596b52f10efb0c9befc0c4ae850cc5175297d71c",
diff --git a/binderhub/tests/test_repoproviders.py b/binderhub/tests/test_repoproviders.py
index 91b70fb4..4041eec5 100644
--- a/binderhub/tests/test_repoproviders.py
+++ b/binderhub/tests/test_repoproviders.py
@@ -360,14 +360,14 @@ class TestSpecErrorHandling(TestCase):
         assert len(spec_parts) == 3
 
     def test_spec_with_no_suggestion(self):
-        spec = "short/master"
+        spec = "short/HEAD"
         error = "^((?!Did you mean).)*$"  # negative match
         with self.assertRaisesRegex(ValueError, error):
             user, repo, unresolved_ref = tokenize_spec(spec)
 
     def test_spec_with_suggestion(self):
         spec = "short/suggestion"
-        error = f'Did you mean "{spec}/main"?'
+        error = f'Did you mean "{spec}/HEAD"?'
         with self.assertRaisesRegex(ValueError, error):
             user, repo, unresolved_ref = tokenize_spec(spec)