Support GitHub /tree/<branch>/<subdir> URLs in project import — fixes the appstore "Remix on CloudPebble" button

cloudpebble/ide/api/project.py

@@ -20,6 +20,7 @@
 from ide.tasks.build import run_compile
 from ide.tasks.gist import import_gist
 from ide.tasks.git import do_import_github
+from ide.utils.github_urls import parse_github_source
 from ide.utils.alloy_templates import list_alloy_templates, build_template_archive
 from ide.utils.c_templates import list_c_templates, build_c_template_archive
 from utils.td_helper import send_td_event
@@ -1029,14 +1030,24 @@ def import_zip(request):
 def import_github(request):
     name = request.POST['name']
     repo = request.POST['repo']
-    branch = request.POST['branch']
+    branch = request.POST.get('branch', '')
     add_remote = (request.POST['add_remote'] == 'true')
-    match = re.match(r'^(?:https?://|git@|git://)?(?:www\.)?github\.com[/:]([\w.-]+)/([\w.-]+?)(?:\.git|/|$)', repo)
-    if match is None:
+    source = parse_github_source(repo)
+    if source is None:
         raise BadRequest(_("Invalid Github URL."))
 
-    github_user = match.group(1)
-    github_project = match.group(2)
+    github_user = source.user
+    github_project = source.project
+    if source.refpath:
+        # A /tree/, /blob/ or /commit/ URL is authoritative for both the ref
+        # and the subdirectory (the celery task resolves the ambiguous split
+        # against the repository's real refs); a separately-typed branch would
+        # conflict with it, so it is ignored.
+        branch = ''
+        if add_remote:
+            raise BadRequest(_("Linking a repository is not supported for /tree/ or /blob/ "
+                               "imports — import the project first, then add the remote from "
+                               "its settings."))
 
     try:
         project = Project.objects.create(owner=request.user, name=name)
@@ -1045,10 +1056,11 @@ def import_github(request):
 
     if add_remote:
         project.github_repo = "%s/%s" % (github_user, github_project)
-        project.github_branch = branch
+        project.github_branch = branch or None
         project.save()
 
-    task = do_import_github.delay(project.id, github_user, github_project, branch, delete_project=True)
+    task = do_import_github.delay(project.id, github_user, github_project, branch, delete_project=True,
+                                  github_refpath=source.refpath, github_kind=source.kind)
     return {'task_id': task.task_id, 'project_id': project.id}
 
 

cloudpebble/ide/static/ide/js/project_list.js

