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

Set incremental = true in config.toml.example #76417

Closed
wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Sep 6, 2020

This does not affect any x.py defaults, or any contributors that have an
existing config.toml. It only affects the settings for new contributors.

This has been recommended by rustc-dev-guide for many months.
I'm not aware of people running into much trouble with it and it saves
an enormous amount of time when recompiling. It also requires
recompiling all of rustc when changed.

r? @Mark-Simulacrum

This does not affect any x.py defaults, or any contributors that have an
existing config.toml. It only affects the settings for new contributors.

This has been recommended by rustc-dev-guide for many months.
I'm not aware of people running into much trouble with it and it saves
an *enormous* amount of time when recompiling. It also requires
recompiling all of `rustc` when changed.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2020
@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 6, 2020
@Mark-Simulacrum
Copy link
Member

I don't think we should have the example configuration provide just this value as a non-default. If we truly believe that it is a good default (I would like to see a comparison of edit-rerun cycle times for a change to something like rustc_middle), then it seems like we should set it by default for everyone.

One thing I would be fine with is adding a config.toml.developers or so, that explicitly sets a subset of options we believe to be good for local development (vs distros or CI). It could even also be all commented out and simply declare developer=true at the top level, which would change defaults in bootstrap to whatever the latest "best" configuration is.

@jyn514
Copy link
Member Author

jyn514 commented Sep 6, 2020

Hmm ... I guess I was imagining config.toml.example as already being for developers. If we added .developers, who would the .example file be for?

That said the rest of your points make sense. Do you think this would be useful for CI/distros? I'm worried I'll get more angry emails from @infinity0 :P

@ehuss
Copy link
Contributor

ehuss commented Sep 6, 2020

I agree with what Mark said, but also that incremental often breaks for me, so I'm not sure it is a great thing to recommend without some explanation of the downsides. It also takes about 5GB of extra disk space, which might be too much for some people.

I would also encourage to keep in mind that there are people who want to build from source for whatever reason. It's not just contributors or CI or distros.

@mati865
Copy link
Contributor

mati865 commented Sep 7, 2020

Among the issues mentioned above it often causes 32-bit builds on Windows to fail because rustc needs more memory with incemental objects.


# Build a multi-threaded rustc
# FIXME(#75760): Some UI tests fail when this option is enabled.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this could be added separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #76441 for this.

@pickfire
Copy link
Contributor

pickfire commented Sep 7, 2020

I don't think we should set it to true by default but we could add a note that is is recommended to turn it on. Since there are some drawbacks, maybe we could mention why it wasn't on by default there too.

@jyn514
Copy link
Member Author

jyn514 commented Sep 7, 2020

Ok, I'll close this in favor of the rustc-dev-guide instructions then.

@jyn514 jyn514 closed this Sep 7, 2020
@pickfire
Copy link
Contributor

pickfire commented Sep 7, 2020

@jyn514 In my humble opinion, I think it is good to mention it in the comment that it is recommended like you did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

7 participants