Git backend: make default_branch to point to VCS' default branch

docs/user/integrations.rst

@@ -153,10 +153,15 @@ token
     The integration token found on the project's **Integrations** dashboard page
     (:guilabel:`Admin` > :guilabel:`Integrations`).
 
+default_branch
+    This is the default branch of the repository
+    (ie. the one checked out when cloning the repository without arguments)
+
 For example, the cURL command to build the ``dev`` branch, using the token
 ``1234``, would be::
 
-    curl -X POST -d "branches=dev" -d "token=1234" https://readthedocs.org/api/v2/webhook/example-project/1/
+    curl -X POST -d "branches=dev" -d "token=1234" -d "default_branch=main"
+    https://readthedocs.org/api/v2/webhook/example-project/1/
 
 A command like the one above could be called from a cron job or from a hook
 inside Git_, Subversion_, Mercurial_, or Bazaar_.

readthedocs/api/v2/views/integrations.py

@@ -18,6 +18,7 @@
 from readthedocs.builds.constants import (
     EXTERNAL_VERSION_STATE_CLOSED,
     EXTERNAL_VERSION_STATE_OPEN,
+    LATEST,
 )
 from readthedocs.core.signals import webhook_bitbucket, webhook_github, webhook_gitlab
 from readthedocs.core.views.hooks import (
@@ -280,6 +281,30 @@ def get_closed_external_version_response(self, project):
             "versions": [version_closed] if version_closed else [],
         }
 
+    def update_default_branch(self, default_branch):
+        """
+        Update the `Version.identifer` for `latest` with the VCS's `default_branch`.
+
+        The VCS's `default_branch` is the branch cloned when there is no specific branch specified
+        (e.g. `git clone <URL>`).
+
+        Some VCS providers (GitHub and GitLab) send the `default_branch` via incoming webhooks.
+        We use that data to update our database and keep it in sync.
+
+        This solves the problem about "changing the default branch in GitHub"
+        and also importing repositories with a different `default_branch` than `main` manually:
+        https://github.com/readthedocs/readthedocs.org/issues/9367
+
+        In case the user already selected a `default-branch` from the "Advanced settings",
+        it does not override it.
+        """
+        if not self.project.default_branch:
+            (
+                self.project.versions.filter(slug=LATEST).update(
+                    identifier=default_branch
+                )
+            )
+
 
 class GitHubWebhookView(WebhookMixin, APIView):
 
@@ -417,6 +442,12 @@ def handle_webhook(self):
             event=event,
         )
 
+        # Always update `latest` branch to point to the default branch in the repository
+        # even if the event is not gonna be handled. This helps us to keep our db in sync.
+        default_branch = self.data.get("repository", {}).get("default_branch", None)
+        if default_branch:
+            self.update_default_branch(default_branch)
+
         if event == GITHUB_PING:
             return {"detail": "Webhook configured correctly"}
 
@@ -467,14 +498,15 @@ def handle_webhook(self):
         # Trigger a build for all branches in the push
         if event == GITHUB_PUSH:
             try:
-                branches = [self._normalize_ref(self.data['ref'])]
-                return self.get_response_push(self.project, branches)
+                branch = self._normalize_ref(self.data["ref"])
+                return self.get_response_push(self.project, [branch])
             except KeyError:
                 raise ParseError('Parameter "ref" is required')
 
         return None
 
     def _normalize_ref(self, ref):
+        """Remove `ref/(heads|tags)/` from the reference to match a Version on the db."""
         pattern = re.compile(r'^refs/(heads|tags)/')
         return pattern.sub('', ref)
 
@@ -568,6 +600,13 @@ def handle_webhook(self):
             data=self.request.data,
             event=event,
         )
+
+        # Always update `latest` branch to point to the default branch in the repository
+        # even if the event is not gonna be handled. This helps us to keep our db in sync.
+        default_branch = self.data.get("project", {}).get("default_branch", None)
+        if default_branch:
+            self.update_default_branch(default_branch)
+
         # Handle push events and trigger builds
         if event in (GITLAB_PUSH, GITLAB_TAG_PUSH):
             data = self.request.data
