rewrite bootstrapping stages #1458

mightyiam · 2022-09-08T05:14:43Z

Continuing from #1327.

mightyiam · 2022-09-08T05:34:23Z

I did put in time and thought and feeling into this rewrite and I feel it's an improvement with regards to passing of information and use of language. Yet, it should go technical review, because I'm not savvy in most of what is involved.

JohnTitor · 2022-09-09T12:41:07Z

r? @jyn514 @tshepang maybe?
I don't feel I'm the best reviewer here.

JohnTitor · 2022-09-09T12:42:14Z

src/building/bootstrapping.md

-Stage 3 is optional. To sanity check our new compiler, we
-can build the libraries with the stage2 compiler. The result ought
-to be identical to before, unless something has broken.
+To verify that the stage 2 libraries that were copied from stage 1 are indeed
+identical to those which would otherwise have been compiled in stage 2, the
+stage 2 compiler is used to compile them and a comparison is made.


I guess emphasizing that stage 3 is optional is important here as most contributors don't need it.

What is it, more specifically, that would require one to compile or use this stage compiler?

Please determine whether it is resolved in HEAD.

jyn514 · 2022-09-09T13:38:14Z

@JohnTitor said this on the other pull request, but I'll repeat it here:

It was written carefully. I'm certain that it's a significant improvement.

Could you elaborate on what improvements your changes are bringing? It's a bit unclear how these changes improve stuff.

I'm sorry you put in a lot of effort to this work, but it's a lot of effort to review as well, and it's unclear how it will help people reading the guide.

mightyiam · 2022-09-09T14:19:53Z

It should be clear to a reviewer how it will help people reading the guide.

mightyiam · 2022-09-09T14:21:10Z

Is there really nothing I can do to have this reviewed? Pinging someone specific? Asking for a budget for contracting a documentation expert?

tshepang · 2022-09-09T18:40:30Z

will have a look within the next week

tshepang · 2022-09-14T01:56:04Z

src/building/bootstrapping.md

-## Stages of bootstrapping
+Each stage involves:
+- An existing compiler and its set of dependencies.
+- Targets ([objects][objects]): `std` and `rustc`.


this can lead to confusion due to target being used differently in Rust... x86_64-unknown-linux-gnu would be an example of a target

All occurrences of variants of "compile" as a verb in sections that this PR edits have been changed to "produce". Please confirm resolution.

src/building/bootstrapping.md

tshepang · 2022-09-14T02:01:24Z

src/building/bootstrapping.md

+Note: the compiler of a stage—e.g. "the stage 1 compiler"—refers to the
+compiler that is compiled at that stage, not the one that already exists.
+
+In the first stage (stage 0) the compiler is usually obtained by downloading a


usually makes me wonder if other ways should be mentioned or linked to

I agree. But I'm not familiar with other ways. And this doesn't seem like an issue created in this PR. Also, I've changed it to "typically". Resolve?

tshepang · 2022-09-14T02:06:43Z

src/building/bootstrapping.md


-### Stage 1
+The stage 0 compiler compiles from current code `rustbuild` and `std` and uses
+them to compile from current code a compiler. This is the stage 1 compiler.


produce (or an equivalent) is an important word to keep here... makes it more clear what is happenin

Resolve in favor of #1458 (comment) ?

tshepang · 2022-09-14T02:09:53Z

src/building/bootstrapping.md

-This means that the symbol names used in the compiler source
-may not match the symbol names that would have been made by the stage1 compiler,
-which can cause problems for dynamic libraries and tests.
+The stage 1 compiler is used to compile from current code a compiler. This is


produce (or equivalent) also missing here

Resolve in favor of #1458 (comment) ?

tshepang · 2022-09-14T02:15:28Z

src/building/bootstrapping.md

+Between the stage 2 and the stage 1 compiler are subtle differences:
+
+The symbol names used in the compiler source may not match the symbol names
+that would have been made by the stage1 compiler. This is important when using
+dynamic linking and due to the lack of ABI compatibility between versions. This
+primarily manifests when tests try to link with any of the `rustc_*` crates or
+use the (now deprecated) plugin infrastructure. These tests are marked with
+`ignore-stage1`.
+
+Also, the stage 2 compiler benefits from the compile-time optimizations
+generated by a compiler that is of the current code.


I would create 2 bullet points from the paragraphs, to show that they flow from "... differences"

Done. Resolve?

mightyiam · 2022-09-14T02:45:56Z

Thanks a lot @tshepang! I will amend.

JohnTitor · 2022-11-20T04:10:05Z

Triage: Friendly-ping @mightyiam, is there any progress since your last comment?

mightyiam · 2022-11-20T14:02:21Z

Sorry. Thank you, @JohnTitor. Will get back to this.

mightyiam · 2022-12-13T15:37:25Z

Scheduled for tomorrow.

mightyiam · 2022-12-14T04:29:26Z

Sorry about my confusion regarding requesting of a review. Apparently, requesting review from one person removes the request for review from another... Anyway, @tshepang , please review.

tshepang · 2022-12-16T01:26:57Z

@mightyiam sorry to make you sad given your efforts and leading you on (by suggesting improvements you have now accepted), but this change does not feel like an overall improvement to existing text.

tshepang · 2022-12-16T01:28:31Z

The one change that may be worth submitting separately as a PR is the following, as it adds new info (but I would not be a proper reviewer for it as am not familiar)...

This primarily manifests when tests try to link with any of the rustc_* crates or
use the (now deprecated) plugin infrastructure. These tests are marked with
ignore-stage1.

mightyiam · 2022-12-16T01:58:47Z

No, I'm sorry. Please provide feedback. Specific feedback, @tshepang. I am certain that this is an improvement.

