Automatically detect default branch of custom repo

AddonManagerOptions.py

@@ -474,7 +474,8 @@ def _add_custom_repo_clicked(self):
         """Callback: show the Add custom repo dialog"""
         dlg = CustomRepositoryDialog()
         url, branch = dlg.exec()
-        if url and branch:
+        if url:
+            # An empty branch is allowed: it means the repository's default branch
             self.table_model.appendData(url, branch)
 
     def _remove_custom_repo_clicked(self):
@@ -494,7 +495,7 @@ def _row_double_clicked(self, item):
         dlg.dialog.urlLineEdit.setText(self.table_model.data(url_index))
         dlg.dialog.branchLineEdit.setText(self.table_model.data(branch_index))
         url, branch = dlg.exec()
-        if url and branch:
+        if url:
             self.table_model.setData(url_index, url)
             self.table_model.setData(branch_index, branch)
 
@@ -515,25 +516,23 @@ def load_model(self):
         pref_entry: str = self.pref.GetString("CustomRepositories", "")
 
         # The entry is saved as a space- and newline-delimited text block: break it into its
-        # constituent parts
+        # constituent parts. A line with no branch means the repository's default branch.
         lines = pref_entry.split("\n")
         self.model = []
         for line in lines:
             if not line:
                 continue
             split_data = line.split()
-            if len(split_data) > 1:
-                branch = split_data[1]
-            else:
-                branch = "master"
+            branch = split_data[1] if len(split_data) > 1 else ""
             url = split_data[0]
             self.model.append([url, branch])
 
     def save_model(self):
         """Save the data into a preferences entry"""
         entry = ""
         for row in self.model:
-            entry += f"{row[0]} {row[1]}\n"
+            url, branch = row[0], row[1]
+            entry += f"{url} {branch}\n" if branch else f"{url}\n"
         self.pref.SetString("CustomRepositories", entry)
 
     def rowCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
@@ -623,12 +622,16 @@ def __init__(self):
             os.path.join(os.path.dirname(__file__), "AddonManagerOptions_AddCustomRepository.ui")
         )
         self.dialog.setObjectName("AddonManager_AddCustomRepositoryDialog")
+        self.dialog.branchLineEdit.setPlaceholderText(
+            translate("AddonsInstaller", "Leave blank to use the repository's default branch")
+        )
 
     def exec(self):
-        """Run the dialog as a modal, and return either None or a tuple of (url,branch)"""
+        """Run the dialog as a modal, and return either None or a tuple of (url,branch). The branch
+        may be empty, meaning the repository's default branch, whatever that turns out to be."""
         result = self.dialog.exec()
         if result == QtWidgets.QDialog.Accepted:
-            url = self.dialog.urlLineEdit.text()
-            branch = self.dialog.branchLineEdit.text()
+            url = self.dialog.urlLineEdit.text().strip()
+            branch = self.dialog.branchLineEdit.text().strip()
             return url, branch
         return None, None

AddonManagerTest/app/test_workers_startup.py

@@ -28,6 +28,7 @@
 import addonmanager_utilities as utils
 import addonmanager_workers_startup
 from Addon import Addon
+from addonmanager_git import GitFailed
 from PySideWrapper import QtCore
 
 
