mirror of
https://github.com/gentoo-mirror/gentoo.git
synced 2025-12-08 00:05:33 +03:00
dev-util/rbtools: remove unused patch
Signed-off-by: Michael Mair-Keimberger <mm1ke@gentoo.org>
This commit is contained in:
parent
97e69b5219
commit
b5e6482732
@ -1,483 +0,0 @@
|
||||
From 36efa0724e3b4d941aa0554b1d82934319d25ce3 Mon Sep 17 00:00:00 2001
|
||||
From: David Trowbridge <trowbrds@gmail.com>
|
||||
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
|
||||
Loading…
x
Reference in New Issue
Block a user