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.

Sign up for GitHub

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

Jump to bottom

check lld version to choose correct option to disable multi-threading in tests #102101

Merged
merged 1 commit into from
Oct 31, 2022
Merged

check lld version to choose correct option to disable multi-threading in tests #102101

merged 1 commit into from
Oct 31, 2022

Conversation

belovdv
Copy link
Contributor

@belovdv belovdv commented Sep 21, 2022

Testing compiler with 'use-lld = true' may be incorrect with old lld.
Flag, disabling multi-threading, should consider lld version.

r? @petrochenkov

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 21, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2022
@belovdv
Copy link
Contributor Author

belovdv commented Sep 21, 2022

@rustbot author

@rustbot rustbot 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 Sep 21, 2022
@belovdv belovdv force-pushed the new-check-lld-version branch from 6d742a4 to 9ac8ce5 Compare September 21, 2022 15:12
@belovdv belovdv changed the title check lld version check lld version to choose correct option to disable multi-threading in tests Sep 21, 2022
@belovdv belovdv force-pushed the new-check-lld-version branch from 9ac8ce5 to 6e4bfc2 Compare September 26, 2022 08:44
@belovdv
Copy link
Contributor Author

belovdv commented Sep 26, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 26, 2022
petrochenkov
petrochenkov reviewed Oct 6, 2022
View reviewed changes
src/bootstrap/dylib_util.rs Outdated Show resolved Hide resolved
src/bootstrap/bin/rustdoc.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov 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 6, 2022
@belovdv belovdv force-pushed the new-check-lld-version branch from 6e4bfc2 to 967b4b1 Compare October 19, 2022 12:27
@belovdv
Copy link
Contributor Author

belovdv commented Oct 19, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 19, 2022
petrochenkov
petrochenkov reviewed Oct 19, 2022
View reviewed changes
src/bootstrap/test.rs Outdated Show resolved Hide resolved
src/bootstrap/dylib_util.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov 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 19, 2022
@belovdv belovdv force-pushed the new-check-lld-version branch from 6d7a1f0 to 3446cc3 Compare October 19, 2022 14:48
@belovdv
Copy link
Contributor Author

belovdv commented Oct 19, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 19, 2022
petrochenkov
petrochenkov reviewed Oct 19, 2022
View reviewed changes
src/bootstrap/dylib_util.rs Outdated Show resolved Hide resolved
src/bootstrap/dylib_util.rs Outdated Show resolved Hide resolved
src/bootstrap/bin/rustdoc.rs Outdated Show resolved Hide resolved
src/bootstrap/bin/rustdoc.rs Outdated Show resolved Hide resolved
src/bootstrap/test.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov 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 19, 2022
@belovdv belovdv force-pushed the new-check-lld-version branch from 856ae4a to 5b904cc Compare October 24, 2022 08:24
@belovdv belovdv force-pushed the new-check-lld-version branch from fe50ec3 to 0c4a01a Compare October 26, 2022 08:18
@belovdv
Copy link
Contributor Author

belovdv commented Oct 26, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2022
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 26, 2022

📌 Commit 0c4a01a has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2022
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Oct 26, 2022
@compiler-errors
Rollup merge of rust-lang#102101 - BelovDV:new-check-lld-version, r=p…
485adb5 
…etrochenkov

check lld version to choose correct option to disable multi-threading in tests

Testing compiler with 'use-lld = true' may be incorrect with old lld.
Flag, disabling multi-threading, should consider lld version.

r? `@petrochenkov`
@compiler-errors compiler-errors mentioned this pull request Oct 26, 2022
Closed
notriddle added a commit to notriddle/rust that referenced this pull request Oct 30, 2022
@notriddle
Rollup merge of rust-lang#102101 - BelovDV:new-check-lld-version, r=p…
c0ee7a1 
…etrochenkov

check lld version to choose correct option to disable multi-threading in tests

Testing compiler with 'use-lld = true' may be incorrect with old lld.
Flag, disabling multi-threading, should consider lld version.

r? `@petrochenkov`
@notriddle notriddle mentioned this pull request Oct 30, 2022
Closed
@notriddle notriddle mentioned this pull request Oct 31, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 31, 2022
@bors
Auto merge of rust-lang#103787 - notriddle:rollup-q1vmxsb, r=notriddle
4596f4f 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#97971 (Enable varargs support for calling conventions other than C or cdecl )
 - rust-lang#101428 (Add mir building test directory)
 - rust-lang#101944 (rustdoc: clean up `#toggle-all-docs`)
 - rust-lang#102101 (check lld version to choose correct option to disable multi-threading in tests)
 - rust-lang#102689 (Add a tier 3 target for the Sony PlayStation 1)
 - rust-lang#103746 (rustdoc: add support for incoherent impls on structs and traits)
 - rust-lang#103758 (Add regression test for reexports in search results)
 - rust-lang#103764 (All verbosity checks in `PrettyPrinter` now go through `PrettyPrinter::should_print_verbose`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f404246 into rust-lang:master Oct 31, 2022
@rustbot rustbot added this to the 1.67.0 milestone Oct 31, 2022
This was referenced Sep 27, 2023
Closed
Closed
Kobzol added a commit to Kobzol/rust that referenced this pull request Sep 29, 2023
@Kobzol
Remove RUSTDOC_FUSE_LD_LLD flag
8f1e28f 
This flag wasn't used for anything after rust-lang#102101.
@Kobzol Kobzol mentioned this pull request Sep 29, 2023
Merged
Kobzol added a commit to Kobzol/rust that referenced this pull request Oct 2, 2023
@Kobzol
Remove RUSTDOC_FUSE_LD_LLD flag
c003042 
This flag wasn't used for anything after rust-lang#102101.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
@bors
Auto merge of rust-lang#116278 - Kobzol:bootstrap-lld-mode, r=albertl…
e425e1c 
…arsan68,petrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2023
@bors
Auto merge of rust-lang#116278 - Kobzol:bootstrap-lld-mode, r=albertl…
0c64c8d 
…arsan68,petrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2023
@bors
Auto merge of rust-lang#116278 - Kobzol:bootstrap-lld-mode, r=albertl…
befd1eb 
…arsan68,petrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 12, 2023
@bors
Auto merge of #116278 - Kobzol:bootstrap-lld-mode, r=albertlarsan68,p…
33b52e4 
…etrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang/rust#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
@bors
Auto merge of #116278 - Kobzol:bootstrap-lld-mode, r=albertlarsan68,p…
a2cd73e 
…etrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang/rust#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #116278 - Kobzol:bootstrap-lld-mode, r=albertlarsan68,p…
3b0eefb 
…etrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since rust-lang/rust#102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@klensy klensy klensy left review comments

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.67.0
Development

Successfully merging this pull request may close these issues.
6 participants
@belovdv @petrochenkov @bors @klensy @rust-highfive @rustbot