@@ -583,8 +622,8 @@ def handle_webhook(self):
                 return self.sync_versions_response(self.project)
             # Normal push to master
             try:
-                branches = [self._normalize_ref(data['ref'])]
-                return self.get_response_push(self.project, branches)
+                branch = self._normalize_ref(data["ref"])
+                return self.get_response_push(self.project, [branch])
             except KeyError:
                 raise ParseError('Parameter "ref" is required')
 
@@ -660,6 +699,11 @@ def handle_webhook(self):
             data=self.request.data,
             event=event,
         )
+
+        # NOTE: we can't call `self.update_default_branch` here because
+        # BitBucket does not tell us what is the `default_branch` for a
+        # repository in these incoming webhooks.
+
         if event == BITBUCKET_PUSH:
             try:
                 data = self.request.data
@@ -676,8 +720,11 @@ def handle_webhook(self):
                 # we don't trigger the sync versions, because that
                 # will be triggered with the normal push.
                 if branches:
-                    return self.get_response_push(self.project, branches)
-                log.debug('Triggered sync_versions.')
+                    return self.get_response_push(
+                        self.project,
+                        branches,
+                    )
+                log.debug("Triggered sync_versions.")
                 return self.sync_versions_response(self.project)
             except KeyError:
                 raise ParseError('Invalid request')
