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

rustbuild: Add cli option --keep-stage #38331

Merged
merged 2 commits into from
Dec 15, 2016
Merged

Conversation

bluss
Copy link
Member

@bluss bluss commented Dec 12, 2016

This option is intended to be used like:

./x.py build --stage 1 --keep-stage 0

Which skips all stage 0 steps, so that stage 1 can be recompiled
directly (even if for example libcore has changes).

This is useful when working on cfg(not(stage0)) parts of the
libraries or when re-running stage 1 tests in libraries in general.

Fixes #38326

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@bluss bluss changed the title qrustbuild: Add cli option --assume-stage rustbuild: Add cli option --assume-stage Dec 12, 2016
@bluss
Copy link
Member Author

bluss commented Dec 12, 2016

r? @alexcrichton tell me if this design is even ok

Maybe I can ask @petrochenkov, didn't you have an idea of more detailed control of which steps that were already built?

@rust-highfive rust-highfive assigned alexcrichton and unassigned aturon Dec 12, 2016
@bluss bluss changed the title rustbuild: Add cli option --assume-stage rustbuild: Add cli option --keep-stage Dec 12, 2016
@bluss
Copy link
Member Author

bluss commented Dec 12, 2016

Switched name to --keep-stage N (stages N and before are not recompiled)

This option is intended to be used like:

./x.py build --stage 1 --keep-stage 0

Which skips all stage 0 steps, so that stage 1 can be recompiled
directly (even if for example libcore has changes).

This is useful when working on `cfg(not(stage0))` parts of the
libraries, or when re-running stage 1 tests in libraries in general.
@petrochenkov
Copy link
Contributor

@bluss
No idea, sorry. Maybe at the end of the week.

@arielb1
Copy link
Contributor

arielb1 commented Dec 13, 2016

r? @alexcrichton

@alexcrichton
Copy link
Member

Thanks for the PR! This is the implementation I'd expect, although I hesitate only due to some planned changes.

  • First we're planning on building the compiler only 2 times instead of today's 3. (more info on this soon).
  • @nikomatsakis would like to use incremental compilation to skip the first compile if you have a "new enough" nightly, which means that we're actually only compiling the compiler once (incrementally).
  • Alternatively, I've often wanted a mode where you compile one compiler, cache that output, and then incrementally work on the compiler from then on.

That's basically a fancy way of saying that hopefully in the near future we obviate the need for this entirely as the builds will all be incremental and fast enough. I'm wary of these sorts of flags where newbies will shoot themselves in the foot almost instantly (e.g. this is difficult to get right unless you know what you're doing).

That being said, it's super small, easy to verify and easy to back out. In that sense I wouldn't mind at all landing.

Thoughts?

@est31
Copy link
Member

est31 commented Dec 14, 2016

Those alternatives all sound great, but I'd like to be able to use this until they become reality, maybe even for longer. You already said its easy to revert this change once it becomes redundant.

I think it won't be confusing for newbies as its easy to see that this only does subset of the full build, and therefore might not test everything, but maybe I'm not newbie enough anymore to assess that, idk.

What is certain though is that there seems to be real demand for the feature (at least @bluss @arielb1 and me), and I think there is no advantage in waiting for one of the other features to be implemented before merging this, after all this change can be removed just as easily as its added.

The alternatives don't cover one use case though: I for the i128 pr, I wanted to find out why rand didn't compile as stage2 artifact. So I locally applied the PR (in fact a simplified version of it) to not recompile stage 0 and 1 artifacts, and minimized the example. How to make this possible without this feature applied?

@bluss
Copy link
Member Author

bluss commented Dec 14, 2016

Since those things sound a lot like local_rebuild, they don't address the use case I had for this PR, which was to first apply some patches to the compiler, then iterate on not(stage0) changes to libcore.

@alexcrichton
Copy link
Member

We discussed this during tools triage today and the conclusion was that we should go ahead and merge this as it's useful for now and such a small change.

@bluss could you also add some documentation for this? Also, it'd be great if we could detect errors when you pass this option and say "try not passing this option", but that may be a little ambitious for here.

@bluss
Copy link
Member Author

bluss commented Dec 15, 2016

Catching failing commands would be something very general, not sure if it's worth it. If this is temporary, too we could make it less visible instead (not show in main usage, just with --help -v or so). But most of all I just want this in, because it's a useful clutch.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Dec 15, 2016

📌 Commit 0e01427 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Dec 15, 2016

⌛ Testing commit 0e01427 with merge f70ad0a...

bors added a commit that referenced this pull request Dec 15, 2016
rustbuild: Add cli option --keep-stage

This option is intended to be used like:

./x.py build --stage 1 --keep-stage 0

Which skips all stage 0 steps, so that stage 1 can be recompiled
directly (even if for example libcore has changes).

This is useful when working on `cfg(not(stage0))` parts of the
libraries or when re-running stage 1 tests in libraries in general.

Fixes #38326
@bors bors merged commit 0e01427 into rust-lang:master Dec 15, 2016
@bluss bluss deleted the assume-stage branch December 15, 2016 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants