Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use separate sysroot directories for check and build #77779

Closed
wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Oct 10, 2020

This allows running both in parallel, I think. I don't know how to test this; I tried running x.py check and x.py build at the same time but it seems to be non-deterministic whether that causes an error.

Hopefully fixes #76661.

r? @Mark-Simulacrum

This allows running both in parallel.
@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Oct 10, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2020
@spastorino
Copy link
Member

👍 but, what happens if you run check twice or build twice?. Maybe a solution like this one is a pragmatic enough one anyway?.

@jyn514
Copy link
Member Author

jyn514 commented Oct 10, 2020

Cargo will block like normal:

Checking std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Blocking waiting for file lock on build directory

The reason this broke before is it wasn't using cargo's locking: it would be building the standard library and rustc at the same time, which was two separate cargo invocations. Now that it's using different sysroots, cargo knows about all the compiles going on, so it can lock correctly.

@Mark-Simulacrum
Copy link
Member

Seems okay, even though I'm not a fan of the solution proposed here (e.g., I would expect we still run into trouble with doc vs. build or other combinations). Maybe we should just do what cargo does except globally (i.e., a global flock file)?

Either solution seems a bit unfortunate...

cc @alexcrichton as well since I imagine you might have thoughts on this (having dealt with locking in Cargo more than I)

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2020
@alexcrichton
Copy link
Member

I don't think I'd bother solving the original issue. Most build systems aren't design to be run in parallel, and rustbuild definitely isn't either. This seems like a pretty big hammer for a pretty niche problem.

@jyn514
Copy link
Member Author

jyn514 commented Oct 14, 2020

This seems like a pretty big hammer for a pretty niche problem.

I don't think I'd describe this as niche. I run into this frequently enough that I've stopped recommending rust-analyzer for rustc itself, because it runs check in the background and messes up your actual build. That said, I'm not attached to this particular solution - do you have a 'smaller hammer' in mind?

@jyn514
Copy link
Member Author

jyn514 commented Oct 14, 2020

cc @RalfJung

@alexcrichton
Copy link
Member

Sorry I don't know what a less invasive solution would look like.

@Mark-Simulacrum
Copy link
Member

I think the least invasive and maybe quite simple solution is to create a file in the root of the build directory when rustbuild starts up and delete it on any exit (maybe in Python would be best); if the file exists we can wait on it's deletion (probably just check every second or something, not sure if there's a better way). It's not great but should work ok and solves this problem for all of rustbuild.

@Mark-Simulacrum
Copy link
Member

@jyn514 to check in, do you think you'd be interested in implementing the solution I suggested above?

@jyn514
Copy link
Member Author

jyn514 commented Oct 24, 2020

@Mark-Simulacrum yes, I just haven't gotten around to it.

@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2020
@jyn514
Copy link
Member Author

jyn514 commented Nov 16, 2020

I'm probably not going to get to this any time soon. Since the current approach is too big a hammer, I'll close this and someone can pick up the solution Mark proposed in #77779 (comment) if they like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running "x.py check" and "x.py test" in parallel leads to errors
6 participants