@@ -355,6 +356,142 @@ def test_a_host_that_changed_software_is_identified_again(self, mock_network_man
         self.assertEqual("My Custom Addon", addon.display_name)
 
 
+class TestCustomAddonDefaultBranch(unittest.TestCase):
+    """A custom repository whose branch the user did not name. Its default branch has to be worked
+    out: guessing "master" has been wrong by default since GitHub renamed it in 2020."""
+
+    _URL = "https://git.example.com/user/addon"
+
+    def setUp(self):
+        # Looking for the branch identifies the git host as a side effect, and that is remembered
+        utils.forget_git_hosts()
+        self.addCleanup(utils.forget_git_hosts)
+
+    def _worker(self) -> addonmanager_workers_startup.CreateAddonListWorker:
+        worker = addonmanager_workers_startup.CreateAddonListWorker()
+        worker.current_thread = MagicMock()
+        worker.current_thread.isInterruptionRequested.return_value = False
+        return worker
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_git_is_asked_for_the_default_branch(self, mock_initialize_git, _):
+        """Git knows the answer exactly, whatever the branch is called and whatever the host is."""
+        mock_initialize_git.return_value.default_branch.return_value = "development"
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("development", branch)
+        mock_initialize_git.return_value.default_branch.assert_called_once_with(self._URL)
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.NetworkManager.AM_NETWORK_MANAGER")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_without_git_the_usual_names_are_tried(self, mock_initialize_git, mock_network, _):
+        """Without git, the repository is asked for a package.xml on each of the usual names."""
+        mock_initialize_git.return_value = None
+        mock_network.blocking_get_with_retries.side_effect = _serve_urls(
+            {f"{self._URL}/-/raw/main/package.xml": _package_xml("1.0.0")}
+        )
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("main", branch)
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.NetworkManager.AM_NETWORK_MANAGER")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_without_git_master_is_used_if_main_does_not_exist(
+        self, mock_initialize_git, mock_network, _
+    ):
+        mock_initialize_git.return_value = None
+        mock_network.blocking_get_with_retries.side_effect = _serve_urls(
+            {f"{self._URL}/-/raw/master/package.xml": _package_xml("1.0.0")}
+        )
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("master", branch)
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.NetworkManager.AM_NETWORK_MANAGER")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_git_failing_falls_back_to_the_usual_names(self, mock_initialize_git, mock_network, _):
+        """Git is there but cannot answer, so the usual names are tried instead of giving up."""
+        mock_initialize_git.return_value.default_branch.side_effect = GitFailed("no such repo")
+        mock_network.blocking_get_with_retries.side_effect = _serve_urls(
+            {f"{self._URL}/-/raw/main/package.xml": _package_xml("1.0.0")}
+        )
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("main", branch)
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.NetworkManager.AM_NETWORK_MANAGER")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_nothing_works_so_the_old_default_is_used(self, mock_initialize_git, mock_network, _):
+        """When the branch cannot be worked out at all, the addon still gets the branch it has
+        always been given, and the user is told to name one themselves."""
+        mock_initialize_git.return_value = None
+        mock_network.blocking_get_with_retries.side_effect = _serve_urls({})
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("master", branch)
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.NetworkManager.AM_NETWORK_MANAGER")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_a_sign_in_page_is_not_mistaken_for_a_branch(
+        self, mock_initialize_git, mock_network, _
+    ):
+        """A git host that requires a login answers every request with a 200 and a sign-in page,
+        which must not be taken as proof that the branch exists."""
+        mock_initialize_git.return_value = None
+        mock_network.blocking_get_with_retries.side_effect = lambda url, *_a, **_k: (
+            _make_network_reply(b"<!DOCTYPE html><html>Please sign in</html>")
+        )
+
+        branch = self._worker()._default_branch_of(self._URL)
+
+        self.assertEqual("master", branch)  # The fallback, not "main"
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.CreateAddonListWorker.addon_repo")
+    @patch("addonmanager_workers_startup.fci.Preferences")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_a_branch_the_user_named_is_never_second_guessed(
+        self, mock_initialize_git, mock_preferences, mock_addon_repo_signal, _
+    ):
+        """The user named a branch, so the repository is not asked about it at all."""
+        mock_preferences.return_value.get.return_value = f"{self._URL} their-branch"
+        worker = self._worker()
+
+        with patch.object(worker, "_create_custom_addon") as mock_create:
+            worker._get_custom_addons()
+
+        mock_initialize_git.assert_not_called()
+        mock_create.assert_called_once_with("addon", self._URL, "their-branch")
+
+    @patch("addonmanager_workers_startup.fci.Console")
+    @patch("addonmanager_workers_startup.CreateAddonListWorker.addon_repo")
+    @patch("addonmanager_workers_startup.fci.Preferences")
+    @patch("addonmanager_workers_startup.initialize_git")
+    def test_the_detected_branch_is_the_one_the_addon_is_built_with(
+        self, mock_initialize_git, mock_preferences, mock_addon_repo_signal, _
+    ):
+        """The branch that was worked out is the one the addon is fetched and installed from."""
+        mock_preferences.return_value.get.return_value = self._URL  # No branch named
+        mock_initialize_git.return_value.default_branch.return_value = "development"
+        worker = self._worker()
+
+        with patch.object(worker, "_create_custom_addon") as mock_create:
+            worker._get_custom_addons()
+
+        mock_create.assert_called_once_with("addon", self._URL, "development")
+
+
 class TestCustomAddons(unittest.TestCase):
     """Tests for the custom repository handling in CreateAddonListWorker."""
 
@@ -394,7 +531,8 @@ def _create_addon(self) -> Addon:
 
     @patch("addonmanager_workers_startup.fci.Preferences")
     def test_parse_custom_repositories(self, mock_preferences):