@@ -710,7 +757,8 @@ class APIWebhookView(WebhookMixin, APIView):
     Expects the following JSON::
 
         {
-            "branches": ["master"]
+            "branches": ["master"],
+            "default_branch": "main"
         }
     """
 
@@ -753,8 +801,13 @@ def handle_webhook(self):
                 'branches',
                 [self.project.get_default_branch()],
             )
+            default_branch = self.request.data.get("default_branch", None)
             if isinstance(branches, str):
                 branches = [branches]
+
+            if default_branch and isinstance(default_branch, str):
+                self.update_default_branch(default_branch)
+
             return self.get_response_push(self.project, branches)
         except TypeError:
             raise ParseError('Invalid request')

readthedocs/builds/managers.py

@@ -1,7 +1,6 @@
 """Build and Version class model Managers."""
 
 import structlog
-
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
 from polymorphic.managers import PolymorphicManager
@@ -50,6 +49,12 @@ def create_stable(self, **kwargs):
             'verbose_name': STABLE_VERBOSE_NAME,
             'machine': True,
             'active': True,
+            # TODO: double-check if we still require the `identifier: STABLE` field.
+            # At the time of creation, we don't really know what's the branch/tag identifier
+            # for the STABLE version. It makes sense to be `None`, probably.
+            #
+            # Note that we removed the `identifier: LATEST` from `create_latest` as a way to
+            # use the default branch.
             'identifier': STABLE,
             'type': TAG,
         }
@@ -62,7 +67,6 @@ def create_latest(self, **kwargs):
             'verbose_name': LATEST_VERBOSE_NAME,
             'machine': True,
             'active': True,
-            'identifier': LATEST,
             'type': BRANCH,
         }
         defaults.update(kwargs)

readthedocs/builds/migrations/0046_identifier_null.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.13 on 2022-07-11 08:53
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("builds", "0045_alter_build_status"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="version",
+            name="identifier",
+            field=models.CharField(
+                blank=True, max_length=255, null=True, verbose_name="Identifier"
+            ),
+        ),
+    ]

readthedocs/builds/models.py

@@ -119,11 +119,15 @@ class Version(TimeStampedModel):
     )
     # used by the vcs backend
 
-    #: The identifier is the ID for the revision this is version is for. This
-    #: might be the revision number (e.g. in SVN), or the commit hash (e.g. in
-    #: Git). If the this version is pointing to a branch, then ``identifier``
-    #: will contain the branch name.
-    identifier = models.CharField(_('Identifier'), max_length=255)
+    #: The identifier is the ID for the revision this is version is for.
+    #: This might be the revision number (e.g. in SVN),
+    #: or the commit hash (e.g. in Git).
+    #: If the this version is pointing to a branch,
+    #: then ``identifier`` will contain the branch name.
+    #: `None`/`null` means it will use the VCS default branch.
+    identifier = models.CharField(
+        _("Identifier"), max_length=255, null=True, blank=True
+    )
 
     #: This is the actual name that we got for the commit stored in
     #: ``identifier``. This might be the tag or branch name like ``"v1.0.4"``.
@@ -363,6 +367,11 @@ def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
     @property
     def identifier_friendly(self):
         """Return display friendly identifier."""
+        if not self.identifier:
+            # Text shown to user when we don't know yet what's the ``identifier`` for this version.
+            # This usually happens when we haven't pulled the ``default_branch`` for LATEST.
+            return "Unknown yet"
+
         if re.match(r'^[0-9a-f]{40}$', self.identifier, re.I):
             return self.identifier[:8]
         return self.identifier

readthedocs/core/views/hooks.py

@@ -52,7 +52,6 @@ def build_branches(project, branch_list):
     not_building = set()
     for branch in branch_list:
         versions = project.versions_from_branch_name(branch)
-
         for version in versions:
             log.debug(
                 'Processing.',

readthedocs/projects/migrations/0095_default_branch_helptext.py

@@ -0,0 +1,37 @@
+# Generated by Django 3.2.13 on 2022-07-11 09:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("projects", "0094_auto_20221221_1045"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="historicalproject",
+            name="default_branch",
+            field=models.CharField(
+                blank=True,
+                default=None,
+                help_text='What branch "latest" points to. Leave empty to use the default value for your VCS.',
+                max_length=255,
+                null=True,
+                verbose_name="Default branch",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="project",
+            name="default_branch",
+            field=models.CharField(
+                blank=True,
+                default=None,
+                help_text='What branch "latest" points to. Leave empty to use the default value for your VCS.',
+                max_length=255,
+                null=True,
+                verbose_name="Default branch",
+            ),
+        ),
+    ]

readthedocs/projects/models.py

@@ -175,8 +175,8 @@ class Project(models.Model):
         default=LATEST,
         help_text=_('The version of your project that / redirects to'),
     )
-    # In default_branch, None means the backend should choose the
-    # appropriate branch. Eg 'master' for git
+    # In default_branch, ``None`` means the backend will use the default branch
+    # cloned for each backend.
     default_branch = models.CharField(
         _('Default branch'),
         max_length=255,
@@ -185,8 +185,7 @@ class Project(models.Model):
         blank=True,
         help_text=_(
             'What branch "latest" points to. Leave empty '
-            'to use the default value for your VCS (eg. '
-            '<code>trunk</code> or <code>master</code>).',
+            "to use the default value for your VCS.",
         ),
     )
     requirements_file = models.CharField(
@@ -480,21 +479,21 @@ def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
             if not self.slug:
                 raise Exception(_('Model must have slug'))
         super().save(*args, **kwargs)
-        try:
-            latest = self.versions.filter(slug=LATEST).first()
-            default_branch = self.get_default_branch()
-            if latest and latest.identifier != default_branch:
-                latest.identifier = default_branch
-                latest.save()
-        except Exception:
-            log.exception('Failed to update latest identifier')
 
         try:
-            branch = self.get_default_branch()
             if not self.versions.filter(slug=LATEST).exists():
-                self.versions.create_latest(identifier=branch)
+                self.versions.create_latest()
         except Exception:
-            log.exception('Error creating default branches')
+            log.exception("Error creating default branches")
+
+        # Update `Version.identifier` for `latest` with the default branch the user has selected,
+        # even if it's `None` (meaning to match the `default_branch` of the repository)
+        # NOTE: this code is required to be *after* ``create_latest()``.
+        # It has to be updated after creating LATEST originally.
+        log.debug(
+            "Updating default branch.", slug=LATEST, identifier=self.default_branch
+        )
+        self.versions.filter(slug=LATEST).update(identifier=self.default_branch)
 
     def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
         from readthedocs.projects.tasks.utils import clean_project_resources