commit: 5c161666a8f8a709f8ff8ea270109f88dc7c7ae6
Author: Hans de Graaff <graaff <AT> gentoo <DOT> org>
AuthorDate: Wed Jun 26 09:20:06 2024 +0000
Commit: Hans de Graaff <graaff <AT> gentoo <DOT> org>
CommitDate: Wed Jun 26 09:52:58 2024 +0000
URL: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=5c161666
dev-util/rbtools: backport upstream fix
This fixes a crash with reviewboard instances without powerpack.
Signed-off-by: Hans de Graaff <graaff <AT> gentoo.org>
.../rbtools/files/rbtools-5.0-scmtool-crash.patch | 483 +++++++++++++++++++++
.../{rbtools-5.0.ebuild => rbtools-5.0-r1.ebuild} | 4 +-
2 files changed, 485 insertions(+), 2 deletions(-)
diff --git a/dev-util/rbtools/files/rbtools-5.0-scmtool-crash.patch
b/dev-util/rbtools/files/rbtools-5.0-scmtool-crash.patch
new file mode 100644
index 000000000000..28783545edef
--- /dev/null
+++ b/dev-util/rbtools/files/rbtools-5.0-scmtool-crash.patch
@@ -0,0 +1,483 @@
+From 36efa0724e3b4d941aa0554b1d82934319d25ce3 Mon Sep 17 00:00:00 2001
+From: David Trowbridge <[email protected]>
+Date: Tue, 11 Jun 2024 17:22:15 -0600
+Subject: [PATCH] Use SCMTool IDs to look up repositories when possible.
+
+Many versions of Review Board currently have a bug where sending an
+unknown value in the tool= parameter to the repository list API would
+cause a crash. We'll be shipping a fix for that, but unfortunately there
+are a variety of released versions where the Git and Clearcase clients
+can trigger this crash on servers that do not have Power Pack installed.
+
+This change fixes the RBTools side to not include the Power Pack SCMTool
+names, which will avoid the problem for existing servers. For new
+servers, we'll be shipping a server-side fix that fixes the crash, adds
+the SCMTool IDs to the capabilities, and allows us to pass SCMTool IDs
+rather than names to the repository list API. If we see the IDs in the
+capability blob, we can assume that we can pass the IDs, including
+potentially unknown IDs.
+
+This also fixes a problem where we were sometimes accessing the
+repository list API twice with exactly the same parameters.
+
+Testing Done:
+- Ran unit tests.
+- Verified that the repository list API was accessed using SCMTool names
+ that did not include potentially missing ones when running against an
+ older server.
+- Verified that the repository list API was accessed with SCMTool IDs
+ when running against a server with the new API fixes.
+
+Reviewed at https://reviews.reviewboard.org/r/13973/
+---
+ rbtools/clients/base/scmclient.py | 59 +++++++++++++++++++++++++------
+ rbtools/clients/bazaar.py | 1 +
+ rbtools/clients/clearcase.py | 10 +++++-
+ rbtools/clients/cvs.py | 1 +
+ rbtools/clients/git.py | 10 +++++-
+ rbtools/clients/mercurial.py | 1 +
+ rbtools/clients/perforce.py | 1 +
+ rbtools/clients/plastic.py | 1 +
+ rbtools/clients/svn.py | 1 +
+ rbtools/clients/tfs.py | 1 +
+ rbtools/commands/base/commands.py | 3 +-
+ rbtools/commands/setup_repo.py | 25 +++++++++----
+ rbtools/utils/repository.py | 58 ++++++++++++++++++++++--------
+ 13 files changed, 139 insertions(+), 33 deletions(-)
+
+diff --git a/rbtools/clients/base/scmclient.py
b/rbtools/clients/base/scmclient.py
+index c053b91b..9f56c084 100644
+--- a/rbtools/clients/base/scmclient.py
++++ b/rbtools/clients/base/scmclient.py
+@@ -9,24 +9,24 @@
+ import argparse
+ import logging
+ import re
+-from typing import (Any, Dict, List, Mapping, Optional, Tuple, Union, cast,
+- TYPE_CHECKING)
++from typing import (Any, cast, ClassVar, Dict, List, Mapping, Optional,
++ TYPE_CHECKING, Tuple, Union)
+
+ from typing_extensions import NotRequired, TypedDict, final
+
+-from rbtools.api.capabilities import Capabilities
+-from rbtools.api.resource import (ItemResource,
+- ListResource,
+- ReviewRequestResource)
+ from rbtools.clients.base.patch import PatchAuthor, PatchResult
+-from rbtools.clients.base.repository import RepositoryInfo
+ from rbtools.clients.errors import SCMClientDependencyError, SCMError
+ from rbtools.deprecation import RemovedInRBTools50Warning
+-from rbtools.diffs.tools.base import BaseDiffTool
+ from rbtools.diffs.tools.registry import diff_tools_registry
+ from rbtools.utils.process import execute
+
+ if TYPE_CHECKING:
++ from rbtools.api.capabilities import Capabilities
++ from rbtools.api.resource import (ItemResource,
++ ListResource,
++ ReviewRequestResource)
++ from rbtools.clients.base.repository import RepositoryInfo
++ from rbtools.diffs.tools.base import BaseDiffTool
+ from rbtools.config import RBToolsConfig
+
+
+@@ -281,14 +281,27 @@ class BaseSCMClient(object):
+ #: str
+ name: str = ''
+
+- #: A comma-separated list of SCMClient names on the server
++ #: A comma-separated list of SCMClient names on the server.
+ #:
+ #: Version Added:
+ #: 3.0
+ #:
+ #: Type:
+ #: str
+- server_tool_names: Optional[str] = None
++ server_tool_names: ClassVar[Optional[str]] = None
++
++ #: A comma-separated list of SCMClient IDs on the server.
++ #:
++ #: This supersedes :py:attr:`server_tool_names` when running on a version
++ #: of Review Board that supports passing tool IDs to the repositories
++ #: list API.
++ #:
++ #: Version Added:
++ #: 5.0.1
++ #:
++ #: Type:
++ #: str
++ server_tool_ids: ClassVar[Optional[List[str]]] = None
+
+ #: Whether this tool requires a command line diff tool.
+ #:
+@@ -692,6 +705,32 @@ def get_diff_tool(self) -> Optional[BaseDiffTool]:
+
+ return diff_tool
+
++ def get_server_tool_names(
++ self,
++ capabilities: Optional[Capabilities],
++ ) -> Optional[str]:
++ """Return the list of supported tool names on the server.
++
++ Version Added:
++ 5.0.1
++
++ Args:
++ capabilities (rbtools.api.capabilities.Capabilities):
++ The server capabilities, if present.
++
++ Returns:
++ str:
++ A comma-separated list of server-side tool names to match with.
++ """
++ if (capabilities is not None and
++ capabilities.get_capability('scmtools', 'supported_tools') and
++ self.server_tool_ids is not None):
++ # Versions of Review Board that have this capability allow us to
++ # pass SCMTool IDs rather than names.
++ return ','.join(self.server_tool_ids)
++ else:
++ return self.server_tool_names
++
+ def find_matching_server_repository(
+ self,
+ repositories: ListResource,
+diff --git a/rbtools/clients/bazaar.py b/rbtools/clients/bazaar.py
+index 0e39315d..4174c010 100644
+--- a/rbtools/clients/bazaar.py
++++ b/rbtools/clients/bazaar.py
+@@ -37,6 +37,7 @@ class BazaarClient(BaseSCMClient):
+ scmclient_id = 'bazaar'
+ name = 'Bazaar'
+ server_tool_names = 'Bazaar'
++ server_tool_ids = ['bazaar']
+ supports_diff_exclude_patterns = True
+ supports_parent_diffs = True
+ can_branch = True
+diff --git a/rbtools/clients/clearcase.py b/rbtools/clients/clearcase.py
+index f9c8762e..e2c04633 100644
+--- a/rbtools/clients/clearcase.py
++++ b/rbtools/clients/clearcase.py
+@@ -392,7 +392,15 @@ class ClearCaseClient(BaseSCMClient):
+
+ scmclient_id = 'clearcase'
+ name = 'VersionVault / ClearCase'
+- server_tool_names = 'ClearCase,VersionVault / ClearCase'
++
++ # Review Board versions that use the old names-based repositories/?tool=
++ # API parameter also have a bug where a missing name could cause a
++ # server-side crash. This was making it so servers that did not have Power
++ # Pack were failing when we tried to make a query that included the
++ # VersionVault name. We therefore only include it when we know the server
++ # can use server_tool_ids instead.
++ server_tool_names = 'ClearCase'
++ server_tool_ids = ['clearcase', 'versionvault']
+
+ requires_diff_tool = True
+
+diff --git a/rbtools/clients/cvs.py b/rbtools/clients/cvs.py
+index 98354e8b..dd2c72ef 100644
+--- a/rbtools/clients/cvs.py
++++ b/rbtools/clients/cvs.py
+@@ -28,6 +28,7 @@ class CVSClient(BaseSCMClient):
+ scmclient_id = 'cvs'
+ name = 'CVS'
+ server_tool_names = 'CVS'
++ server_tool_ids = ['cvs']
+ supports_diff_exclude_patterns = True
+ supports_patch_revert = True
+
+diff --git a/rbtools/clients/git.py b/rbtools/clients/git.py
+index f4b96cb6..431fd7c6 100644
+--- a/rbtools/clients/git.py
++++ b/rbtools/clients/git.py
+@@ -76,7 +76,15 @@ class GitClient(BaseSCMClient):
+
+ scmclient_id = 'git'
+ name = 'Git'
+- server_tool_names = 'Git,Perforce,Subversion,Team Foundation Server (git)'
++
++ # Review Board versions that use the old names-based repositories/?tool=
++ # API parameter also have a bug where a missing name could cause a
++ # server-side crash. This was making it so servers that did not have Power
++ # Pack were failing when we tried to make a query that included the
TFS-Git
++ # name. We therefore only include it when we know the server can use
++ # server_tool_ids instead.
++ server_tool_names = 'Git,Perforce,Subversion'
++ server_tool_ids = ['git', 'perforce', 'subversion', 'tfs_git']
+
+ supports_commit_history = True
+ supports_diff_exclude_patterns = True
+diff --git a/rbtools/clients/mercurial.py b/rbtools/clients/mercurial.py
+index 2ee473c9..6f1c5ea0 100644
+--- a/rbtools/clients/mercurial.py
++++ b/rbtools/clients/mercurial.py
+@@ -64,6 +64,7 @@ class MercurialClient(BaseSCMClient):
+ scmclient_id = 'mercurial'
+ name = 'Mercurial'
+ server_tool_names = 'Mercurial,Subversion'
++ server_tool_ids = ['mercurial', 'subversion']
+
+ supports_commit_history = True
+ supports_diff_exclude_patterns = True
+diff --git a/rbtools/clients/perforce.py b/rbtools/clients/perforce.py
+index f5892073..4f5d7fb1 100644
+--- a/rbtools/clients/perforce.py
++++ b/rbtools/clients/perforce.py
+@@ -445,6 +445,7 @@ class PerforceClient(BaseSCMClient):
+ scmclient_id = 'perforce'
+ name = 'Perforce'
+ server_tool_names = 'Perforce'
++ server_tool_ids = ['perforce']
+
+ requires_diff_tool = True
+
+diff --git a/rbtools/clients/plastic.py b/rbtools/clients/plastic.py
+index c45c7abd..3bec215a 100644
+--- a/rbtools/clients/plastic.py
++++ b/rbtools/clients/plastic.py
+@@ -28,6 +28,7 @@ class PlasticClient(BaseSCMClient):
+ scmclient_id = 'plastic'
+ name = 'Plastic'
+ server_tool_names = 'Plastic SCM'
++ server_tool_ids = ['plastic']
+ supports_changesets = True
+ supports_patch_revert = True
+
+diff --git a/rbtools/clients/svn.py b/rbtools/clients/svn.py
+index 34cdffcb..6d0352cc 100644
+--- a/rbtools/clients/svn.py
++++ b/rbtools/clients/svn.py
+@@ -52,6 +52,7 @@ class SVNClient(BaseSCMClient):
+ scmclient_id = 'svn'
+ name = 'Subversion'
+ server_tool_names = 'Subversion'
++ server_tool_ids = ['subversion']
+
+ requires_diff_tool = True
+
+diff --git a/rbtools/clients/tfs.py b/rbtools/clients/tfs.py
+index 2b70fae3..7b02d3de 100644
+--- a/rbtools/clients/tfs.py
++++ b/rbtools/clients/tfs.py
+@@ -1321,6 +1321,7 @@ class TFSClient(BaseSCMClient):
+ scmclient_id = 'tfs'
+ name = 'Team Foundation Server'
+ server_tool_names = 'Team Foundation Server'
++ server_tool_ids = ['tfs']
+
+ requires_diff_tool = True
+
+diff --git a/rbtools/commands/base/commands.py
b/rbtools/commands/base/commands.py
+index 3313a689..d4e20f61 100644
+--- a/rbtools/commands/base/commands.py
++++ b/rbtools/commands/base/commands.py
+@@ -939,7 +939,8 @@ def initialize(self) -> None:
+ api_root=self.api_root,
+ tool=tool,
+ repository_name=options.repository_name,
+- repository_paths=repository_info.path)
++ repository_paths=repository_info.path,
++ capabilities=self.capabilities)
+ self.repository = repository
+
+ if repository:
+diff --git a/rbtools/commands/setup_repo.py b/rbtools/commands/setup_repo.py
+index 615bc9e3..9905d1b5 100644
+--- a/rbtools/commands/setup_repo.py
++++ b/rbtools/commands/setup_repo.py
+@@ -1,14 +1,20 @@
+ """Implementation of rbt setup-repo."""
+
++from __future__ import annotations
++
+ import difflib
+ import os
+ import textwrap
++from typing import Optional, TYPE_CHECKING, Union
+
+ from rbtools.commands.base import BaseCommand, CommandError
+ from rbtools.config.loader import CONFIG_FILENAME
+ from rbtools.utils.console import confirm, confirm_select
+ from rbtools.utils.repository import get_repository_resource
+
++if TYPE_CHECKING:
++ from rbtools.api.resource import ItemResource, RootResource
++
+
+ class SetupRepo(BaseCommand):
+ """Configure a repository to point to a Review Board server.
+@@ -38,22 +44,27 @@ class SetupRepo(BaseCommand):
+ BaseCommand.tfs_options,
+ ]
+
+- def prompt_rb_repository(self, local_tool_name, server_tool_names,
+- repository_paths, api_root):
++ def prompt_rb_repository(
++ self,
++ local_tool_name: str,
++ server_tool_names: Optional[str],
++ repository_paths: Optional[Union[str, list[str]]],
++ api_root: RootResource,
++ ) -> Optional[ItemResource]:
+ """Interactively prompt to select a matching repository.
+
+ The user is prompted to choose a matching repository found on the
+ Review Board server.
+
+ Args:
+- local_tool_name (unicode):
++ local_tool_name (str):
+ The local name of the detected tool.
+
+- server_tool_names (unicode):
++ server_tool_names (str):
+ A comma-separated list of potentially matching SCMTool names
in
+ the Review Board server.
+
+- repository_paths (list or unicode, optional):
++ repository_paths (list or str, optional):
+ A list of potential paths to match for the repository.
+
+ api_root (rbtools.api.resource.RootResource):
+@@ -192,9 +203,11 @@ def main(self, *args):
+ while True:
+ self.stdout.new_line()
+ self.stdout.write('Current server: %s' % server)
++
++ tool_names = tool.get_server_tool_names(self.capabilities)
+ selected_repo = self.prompt_rb_repository(
+ local_tool_name=tool.name,
+- server_tool_names=tool.server_tool_names,
++ server_tool_names=tool_names,
+ repository_paths=repository_info.path,
+ api_root=api_root)
+
+diff --git a/rbtools/utils/repository.py b/rbtools/utils/repository.py
+index d30d2cc0..d3ec280d 100644
+--- a/rbtools/utils/repository.py
++++ b/rbtools/utils/repository.py
+@@ -1,17 +1,34 @@
+ """Utility functions for working with repositories."""
+
++from __future__ import annotations
++
++from typing import Optional, TYPE_CHECKING, Union
++
+ from rbtools.api.errors import APIError
+
++if TYPE_CHECKING:
++ from rbtools.api.capabilities import Capabilities
++ from rbtools.api.resource import ItemResource, RootResource
++ from rbtools.clients.base.repository import RepositoryInfo
++ from rbtools.clients.base.scmclient import BaseSCMClient
++
+
+-def get_repository_resource(api_root,
+- tool=None,
+- repository_name=None,
+- repository_paths=None):
++def get_repository_resource(
++ api_root: RootResource,
++ tool: Optional[BaseSCMClient] = None,
++ repository_name: Optional[str] = None,
++ repository_paths: Optional[Union[str, list[str]]] = None,
++ capabilities: Optional[Capabilities] = None,
++) -> tuple[Optional[ItemResource], Optional[ItemResource]]:
+ """Return the API resource for the matching repository on the server.
+
+ Version Added:
+ 3.0
+
++ Version Changed:
++ 5.0.1:
++ Added the ``capabilities`` argument.
++
+ Args:
+ api_root (rbtools.api.resource.RootResource):
+ The root resource for the API.
+@@ -19,12 +36,15 @@ def get_repository_resource(api_root,
+ tool (rbtools.clients.base.BaseSCMClient, optional):
+ The SCM client corresponding to the local working directory.
+
+- repository_name (unicode, optional):
++ repository_name (str, optional):
+ An explicit repository name provided by the local configuration.
+
+- repository_paths (list or unicode, optional):
++ repository_paths (list or str, optional):
+ A list of potential paths to match for the repository.
+
++ capabilities (rbtools.api.capabilities.Capabilities, optional):
++ The capabilities fetched from the server.
++
+ Returns:
+ tuple of rbtools.api.resource.ItemResource:
+ A 2-tuple of :py:class:`~rbtools.api.resource.ItemResource`. The first
+@@ -47,8 +67,11 @@ def _get_info(repository):
+ 'only_links': 'info,diff_file_attachments',
+ }
+
+- if tool and tool.server_tool_names:
+- query['tool'] = tool.server_tool_names
++ if tool:
++ server_tool_names = tool.get_server_tool_names(capabilities)
++
++ if server_tool_names:
++ query['tool'] = server_tool_names
+
+ if repository_name:
+ query['name'] = repository_name
+@@ -70,9 +93,12 @@ def _get_info(repository):
+ # configured path than the client. In that case, we want to try again
+ # without filtering by path, and ask each tool to match based on other
+ # conditions.
+- query.pop('path', None)
++ if 'path' in query:
++ query.pop('path', None)
+
+- all_repositories = api_root.get_repositories(**query)
++ all_repositories = api_root.get_repositories(**query)
++ else:
++ all_repositories = repositories
+
+ if all_repositories.total_results > 0 and tool:
+ repository, info = tool.find_matching_server_repository(
+@@ -93,7 +119,11 @@ def _get_info(repository):
+ return None, None
+
+
+-def get_repository_id(repository_info, api_root, repository_name=None):
++def get_repository_id(
++ repository_info: RepositoryInfo,
++ api_root: RootResource,
++ repository_name: Optional[str] = None,
++) -> Optional[int]:
+ """Return the ID of a repository from the server.
+
+ This will look up all accessible repositories on the server and try to
+@@ -106,18 +136,18 @@ def get_repository_id(repository_info, api_root,
repository_name=None):
+ api_root (rbtools.api.resource.RootResource):
+ The root resource for the API.
+
+- repository_name (unicode, optional):
++ repository_name (str, optional):
+ An explicit repository name provided by local configuration.
+
+ Returns:
+ int:
+ The ID of the repository, or ``None`` if not found.
+ """
+- repository, info = get_repository_resource(
++ repository = get_repository_resource(
+ api_root,
+ tool=None,
+ repository_name=repository_name,
+- repository_paths=repository_info.path)
++ repository_paths=repository_info.path)[0]
+
+ if repository:
+ return repository.id
diff --git a/dev-util/rbtools/rbtools-5.0.ebuild
b/dev-util/rbtools/rbtools-5.0-r1.ebuild
similarity index 93%
rename from dev-util/rbtools/rbtools-5.0.ebuild
rename to dev-util/rbtools/rbtools-5.0-r1.ebuild
index 6a99f1af7e31..e3d01d77608e 100644
--- a/dev-util/rbtools/rbtools-5.0.ebuild
+++ b/dev-util/rbtools/rbtools-5.0-r1.ebuild
@@ -16,7 +16,7 @@ S="${WORKDIR}/rbtools-release-${PV}"
LICENSE="MIT"
SLOT="0"
KEYWORDS="~amd64 ~x86"
-IUSE=""
+IUSE="test"
RDEPEND="
>=dev-python/certifi-2023.5.7[${PYTHON_USEDEP}]
@@ -40,7 +40,7 @@ BDEPEND="
)
"
-PATCHES=( "${FILESDIR}/${P}-importlib-resources.patch" )
+PATCHES=( "${FILESDIR}/${P}-importlib-resources.patch"
"${FILESDIR}/${P}-scmtool-crash.patch" )
DOCS=( AUTHORS NEWS README.md )
