[rust] Replace cargo vet
with a lighterweight process.
This CL: * Removes the `cargo vet` presubmit for `//third_party/rust`. After this CL, changes to `//third_party/rust` can land with an LGTM from one of `//third_party/rust/OWNERS` and won't anymore require recording a `cargo vet` audit. * Documents an initial set of security review guidelines and puts them into `//third_party/rust/OWNERS-security-review.md`. Some minor follow-up doc changes affect `//tools/crates/gnrt`. Bug: 405980483 Change-Id: I0fedfc012651cee7057ddad3d74a9481a1299945 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6388195 Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1437597}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
9552a26a17
commit
f6888b7bac
@ -1,107 +0,0 @@
|
||||
# `unsafe` Rust Guidelines
|
||||
|
||||
## Code Review Policy {#code-review-policy}
|
||||
|
||||
All `unsafe` Rust code in Chromium needs to be reviewed and LGTM-ed by a
|
||||
reviewer from `//third_party/rust/UNSAFE_RUST_OWNERS`.
|
||||
This policy applies to both third-party code
|
||||
(e.g. under `//third_party/rust`) and first-party code.
|
||||
|
||||
### How to request a review
|
||||
|
||||
To facilitate a code review please:
|
||||
|
||||
* For each new or modified `unsafe` block, function, `impl`, etc.,
|
||||
add an unresolved "TODO: `unsafe` review" comment in Gerrit.
|
||||
You can consider using `tools/crates/create_draft_comments.py` to streamline
|
||||
creating such comments.
|
||||
|
||||
* Add `chrome-unsafe-rust-reviews@google.com` as a reviewer.
|
||||
|
||||
### Scope of review
|
||||
|
||||
Note that changes _anywhere_ in a crate that uses `unsafe` blocks may violate
|
||||
the internal invariants on which those `unsafe` blocks rely. It is unrealistic
|
||||
to require an `unsafe` review to re-audit all the
|
||||
`unsafe` blocks each time a crate is updated, but the crate `OWNERS` and other
|
||||
reviewers should be on the lookout for code changes which feel as though they
|
||||
could affect invariants on which `unsafe` blocks rely.
|
||||
|
||||
### `OWNERS` files guidance
|
||||
|
||||
To require `unsafe` review for certain `.rs` files
|
||||
(e.g. ones that use `unsafe` Rust)
|
||||
you can forward from the file's `OWNERS` to
|
||||
`//third_party/rust/UNSAFE_RUST_OWNERS`
|
||||
(see comments in the latter for more details).
|
||||
|
||||
### Soft SLA
|
||||
|
||||
For incremental changes (including updating a minor version of a crate under
|
||||
`//third_party/rust/chromium_crates_io`) the usual [Chromium responsiveness
|
||||
expectations](cl_respect.md#expect-responsiveness) apply. (i.e. You should expect
|
||||
reviewer input within 1 business day.)
|
||||
|
||||
For bulk changes (e.g. importing a new crate and its transitive dependencies)
|
||||
the turnaround time may be longer. This depends mostly on the amount of
|
||||
`unsafe` code. To streamline reviews and future maintainability, we ask you
|
||||
kindly to prefer crates that do *not* use `unsafe` Rust code.
|
||||
|
||||
### Other notes
|
||||
|
||||
Bugs that track streamlining application of this policy are tracked under
|
||||
the umbrella of https://crbug.com/393394872/dependencies.
|
||||
|
||||
## `cargo vet` Policy {#cargo-vet-policy}
|
||||
|
||||
Crates in `//third_party/rust/chromium_crates_io` need to be covered by `cargo
|
||||
vet` audits. In other words, `tools/crates/run_cargo_vet.py check` should
|
||||
always succeed (this is enforced by `//third_party/rust/PRESUBMIT.py`).
|
||||
|
||||
### Audit criteria required for most crates
|
||||
|
||||
Audit criteria required for a given crate depend on how the crate is used. The
|
||||
criteria are written to
|
||||
`third_party/rust/chromium_crates_io/supply-chain/config.toml` by
|
||||
`tools/crates/run_gnrt.py vendor` based on whether
|
||||
`third_party/rust/chromium_crates_io/gnrt_config.toml` declares that the crate
|
||||
is meant to be used (maybe transitively) in a `safe`, `sandbox`, or `test`
|
||||
environment. For example, to declare that a crate is `safe` to be used in the
|
||||
browser process, it needs to be audited and certified to be `safe-to-deploy`,
|
||||
`ub-risk-2` or lower, and either `does-not-implement-crypto` or `crypto-safe`.
|
||||
|
||||
Note that some audits can be done by any engineer ("ub-risk-0" and
|
||||
"safe-to-run") while others will require specialists from the
|
||||
`unsafe-rust-in-chrome@google.com` group (see the ["Code Review Policy"
|
||||
above](#code-review-policy). More details about audit criteria and the required
|
||||
expertise are explained in the
|
||||
[auditing_standards.md](https://github.com/google/rust-crate-audits/blob/main/auditing_standards.md),
|
||||
which also provides guidance for conducting delta audits.
|
||||
|
||||
### Some crates don't require an audit
|
||||
|
||||
Chromium implicitly trusts certain crate publishers. Currently
|
||||
there are two scenarios where such trust relationship may be established:
|
||||
|
||||
* Trusting crates authored and maintained under https://github.com/rust-lang/
|
||||
(e.g. `libc`, `hashbrown`), because they are closely related to the Rust
|
||||
toolchain (i.e. the same group managed and publishes `rustc`,
|
||||
`rustfmt`, `cargo`, `rustup`, etc.).
|
||||
* Trusting crates that are part of an OS SDK (e.g. `windows-...` crates).
|
||||
|
||||
Chromium uses both our own audits
|
||||
(stored in `third_party/rust/chromium_crates_io/supply-chain/audits.toml`)
|
||||
as well as audits imported from other parts of Google
|
||||
(e.g. Android, Fuchsia, etc.). This means that adding a new crate does not
|
||||
necessarily require a new audit if the crate has already been audited by
|
||||
other projects (in this case, `cargo vet` will record the imported audit
|
||||
in the `third_party/rust/chromium_crates_io/supply-chain/imports.lock` file).
|
||||
|
||||
### How to run `cargo vet` in Chromium
|
||||
|
||||
See
|
||||
[Cargo Vet documentation](https://mozilla.github.io/cargo-vet/recording-audits.html)
|
||||
for how to record the audit in `audits.toml`.
|
||||
The `tools/crates/run_cargo_vet.py` may be used to invoke Chromium's copy of
|
||||
`cargo-vet`.
|
||||
|
11
docs/rust.md
11
docs/rust.md
@ -76,21 +76,12 @@ To use a third-party crate "bar" version 3 from first party code:
|
||||
* `vpython3 ./tools/crates/run_gnrt.py gen`
|
||||
* Or, directly through (nightly) cargo:
|
||||
`cargo run --release --manifest-path tools/crates/gnrt/Cargo.toml --target-dir out/gnrt gen`
|
||||
1. Verify if all new dependencies are already audited by running `cargo vet`
|
||||
See [`rust-unsafe.md#cargo-vet-policy`](rust-unsafe.md#cargo-vet-policy) for
|
||||
more details. This boils down to:
|
||||
* `./tools/crates/run_cargo_vet.py check`
|
||||
* If `check` fails, then there are missing audits, which need to be added to
|
||||
`//third_party/rust/chromium_crates_io/supply-chain/audits.toml`.
|
||||
1. Add the new files to git:
|
||||
* `git add -f third_party/rust/chromium_crates_io/vendor`.
|
||||
(The `-f` is important, as files may be skipped otherwise from a
|
||||
`.gitignore` inside the crate.)
|
||||
* `git add third_party/rust`
|
||||
1. Upload the CL. If there is any `unsafe` usage then Security experts will need to
|
||||
audit the "ub-risk" level. See
|
||||
[`rust-unsafe.md#code-review-policy`](rust-unsafe.md#code-review-policy) for
|
||||
more details.
|
||||
1. Upload the CL and get a review from `//third_party/rust/OWNERS`.
|
||||
|
||||
### Cargo features
|
||||
|
||||
|
108
third_party/rust/OWNERS-security-review.md
vendored
Normal file
108
third_party/rust/OWNERS-security-review.md
vendored
Normal file
@ -0,0 +1,108 @@
|
||||
# Chromium Security Review Checklist for `//third_party/rust`
|
||||
|
||||
`//third_party/rust/OWNERS` review crates in this directory
|
||||
(e.g. when importing a new crate,
|
||||
or when updating an existing crate to a new version).
|
||||
|
||||
This document provides a checklist that
|
||||
|
||||
1) helps the reviewers remember what to look for, and
|
||||
2) helps to set the expectations
|
||||
so that the reviews provide the right level of assurance and
|
||||
take the right amount of reviewer time
|
||||
(not too little, but also not too much).
|
||||
|
||||
These are very high level guidelines that explicitly avoid
|
||||
trying to codify what exactly to look for and how to do the reviews,
|
||||
because ultimately we trust the reviewer’s judgement here.
|
||||
|
||||
## Review checklist
|
||||
|
||||
* **ATL+Director approval**: Is it an import of a new crate?
|
||||
If so, then check if 1) ATL approval has been obtained
|
||||
and 2) a Director has been CC-ed on a post to rust-fyi@ group
|
||||
(step 3 from
|
||||
[go/chrome-rust](https://goto2.corp.google.com/chrome-rust)
|
||||
[Google-internal]).
|
||||
- High-level goal: Ensure that an LGTM from `//third_party/rust/OWNERS`
|
||||
doesn’t accidentally bypass the requirement for a generic third-party
|
||||
review by `//third_party/OWNERS`.
|
||||
- If the new crate is a new transitive dependency of an already approved
|
||||
crate, then we should send an FYI email to ATLs (as discussed
|
||||
[here](https://groups.google.com/a/google.com/g/chrome-atls-discuss/c/xa-tFeJ6BnE/m/vkg89NC1AQAJ)).
|
||||
|
||||
* **`unsafe` code**: Does the crate use `unsafe` Rust? If so, then check that
|
||||
the `unsafe` code is small, encapsulated, and documented as explained in
|
||||
[the `rule-of-2.md` doc](https://chromium.googlesource.com/chromium/src/+/main/docs/security/rule-of-2.md#unsafe-code-in-safe-languages).
|
||||
- High-level goals:
|
||||
- Ensure that one of `//third_party/rust/OWNERS` looks
|
||||
at each `unsafe` block of code.
|
||||
- Introduce some friction for importing libraries with worrying
|
||||
or unnecessary use of `unsafe`.
|
||||
- Not a goal:
|
||||
- Leaving a written trail that for each `unsafe` block explains
|
||||
reviewer's thinking why the safety requirements are met.
|
||||
- Spending more than a minute or two per `unsafe` block
|
||||
|
||||
* **Proc macros and `build.rs`**: Does the crate execute any code at build time?
|
||||
If so, then check if the build is still deterministic and hermetic.
|
||||
- High-level goal: Avoid surprises and/or identify extra complexity early.
|
||||
We want to preemptively review this aspect even though in theory
|
||||
non-hermetic or non-deterministic builds should be caught by Chromium CI
|
||||
at a later point, and depending on crates like `cc` wouldn’t work with
|
||||
Chromium’s build system at all.
|
||||
|
||||
* **Other concerns**: Do you have any (generic or specific) concerns or doubts
|
||||
about some aspects of the crate? If so, please don’t hesitate to escalate and
|
||||
discuss with others, and then LGTM (or not) based on the discussion.
|
||||
- Possible escalation routes:
|
||||
- For any question:
|
||||
[Cr2O3 chatroom](https://chat.google.com/room/AAAAk1UCFGg?cls=7)
|
||||
[Google-internal]
|
||||
- For `unsafe` questions:
|
||||
[Unsafe Rust Crabal chatroom](https://chat.google.com/room/AAAAhLsgrQ4?cls=7)
|
||||
[Google-internal]
|
||||
|
||||
## Other notes
|
||||
|
||||
* The review can be quite minimal or lightweight if Chromium already has a trust
|
||||
relationship with the crate authors. For example, we already trust:
|
||||
- Rust maintainers - e.g. we trust `libc`, `hashbrown`, and similar crates
|
||||
authored by https://github.com/rust-lang
|
||||
- OS SDKs - e.g. we trust `windows-sys`, `windows_aarch64_msvc` and similar
|
||||
crates that are authored by Microsoft and expose OS APIs to Rust
|
||||
- Google or Chromium engineers - e.g. Fontations, ICU4x, etc.
|
||||
|
||||
* If needed the review can be scoped down by using `ban_features` in
|
||||
`gnrt_config.toml` to ensure that a given crate feature is disabled.
|
||||
|
||||
* There is no need to review tests, benchmarks, nor examples.
|
||||
|
||||
* Tools that may be helpful during a review:
|
||||
- `tools/crates/grep_for_vet_relevant_keywords.sh`
|
||||
- Tools for looking at a diff when updating a crate to a new version
|
||||
- <https://diff.rs/>
|
||||
- In Gerrit, you can use patchset 2
|
||||
(results of `git mv old_dir new_dir`)
|
||||
as the baseline.
|
||||
(This should become easier once we fix https://crbug.com/396397336.)
|
||||
|
||||
## Explicitly *not* part of the review checklist
|
||||
|
||||
* **Licensing**: `gnrt` already checks for known/approved licenses when
|
||||
generating `README.chromium`. If an import requires updating
|
||||
`gnrt/lib/readme.rs` to account for new license kinds, then
|
||||
`//tools/crates/gnrt/lib/readme.rs-third-party-license-review.md`
|
||||
can say how to review the changes.
|
||||
|
||||
## Other related docs
|
||||
|
||||
* Corresponding checklist used by Android:
|
||||
[go/android-rust-reviewing-crates](https://goto2.corp.google.com/android-rust-reviewing-crates)
|
||||
[Google-internal]
|
||||
* [Initial draft of this checklist](https://docs.google.com/document/d/1WIwaifxnNK2slmIDoADJX8yeVtoW23yIIxe5F2s0lEg/edit?usp=sharing&resourcekey=0-NMY_YlQzZiN-trOQuVYFOw)
|
||||
[Google internal]
|
||||
|
||||
|
||||
|
||||
|
21
third_party/rust/PRESUBMIT.py
vendored
21
third_party/rust/PRESUBMIT.py
vendored
@ -10,22 +10,5 @@ for more details about the presubmit API built into depot_tools.
|
||||
|
||||
PRESUBMIT_VERSION = '2.0.0'
|
||||
|
||||
def CheckCargoVet(input_api, output_api):
|
||||
vet_args = ['check']
|
||||
|
||||
# Hermetic and idempotent.
|
||||
vet_args += ['--locked', '--frozen', '--no-minimize-exemptions']
|
||||
|
||||
run_cargo_vet_path = input_api.os_path.join(
|
||||
input_api.PresubmitLocalPath(),
|
||||
'..', '..', 'tools', 'crates', 'run_cargo_vet.py')
|
||||
cmd_name = '//tools/crates/run_cargo_vet.py check'
|
||||
|
||||
test_cmd = input_api.Command(
|
||||
name=cmd_name,
|
||||
cmd=[input_api.python3_executable, run_cargo_vet_path] + vet_args,
|
||||
kwargs={},
|
||||
message=output_api.PresubmitError)
|
||||
if input_api.verbose:
|
||||
print('Running ' + cmd_name)
|
||||
return input_api.RunTests([test_cmd])
|
||||
# TODO(https://crbug.com/399934630): Check that `gnrt gen` doesn't result in
|
||||
# any additional changes.
|
||||
|
46
third_party/rust/UNSAFE_RUST_OWNERS
vendored
46
third_party/rust/UNSAFE_RUST_OWNERS
vendored
@ -1,46 +0,0 @@
|
||||
# Overview
|
||||
# ========
|
||||
#
|
||||
# This file lists reviewers for CLs that add or interact with `unsafe` Rust.
|
||||
# See `//docs/rust-unsafe.md` for Chromium policy for `unsafe` Rust code.
|
||||
|
||||
# How to assign reviews
|
||||
# =====================
|
||||
#
|
||||
# Consider assigning reviews to chrome-unsafe-rust-reviews@google.com, rather
|
||||
# than to a specific reviewer. This helps distribute review load and knowledge
|
||||
# more evenly amongst the team. Automation will select a random reviewer and
|
||||
# reassign the review within ~5 minutes. Googlers can read more about this at
|
||||
# go/gwsq-gerrit.
|
||||
chrome-unsafe-rust-reviews@google.com
|
||||
|
||||
# `OWNERS` boilerplate to copy
|
||||
# ============================
|
||||
#
|
||||
# This file should be used to set ownership of `.rs` files that contain
|
||||
# `unsafe` Rust code as follows:
|
||||
#
|
||||
# ```
|
||||
# # In //foo/bar/OWNERS file:
|
||||
# per-file unsafe_module.rs=set noparent
|
||||
# per-file unsafe_module.rs=file://third_party/rust/UNSAFE_REVIEW_OWNERS
|
||||
# ```
|
||||
#
|
||||
# Depending on how well the `unsafe` code is encapsulated, it may be necessary
|
||||
# to add other `.rs` files from the same crate, even if those other Rust
|
||||
# modules don't use `unsafe` themselves.
|
||||
|
||||
# Actual list of reviewers
|
||||
# ========================
|
||||
#
|
||||
# The find-owners plugin does not support groups, so the actual `unsafe` Rust
|
||||
# reviewers are listed below but tagged with {LAST_RESORT_SUGGESTION} so that
|
||||
# Gerrit will preferentially suggest the gwsq review alias above.
|
||||
#
|
||||
# Please keep this list alphabetized and in sync with the gwsq config (see
|
||||
# go/chrome-unsafe-rust-reviews).
|
||||
dcheng@chromium.org #{LAST_RESORT_SUGGESTION}
|
||||
djmitche@chromium.org #{LAST_RESORT_SUGGESTION}
|
||||
liza@chromium.org #{LAST_RESORT_SUGGESTION}
|
||||
lukasza@chromium.org #{LAST_RESORT_SUGGESTION}
|
||||
phao@chromium.org #{LAST_RESORT_SUGGESTION}
|
@ -49,102 +49,22 @@ The script should Just Work in most cases, but sometimes it may fail when
|
||||
dealing with a specific create update. See the "Recovering from script
|
||||
failures" section below for what to do when that happens.
|
||||
|
||||
Before the auto-generated CLs can be landed, some additional manual steps need
|
||||
to be done first - see the sections below.
|
||||
|
||||
## Manual step: `run_cargo_vet.py`
|
||||
|
||||
The changes in the auto-generated CL need to go through a security audit, which
|
||||
will ensure that `cargo vet` criteria (e.g. `ub-risk-0`, `safe-to-deploy`,
|
||||
etc.). still hold for the new versions. The CL description specifies what are
|
||||
the _minimum_ criteria required for the updated crates (note that
|
||||
`supply-chain/audits.toml` can and should record a stricter certification if
|
||||
possible).
|
||||
See the `//docs/rust-unsafe.md` doc for details on how to audit and certify
|
||||
the new crate versions (this may require looping in `unsafe` Rust experts
|
||||
and/or cryptography experts).
|
||||
|
||||
For each update CL, there is a separate git branch created. An audit will most
|
||||
likely need to be recorded in
|
||||
`third_party/rust/chromium_crates_io/supply-chain/audits.toml` and committed to
|
||||
the git branch for each update CL. There are some known corner cases where
|
||||
`audits.toml` changes are not needed:
|
||||
|
||||
* Updates of crates listed in `remove_crates` in
|
||||
`third_party/rust/chromium_crates_io/gnrt_config.toml` (e.g. the `cc` crate
|
||||
which is a dependency of `cxx` but is not actually used/needed in Chromium).
|
||||
This case should be recognized by the `create_update_cl.py` script and noted
|
||||
in the CL description.
|
||||
* Updates of grand-parented-in crates that are covered by exemptions in
|
||||
`third_party/rust/chromium_crates_io/supply-chain/config.toml` instead of
|
||||
being covered by real audits from `audits.toml`. For such crates, skim the
|
||||
delta and use your best judgement on whether to bump the crate version that
|
||||
the exemption applies to. Note that `supply-chain/config.toml` is generated
|
||||
by `gnrt vendor` and should not be edited directly - please instead edit
|
||||
`third_party/rust/chromium_crates_io/vet_config.toml.hbs` and then run
|
||||
`tools/crates/run_gnrt.py vendor` to regenerate `supply-chain/config.toml`.
|
||||
* Update to a crate version that is already covered by `audits.toml` of other
|
||||
projects that Chromium's `run_cargo_vet.py` imports. In such case you may
|
||||
need to commit changes that `cargo vet` generates in
|
||||
`third_party/rust/chromium_crates_io/supply-chain/imports.lock`.
|
||||
|
||||
This step may require one or more of the commands below, for each git branch
|
||||
associated with an update CL (starting with the earliest branches - ones
|
||||
closest to `origin/main`):
|
||||
|
||||
1. `git checkout rust-crates-update--...`
|
||||
- If this is the second or subsequent branch, then also run `git rebase` to
|
||||
rebase it on top of the manual `audits.toml` changes in the upstream
|
||||
branches
|
||||
1. Check which crate (or crates) and which audit criteria need to be reviewed in
|
||||
this branch / CL: `tools/crates/run_cargo_vet.py check`
|
||||
- Note that `run_cargo_vet.py check` may list a _subset_ of the criteria
|
||||
that the automated script has listed in the CL description (e.g. if some
|
||||
of the criteria are already covered by `audits.toml` imported from other
|
||||
projects).
|
||||
- Also note that `cargo vet` will list the _minimum_ required criteria
|
||||
and `audits.toml` can and should record stricter certification if
|
||||
possible. In particular:
|
||||
- Record `does-not-implement-crypto` instead of `crypto-safe` if the
|
||||
crate does not implement crypto.
|
||||
- Record a lower-numbered `ub-risk-N` if appropriate.
|
||||
- And also note that if the crate is currently covered by an exemption
|
||||
in `config.toml`, then we want to bump the exemption instead of providing
|
||||
a delta audit that is baselined on an exemption.
|
||||
Note that `config.toml` shouldn't be edited manually - please edit
|
||||
`vet_config.toml.hbs` and regenerate `config.toml` by running
|
||||
`tools/crates/run_gnrt.py vendor`.
|
||||
1. Follow the cargo vet instructions to inspect diffs and certify the results
|
||||
- Note that special guidelines may apply to
|
||||
[delta audits](https://github.com/google/rust-crate-audits/blob/main/auditing_standards.md#delta-audits-should-describe-the-final-version)
|
||||
- When performing delta audits, <https://diff.rs/> may be a helpful
|
||||
resource.
|
||||
- The result of an audit can be recorded by simply editing `audits.toml`
|
||||
directly, or running a command like the following:
|
||||
```
|
||||
tools/crates/run_cargo_vet.py certify autocfg 1.2.0 1.4.0 \
|
||||
--criteria=safe-to-deploy \
|
||||
--criteria=does-not-implement-crypto \
|
||||
--criteria=ub-risk-0
|
||||
```
|
||||
1. `git add third_party/rust/chromium_crates_io/supply-chain`.
|
||||
1. `git commit -m 'cargo vet'`
|
||||
1. `git cl upload -m 'cargo vet'`
|
||||
Before the auto-generated CLs can be landed, you will need to get an LGTM from
|
||||
`//third_party/rust/OWNERS`. See `//third_party/rust/OWNERS-security-review.md`
|
||||
for a review checklist.
|
||||
|
||||
## New transitive dependencies
|
||||
|
||||
If the roll brings in a new transitive dependency, it will need to be
|
||||
audited in its entirety and the results recorded in
|
||||
`third_party/rust/chromium_crates_io/supply-chain/audits.toml`.
|
||||
Notes from `//third_party/rust/OWNERS-security-review.md` apply:
|
||||
|
||||
The addition of transitive Rust dependencies does not need ATL approval,
|
||||
but an FYI email should be sent to
|
||||
[chrome-atls-discuss@google.com](mailto:chrome-atls-discuss@google.com)
|
||||
in order to record the addition.
|
||||
* The dependency will need to go through security review.
|
||||
* An FYI email should be sent to
|
||||
[chrome-atls-discuss@google.com](mailto:chrome-atls-discuss@google.com)
|
||||
in order to record the addition.
|
||||
|
||||
### Optional: Adding the transitive dependency in its own CL.
|
||||
|
||||
If the crate and/or audit are non-trivial, it's possible to split the
|
||||
If the new crate is non-trivial, it's possible to split the
|
||||
additional crate into its own CL, however then it will default to global
|
||||
visibility and allowing non-test use.
|
||||
* `gnrt add` and `gnrt vendor` can add the dependency to a fresh checkout.
|
||||
@ -218,9 +138,8 @@ introduce breaking changes in the _behavior_ of the existing APIs.
|
||||
To update:
|
||||
|
||||
1. `tools/crates/create_update_cl.py auto -- some_crate_name --breaking`
|
||||
1. Follow the manual steps from the minor version update rotation:
|
||||
1. `cargo vet` audit
|
||||
1. Review, landing, etc.
|
||||
1. Follow the manual steps from the minor version update rotation for
|
||||
review, landing, etc.
|
||||
|
||||
### Major version update: Workflow B: Incremental transition
|
||||
|
||||
|
@ -40,7 +40,6 @@ THIRD_PARTY_RUST = os.path.join(CHROMIUM_DIR, "third_party", "rust")
|
||||
CRATES_DIR = os.path.join(THIRD_PARTY_RUST, "chromium_crates_io")
|
||||
VENDOR_DIR = os.path.join(CRATES_DIR, "vendor")
|
||||
CARGO_LOCK = os.path.join(CRATES_DIR, "Cargo.lock")
|
||||
VET_CONFIG = os.path.join(CRATES_DIR, "supply-chain", "config.toml")
|
||||
INCLUSIVE_LANG_SCRIPT = os.path.join(
|
||||
CHROMIUM_DIR, "infra", "update_inclusive_language_presubmit_exempt_dirs.sh")
|
||||
INCLUSIVE_LANG_CONFIG = os.path.join(
|
||||
@ -150,23 +149,6 @@ def GetCurrentCrateIds() -> Set[str]:
|
||||
return result
|
||||
|
||||
|
||||
def GetVetExemptedCrateIds() -> Set[str]:
|
||||
"""Parses supply-chain/config.toml and returns a set of crate ids
|
||||
that have `exemptions` entries."""
|
||||
vet_config = toml.load(open(VET_CONFIG))
|
||||
result = set()
|
||||
if "exemptions" not in vet_config:
|
||||
return result
|
||||
for crate_name, exemptions in vet_config["exemptions"].items():
|
||||
for exemption in exemptions:
|
||||
# Ignoring which criteria are covered by the exemption
|
||||
# (it is not needed if we just want to emit a warning).
|
||||
crate_version = exemption["version"]
|
||||
crate_id = f"{crate_name}@{crate_version}"
|
||||
result.add(crate_id)
|
||||
return result
|
||||
|
||||
|
||||
@dataclass(eq=True, order=True)
|
||||
class UpdatedCrate:
|
||||
old_crate_id: str
|
||||
@ -371,55 +353,6 @@ def SortedMarkdownList(input_list: List[str]) -> str:
|
||||
return "\n".join(input_list)
|
||||
|
||||
|
||||
def CreateVetPolicyDescription(crate_ids: List[str]) -> str:
|
||||
"""Returns a textual description of the required `cargo vet`'s
|
||||
certifications.
|
||||
|
||||
Args:
|
||||
crate_ids: List of crate ids - e.g. `["clap@1.2.3","clap_derive@1.2.3"]`
|
||||
|
||||
Returns:
|
||||
String suitable for including in the CL description.
|
||||
"""
|
||||
vet_config = toml.load(open(VET_CONFIG))
|
||||
crate_id_to_criteria = dict()
|
||||
for crate_id in crate_ids:
|
||||
crate_name = ConvertCrateIdToCrateName(crate_id)
|
||||
crate_version = ConvertCrateIdToCrateVersion(crate_id)
|
||||
policy = vet_config["policy"][f"{crate_name}:{crate_version}"][
|
||||
"criteria"]
|
||||
policy.sort()
|
||||
crate_id_to_criteria[crate_id] = policy
|
||||
|
||||
criteria_to_crate_ids = dict()
|
||||
for crate_id, criteria in crate_id_to_criteria.items():
|
||||
criteria = ', '.join(criteria)
|
||||
if criteria not in criteria_to_crate_ids:
|
||||
criteria_to_crate_ids[criteria] = []
|
||||
criteria_to_crate_ids[criteria].append(crate_id)
|
||||
|
||||
items = []
|
||||
for criteria, crate_ids in criteria_to_crate_ids.items():
|
||||
crate_ids.sort()
|
||||
crate_ids = ', '.join(crate_ids)
|
||||
if not criteria:
|
||||
criteria = "No audit criteria found. Crates with no audit " \
|
||||
"criteria can be submitted without an update to " \
|
||||
"audits.toml."
|
||||
items.append(f"{crate_ids}: {criteria}")
|
||||
|
||||
description = \
|
||||
"""Chromium `supply-chain/config.toml` policy requires that the following
|
||||
audit criteria are met (note that these are the *minimum* required
|
||||
criteria and `supply-chain/audits.toml` can and should record a stricter
|
||||
certification if possible; see also //docs/rust-unsafe.md):
|
||||
|
||||
"""
|
||||
description += SortedMarkdownList(items)
|
||||
|
||||
return description
|
||||
|
||||
|
||||
def CreateCommitTitle(old_crate_id: str, new_crate_id: str) -> str:
|
||||
crate_name = ConvertCrateIdToCrateName(old_crate_id)
|
||||
old_version = ConvertCrateIdToCrateVersion(old_crate_id)
|
||||
@ -437,8 +370,7 @@ def CreateCommitTitleForBreakingUpdate(diff: CratesDiff) -> str:
|
||||
return textwrap.shorten(title, width=72, placeholder="...")
|
||||
|
||||
|
||||
def CreateCommitDescription(title: str, diff: CratesDiff,
|
||||
include_vet_criteria: bool) -> str:
|
||||
def CreateCommitDescription(title: str, diff: CratesDiff) -> str:
|
||||
description = f"""{title}
|
||||
|
||||
This CL has been created semi-automatically. The expected review
|
||||
@ -459,9 +391,6 @@ process and other details can be found at
|
||||
|
||||
new_or_updated_crate_ids = diff.added_crate_ids + \
|
||||
[update.new_crate_id for update in diff.updates]
|
||||
if include_vet_criteria:
|
||||
vet_policies = CreateVetPolicyDescription(new_or_updated_crate_ids)
|
||||
description += f"\n{vet_policies}"
|
||||
|
||||
description += """
|
||||
Bug: None
|
||||
@ -504,10 +433,11 @@ def UpdateCrate(args, old_crate_id: str, new_crate_id: str,
|
||||
return upstream_branch
|
||||
diff = DiffCrateIds(old_crate_ids, new_crate_ids, only_minor_updates)
|
||||
title = CreateCommitTitle(old_crate_id, new_crate_id)
|
||||
description = CreateCommitDescription(title, diff, False)
|
||||
description = CreateCommitDescription(title, diff)
|
||||
|
||||
# Checkout a new git branch + `git cl upload`
|
||||
new_branch = f"{BRANCH_BASENAME}--{branch_number:02}-{old_crate_id.replace('@', '-')}"
|
||||
branch_suffix = f"{branch_number:02}-{old_crate_id.replace('@', '-')}"
|
||||
new_branch = f"{BRANCH_BASENAME}--{branch_suffix}"
|
||||
Git("checkout", upstream_branch, "-b", new_branch)
|
||||
Git("branch", "--set-upstream-to", upstream_branch)
|
||||
GitAddRustFiles()
|
||||
@ -522,7 +452,6 @@ def UpdateCrate(args, old_crate_id: str, new_crate_id: str,
|
||||
|
||||
|
||||
def FinishUpdatingCrate(args, title: str, diff: CratesDiff):
|
||||
vet_exempted_crate_ids = GetVetExemptedCrateIds()
|
||||
updated_old_crate_ids = set()
|
||||
|
||||
# git mv <vendor/old version> <vendor/new version>
|
||||
@ -555,11 +484,6 @@ def FinishUpdatingCrate(args, title: str, diff: CratesDiff):
|
||||
f.write(new_content)
|
||||
Git("add", INCLUSIVE_LANG_CONFIG)
|
||||
GitCommit(args, "gnrt vendor")
|
||||
if args.upload:
|
||||
print(f" Running `git cl upload --commit-description=...` ...")
|
||||
description = CreateCommitDescription(title, diff, True)
|
||||
GitClUpload(f"--commit-description={description}", "-t",
|
||||
"Edit CL description to include vet policy")
|
||||
|
||||
# gnrt gen
|
||||
print(f" Running `gnrt gen`...")
|
||||
@ -629,15 +553,6 @@ def FinishUpdatingCrate(args, title: str, diff: CratesDiff):
|
||||
issue = Git("cl", "issue")
|
||||
print(f" {issue}")
|
||||
|
||||
for exempted_crate_id in (
|
||||
updated_old_crate_ids.intersection(vet_exempted_crate_ids)):
|
||||
exempted_crate_name = ConvertCrateIdToCrateName(exempted_crate_id)
|
||||
print(f" WARNING: The `{exempted_crate_name}` crate "\
|
||||
"is covered by an exemption rather than an audit. "\
|
||||
"Please bump the exemption in "\
|
||||
"`third_party/rust/chromium_crates_io/vet_config.toml.hbs` "\
|
||||
"and run `tools/crates/run_gnrt.py vendor` again.")
|
||||
|
||||
|
||||
def IsGitDirty():
|
||||
# Make sure there are no uncommitted changes in //third_party/rust,
|
||||
@ -685,8 +600,8 @@ def CheckoutInitialBranch(branch):
|
||||
|
||||
|
||||
def GitClUpload(*args):
|
||||
# `--bypass-hooks` because the uploaded CL will initially fail
|
||||
# `tools/crates/run_cargo_vet.py check`.
|
||||
# TODO(https://crbug.com/405980483): Remove `--bypass-hooks`, or document
|
||||
# why this is still needed.
|
||||
#
|
||||
# `-o banned-words-skip` is used, because the CL is auto-generated and only
|
||||
# modifies third-party libraries (where any banned words would be purely
|
||||
@ -758,7 +673,7 @@ def BreakingUpdate(args):
|
||||
return
|
||||
diff = DiffCrateIds(old_crate_ids, new_crate_ids, only_minor_updates)
|
||||
title = CreateCommitTitleForBreakingUpdate(diff)
|
||||
description = CreateCommitDescription(title, diff, False)
|
||||
description = CreateCommitDescription(title, diff)
|
||||
|
||||
# Checkout a new git branch + `git cl upload`
|
||||
new_branch = f"{BRANCH_BASENAME}--major-version-update"
|
||||
|
@ -99,7 +99,7 @@ class CommitDescriptionTests(unittest.TestCase):
|
||||
after = set(["updated_crate@2.0.2", "added@5.0.1"])
|
||||
diff = DiffCrateIds(before, after, only_minor_updates=True)
|
||||
|
||||
actual_desc = CreateCommitDescription("Commit title.", diff, False)
|
||||
actual_desc = CreateCommitDescription("Commit title.", diff)
|
||||
expected_desc = \
|
||||
"""Commit title.
|
||||
|
||||
|
@ -179,6 +179,8 @@ pub fn readme_file_from_package<'a>(
|
||||
Ok((dir, readme))
|
||||
}
|
||||
|
||||
/// REVIEW REQUIREMENT: When adding a new `LicenseKind`, please consult
|
||||
/// `readme.rs-third-party-license-review.md`.
|
||||
#[allow(clippy::upper_case_acronyms)]
|
||||
#[derive(Debug, Eq, Hash, PartialEq, Clone, Copy)]
|
||||
enum LicenseKind {
|
||||
|
@ -0,0 +1,11 @@
|
||||
## Quick note how to review license-related changes in `readme.rs`
|
||||
|
||||
For changes that add support for a new license kind, please get a review from
|
||||
chromium-third-party@google.com (as documented in
|
||||
`//docs/adding_to_third_party.md` - see
|
||||
[a link to an old revision of that doc here](https://source.chromium.org/chromium/chromium/src/+/main:docs/adding_to_third_party.md;l=396-403;drc=933b35e0c349c6e813dbf9d6ff98ecb4428466ab)
|
||||
|
||||
For benign changes (e.g.
|
||||
[adding](https://chromium-review.googlesource.com/c/chromium/src/+/5773797/2/tools/crates/gnrt/lib/readme.rs)
|
||||
a new free-form spelling of "MIT or Apache-2")
|
||||
there is no need for any additional review.
|
@ -2,6 +2,10 @@
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
// TODO(https://crbug.com/405980483): Evaluate whether to keep generating `cargo vet`'s
|
||||
// `config.toml`. Note that we have removed `cargo vet` presubmits (as tracked
|
||||
// in https://crbug.com/405980483).
|
||||
|
||||
use crate::group::Group;
|
||||
use anyhow::Result;
|
||||
|
||||
|
@ -7,6 +7,11 @@
|
||||
Arguments are passed through to `cargo vet`.
|
||||
'''
|
||||
|
||||
# TODO(https://crbug.com/405980483): Evaluate whether to keep supporting
|
||||
# `tools/crates/run_cargo_vet.py` (see similar note in
|
||||
# `tools/rust/build_vet.py`). Note that we have removed `cargo vet` presubmits
|
||||
# (as tracked in https://crbug.com/405980483).
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import platform
|
||||
@ -79,9 +84,7 @@ def main():
|
||||
if not success:
|
||||
is_presubmit = '--locked' in unrecognized_args and \
|
||||
'--frozen' in unrecognized_args
|
||||
if is_presubmit:
|
||||
print("INFO: Chromium's `cargo vet` policy and presubmit " \
|
||||
"are described in `//docs/rust-unsafe.md`.")
|
||||
assert not is_presubmit
|
||||
|
||||
return 0 if success else 1
|
||||
|
||||
|
@ -4,6 +4,11 @@
|
||||
# found in the LICENSE file.
|
||||
'''Builds the cargo-vet tool.'''
|
||||
|
||||
# TODO(https://crbug.com/405980483): Evaluate whether to keep supporting
|
||||
# `tools/crates/run_cargo_vet.py` (and therefore whether to keep building
|
||||
# `cargo-vet` binary). Note that we have removed `cargo vet` presubmits (as
|
||||
# tracked in https://crbug.com/405980483).
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import sys
|
||||
|
Reference in New Issue
Block a user