1835274 - mach vendor rust seem to resolve dependencies of a new crate without importing them in third_party/rust

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, defect, P3)

Description

[Benjamin Beurdouche [:beurdouche]]

When running mach vendor rust, all the right dependencies are listed as "Add"ed but the packages are not downloaded and vendored in third_party/rust.

It is unclear whether mach cargo vet is needed or not in order to vendor the crates during/after mach vendor rust

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::Mach Core' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Gabriele Svelto [:gsvelto]]

IIRC ./mach vendor rust won't import the crates until they've been audited. You can add exemptions to supply-chain/config.toml to vendor the sources and do the auditing later.

Comment 3

[Gabriele Svelto [:gsvelto]]

Yeah, cargo vendor won't run unless cargo vet succeeded.

Comment 4

[Gabriele Svelto [:gsvelto]]

We should update the docs.

Comment 5

[Alex Franchuk [:afranchuk]]

Note that while adding exemptions is okay to get things building, it's very unlikely that a patch with new exemptions will be accepted. Auditing isn't too tough to do (it's not the same thing as a full code review).

Comment 6

[Paul Adenot (:padenot)]

(In reply to Gabriele Svelto [:gsvelto] from comment #4)

We should update the docs.

We should make mach vendor says that it didn't vendor, and explain that vetting is necessary to vendor something.

Comment 7

[Benjamin Beurdouche [:beurdouche]]

Adding the exemptions still didn't work for me. I don't have the missing audits anymore but instead:

 0:13.64 Limiting kinds to toolchain and dependencies
 0:13.65 Generating full task set
 0:13.75 Generated 98 tasks for kind fetch
 0:13.76 Generated 14 tasks for kind packages
 0:14.25 Generated 215 tasks for kind toolchain
 0:14.34 Generated 47 tasks for kind docker-image
 WARN Couldn't find cargo registry: reference 'refs/heads/main' not found; class=Reference (4); code=NotFound (-3)    
 WARN Couldn't find cargo registry: reference 'refs/heads/main' not found; class=Reference (4); code=NotFound (-3)    
 WARN Your supply-chain has unnecessary exemptions which could be relaxed or pruned.
 WARN Consider running `cargo vet prune` to prune unnecessary exemptions and imports.

Comment 8

[Benjamin Beurdouche [:beurdouche]]

refs/heads/main looks like a git related message and I am using git-cinnabar. Could that be related?

Comment 9

[Benjamin Beurdouche [:beurdouche]]

Apparently this is unrelated to using git or mercurial, using the second and a fresh setup leads to the same issue. Warning and nothing happen in a background.

Comment 10

[Benjamin Beurdouche [:beurdouche]]

I tried to add exemptions manually into supply-chain/config.toml but that didn't seem to work.

So, after using cargo vet add-exemption to all the dependencies and adding more TOLERATED_DUPES in vendor_rust.py I ended up having the following

(...)
   Vendoring evercrypt v0.0.11 (/Users/bbeurdouche/.cargo/registry/src/index.crates.io-6f17d22bba15001f/evercrypt-0.0.11) to /opt/mozilla/relay-firefox/third_party/rust/evercrypt
   Vendoring evercrypt-sys v0.0.9 (/Users/bbeurdouche/.cargo/registry/src/index.crates.io-6f17d22bba15001f/evercrypt-sys-0.0.9) to /opt/mozilla/relay-firefox/third_party/rust/evercrypt-sys
(...)

The list of vendored is followed by

To use vendored sources, add this to your .cargo/config.toml for this project:

 0:18.62 Package bindgen-0.58.1 has a license that is approved for build-time dependencies:
    BSD-3-Clause
    but the package itself is not whitelisted as being a build-time only package.

    If your package is build-time only, please add it to the whitelist of build-time
    only packages. Otherwise, you need to request license review on the package's license.
    If the package's license is approved, please add it to the whitelist of suitable licenses.
    
 0:18.82 The changes from `mach vendor rust` will NOT be added to version control.


NOTE: `cargo vendor` may have made changes to your Cargo.lock. To restore your
Cargo.lock to the HEAD version, run `git checkout -- Cargo.lock` or
`hg revert Cargo.lock`.

Then nothing happens and the third-party/rust folder does not get populated.
Note that I don't understand the message...

Comment 11

[Benjamin Beurdouche [:beurdouche]]

Those are actually all Error messages in disguise, only after modifying the dependency that relied on that version of bindgen did I finally manage to get everything vendored in...

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:ahochheiden, could you have a look please?

For more information, please visit BugBot documentation.

Comment 13

[Benjamin Beurdouche [:beurdouche]]

To use vendored sources, add this to your .cargo/config.toml for this project:

 0:17.15 Package ring has an unreviewed license file: LICENSE.

Please request review on the provided license; if approved, the package can be added
to the whitelist of packages whose licenses are suitable.

Another one that makes fails without warning. I actually don't yet know how to bypass this.

Comment 14

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1835274 - mach vendor rust seem to resolve dependencies of a new crate without importing them in third_party/rust r=#firefox-build-system-reviewers

This makes a few changes to clarify messaging:

• Documentation now mentions cargo vet and says that all vendored crates must be audited.
• A message is printed when vendoring fails, making it clear that no new crates were vendored.
• Logging now shows the log level for messages, making it easy to find actionable messages in the stream.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9afe3495a11
mach vendor rust seem to resolve dependencies of a new crate without importing them in third_party/rust r=firefox-build-system-reviewers,ahochheiden

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c9afe3495a11

Comment 17

[Benjamin Beurdouche [:beurdouche]]

Thanks for the patch Alex !

Comment 18

[Mike Hommey [:glandium]]

(In reply to Benjamin Beurdouche [:beurdouche] from comment #13)

To use vendored sources, add this to your .cargo/config.toml for this project:

 0:17.15 Package ring has an unreviewed license file: LICENSE.

Please request review on the provided license; if approved, the package can be added
to the whitelist of packages whose licenses are suitable.

Another one that makes fails without warning. I actually don't yet know how to bypass this.

bug 1835972 added a --force flag that bypasses everything for testing purposes, but you'd still have to solve the issues before landing. Adding a dependency on bindgen 0.58 is certainly not something we'd accept (that can be tweaked, we have hacks in place for other versions of bindgen, check the top-level Cargo.toml), and you'd really have to check with legal whether we can use ring, which is AIUI essentially under the openssl license, which has known license compatibility problems that may prevent us from using it (notwithstanding the fact that we may not want to add another SSL stack in Firefox).

Comment 19

[Benjamin Beurdouche [:beurdouche]]

That's super nice, thanks for the tip glandium.

Comment 20

[Gabriele Svelto [:gsvelto]]

(In reply to Mike Hommey [:glandium] from comment #18)

and you'd really have to check with legal whether we can use ring, which is AIUI essentially under the openssl license, which has known license compatibility problems that may prevent us from using it (notwithstanding the fact that we may not want to add another SSL stack in Firefox).

Besides the last stable version of ring doesn't support Windows/AArch64 which we must support.

Comment 21

[Benjamin Beurdouche [:beurdouche]]

Generally, we require crypto to be part of NSS for sensitive computations anyway, because we have higher assurance requirements than most crypto libraries. I am using these crate to make fast progress for a demo, but those dependencies won't ever land in MC. Anna has written code for NSS already and we'll use that one for everything when it is ready... It's too bad that we don't have great rust bindings for NSS yet...