tshepang · 2022-12-16T02:17:55Z

One example is you are saying the same thing, but making it more verbose, and/or less readable...

source code is compiled with the stage0 compiler -> stage 0 compiler produces from current code

The explicit mention of current code feels overkill since no one is talking about old code.

One thing you may also submit as a separate PR, if not mentioned elsewhere, is...

Note: the compiler of a stage—e.g. "the stage 1 compiler"—refers to the compiler that is produced at that stage, not the one that already exists.

mightyiam · 2022-12-16T09:09:58Z

One example is you are saying the same thing, but making it more verbose, and/or less readable...

source code is compiled with the stage0 compiler -> stage 0 compiler produces from current code

The explicit mention of current code feels overkill since no one is talking about old code.

The existence of a compiler that is from earlier code is central to the concept of bootstrapping.

What I am doing is giving each stage a title:

Stage 0: the pre-compiled compiler

Stage 1: from current code, by an earlier compiler

Stage 2: the truly current compiler

Stage 3: the same-result test

The phrasing "current code" originates in this context. And in this context I feel it is useful.

I have amended that sentence to include commas:

The stage 0 compiler produces, from current code, src/bootstrap and std and uses them to produce, from current code, a compiler. This is the stage 1 compiler.

jyn514 · 2022-12-16T13:43:46Z

No, I'm sorry. Please provide feedback. Specific feedback, @tshepang. I am certain that this is an improvement.

@mightyiam This is not an acceptable tone to use with people volunteering their time. Don't repeat it again in the future.

jyn514 · 2022-12-16T13:44:33Z

We are not obligated to merge your code. The time you have spent so far has no impact on whether or not we merge your code. The only reason we would accept this change is because we think it is an improvement, not because you've worked hard on it.

mightyiam · 2022-12-16T15:13:57Z

Thank you, @jyn514. Sorry about the tone, @tshepang.

tshepang · 2022-12-16T15:49:01Z

@mightyiam I hope this does not discourage you... feel free to submit future improvements, and try to keep them small, as that will make them more easy to review, and provide better feedback in case of disagreement

mightyiam · 2022-12-16T15:56:34Z

@tshepang and @jyn514, I am not discouraged. I certain that this PR is a significant improvement. I would like it reopened, please.

mightyiam · 2022-12-16T15:59:03Z

Documentation rewrites are sometimes the right thing to do. Are they impossible to review? This is not that long. I dispute the rejection of it.

jyn514 · 2022-12-16T16:38:14Z

@JohnTitor said this on the other pull request, but I'll repeat it here:

It was written carefully. I'm certain that it's a significant improvement.

Could you elaborate on what improvements your changes are bringing? It's a bit unclear how these changes improve stuff.

I'm sorry you put in a lot of effort to this work, but it's a lot of effort to review as well, and it's unclear how it will help people reading the guide.

@mightyiam you still 6 months later have not answered this question.

jyn514 · 2022-12-16T16:39:36Z

The burden of proof is on you to show that it is an improvement, not on us to find fault with it.

mightyiam force-pushed the bootstrapping-stages-re branch from e629f4e to f4bcf2b Compare September 8, 2022 05:30

mightyiam mentioned this pull request Sep 8, 2022

rewrite bootstrapping stages #1327

Closed

JohnTitor added the S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content label Sep 9, 2022

JohnTitor reviewed Sep 9, 2022

View reviewed changes

tshepang reviewed Sep 14, 2022

View reviewed changes

mightyiam force-pushed the bootstrapping-stages-re branch 2 times, most recently from f29e27e to e85d43c Compare December 14, 2022 04:19

rewrite bootstrapping stages

c74e2b5

mightyiam force-pushed the bootstrapping-stages-re branch from e85d43c to c74e2b5 Compare December 14, 2022 04:24

mightyiam requested review from tshepang and JohnTitor and removed request for tshepang and JohnTitor December 14, 2022 04:27

tshepang closed this Dec 16, 2022

mightyiam mentioned this pull request Jan 13, 2023

bootsrapping stages overview list #1555

Merged

mightyiam mentioned this pull request Mar 23, 2023

rewrite bootstrapping stages #1650

Draft

rewrite bootstrapping stages #1458

rewrite bootstrapping stages #1458

Conversation

mightyiam commented Sep 8, 2022

mightyiam commented Sep 8, 2022

JohnTitor commented Sep 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyn514 commented Sep 9, 2022

mightyiam commented Sep 9, 2022

mightyiam commented Sep 9, 2022

tshepang commented Sep 9, 2022

Choose a reason for hiding this comment

mightyiam Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tshepang Sep 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mightyiam commented Sep 14, 2022

JohnTitor commented Nov 20, 2022

mightyiam commented Nov 20, 2022

mightyiam commented Dec 13, 2022

mightyiam commented Dec 14, 2022

tshepang commented Dec 16, 2022

tshepang commented Dec 16, 2022

mightyiam commented Dec 16, 2022

tshepang commented Dec 16, 2022 • edited Loading

mightyiam commented Dec 16, 2022

jyn514 commented Dec 16, 2022

jyn514 commented Dec 16, 2022

mightyiam commented Dec 16, 2022

tshepang commented Dec 16, 2022 • edited Loading

mightyiam commented Dec 16, 2022

mightyiam commented Dec 16, 2022

jyn514 commented Dec 16, 2022

jyn514 commented Dec 16, 2022

mightyiam Dec 14, 2022 •

edited

Loading

tshepang Sep 14, 2022 •

edited

Loading

tshepang commented Dec 16, 2022 •

edited

Loading

tshepang commented Dec 16, 2022 •

edited

Loading