Automate vendoring by invoking cargo-vendor when building src dist tarballs. #39728

eddyb · 2017-02-10T21:40:46Z

This avoids #39633 bringing the src/vendor checked into git by #37524, past 200,000 lines of code.

I believe the strategy of having rustbuild run cargo vendor during the dist src step is sound.

However, the only way to be sure cargo-vendor exists is to run cargo install --force cargo-vendor, which will recompile it every time (not passing --force means you can't tell between "already exists" and "build error"). ~~This is quite suboptimal and I'd like to somehow do it in each Dockerfile that would need it.~~

Cache CARGO_HOME (i.e. ~/.cargo) between CI runs
- bin/cargo-vendor and the actual caches are the relevant bits
Do not build cargo-vendor all the time
- ~~Maybe detect ~/.cargo/bin/cargo-vendor already exists?~~
- ~~Could also try to build it in a Dockerfile but do we have cargo/rustc there?~~
- Final solution: check cargo install --list for a line starting with cargo-vendor

cc @rust-lang/tools

rust-highfive · 2017-02-10T21:41:00Z

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

steveklabnik · 2017-02-10T22:08:30Z

src/ci/run.sh

why comment this out rather than delete all this?

Because I'm not sure yet what's supposed to happen. I guess I should remove it if we want to cache .cargo instead through some other means.

alexcrichton · 2017-02-11T01:17:14Z

Thanks for taking the time to make a PR @eddyb!

Taking a look at this, here's my thoughts:

We'll still want to test --enable-vendor I believe. The best opportunity for this is the "distcheck" bot. Can you update the distcheck step to unconditionally pass --enable-vendor?
I think we'll still want the license check in tidy, but we can skip it in most cases unless --enable-vendor is passed. That way we'll run this on the distcheck bot but nowhere else.
Another purpose of --enable-vendor is to verify that Cargo.lock in-tree is up-to-date (e.g. doesn't need changes to make it correct). I believe that if we only run --enable-vendor on distcheck we won't catch mistakes. Can you add a new configure flag to pass --locked to Cargo on the bots? We still shouldn't pass the flag by default, only on Travis/AppVeyor.

Other than that I think the biggest question is where does the cargo-vendor binary itself come from. Using cargo install just before can be annoying as it unconditionally compiles the project which currently takes quite awhile (depends on Cargo as a library). I don't think that many users are running this command, though, so this is likely just a pain that we can stomach.

That being said I think we could improve the logic here with:

Don't install --force if cargo-vendor is somewhere in PATH. There should be example code in bootstrap/sanity.rs of probing PATH for a binary.
Install with --debug because we don't need a speedy version and otherwise Cargo can take a significant amount of time to optimize.

I believe those points would cover my concerns at least!

alexcrichton · 2017-02-12T20:01:23Z

Everything looks great to me, thanks @eddyb!

As soon as travis goes green on the discheck bot (temporarily enabled here) I think this is good to throw at @bors

eddyb · 2017-02-12T23:08:43Z

Successful build is at https://travis-ci.org/rust-lang/rust/builds/200955072, I'm killing this one.

@bors r=alexcrichton

bors · 2017-02-12T23:08:44Z

📌 Commit aa44186 has been approved by alexcrichton

…hton Automate vendoring by invoking cargo-vendor when building src dist tarballs. This avoids rust-lang#39633 bringing the `src/vendor` checked into git by rust-lang#37524, past 200,000 lines of code. I believe the strategy of having rustbuild run `cargo vendor` during the `dist src` step is sound. However, the only way to be sure `cargo-vendor` exists is to run `cargo install --force cargo-vendor`, which will recompile it every time (not passing `--force` means you can't tell between "already exists" and "build error"). ~~This is quite suboptimal and I'd like to somehow do it in each `Dockerfile` that would need it.~~ * [ ] Cache `CARGO_HOME` (i.e. `~/.cargo`) between CI runs * `bin/cargo-vendor` and the actual caches are the relevant bits * [x] Do not build `cargo-vendor` all the time * ~~Maybe detect `~/.cargo/bin/cargo-vendor` already exists?~~ * ~~Could also try to build it in a `Dockerfile` but do we have `cargo`/`rustc` there?~~ * Final solution: check `cargo install --list` for a line starting with `cargo-vendor ` cc @rust-lang/tools

bors · 2017-02-13T06:32:12Z

⌛ Testing commit aa44186 with merge 311db02...

bors · 2017-02-13T08:02:05Z

💔 Test failed - status-travis

eddyb · 2017-02-13T09:50:25Z

@bors r=alexcrichton

bors · 2017-02-13T09:50:27Z

📌 Commit 915d2f0 has been approved by alexcrichton

bors · 2017-02-13T11:42:47Z

⌛ Testing commit 915d2f0 with merge ba56cf2...

bors · 2017-02-13T20:45:23Z

💔 Test failed - status-travis

eddyb · 2017-02-13T21:38:49Z

@bors r=alexcrichton

bors · 2017-02-13T21:38:50Z

📌 Commit e104705 has been approved by alexcrichton

eddyb · 2017-02-13T21:44:41Z

@bors r=alexcrichton

bors · 2017-02-13T21:44:42Z

📌 Commit 7bfd156 has been approved by alexcrichton

bors · 2017-02-13T23:30:43Z

⌛ Testing commit 7bfd156 with merge 702160d...

bors · 2017-02-13T23:48:07Z

💔 Test failed - status-travis

eddyb · 2017-02-13T23:52:32Z

@bors r=alexcrichton

bors · 2017-02-13T23:52:32Z

📌 Commit d29f0bc has been approved by alexcrichton

bors · 2017-02-13T23:58:30Z

⌛ Testing commit d29f0bc with merge 19dcf06...

bors · 2017-02-14T01:26:05Z

💔 Test failed - status-travis

steveklabnik · 2017-02-14T01:29:44Z

@bors: retry

bors · 2017-02-14T04:15:31Z

⌛ Testing commit d29f0bc with merge c03f12e...

Automate vendoring by invoking cargo-vendor when building src dist tarballs. This avoids #39633 bringing the `src/vendor` checked into git by #37524, past 200,000 lines of code. I believe the strategy of having rustbuild run `cargo vendor` during the `dist src` step is sound. However, the only way to be sure `cargo-vendor` exists is to run `cargo install --force cargo-vendor`, which will recompile it every time (not passing `--force` means you can't tell between "already exists" and "build error"). ~~This is quite suboptimal and I'd like to somehow do it in each `Dockerfile` that would need it.~~ * [ ] Cache `CARGO_HOME` (i.e. `~/.cargo`) between CI runs * `bin/cargo-vendor` and the actual caches are the relevant bits * [x] Do not build `cargo-vendor` all the time * ~~Maybe detect `~/.cargo/bin/cargo-vendor` already exists?~~ * ~~Could also try to build it in a `Dockerfile` but do we have `cargo`/`rustc` there?~~ * Final solution: check `cargo install --list` for a line starting with `cargo-vendor ` cc @rust-lang/tools

bors · 2017-02-14T04:22:49Z

💔 Test failed - status-appveyor

eddyb · 2017-02-14T06:06:05Z

@bors retry

more AppVeyor network nonsense

bors · 2017-02-14T07:06:30Z

⌛ Testing commit d29f0bc with merge 48bc082...

Automate vendoring by invoking cargo-vendor when building src dist tarballs. This avoids #39633 bringing the `src/vendor` checked into git by #37524, past 200,000 lines of code. I believe the strategy of having rustbuild run `cargo vendor` during the `dist src` step is sound. However, the only way to be sure `cargo-vendor` exists is to run `cargo install --force cargo-vendor`, which will recompile it every time (not passing `--force` means you can't tell between "already exists" and "build error"). ~~This is quite suboptimal and I'd like to somehow do it in each `Dockerfile` that would need it.~~ * [ ] Cache `CARGO_HOME` (i.e. `~/.cargo`) between CI runs * `bin/cargo-vendor` and the actual caches are the relevant bits * [x] Do not build `cargo-vendor` all the time * ~~Maybe detect `~/.cargo/bin/cargo-vendor` already exists?~~ * ~~Could also try to build it in a `Dockerfile` but do we have `cargo`/`rustc` there?~~ * Final solution: check `cargo install --list` for a line starting with `cargo-vendor ` cc @rust-lang/tools

bors · 2017-02-14T09:37:34Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 48bc082 to master...

rust-highfive assigned brson Feb 10, 2017

eddyb force-pushed the vendeur-tres-bien branch from bf4b5fa to 919730f Compare February 10, 2017 21:53

steveklabnik reviewed Feb 10, 2017

View reviewed changes

eddyb force-pushed the vendeur-tres-bien branch 2 times, most recently from 69b92e0 to 38b67b7 Compare February 10, 2017 23:28

steveklabnik mentioned this pull request Feb 10, 2017

Port books to mdbook #39633

Merged

Nuke src/vendor.

42c1ea2

eddyb force-pushed the vendeur-tres-bien branch 4 times, most recently from ffc1f6a to 1bd57bf Compare February 12, 2017 20:00

eddyb force-pushed the vendeur-tres-bien branch 4 times, most recently from a143460 to aa44186 Compare February 12, 2017 23:08

frewsxcv mentioned this pull request Feb 13, 2017

Rollup of 4 pull requests #39780

Closed

eddyb force-pushed the vendeur-tres-bien branch from aa44186 to 915d2f0 Compare February 13, 2017 09:49

eddyb force-pushed the vendeur-tres-bien branch from f25be51 to e104705 Compare February 13, 2017 21:38

eddyb force-pushed the vendeur-tres-bien branch from e104705 to 7bfd156 Compare February 13, 2017 21:42

Automatically vendor Cargo deps when building the source tarballs.

d29f0bc

eddyb force-pushed the vendeur-tres-bien branch from 7bfd156 to d29f0bc Compare February 13, 2017 23:52

bors merged commit d29f0bc into rust-lang:master Feb 14, 2017

eddyb deleted the vendeur-tres-bien branch February 14, 2017 09:38

o01eg mentioned this pull request Feb 15, 2017

rust-9999 fails at src_install phase. gentoo/gentoo-rust#230

Closed

alexcrichton mentioned this pull request Mar 24, 2017

Allow overriding vendored sources under sudo #40791

Closed

xuchunmei000 mentioned this pull request Jul 8, 2020

offline build failed with release version occlum/occlum#108

Closed

Uh oh!

Automate vendoring by invoking cargo-vendor when building src dist tarballs. #39728

Automate vendoring by invoking cargo-vendor when building src dist tarballs. #39728

Uh oh!

Conversation

eddyb commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Feb 10, 2017

Uh oh!

steveklabnik Feb 10, 2017

Choose a reason for hiding this comment

Uh oh!

eddyb Feb 10, 2017

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Feb 11, 2017

Uh oh!

alexcrichton commented Feb 12, 2017

Uh oh!

eddyb commented Feb 12, 2017

Uh oh!

bors commented Feb 12, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

eddyb commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

eddyb commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

eddyb commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

eddyb commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 13, 2017

Uh oh!

bors commented Feb 14, 2017

Uh oh!

steveklabnik commented Feb 14, 2017

Uh oh!

bors commented Feb 14, 2017

Uh oh!

bors commented Feb 14, 2017

Uh oh!

eddyb commented Feb 14, 2017

Uh oh!

bors commented Feb 14, 2017

Uh oh!

bors commented Feb 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eddyb commented Feb 10, 2017 •

edited

Loading