-        """Each line is parsed into a URL and a branch, with "master" as the default branch."""
+        """Each line is parsed into a URL and a branch. A line with no branch yields an empty one:
+        the repository is asked what its default branch is, rather than being assumed."""
         mock_preferences.return_value.get.return_value = "\n".join(
             [
                 "https://github.com/myorg/no-branch-given",
@@ -409,10 +547,10 @@ def test_parse_custom_repositories(self, mock_preferences):
 
         self.assertEqual(
             [
-                ("https://github.com/myorg/no-branch-given", "master"),
+                ("https://github.com/myorg/no-branch-given", ""),
                 ("https://github.com/myorg/branch-given", "other-branch"),
-                ("https://github.com/myorg/trailing-slash", "master"),
-                ("https://github.com/myorg/dot-git", "master"),
+                ("https://github.com/myorg/trailing-slash", ""),
+                ("https://github.com/myorg/dot-git", ""),
             ],
             result,
         )

addonmanager_git.py

@@ -270,6 +270,22 @@ def repair(self, remote, local_path):
         os.chdir(original_cwd)
         shutil.rmtree(backup_path, ignore_errors=True)
 
+    def default_branch(self, remote: str) -> str:
+        """Get the name of the default branch of a remote repository, without cloning it. Works with
+        any git host, since it asks the repository itself rather than the software hosting it.
+        Raises GitFailed if the default branch cannot be determined."""
+
+        response = self._synchronous_call_git(["ls-remote", "--symref", remote, "HEAD"])
+        for line in response.split("\n"):
+            # The line we want looks like:
+            # ref: refs/heads/main    HEAD
+            if line.startswith("ref:"):
+                reference = line.split()[1]
+                prefix = "refs/heads/"
+                if reference.startswith(prefix):
+                    return reference[len(prefix) :]  # A branch name may itself contain a slash
+        raise GitFailed(f"Could not determine the default branch of {remote}")
+
     def get_remote(self, local_path) -> str:
         """Get the repository that this local path is set to fetch from"""
         old_dir = os.getcwd()

addonmanager_workers_startup.py

@@ -66,6 +66,11 @@ class CreateAddonListWorker(QtCore.QThread):
     RETRY_DELAY_MS = 3000
     ATTEMPT_TIMEOUT_MS = 30000
 
+    # The names a default branch is given, tried in this order when git is not available to ask the
+    # repository what its default branch actually is. The last of them is the fallback when nothing
+    # else works, which is what a custom repository has always used.
+    DEFAULT_BRANCH_NAMES = ("main", "master")
+
     def __init__(self):
         QtCore.QThread.__init__(self)
         self.setObjectName("CreateAddonListWorker")
@@ -111,23 +116,77 @@ def _get_custom_addons(self):
                     + "\n"
                 )
                 continue
+            if not branch:
+                branch = self._default_branch_of(url)
             fci.Console.PrintLog(f"Adding custom location {url} with branch {branch}\n")
             self.package_names.append(name)
             self.addon_repo.emit(self._create_custom_addon(name, url, branch))
 
     @staticmethod
     def _parse_custom_repositories() -> List[Tuple[str, str]]:
         """Parse the CustomRepositories preference into a list of (url, branch) pairs. Each line of
-        the preference is a repository URL, optionally followed by a space and a branch name."""
+        the preference is a repository URL, optionally followed by a space and a branch name. The
+        branch is empty when the user did not name one, and has to be worked out from the
+        repository itself."""
 
         repositories = []
         for line in fci.Preferences().get("CustomRepositories").split("\n"):
             url, _, branch = line.strip().partition(" ")
             url = url.rstrip("/").split(".git")[0]
             if url:
-                repositories.append((url, branch.strip() if branch.strip() else "master"))
+                repositories.append((url, branch.strip()))
         return repositories
 
+    def _default_branch_of(self, url: str) -> str:
+        """Work out which branch to use for a custom repository whose branch the user did not name.
+
+        Asking git is exact and works with any host, because the repository is asked about itself
+        rather than the software hosting it. Without git, the two names that a default branch almost
+        always has are tried instead, which does not cover a repository whose default branch is
+        named anything else."""
+
+        git_manager = initialize_git()
+        if git_manager:
+            try:
+                branch = git_manager.default_branch(url)
+                fci.Console.PrintLog(f"The default branch of {url} is '{branch}'\n")
+                return branch
+            except GitFailed as e:
+                fci.Console.PrintLog(f"Could not ask git for the default branch of {url}: {e}\n")
+
+        for candidate in CreateAddonListWorker.DEFAULT_BRANCH_NAMES:
+            if self._repository_has_branch(url, candidate):
+                fci.Console.PrintLog(f"Using branch '{candidate}' of {url}\n")
+                return candidate
+
+        fci.Console.PrintWarning(
+            translate(
+                "AddonsInstaller",
+                "Could not determine the default branch of the custom repository {}, so '{}' is "
+                "being used. If that is the wrong branch, set the correct one in the Addon Manager "
+                "preferences.",
+            ).format(url, CreateAddonListWorker.DEFAULT_BRANCH_NAMES[-1])
+            + "\n"
+        )
+        return CreateAddonListWorker.DEFAULT_BRANCH_NAMES[-1]
+
+    def _repository_has_branch(self, url: str, branch: str) -> bool:
+        """Whether the repository serves an addon's package.xml on the given branch.
+
+        If the software running the git host has not been identified yet, then which URL the file
+        would live at is not known either, so every layout is asked: the host gets identified in the
+        process, and the answer is kept for later."""
+
+        location = SimpleNamespace(url=url, branch=branch)
+        if utils.git_host_of(location) is not None:
+            return self._serves_package_xml(utils.construct_git_url(location, "package.xml"))
+
+        host = utils.identify_git_host(location, self._serves_package_xml)
+        if host is None:
+            return False
+        utils.remember_git_host(location, host)
+        return True
+
     def _create_custom_addon(self, name: str, url: str, branch: str) -> Addon:
         """Create an Addon for a custom repository by synthesizing the catalog entry that the
         remote addon catalog would have contained for it, so that a custom addon is constructed by