Fix Pylint and Mypy errors and refactor
Test: TreeHugger
Change-Id: Ie7ec805a360b859f3d24301cb5d05443e8ee4640
diff --git a/archive_utils.py b/archive_utils.py
index 3c633d4..fd3cb65 100644
--- a/archive_utils.py
+++ b/archive_utils.py
@@ -119,7 +119,7 @@
"""Finds the real root of an extracted archive.
Sometimes archives has additional layers of directories. This function tries
- to guess the right 'root' path by entering all single sub-directories.
+ to guess the right 'root' path by entering all single subdirectories.
Args:
path: Path to the extracted archive.
diff --git a/base_updater.py b/base_updater.py
index e11eab8..5339fb3 100644
--- a/base_updater.py
+++ b/base_updater.py
@@ -32,7 +32,7 @@
self._new_identifier = metadata_pb2.Identifier()
self._new_identifier.CopyFrom(old_identifier)
- self._suggested_new_ver = None
+ self._alternative_new_ver = None
self._has_errors = False
@@ -107,9 +107,9 @@
return self._has_errors
@property
- def suggested_latest_version(self) -> str | None:
- """Gets suggested latest version."""
- return self._suggested_new_ver
+ def alternative_latest_version(self) -> str | None:
+ """Gets alternative latest version."""
+ return self._alternative_new_ver
def refresh_without_upgrading(self) -> None:
"""Uses current version and url as the latest to refresh project."""
diff --git a/crates_updater.py b/crates_updater.py
index 053cf6e..7158836 100644
--- a/crates_updater.py
+++ b/crates_updater.py
@@ -139,7 +139,7 @@
self.download_url = "https://crates.io" + data["version"]["dl_path"]
def set_new_version_to_old(self):
- super().set_new_version_to_old()
+ super().refresh_without_upgrading()
# A shortcut to use the static download path.
self.download_url = f"https://static.crates.io/crates/{self.package}/" \
f"{self.package}-{self._new_identifier.version}.crate"
@@ -177,8 +177,10 @@
updated_metadata = super().update_metadata(metadata)
for identifier in updated_metadata.third_party.identifier:
if identifier.version:
- identifier.value = f"https://static.crates.io/crates/{updated_metadata.name}/" \
- f"{updated_metadata.name}-{self.latest_identifier.version}.crate"
+ identifier.value = f"https://static.crates.io/crates/" \
+ f"{updated_metadata.name}/"\
+ f"{updated_metadata.name}" \
+ f"-{self.latest_identifier.version}.crate"
break
# copy description from Cargo.toml to METADATA
cargo_toml = os.path.join(self.project_path, "Cargo.toml")
diff --git a/external_updater.py b/external_updater.py
index 83a840f..2871889 100644
--- a/external_updater.py
+++ b/external_updater.py
@@ -87,7 +87,7 @@
metadata = fileutils.read_metadata(proj_path)
metadata = fileutils.convert_url_to_identifier(metadata)
updater = updater_utils.create_updater(metadata, proj_path, UPDATERS)
- return (updater, metadata)
+ return updater, metadata
def _do_update(args: argparse.Namespace, updater: Updater,
@@ -137,7 +137,7 @@
try:
updater_utils.build(full_path)
except subprocess.CalledProcessError as err:
- logging.exception(f"Build failed, aborting upload")
+ logging.exception("Build failed, aborting upload")
return
except Exception as err:
if updater.rollback():
@@ -156,7 +156,7 @@
Args:
args: commandline arguments
proj_path: Absolute or relative path to the project.
- update: If false, will only check for new version, but not update.
+ update_lib: If false, will only check for new version, but not update.
"""
try:
@@ -168,9 +168,9 @@
current_version = updater.current_version
latest_version = updater.latest_version
print(f'Current version: {current_version}\nLatest version: {latest_version}')
- suggested_version = updater.suggested_latest_version
- if suggested_version is not None:
- print(f'Suggested latest version: {suggested_version}')
+ alternative_version = updater.alternative_latest_version
+ if alternative_version is not None:
+ print(f'Alternative latest version: {alternative_version}')
has_new_version = current_version != latest_version
if has_new_version:
@@ -183,17 +183,17 @@
updater.refresh_without_upgrading()
answer = 'n'
- if update_lib and suggested_version is not None:
- suggested_ver_type = (
+ if update_lib and alternative_version is not None:
+ alternative_ver_type = (
'tag' if git_utils.is_commit(current_version) else 'SHA'
)
answer = input(
- f'There is a new {suggested_ver_type} available:'
- f' {suggested_version}. Would you like to switch to'
- f' the latest {suggested_ver_type} instead? (y/n) '
+ f'There is a new {alternative_ver_type} available:'
+ f' {alternative_version}. Would you like to switch to'
+ f' the latest {alternative_ver_type} instead? (y/n) '
)
if answer == 'y':
- updater.set_new_version(suggested_version)
+ updater.set_new_version(alternative_version)
if update_lib and (has_new_version or args.force or args.refresh or answer == 'y'):
_do_update(args, updater, metadata)
diff --git a/fileutils.py b/fileutils.py
index 1f35f58..d211437 100644
--- a/fileutils.py
+++ b/fileutils.py
@@ -15,7 +15,6 @@
import datetime
import enum
-import glob
import os
from pathlib import Path
import textwrap
@@ -44,23 +43,24 @@
def find_tree_containing(project: Path) -> Path:
"""Returns the path to the repo tree parent of the given project.
- The parent tree is found by searching up the directory tree until a directory is
- found that contains a .repo directory. Other methods of finding this directory won't
- necessarily work:
+ The parent tree is found by searching up the directory tree until a
+ directory is found that contains a .repo directory. Other methods of
+ finding this directory won't necessarily work:
- * Using ANDROID_BUILD_TOP might find the wrong tree (if external_updater is used to
- manage a project that is not in AOSP, as it does for CMake, rr, and a few others),
- since ANDROID_BUILD_TOP will be the one that built external_updater rather than
- the given project.
- * Paths relative to __file__ are no good because we'll run from a "built" PAR
- somewhere in the soong out directory, or possibly somewhere more arbitrary when
- run from CI.
- * Paths relative to the CWD require external_updater to be run from a predictable
- location. Doing so prevents the user from using relative paths (and tab complete)
- from directories other than the expected location.
+ * Using ANDROID_BUILD_TOP might find the wrong tree (if external_updater
+ is used to manage a project that is not in AOSP, as it does for CMake,
+ rr, and a few others), since ANDROID_BUILD_TOP will be the one that built
+ external_updater rather than the given project.
+ * Paths relative to __file__ are no good because we'll run from a "built"
+ PAR somewhere in the soong out directory, or possibly somewhere more
+ arbitrary when run from CI.
+ * Paths relative to the CWD require external_updater to be run from a
+ predictable location. Doing so prevents the user from using relative
+ paths (and tab complete) from directories other than the expected location.
- The result for one project should not be reused for other projects, as it's possible
- that the user has provided project paths from multiple trees.
+ The result for one project should not be reused for other projects,
+ as it's possible that the user has provided project paths from multiple
+ trees.
"""
if (project / ".repo").exists():
return project
@@ -74,16 +74,17 @@
def external_path() -> Path:
"""Returns the path to //external.
- We cannot use the relative path from this file to find the top of the tree because
- this will often be run in a "compiled" form from an arbitrary location in the out
- directory. We can't fully rely on ANDROID_BUILD_TOP because not all contexts will
- have run envsetup/lunch either. We use ANDROID_BUILD_TOP whenever it is set, but if
- it is not set we instead rely on the convention that the CWD is the root of the tree
- (updater.sh will cd there before executing).
+ We cannot use the relative path from this file to find the top of the
+ tree because this will often be run in a "compiled" form from an
+ arbitrary location in the out directory. We can't fully rely on
+ ANDROID_BUILD_TOP because not all contexts will have run envsetup/lunch
+ either. We use ANDROID_BUILD_TOP whenever it is set, but if it is not set
+ we instead rely on the convention that the CWD is the root of the tree (
+ updater.sh will cd there before executing).
- There is one other context where this function cannot succeed: CI. Tests run in CI
- do not have a source tree to find, so calling this function in that context will
- fail.
+ There is one other context where this function cannot succeed: CI. Tests
+ run in CI do not have a source tree to find, so calling this function in
+ that context will fail.
"""
android_top = Path(os.environ.get("ANDROID_BUILD_TOP", os.getcwd()))
top = android_top / 'external'
@@ -109,9 +110,9 @@
def resolve_command_line_paths(paths: list[str]) -> list[Path]:
"""Resolves project paths provided by the command line.
- Both relative and absolute paths are resolved to fully qualified paths and returned.
- If any path does not exist relative to the CWD, a message will be printed and that
- path will be pruned from the list.
+ Both relative and absolute paths are resolved to fully qualified paths
+ and returned. If any path does not exist relative to the CWD, a message
+ will be printed and that path will be pruned from the list.
"""
resolved: list[Path] = []
for path_str in paths:
@@ -136,23 +137,25 @@
def canonicalize_project_path(proj_path: Path) -> Path:
"""Returns the canonical representation of the project path.
- For paths that are in the same tree as external_updater (the common case), the
- canonical path is the path of the project relative to //external.
+ For paths that are in the same tree as external_updater (the common
+ case), the canonical path is the path of the project relative to //external.
- For paths that are in a different tree (an uncommon case used for updating projects
- in other builds such as the NDK), the canonical path is the absolute path.
+ For paths that are in a different tree (an uncommon case used for
+ updating projects in other builds such as the NDK), the canonical path is
+ the absolute path.
"""
try:
return get_relative_project_path(proj_path)
except ValueError:
- # A less common use case, but the path might be to a non-local tree, in which case
- # the path will not be relative to our tree. This happens when using
- # external_updater in another project like the NDK or rr.
+ # A less common use case, but the path might be to a non-local tree,
+ # in which case the path will not be relative to our tree. This
+ # happens when using external_updater in another project like the NDK
+ # or rr.
if proj_path.is_absolute():
return proj_path
- # Not relative to //external, and not an absolute path. This case hasn't existed
- # before, so it has no canonical form.
+ # Not relative to //external, and not an absolute path. This case
+ # hasn't existed before, so it has no canonical form.
raise ValueError(
f"{proj_path} must be either an absolute path or relative to {external_path()}"
)
@@ -213,9 +216,9 @@
try:
rel_proj_path = str(get_relative_project_path(proj_path))
except ValueError:
- # Absolute paths to other trees will not be relative to our tree. There are
- # not portable instructions for upgrading that project, since the path will
- # differ between machines (or checkouts).
+ # Absolute paths to other trees will not be relative to our tree.
+ # There are no portable instructions for upgrading that project,
+ # since the path will differ between machines (or checkouts).
rel_proj_path = "<absolute path to project>"
usage_hint = textwrap.dedent(f"""\
# This project was upgraded with external_updater.
diff --git a/git_updater.py b/git_updater.py
index 293270c..323f6c9 100644
--- a/git_updater.py
+++ b/git_updater.py
@@ -13,13 +13,10 @@
# limitations under the License.
"""Module to check updates from Git upstream."""
-from pathlib import Path
-
import base_updater
import fileutils
import git_utils
# pylint: disable=import-error
-import metadata_pb2
from manifest import Manifest
@@ -33,9 +30,10 @@
@staticmethod
def _is_likely_android_remote(url: str) -> bool:
"""Returns True if the URL is likely to be the project's Android remote."""
- # There isn't a strict rule for finding the correct remote for upstream-master/main,
- # so we have to guess. Be careful to filter out things that look almost right
- # but aren't. Here's an example of a project that has a lot of false positives:
+ # There isn't a strict rule for finding the correct remote for
+ # upstream-master/main, so we have to guess. Be careful to filter out
+ # things that look almost right but aren't. Here's an example of a
+ # project that has a lot of false positives:
#
# aosp /usr/local/google/home/danalbert/src/mirrors/android/refs/aosp/toolchain/rr.git (fetch)
# aosp persistent-https://android.git.corp.google.com/toolchain/rr (push)
@@ -48,9 +46,9 @@
# upstream https://github.com/rr-debugger/rr.git (fetch)
# upstream https://github.com/rr-debugger/rr.git (push)
#
- # unmirrored is the correct remote here. It's not a local path, and contains
- # either /platform/external/ or /toolchain/ (the two common roots for third-
- # party Android imports).
+ # unmirrored is the correct remote here. It's not a local path,
+ # and contains either /platform/external/ or /toolchain/ (the two
+ # common roots for third- party Android imports).
if '://' not in url:
# Skip anything that's likely a local GoB mirror.
return False
@@ -99,16 +97,16 @@
# Update to remote head.
self._new_identifier.version = self.current_head_of_upstream_default_branch()
# Some libraries don't have a tag. We only populate
- # _suggested_new_ver if there is a tag newer than _old_ver.
+ # _alternative_new_ver if there is a tag newer than _old_ver.
# Checks if there is a tag newer than AOSP's SHA
- possible_suggested_new_ver = self.latest_tag_of_upstream_default_branch()
+ possible_alternative_new_ver = self.latest_tag_of_upstream_default_branch()
else:
- # Update to latest version tag.
+ # Update to the latest version tag.
self._new_identifier.version = self.latest_tag_of_upstream_default_branch()
# Checks if there is a SHA newer than AOSP's tag
- possible_suggested_new_ver = self.current_head_of_upstream_default_branch()
- if git_utils.is_ancestor(self._proj_path, self._old_identifier.version, possible_suggested_new_ver):
- self._suggested_new_ver = possible_suggested_new_ver
+ possible_alternative_new_ver = self.current_head_of_upstream_default_branch()
+ if git_utils.is_ancestor(self._proj_path, self._old_identifier.version, possible_alternative_new_ver):
+ self._alternative_new_ver = possible_alternative_new_ver
def latest_tag_of_upstream_default_branch(self) -> str:
branch = git_utils.detect_default_branch(self._proj_path,
@@ -131,10 +129,11 @@
def _determine_android_fetch_ref(self) -> str:
"""Returns the ref that should be fetched from the android remote."""
- # It isn't particularly efficient to reparse the tree for every project, but we
- # don't guarantee that all paths passed to updater.sh are actually in the same
- # tree so it wouldn't necessarily be correct to do this once at the top level.
- # This isn't the slow part anyway, so it can be dealt with if that ever changes.
+ # It isn't particularly efficient to reparse the tree for every
+ # project, but we don't guarantee that all paths passed to updater.sh
+ # are actually in the same tree so it wouldn't necessarily be correct
+ # to do this once at the top level. This isn't the slow part anyway,
+ # so it can be dealt with if that ever changes.
root = fileutils.find_tree_containing(self._proj_path)
manifest = Manifest.for_tree(root)
manifest_path = str(self._proj_path.relative_to(root))
diff --git a/git_utils.py b/git_utils.py
index e6142f2..eed9602 100644
--- a/git_utils.py
+++ b/git_utils.py
@@ -29,12 +29,13 @@
UNWANTED_TAGS = ["*alpha*", "*Alpha*", "*beta*", "*Beta*", "*rc*", "*RC*", "*test*"]
+
def fetch(proj_path: Path, remote_name: str, branch: str | None = None) -> None:
"""Runs git fetch.
Args:
proj_path: Path to Git repository.
- remote_names: Array of string to specify remote names.
+ remote_name: A string to specify remote names.
"""
cmd = ['git', 'fetch', '--tags', remote_name] + ([branch] if branch is not None else [])
subprocess.run(cmd, capture_output=True, cwd=proj_path, check=True)
diff --git a/github_archive_updater.py b/github_archive_updater.py
index c4e3b9e..cb43d8a 100644
--- a/github_archive_updater.py
+++ b/github_archive_updater.py
@@ -23,7 +23,6 @@
from base_updater import Updater
import git_utils
# pylint: disable=import-error
-import metadata_pb2 # type: ignore
import updater_utils
GITHUB_URL_PATTERN: str = (r'^https:\/\/github.com\/([-\w]+)\/([-\w]+)\/' +
@@ -102,7 +101,7 @@
a['browser_download_url'] for a in data['assets']
if archive_utils.is_supported_archive(a['browser_download_url'])
]
- return (data[self.VERSION_FIELD], supported_assets)
+ return data[self.VERSION_FIELD], supported_assets
def setup_remote(self) -> None:
homepage = f'https://github.com/{self.owner}/{self.repo}'
@@ -138,8 +137,10 @@
or self._fetch_latest_tag())
# Adds source code urls.
- urls.append(f'https://github.com/{self.owner}/{self.repo}/archive/{self._new_identifier.version}.tar.gz')
- urls.append(f'https://github.com/{self.owner}/{self.repo}/archive/{self._new_identifier.version}.zip')
+ urls.append(f'https://github.com/{self.owner}/{self.repo}/archive/'
+ f'{self._new_identifier.version}.tar.gz')
+ urls.append(f'https://github.com/{self.owner}/{self.repo}/archive/'
+ f'{self._new_identifier.version}.zip')
self._new_identifier.value = choose_best_url(urls, self._old_identifier.value)
diff --git a/updater_utils.py b/updater_utils.py
index 299f29a..3271ce0 100644
--- a/updater_utils.py
+++ b/updater_utils.py
@@ -95,7 +95,7 @@
try:
prefix, version, suffix = match.group('prefix', 'version', 'suffix')
versions = [int(v) for v in VERSION_SPLITTER_RE.split(version)]
- return (versions, str(prefix), str(suffix))
+ return versions, str(prefix), str(suffix)
except IndexError:
# pylint: disable=raise-missing-from
raise ValueError('Invalid version.')
@@ -106,12 +106,12 @@
try:
new_ver = _parse_version(version)
except ValueError:
- return (False, False, [])
+ return False, False, []
right_format = new_ver[1:] == old_ver[1:]
right_length = len(new_ver[0]) == len(old_ver[0])
- return (right_format, right_length, new_ver[0])
+ return right_format, right_length, new_ver[0]
def get_latest_version(current_version: str, version_list: List[str]) -> str:
