diff options
author | Joel Galenson <jgalenson@google.com> | 2021-04-07 12:44:31 -0700 |
---|---|---|
committer | Joel Galenson <jgalenson@google.com> | 2021-04-07 12:44:31 -0700 |
commit | 58eeae7363dbc8a83d11acd55c8c1b2ae1ef1fef (patch) | |
tree | e3d51cacd3853277aaaa14abb691b19475caa133 | |
parent | 95dcb8601107e7400f2d344ebac6ab33e90e1b4a (diff) | |
download | external_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.py | 7 | ||||
-rw-r--r-- | crates_updater.py | 19 | ||||
-rw-r--r-- | external_updater.py | 2 | ||||
-rw-r--r-- | git_utils.py | 4 |
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) |