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

bypass linker configuration and cross target check for specific commands #128871

Merged
merged 2 commits into from
Sep 7, 2024

Conversation

onur-ozkan
Copy link
Member

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves #128180

cc @ChrisDenton

Calling `Cargo::configure_linker` unconditionally slows down certain
commands (e.g., "check" command) without providing any benefit.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@rustbot
Copy link
Collaborator

rustbot commented Aug 9, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added 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) labels Aug 9, 2024
@onur-ozkan onur-ozkan changed the title 128180 bypass linker configuration and cross target check for specific commands Aug 9, 2024
For commands like check/clean there is no need to check for target tools.
Avoiding this check can also speed up the process.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@Kobzol
Copy link
Contributor

Kobzol commented Sep 2, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 2, 2024

📌 Commit 94fbe14 has been approved by Kobzol

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 Sep 2, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 2, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc `@ChrisDenton`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 3, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header)
 - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks )
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir)
 - rust-lang#129789 (rustdoc: use strategic boxing to shrink `clean::Item`)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 3, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc ``@ChrisDenton``
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 3, 2024
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir)
 - rust-lang#129796 (Unify scraped examples with other code examples)
 - rust-lang#129939 (explain why Rvalue::Len still exists)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 4, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc ```@ChrisDenton```
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 4, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129796 (Unify scraped examples with other code examples)
 - rust-lang#129939 (explain why Rvalue::Len still exists)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 4, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129796 (Unify scraped examples with other code examples)
 - rust-lang#129939 (explain why Rvalue::Len still exists)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 4, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129796 (Unify scraped examples with other code examples)
 - rust-lang#129939 (explain why Rvalue::Len still exists)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds )
 - rust-lang#127021 (Add target support for RTEMS Arm)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129796 (Unify scraped examples with other code examples)
 - rust-lang#129939 (explain why Rvalue::Len still exists)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 6, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc `@ChrisDenton`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 6, 2024
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#119229 (Update mingw-w64 + GNU toolchain)
 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#128939 (Distribute rustc_codegen_cranelift for Windows)
 - rust-lang#129529 (Add test to build crates used by r-a on stable)
 - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`)
 - rust-lang#129835 (enable const-float-classify test, and test_next_up/down on 32bit x86)
 - rust-lang#130009 (rustdoc-search: allow trailing `Foo ->` arg search)

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez
Copy link
Member

I wonder if this the one which failed in #130027...

@onur-ozkan
Copy link
Member Author

I wonder if this the one which failed in #130027...

Could be.

@bors rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Testing commit 94fbe14 with merge 2122300...

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 7, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Sep 7, 2024

@bors retry

@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 Sep 7, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc `@ChrisDenton`
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Testing commit 94fbe14 with merge d29eebd...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.469
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Sat, Sep  7, 2024 11:08:18 AM
  network time: Sat, 07 Sep 2024 11:08:18 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Sep 7, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 7, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Sep 7, 2024

@bors retry

@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 Sep 7, 2024
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Sep 7, 2024
bypass linker configuration and cross target check for specific commands

Avoids configuring the linker and checking cross-target-specific tools unless necessary.

Resolves rust-lang#128180

cc `@ChrisDenton`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129468 ([testsuite][cleanup] Remove all usages of `dont_merge` hack to avoid function merging)
 - rust-lang#129614 (Adjust doc comment of Condvar::wait_while)
 - rust-lang#129840 (Implement suggestions for `elided_named_lifetimes`)
 - rust-lang#129891 (Do not request sanitizers for naked functions)
 - rust-lang#129899 (Add Suggestions for Misspelled Keywords)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129987 (Don't store region in `CapturedPlace`)
 - rust-lang#130054 (Add missing quotation marks)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Testing commit 94fbe14 with merge 7468b69...

@bors
Copy link
Contributor

bors commented Sep 7, 2024

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 7468b69 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2024
@bors bors merged commit 7468b69 into rust-lang:master Sep 7, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 7, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128871 (bypass linker configuration and cross target check for specific commands)
 - rust-lang#129468 ([testsuite][cleanup] Remove all usages of `dont_merge` hack to avoid function merging)
 - rust-lang#129614 (Adjust doc comment of Condvar::wait_while)
 - rust-lang#129840 (Implement suggestions for `elided_named_lifetimes`)
 - rust-lang#129891 (Do not request sanitizers for naked functions)
 - rust-lang#129899 (Add Suggestions for Misspelled Keywords)
 - rust-lang#129940 (s390x: Fix a regression related to backchain feature)
 - rust-lang#129987 (Don't store region in `CapturedPlace`)
 - rust-lang#130054 (Add missing quotation marks)

r? `@ghost`
`@rustbot` modify labels: rollup
@onur-ozkan onur-ozkan deleted the 128180 branch September 7, 2024 16:11
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7468b69): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.7%, secondary -0.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
-2.7% [-3.2%, -2.2%] 2
Improvements ✅
(secondary)
-3.1% [-3.1%, -3.1%] 1
All ❌✅ (primary) -2.7% [-3.2%, -2.2%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 756.798s -> 756.576s (-0.03%)
Artifact size: 341.03 MiB -> 341.08 MiB (0.02%)

onur-ozkan added a commit to onur-ozkan/rust that referenced this pull request Sep 9, 2024
…ic commands rust-lang#128871"

Signed-off-by: onur-ozkan <work@onurozkan.dev>
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 9, 2024
…r-ozkan

Do not skip linker configuration for `check` builds

This essentially reverts a part of rust-lang#128871, to fix rust-lang#130108. This is mostly a hotfix until we can figure out a better way to both avoid rebuilds and avoid configuring the linker for `x check`.

Skipping linker for check builds was causing unexpected rebuilds.

Fixes: rust-lang#130108
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Sep 10, 2024
Do not skip linker configuration for `check` builds

This essentially reverts a part of rust-lang/rust#128871, to fix rust-lang/rust#130108. This is mostly a hotfix until we can figure out a better way to both avoid rebuilds and avoid configuring the linker for `x check`.

Skipping linker for check builds was causing unexpected rebuilds.

Fixes: rust-lang/rust#130108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

Bootstrap should skip preparing a linker for targets when checking std
7 participants