aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoel Galenson <jgalenson@google.com>2021-04-07 12:44:31 -0700
committerJoel Galenson <jgalenson@google.com>2021-04-07 12:44:31 -0700
commit58eeae7363dbc8a83d11acd55c8c1b2ae1ef1fef (patch)
treee3d51cacd3853277aaaa14abb691b19475caa133
parent95dcb8601107e7400f2d344ebac6ab33e90e1b4a (diff)
downloadexternal_updater-58eeae7363dbc8a83d11acd55c8c1b2ae1ef1fef.tar.gz
Mark CLs as Verified-1 if they have errors.
We currently upload CLs that can be broken. This is potentially fine, as it's a signal to the reviewer that an upgrade needs work. But we should mark them as failing in some way to give the reviewer that hint. This CL does that by setting Verified-1. We currently only implement it for Rust crates. Rather than modifying the existing mechanisms to detect various errors (which seems difficult e.g., in the case of patches) we do checks for errors after the fact. We currently check for .rej files (meaning failed patches) and errors in Android.bp. These checks are both heuristics and so could fail (in either way). Test: Created aosp/1665704, aosp/1667706, aosp/1667707 Change-Id: I7e148d4d0cc5c1f2434cfdb98a969e7969a3893a
-rw-r--r--base_updater.py7
-rw-r--r--crates_updater.py19
-rw-r--r--external_updater.py2
-rw-r--r--git_utils.py4
4 files changed, 30 insertions, 2 deletions
diff --git a/base_updater.py b/base_updater.py
index 8cf3255..18d4435 100644
--- a/base_updater.py
+++ b/base_updater.py
@@ -32,6 +32,8 @@ class Updater:
self._new_url.CopyFrom(old_url)
self._new_ver = old_ver
+ self._has_errors = False
+
def is_supported_url(self) -> bool:
"""Returns whether the url is supported."""
raise NotImplementedError()
@@ -72,6 +74,11 @@ class Updater:
"""Gets URL for latest version."""
return self._new_url
+ @property
+ def has_errors(self) -> bool:
+ """Gets whether this update had an error."""
+ return self._has_errors
+
def use_current_as_latest(self):
"""Uses current version/url as the latest to refresh project."""
self._new_ver = self._old_ver
diff --git a/crates_updater.py b/crates_updater.py
index 78fa9b5..8207375 100644
--- a/crates_updater.py
+++ b/crates_updater.py
@@ -117,6 +117,7 @@ class CratesUpdater(Updater):
temporary_dir = archive_utils.download_and_extract(self.download_url)
package_dir = archive_utils.find_archive_root(temporary_dir)
updater_utils.replace_package(package_dir, self._proj_path)
+ self.check_for_errors()
finally:
urllib.request.urlcleanup()
@@ -143,6 +144,24 @@ class CratesUpdater(Updater):
print("New METADATA description:", description)
metadata.description = description
+ def check_for_errors(self) -> None:
+ # Check for .rej patches from failing to apply patches.
+ # If this has too many false positives, we could either
+ # check if the files are modified by patches or somehow
+ # track which files existed before the patching.
+ rejects = list(self._proj_path.glob('**/*.rej'))
+ if len(rejects) > 0:
+ print("Error: Found patch reject files: %s" % str(rejects))
+ self._has_errors = True
+ # Check for Cargo errors embedded in Android.bp.
+ # Note that this should stay in sync with cargo2android.py.
+ with open('%s/Android.bp' % self._proj_path, 'r') as bp_file:
+ for line in bp_file:
+ if line.strip() == "Errors in cargo.out:":
+ print("Error: Found Cargo errors in Android.bp")
+ self._has_errors = True
+ return
+
def _toml2str(self, line: str) -> str:
"""Convert a quoted toml string to a Python str without quotes."""
if line.startswith("\"\"\""):
diff --git a/external_updater.py b/external_updater.py
index 7f648b2..4d40fa0 100644
--- a/external_updater.py
+++ b/external_updater.py
@@ -115,7 +115,7 @@ def _do_update(args: argparse.Namespace, updater: Updater,
git_utils.commit(full_path, msg)
if args.push_change:
- git_utils.push(full_path, args.remote_name)
+ git_utils.push(full_path, args.remote_name, updater.has_errors)
if args.branch_and_commit:
git_utils.checkout(full_path, args.remote_name + '/master')
diff --git a/git_utils.py b/git_utils.py
index 541e860..1825c37 100644
--- a/git_utils.py
+++ b/git_utils.py
@@ -161,11 +161,13 @@ def checkout(proj_path: Path, branch_name: str) -> None:
_run(['git', 'checkout', branch_name], cwd=proj_path)
-def push(proj_path: Path, remote_name: str) -> None:
+def push(proj_path: Path, remote_name: str, has_errors: bool) -> None:
"""Pushes change to remote."""
cmd = ['git', 'push', remote_name, 'HEAD:refs/for/master']
if revs := reviewers.find_reviewers(str(proj_path)):
cmd.extend(['-o', revs])
if tag := hashtags.find_hashtag(proj_path):
cmd.extend(['-o', 't=' + tag])
+ if has_errors:
+ cmd.extend(['-o', 'l=Verified-1'])
_run(cmd, cwd=proj_path)