Unify tools building

[ishitatsuyuki]

Close #41601

Time saving for up to 10 minutes. Cargo is now only compiled once.

Downsides:

• Out of tree Cargo.lock maintenance
• Cargo.toml [replace] version maintenance

[rust-highfive]

r? @nikomatsakis

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

[ishitatsuyuki]

r? @alexcrichton

Currently the RLS dependencies are fucked up, and I have opened rust-lang/rls#285 to adapt the current state. I'm too lazy to hand-rewrite Cargo.lock, so this is blocked on that PR.

[kennytm]

What's with all these spacing changes and reformatting?

[ishitatsuyuki]

I used rustfmt, but seems it screwed up something else. Will remove.

[ishitatsuyuki]

According to the build results, this saves up to 300 seconds. Not sure if this worth the out-of-tree lockfile maintenance cost.

[ishitatsuyuki]

r? @alexcrichton

Review points:

• Cargo.lock now diverges from upstream. It also needs to be updated on each submodule update. (A simple cargo update in the directory is enough)
• The [replace] section also needs to be updated on version bump. This shouldn't be a big hit though.

[ishitatsuyuki]

By the way, this could be a little bit even faster by swapping the build order of rls and cargo. In that way, it should have more parallelism since rls includes the cargo build and more crates are built in one pass.

[alexcrichton]

Thanks for the PR! I think this is the lowest hanging fruit of speeding up the dist builders right now and this is also an issue that must be resolved before the RLS hits stable.

The downsides here are unfortunate, namely disregarding the lock files in the rls/cargo repositories. Unfortunately though I don't see another way of doing this. I discussed this strategy with @brson and @nrc yesterday and we're comfortable having a separate lock file in-tree for the Cargo/RLS binaries that we ship.

Could this actually, though, move Cargo/RLS into the global rustc workspace? That way all their dependencies will get vendored as usual and should be buildable from source tarballs as well.

[ishitatsuyuki]

I have no idea what will happen if we moved them into global workspace. The thing I most fear is that it mixes things up with the compiler build.

As an alternative approach, can we run cargo vendor separatedly for this directory?

[alexcrichton]

The end goal is to have everything in one workspace, and I don't believe many issues will arise. Mind trying it out?

[ishitatsuyuki]

Old branch backed up, let's see what will happen now.

[ishitatsuyuki]

@alexcrichton the well known submodule delayed manifest problem is here, how should I deal with that?

[ishitatsuyuki]

[01:09:29] error[E0523]: found two different crates with name `rustc_serialize` that are not distinguished by differing `-C metadata`. This will result in symbol conflicts between the two.
[01:09:29]   --> /checkout/src/tools/rls/src/build.rs:13:1
[01:09:29]    |
[01:09:29] 13 | extern crate rustc_driver;
[01:09:29]    | ^^^^^^^^^^^^^^^^^^^^^^^^^^
[01:09:29] 
[01:09:29] error: Could not compile `rls`.

Something's wrong. No idea.

[Mark-Simulacrum]

@alexcrichton Is there a chance you could help diagnose the failure above?

[ishitatsuyuki]

@alexcrichton I recommend to roll back to the original implementation (backed up as a branch, and can be merged with resolving Cargo.lock conflicts). Here are the points:

• rustc_serialize is complexly reused/re-exported inside the compiler code, and it caused the linkage problem.
• We shouldn't give stage2 tools the same workspace, since they're intended for compiling independently from rustc.
• Putting submodules into workspace causes bootstrap failure (missing submodule = missing Cargo.toml).
• Regarding your intense to vendor stage2 deps, I believe it can be just solved by running cargo-vendor twice.

[alexcrichton]

It's worth being clear up front that rustbuild with respect to crates.io is pretty precarious right now, and builds like this are showing how it's really starting to stress many underlying assumptions. I'm basically just saying that this is not going to be an easy integration do "do right," and there's likely tons of little surprises along the way like this.

Unfortunately I don't know why that error is showing up. The only difference here is a change in workspace, but today we're already compiling rustc-serialize a bunch of times. For example the compiler currently links to rustc-serialize 0.3.23 and so does the rls, so why the linking error only shows up with this PR and not currently on master is confusing to me.

In general we want to make these all one workspace to have one location that we're managing dependencies, not a whole bunch. The linkage problem here sounds like either a bug in the compiler or a bug in the setup, I'm not sure which. These tools are intended to be compiled with rustc as well for a full distribution, so we want them in the same workspace. I had forgotten about the submodule problem, yes, but I think we'll just need to bite the bullet and move git submodule management into bootstrap.py from src/bootstrap/lib.rs. It's worth pointing out that cargo-vendor does not currently support being invoked twice, there's an open bug about that.

[bors]

☔ The latest upstream changes (presumably #41846) made this pull request unmergeable. Please resolve the merge conflicts.

[ishitatsuyuki]

Ready to roll.

[alexcrichton]

@ishitatsuyuki did you want to update the git logic to be line-for-line equivalent to the current manifestation in rustbuild and update the logic later in a future PR? (discussion here)

[ishitatsuyuki]

I don't want to do line-by-line translation personally (due to I having not so much time these days). I'm aware that this probably still has some flaw not found in review, but it's working for most of my local usecases and I consider it OK.

[alexcrichton]

Ok I think I agree with @aidanhs that we don't want to land this until it's the same as before. The submodule logic has been careful crafted over a long time, so without a reason to change it I don't think we should. If you're busy though I can push up the relevant logic.

[ishitatsuyuki]

I understand. I will create a line2line version tomorrow.

[alexcrichton]

@bors: r+

Ah no worries, I went ahead and did so

[bors]

📌 Commit db69d89 has been approved by alexcrichton

[alexcrichton]

@bors: p=1

(fixes breakage on master related to submodules)

[bors]

⌛ Testing commit db69d89 with merge be1d1aa...

[jwilm]

I just published an update for Racer relaxing dependencies. 2.0.7 on crates.io

[bors]

💔 Test failed - status-travis

[Mark-Simulacrum]

I suspect this is a spurious failure since we timed out, so retrying.

@bors retry

[bors]

⌛ Testing commit db69d89 with merge 31bfa96...

[bors]

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

[alexcrichton]

🎊

Thanks again for working on this @ishitatsuyuki!