@@ -178,14 +178,14 @@ $(function() {
             active_set.find('.errors').removeClass('hide').text(gettext("You must specify a project name."));
             return;
         }
-        // This is identical to the regex used on the server.
+        // A prefix check only — the server (ide.utils.github_urls) is the
+        // authority and also understands /tree/<ref>/<subdir> and /blob/ URLs.
         if(!/^(?:https?:\/\/|git@|git:\/\/)?(?:www\.)?github\.com[\/:]([\w.-]+)\/([\w.-]+?)(?:\.git|\/|$)/.test(url)) {
             active_set.find('.errors').removeClass('hide').text(gettext("You must specify a complete GitHub project URL"));
             return;
         }
-        if(branch.length == 0) {
-            branch = 'master';
-        }
+        // An empty branch imports the repository's default branch (the old
+        // hardcoded 'master' fallback broke every main-default repository).
         disable_import_controls();
         active_set.find('.progress').removeClass('hide');
         do_import(Ajax.Post('/ide/import/github', {
@@ -227,10 +227,20 @@ $(function() {
     if (path.indexOf('/ide/import/github/') === 0) {
         var parts = path.substr(1).split('/');
         $('#import-prompt').modal();
-        $('#import-github-name').val(parts[3]);
-        $('#import-github-url').val('github.com/' + parts[3] + '/' + parts[4]);
-        if (parts.length > 5) {
-            $('#import-github-branch').val(parts.slice(5).join('/'));
+        var tail = parts.slice(5).filter(function(p) { return p.length > 0; });
+        if (tail.length > 1 && (tail[0] == 'tree' || tail[0] == 'blob' || tail[0] == 'commit')) {
+            // A GitHub web URL shape (e.g. the appstore's "Remix" button:
+            // .../import/github/<user>/<repo>/tree/<branch>/<subdir>). Hand
+            // the whole thing to the server, which resolves branch vs
+            // subdirectory against the repository's real refs.
+            $('#import-github-name').val(tail.length > 2 ? tail[tail.length - 1] : parts[4]);
+            $('#import-github-url').val('github.com/' + parts.slice(3).concat([]).join('/'));
+        } else {
+            $('#import-github-name').val(parts[3]);
+            $('#import-github-url').val('github.com/' + parts[3] + '/' + parts[4]);
+            if (tail.length) {
+                $('#import-github-branch').val(tail.join('/'));
+            }
         }
         $('a[href=#import-github]').tab('show');
     }

cloudpebble/ide/tasks/archive.py

@@ -145,7 +145,7 @@ def ends_with_any(s, options):
 
 
 @shared_task(acks_late=True)
-def do_import_archive(project_id, archive, delete_project=False, wipe_existing=False):
+def do_import_archive(project_id, archive, delete_project=False, wipe_existing=False, root_hint=None):
     project = Project.objects.get(pk=project_id)
     try:
         with tempfile.NamedTemporaryFile(suffix='.zip') as archive_file:
@@ -172,7 +172,7 @@ def do_import_archive(project_id, archive, delete_project=False, wipe_existing=F
                     raise InvalidProjectArchiveException("Too many files in zip file.")
 
                 archive_items = [ArchiveProjectItem(z, x) for x in contents]
-                base_dir, manifest_item = find_project_root_and_manifest(archive_items)
+                base_dir, manifest_item = find_project_root_and_manifest(archive_items, root_hint=root_hint)
                 dir_end = len(base_dir)
 
                 def make_valid_filename(zip_entry):

cloudpebble/ide/tasks/git.py

@@ -1,5 +1,6 @@
 import base64
 import io
+import posixpath
 from concurrent.futures import ThreadPoolExecutor
 from io import BytesIO
 from urllib.request import urlopen, Request
@@ -22,6 +23,7 @@
 from ide.tasks import do_import_archive, run_compile
 from ide.tasks.archive import get_filename_variant
 from ide.utils.git import git_sha, git_blob
+from ide.utils.github_urls import split_ref_and_path, normalize_subpath
 from ide.utils.project import find_project_root_and_manifest, BaseProjectItem, InvalidProjectArchiveException, MANIFEST_KINDS
 from ide.utils.sdk import generate_manifest_dict, generate_manifest, generate_wscript_file, load_manifest_dict, manifest_name_for_project
 from utils.td_helper import send_td_event
@@ -40,7 +42,8 @@ def exception_reason(error):
 
 
 @shared_task(acks_late=True)
-def do_import_github(project_id, github_user, github_project, github_branch, delete_project=False):
+def do_import_github(project_id, github_user, github_project, github_branch, delete_project=False,
+                     github_refpath=None, github_kind=None):
     project = None
     user = None
     try:
@@ -50,6 +53,18 @@ def do_import_github(project_id, github_user, github_project, github_branch, del
         except:
             pass
 
+        github_path = ''
+        if github_refpath:
+            # A /tree/ or /blob/ URL: the branch and the subdirectory are one
+            # ambiguous string until matched against the repository's refs.
+            github_branch, github_path = resolve_ref_and_path(
+                user, github_user, github_project, github_refpath, github_kind)
+        if not github_branch:
+            # codeload resolves HEAD to the repository's default branch, so an
+            # empty branch imports what visitors see — instead of the old
+            # hardcoded 'master', which broke every main-default repository.
+            github_branch = 'HEAD'
+
         url = "https://github.com/%s/%s/archive/%s.zip" % (github_user, github_project, github_branch)
         auth_url = get_authenticated_archive_url(user, github_user, github_project, github_branch)
         archive = None
@@ -68,7 +83,7 @@ def do_import_github(project_id, github_user, github_project, github_branch, del
                 % (github_user, github_project, github_branch)
             )
 
-        return do_import_archive(project_id, archive.read())
+        return do_import_archive(project_id, archive.read(), root_hint=github_path or None)
     except Exception as e:
         if delete_project and project is not None:
             try:
@@ -80,12 +95,65 @@ def do_import_github(project_id, github_user, github_project, github_branch, del
                 'reason': exception_reason(e),
                 'github_user': github_user,
                 'github_project': github_project,
-                'github_branch': github_branch
+                'github_branch': github_branch,
+                'github_refpath': github_refpath
             }
         }, user=user)
         raise
 
 
+def resolve_ref_and_path(user, github_user, github_project, refpath, kind):
+    """ Resolve a /tree/ or /blob/ remainder ("<ref>[/<path>]") into a real
+    (ref, subdirectory) pair. Branch names may contain slashes, so the split
+    is decided against the repository's actual refs — via the GitHub API when
+    possible, else by probing which archive actually exists (longest candidate
+    first), the way gitpick and gitingest resolve GitHub web URLs. """
+    if kind == 'commit':
+        return refpath, ''
+
+    ref, path = None, None
+    ref_names = get_ref_names(user, github_user, github_project)
+    if ref_names is not None:
+        ref, path = split_ref_and_path(refpath, ref_names)
+    elif '/' in refpath:
+        # No API access (rate limit, private repo without a token): probe the
+        # public archive endpoint. Longest first, so 'feature/foo/src' tries
+        # branch 'feature/foo' before branch 'feature'. The refs-qualified
+        # codeload form MUST be used here: bare archive/<x>.zip answers 200
+        # for junk like 'main/anything', while zip/refs/heads/<x> is strict
+        # (measured: refs/heads/main 200, refs/heads/main/faces 404).
+        parts = refpath.split('/')
+        start = len(parts) if kind == 'tree' else len(parts) - 1
+        for i in range(start, 0, -1):
+            candidate = '/'.join(parts[:i])
+            if any(file_exists("https://codeload.github.com/%s/%s/zip/refs/%s/%s"
+                               % (github_user, github_project, refkind, candidate))
+                   for refkind in ('heads', 'tags')):
+                ref, path = candidate, '/'.join(parts[i:])
+                break
+    if ref is None:
+        ref, path = split_ref_and_path(refpath, None)
+
+    if kind == 'blob':
+        # A blob URL names a file; the project is the directory it lives in.
+        path = posixpath.dirname(path)
+    try:
+        path = normalize_subpath(path)
+    except ValueError:
+        raise Exception("Invalid project path in GitHub URL: '%s'" % refpath)
+    return ref, path
+
+
+def get_ref_names(user, github_user, github_project):
+    """ Branch + tag names, or None when the API is unavailable. """
+    try:
+        g = get_github(user) if user is not None else Github()
+        repo = g.get_repo("%s/%s" % (github_user, github_project))
+        return [b.name for b in repo.get_branches()] + [t.name for t in repo.get_tags()]
+    except Exception:
+        return None
+
+
 def file_exists(url):
     request = Request(url)
     request.get_method = lambda: 'HEAD'

cloudpebble/ide/tests/test_find_project_root.py

@@ -149,3 +149,53 @@ def test_skip_build_and_node_modules_together(self):
             "project/src/c/main.c",
             "project/resources/fonts/font.ttf",
         ], "project/", "project/package.json")
+
+
+class TestFindProjectRootWithHint(TestCase):
+    """ root_hint pins the search to one subdirectory — the /tree/<ref>/<subdir>
+    import case, where the repository may contain several projects. """
+
+    REPO = [
+        # A repository that is ALSO a valid project at its root (a library
+        # shipping its own demo), plus two nested projects — the shape that
+        # made unhinted imports pick the wrong one.
+        "repo-main/package.json",
+        "repo-main/src/",
+        "repo-main/faces/slothvec/package.json",
+        "repo-main/faces/slothvec/src/",
+        "repo-main/examples/other/package.json",
+    ]
+
+    def find(self, contents, root_hint):
+        return find_project_root_and_manifest(
+            [FakeProjectItem(item) for item in contents], root_hint=root_hint)
+
+    def test_hint_picks_nested_project_over_root(self):
+        base_dir, manifest = self.find(self.REPO, "faces/slothvec")
+        self.assertEqual(base_dir, "repo-main/faces/slothvec/")
+        self.assertEqual(manifest.name, "repo-main/faces/slothvec/package.json")
+
+    def test_hint_matches_archive_without_wrapper_dir(self):
+        base_dir, manifest = self.find(
+            ["faces/slothvec/package.json", "faces/slothvec/src/"], "faces/slothvec")
+        self.assertEqual(base_dir, "faces/slothvec/")
+
+    def test_hint_with_no_project_there_throws(self):
+        with self.assertRaises(InvalidProjectArchiveException):
+            self.find(self.REPO, "faces/missing")
+
+    def test_hint_does_not_match_deeper_manifests(self):
+        with self.assertRaises(InvalidProjectArchiveException):
+            self.find(self.REPO, "faces")
+
+    def test_hint_suffix_alone_is_not_enough(self):
+        # 'slothvec' names the right LEAF, but the project lives at
+        # faces/slothvec — a lazy suffix match would import it anyway and
+        # mask a wrong ref/path split upstream. It must fail loudly instead.
+        with self.assertRaises(InvalidProjectArchiveException):
+            self.find(self.REPO, "slothvec")
+
+    def test_no_hint_keeps_existing_behavior(self):
+        base_dir, manifest = find_project_root_and_manifest(
+            [FakeProjectItem(item) for item in self.REPO])
+        self.assertEqual(base_dir, "repo-main/")