diff options
| -rw-r--r-- | base_updater.py | 7 | ||||
| -rw-r--r-- | crates_updater.py | 21 | ||||
| -rw-r--r-- | external_updater.py | 101 | ||||
| -rw-r--r-- | external_updater_reviewers_test.py | 7 | ||||
| -rw-r--r-- | git_utils.py | 10 | ||||
| -rw-r--r-- | metadata.proto | 45 | ||||
| -rwxr-xr-x | regen_bp.sh | 5 | ||||
| -rw-r--r-- | reviewers.py | 13 | ||||
| -rw-r--r-- | update_package.sh | 26 | ||||
| -rwxr-xr-x | updater.sh | 2 | ||||
| -rw-r--r-- | updater_utils.py | 5 |
11 files changed, 160 insertions, 82 deletions
diff --git a/base_updater.py b/base_updater.py index 18d4435..78dde1c 100644 --- a/base_updater.py +++ b/base_updater.py @@ -49,6 +49,13 @@ class Updater: """ raise NotImplementedError() + def rollback(self) -> bool: + """Rolls the current update back. + + This is an optional operation. Returns whether the rollback succeeded. + """ + return False + @property def project_path(self) -> Path: """Gets absolute path to the project.""" diff --git a/crates_updater.py b/crates_updater.py index 8207375..ee476b5 100644 --- a/crates_updater.py +++ b/crates_updater.py @@ -18,6 +18,8 @@ import os # pylint: disable=g-importing-member from pathlib import Path import re +import shutil +import tempfile import urllib.request import archive_utils @@ -48,6 +50,8 @@ class CratesUpdater(Updater): download_url: str package: str + package_dir: str + temp_file: tempfile.NamedTemporaryFile def is_supported_url(self) -> bool: if self._old_url.type != metadata_pb2.URL.HOMEPAGE: @@ -115,12 +119,25 @@ class CratesUpdater(Updater): """ try: 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.package_dir = archive_utils.find_archive_root(temporary_dir) + self.temp_file = tempfile.NamedTemporaryFile() + updater_utils.replace_package(self.package_dir, self._proj_path, + self.temp_file.name) self.check_for_errors() finally: urllib.request.urlcleanup() + def rollback(self) -> bool: + # Only rollback if we have already swapped, + # which we denote by writing to this file. + if os.fstat(self.temp_file.fileno()).st_size > 0: + tmp_dir = tempfile.TemporaryDirectory() + shutil.move(self._proj_path, tmp_dir.name) + shutil.move(self.package_dir, self._proj_path) + shutil.move(Path(tmp_dir.name) / self.package, self.package_dir) + return True + return False + # pylint: disable=no-self-use def update_metadata(self, metadata: metadata_pb2.MetaData, full_path: Path) -> None: diff --git a/external_updater.py b/external_updater.py index 4d40fa0..c15e92d 100644 --- a/external_updater.py +++ b/external_updater.py @@ -22,6 +22,7 @@ updater.sh update --refresh --keep_date rust/crates/libc import argparse import enum +import glob import json import os import sys @@ -94,25 +95,32 @@ def _do_update(args: argparse.Namespace, updater: Updater, git_utils.checkout(full_path, args.remote_name + '/master') git_utils.start_branch(full_path, TMP_BRANCH_NAME) - updater.update() - - updated_metadata = metadata_pb2.MetaData() - updated_metadata.CopyFrom(metadata) - updated_metadata.third_party.version = updater.latest_version - for metadata_url in updated_metadata.third_party.url: - if metadata_url == updater.current_url: - metadata_url.CopyFrom(updater.latest_url) - # For Rust crates, replace GIT url with ARCHIVE url - if isinstance(updater, CratesUpdater): - updater.update_metadata(updated_metadata, full_path) - fileutils.write_metadata(full_path, updated_metadata, args.keep_date) - git_utils.add_file(full_path, 'METADATA') - - if args.branch_and_commit: - msg = 'Upgrade {} to {}\n\nTest: make\n'.format( - args.path, updater.latest_version) - git_utils.add_file(full_path, '*') - git_utils.commit(full_path, msg) + try: + updater.update() + + updated_metadata = metadata_pb2.MetaData() + updated_metadata.CopyFrom(metadata) + updated_metadata.third_party.version = updater.latest_version + for metadata_url in updated_metadata.third_party.url: + if metadata_url == updater.current_url: + metadata_url.CopyFrom(updater.latest_url) + # For Rust crates, replace GIT url with ARCHIVE url + if isinstance(updater, CratesUpdater): + updater.update_metadata(updated_metadata, full_path) + fileutils.write_metadata(full_path, updated_metadata, args.keep_date) + git_utils.add_file(full_path, 'METADATA') + + if args.branch_and_commit: + rel_proj_path = fileutils.get_relative_project_path(full_path) + msg = 'Upgrade {} to {}\n\nTest: make\n'.format( + rel_proj_path, updater.latest_version) + git_utils.remove_gitmodules(full_path) + git_utils.add_file(full_path, '*') + git_utils.commit(full_path, msg) + except Exception as err: + if updater.rollback(): + print('Rolled back.') + raise err if args.push_change: git_utils.push(full_path, args.remote_name, updater.has_errors) @@ -162,12 +170,13 @@ def check_and_update(args: argparse.Namespace, return str(err) -def _check_path(args: argparse.Namespace, paths: Iterator[str], - delay: int) -> Dict[str, Dict[str, str]]: +def check_and_update_path(args: argparse.Namespace, paths: Iterator[str], + update_lib: bool, + delay: int) -> Dict[str, Dict[str, str]]: results = {} for path in paths: res = {} - updater = check_and_update(args, Path(path)) + updater = check_and_update(args, Path(path), update_lib) if isinstance(updater, str): res['error'] = updater else: @@ -188,19 +197,45 @@ def _list_all_metadata() -> Iterator[str]: dirs.sort(key=lambda d: d.lower()) +def get_paths(paths: List[str]) -> List[str]: + """Expand paths via globs.""" + # We want to use glob to get all the paths, so we first convert to absolute. + abs_paths = [fileutils.get_absolute_project_path(Path(path)) + for path in paths] + result = [path for abs_path in abs_paths + for path in sorted(glob.glob(str(abs_path)))] + if paths and not result: + print('Could not find any valid paths in %s' % str(paths)) + return result + + +def write_json(json_file: str, results: Dict[str, Dict[str, str]]) -> List[str]: + """Output a JSON report.""" + with Path(json_file).open('w') as res_file: + json.dump(results, res_file, sort_keys=True, indent=4) + + def check(args: argparse.Namespace) -> None: """Handler for check command.""" - paths = _list_all_metadata() if args.all else args.paths - results = _check_path(args, paths, args.delay) + paths = _list_all_metadata() if args.all else get_paths(args.paths) + results = check_and_update_path(args, paths, False, args.delay) if args.json_output is not None: - with Path(args.json_output).open('w') as res_file: - json.dump(results, res_file, sort_keys=True, indent=4) + write_json(args.json_output, results) def update(args: argparse.Namespace) -> None: """Handler for update command.""" - check_and_update(args, args.path, update_lib=True) + all_paths = get_paths(args.paths) + # Remove excluded paths. + excludes = set() if args.exclude is None else set(args.exclude) + filtered_paths = [path for path in all_paths + if not Path(path).name in excludes] + # Now we can update each path. + results = check_and_update_path(args, filtered_paths, True, 0) + + if args.json_output is not None: + write_json(args.json_output, results) def parse_args() -> argparse.Namespace: @@ -235,9 +270,12 @@ def parse_args() -> argparse.Namespace: # Creates parser for update command. update_parser = subparsers.add_parser('update', help='Update one project.') update_parser.add_argument( - 'path', - help='Path of the project. ' + 'paths', + nargs='*', + help='Paths of the project as globs. ' 'Relative paths will be resolved from external/.') + update_parser.add_argument('--json_output', + help='Path of a json file to write result to.') update_parser.add_argument( '--force', help='Run update even if there\'s no new version.', @@ -260,6 +298,11 @@ def parse_args() -> argparse.Namespace: default='aosp', required=False, help='Upstream remote name.') + update_parser.add_argument('--exclude', + action='append', + help='Names of projects to exclude. ' + 'These are just the final part of the path ' + 'with no directories.') update_parser.set_defaults(func=update) return parser.parse_args() diff --git a/external_updater_reviewers_test.py b/external_updater_reviewers_test.py index d6e4b71..18497f5 100644 --- a/external_updater_reviewers_test.py +++ b/external_updater_reviewers_test.py @@ -28,7 +28,6 @@ class ExternalUpdaterReviewersTest(unittest.TestCase): self.saved_proj_reviewers = reviewers.PROJ_REVIEWERS self.saved_rust_reviewers = reviewers.RUST_REVIEWERS self.saved_rust_reviewer_list = reviewers.RUST_REVIEWER_LIST - self.saved_num_rust_projects = reviewers.NUM_RUST_PROJECTS self.saved_rust_crate_owners = reviewers.RUST_CRATE_OWNERS def tearDown(self): @@ -37,7 +36,6 @@ class ExternalUpdaterReviewersTest(unittest.TestCase): reviewers.PROJ_REVIEWERS = self.saved_proj_reviewers reviewers.RUST_REVIEWERS = self.saved_rust_reviewers reviewers.RUST_REVIEWER_LIST = self.saved_rust_reviewer_list - reviewers.NUM_RUST_PROJECTS = self.saved_num_rust_projects reviewers.RUST_CRATE_OWNERS = self.saved_rust_crate_owners # pylint: disable=no-self-use @@ -74,16 +72,11 @@ class ExternalUpdaterReviewersTest(unittest.TestCase): """Check the constants associated to the reviewers.""" # There should be enough people in the reviewers pool. self.assertGreaterEqual(len(reviewers.RUST_REVIEWERS), 3) - # The NUM_RUST_PROJECTS should not be too small. - self.assertGreaterEqual(reviewers.NUM_RUST_PROJECTS, 50) - self.assertGreaterEqual(reviewers.NUM_RUST_PROJECTS, - len(reviewers.RUST_CRATE_OWNERS)) # Assume no project reviewers and recreate RUST_REVIEWER_LIST reviewers.PROJ_REVIEWERS = {} reviewers.RUST_REVIEWER_LIST = reviewers.create_rust_reviewer_list() sum_projects = sum(reviewers.RUST_REVIEWERS.values()) self.assertEqual(sum_projects, len(reviewers.RUST_REVIEWER_LIST)) - self.assertGreaterEqual(sum_projects, reviewers.NUM_RUST_PROJECTS) def test_reviewers_randomness(self): """Check random selection of reviewers.""" diff --git a/git_utils.py b/git_utils.py index f96a600..5186eb3 100644 --- a/git_utils.py +++ b/git_utils.py @@ -142,8 +142,9 @@ def merge(proj_path: Path, branch: str) -> None: """Merges a branch.""" try: _run(['git', 'merge', branch, '--no-commit'], cwd=proj_path) - except subprocess.CalledProcessError: - # Merge failed. Error is already written to console. + except subprocess.CalledProcessError as err: + if hasattr(err, "output"): + print(err.output) _run(['git', 'merge', '--abort'], cwd=proj_path) raise @@ -153,6 +154,11 @@ def add_file(proj_path: Path, file_name: str) -> None: _run(['git', 'add', file_name], cwd=proj_path) +def remove_gitmodules(proj_path: Path) -> None: + """Deletes .gitmodules files.""" + _run(['find', '.', '-name', '.gitmodules', '-delete'], cwd=proj_path) + + def delete_branch(proj_path: Path, branch_name: str) -> None: """Force delete a branch.""" _run(['git', 'branch', '-D', branch_name], cwd=proj_path) diff --git a/metadata.proto b/metadata.proto index f517021..ed72c34 100644 --- a/metadata.proto +++ b/metadata.proto @@ -1,4 +1,4 @@ -// copyright (C) 2018 The Android Open Source Project +// Copyright (C) 2018 The Android Open Source Project // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,14 +17,17 @@ // This proto will only contain fields and values the updater cares about. // It is not intended to be the formal definition of METADATA file. -syntax = "proto3"; +// See google3/third_party/metadata.proto if you need to add more stuff to match +// upstream. + +syntax = "proto2"; // As long as upstream is proto2... package external_updater; message MetaData { - string name = 1; - string description = 3; - ThirdPartyMetaData third_party = 13; + optional string name = 1; + optional string description = 3; + optional ThirdPartyMetaData third_party = 13; } enum LicenseType { @@ -40,10 +43,11 @@ enum LicenseType { message ThirdPartyMetaData { repeated URL url = 1; - string version = 2; - LicenseType license_type = 4; - string license_note = 5; - Date last_upgrade_date = 10; + optional string version = 2; + optional LicenseType license_type = 4; + optional string license_note = 5; + optional Security security = 7; + optional Date last_upgrade_date = 10; } message URL { @@ -58,13 +62,26 @@ message URL { OTHER = 11; } - Type type = 1; + optional Type type = 1; - string value = 2; + optional string value = 2; } message Date { - int32 year = 1; - int32 month = 2; - int32 day = 3; + optional int32 year = 1; + optional int32 month = 2; + optional int32 day = 3; +} + +message Security { + enum Category { + SANDBOXED_ONLY = 1; + TRUSTED_DATA_ONLY = 2; + REVIEWED_AND_SECURE = 3; + } + + optional Category category = 1; + optional string note = 2; + repeated string tag = 3; + repeated string mitigated_security_patch = 5; } diff --git a/regen_bp.sh b/regen_bp.sh index c84ff53..ddeab8a 100755 --- a/regen_bp.sh +++ b/regen_bp.sh @@ -26,14 +26,15 @@ set -e # Wrapper around cargo2android. C2A_WRAPPER="/google/bin/releases/android-rust/cargo2android/sandbox.par" +C2A_WRAPPER_FLAGS="--updater" function main() { check_files $* update_files_with_cargo_pkg_vars # Save Cargo.lock if it existed before this update. [ ! -f Cargo.lock ] || mv Cargo.lock Cargo.lock.saved - echo "Updating Android.bp: $C2A_WRAPPER -- $FLAGS" - $C2A_WRAPPER -- $FLAGS + echo "Updating Android.bp: $C2A_WRAPPER $C2A_WRAPPER_FLAGS -- $FLAGS" + $C2A_WRAPPER $C2A_WRAPPER_FLAGS -- $FLAGS copy_cargo_out_files $* rm -rf target.tmp cargo.out Cargo.lock # Restore Cargo.lock if it existed before this update. diff --git a/reviewers.py b/reviewers.py index 6f826f1..39a7349 100644 --- a/reviewers.py +++ b/reviewers.py @@ -31,6 +31,7 @@ ProjMapping = Mapping[str, Union[str, List[str], Set[str]]] RUST_CRATE_OWNERS: ProjMapping = { 'rust/crates/anyhow': 'mmaurer@google.com', # more rust crate owners could be added later + # if so, consider modifying the quotas in RUST_REVIEWERS } PROJ_REVIEWERS: ProjMapping = { @@ -40,20 +41,14 @@ PROJ_REVIEWERS: ProjMapping = { # Combine all roject reviewers. PROJ_REVIEWERS.update(RUST_CRATE_OWNERS) -# Estimated number of rust projects, not the actual number. -# It is only used to make random distribution "fair" among RUST_REVIEWERS. -# It should not be too small, to spread nicely to multiple reviewers. -# It should be larger or equal to len(RUST_CRATES_OWNERS). -NUM_RUST_PROJECTS = 120 - # Reviewers for external/rust/crates projects not found in PROJ_REVIEWER. # Each person has a quota, the number of projects to review. -# Sum of these numbers should be greater or equal to NUM_RUST_PROJECTS -# to avoid error cases in the creation of RUST_REVIEWER_LIST. +# The sum of these quotas should ideally be at least the number of Rust +# projects, but this only matters if we have many entries in RUST_CRATE_OWNERS, +# as we subtract a person's owned crates from their quota. RUST_REVIEWERS: Mapping[str, int] = { 'ivanlozano@google.com': 20, 'jeffv@google.com': 20, - 'jgalenson@google.com': 20, 'mmaurer@google.com': 20, 'srhines@google.com': 20, 'tweek@google.com': 20, diff --git a/update_package.sh b/update_package.sh index 219c566..4715db7 100644 --- a/update_package.sh +++ b/update_package.sh @@ -21,6 +21,7 @@ set -e tmp_dir=$1 external_dir=$2 +tmp_file=$3 # root of Android source tree root_dir=`pwd` @@ -45,19 +46,14 @@ CopyIfPresent "METADATA" CopyIfPresent "TEST_MAPPING" CopyIfPresent ".git" CopyIfPresent ".gitignore" -CopyIfPresent "cargo2android.json" +if compgen -G "$external_dir/cargo2android*"; then + cp -a -f -n $external_dir/cargo2android* . +fi CopyIfPresent "patches" CopyIfPresent "post_update.sh" CopyIfPresent "OWNERS" CopyIfPresent "README.android" -if [ -f $tmp_dir/Cargo.toml -a -f $tmp_dir/Android.bp ] -then - # regenerate Android.bp before local patches, so it is - # possible to patch the generated Android.bp after this. - /bin/bash `dirname $0`/regen_bp.sh $root_dir $external_dir -fi - echo "Applying patches..." for p in $tmp_dir/patches/*.{diff,patch} do @@ -87,13 +83,15 @@ then fi echo "Swapping old and new..." -rm -rf $external_dir +second_tmp_dir=`mktemp -d` +mv $external_dir $second_tmp_dir mv $tmp_dir $external_dir - -echo "Updating TEST_MAPPING..." -UCT="$root_dir/development/scripts/update_crate_tests.py" -[ -f "$UCT" ] || abort "ERROR: cannot find $UCT" -$UCT $external_dir +mv $second_tmp_dir/* $tmp_dir +rm -rf $second_tmp_dir +if [ -n "$tmp_file" ]; then + # Write to the temporary file to show we have swapped. + echo "Swapping" > $tmp_file +fi cd $external_dir git add . @@ -20,4 +20,4 @@ cd $(dirname "$0")/../.. source build/envsetup.sh lunch aosp_arm-eng mmma tools/external_updater -out/soong/host/linux-x86/bin/external_updater $@ +out/host/linux-x86/bin/external_updater $@ diff --git a/updater_utils.py b/updater_utils.py index 1bcd567..209a1cd 100644 --- a/updater_utils.py +++ b/updater_utils.py @@ -48,7 +48,7 @@ def create_updater(metadata: metadata_pb2.MetaData, proj_path: Path, raise ValueError('No supported URL.') -def replace_package(source_dir, target_dir) -> None: +def replace_package(source_dir, target_dir, temp_file=None) -> None: """Invokes a shell script to prepare and update a project. Args: @@ -59,7 +59,8 @@ def replace_package(source_dir, target_dir) -> None: print('Updating {} using {}.'.format(target_dir, source_dir)) script_path = os.path.join(os.path.dirname(sys.argv[0]), 'update_package.sh') - subprocess.check_call(['bash', script_path, source_dir, target_dir]) + subprocess.check_call(['bash', script_path, source_dir, target_dir, + "" if temp_file is None else temp_file]) VERSION_SPLITTER_PATTERN: str = r'[\.\-_]' |