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

Use new cargo argument in bootstrap for cfg checking #97529

Merged
merged 1 commit into from
Jun 4, 2022

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented May 29, 2022

This PR use new cargo argument in bootstrap for doing cfg checking.

Follow-up to #97044 and #97214.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jun 1, 2022

📌 Commit 71f60b43514cc14abbf18c5336b165742008d7d8 has been approved by Mark-Simulacrum

@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 Jun 1, 2022
@bors
Copy link
Contributor

bors commented Jun 1, 2022

⌛ Testing commit 71f60b43514cc14abbf18c5336b165742008d7d8 with merge 549c79c9bda4ba63fa8e8cb466e9697a98bf739d...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 1, 2022

💔 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 Jun 1, 2022
@Urgau
Copy link
Member Author

Urgau commented Jun 1, 2022

@jyn514 @Mark-Simulacrum (bootstrap team, idk if there things are already setup or not)

Would any of you know why when at this stage Building rustdoc for stage2 (x86_64-unknown-illumos) only the rustflags are added and not the cargo ones ?

Context: This PR move some args handling from the rustflags var to the cargo var but it as still leave some unstable things in rustflags. If the cargo flags where added they would also cover the rustflags but it seems that only the rustflags are added and not the cargo flags otherwise it wouldn't failed saying that it's missing -Zunstable-options (passed by the cargo flags). Also note that it has worked for many previous steps.

@rust-log-analyzer

This comment has been minimized.

@Urgau
Copy link
Member Author

Urgau commented Jun 2, 2022

Hum... It's seems that from the last run that the problem lies in the cargo implementation of check cfg and not in bootstrap as the arg is indeed passed but cargo doesn't create the args for rustc.

I will figure this out and fix it on the cargo side but in the mean time I will just back-up the merging of all the args (as I don't think it's a huge issue that checking of features is not done when cross-compiling).

EDIT: Actually I'm not so sure anymore. I'm not even sure cargo is involved at all. Which would be the reason why it's failing (as the code assume that cargo is used).

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the bootstrap-check-cfg-features branch from cd6b8d8 to 16b081b Compare June 2, 2022 19:37
@Urgau Urgau marked this pull request as draft June 3, 2022 11:33
@Urgau Urgau force-pushed the bootstrap-check-cfg-features branch 2 times, most recently from fdf5967 to 628b50d Compare June 3, 2022 12:53
@Urgau Urgau force-pushed the bootstrap-check-cfg-features branch from 8210be4 to ff33001 Compare June 3, 2022 13:46
@Urgau Urgau marked this pull request as ready for review June 3, 2022 13:46
@Urgau
Copy link
Member Author

Urgau commented Jun 3, 2022

After some more investigation on CI I'm pretty sure that it's not at all a cargo problem (cargo does not seems to be involve at all when the problem occur) but simply bootstrap who use the rustflags without using the cargo arguments, which leads to the error. To fix the problem I just re-added -Zunstable-options to the rustflags.

@rustbot ready

@Mark-Simulacrum
Copy link
Member

@bors r+

Seems OK for now.

@bors
Copy link
Contributor

bors commented Jun 4, 2022

📌 Commit ff33001 has been approved by Mark-Simulacrum

@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 Jun 4, 2022
@bors
Copy link
Contributor

bors commented Jun 4, 2022

⌛ Testing commit ff33001 with merge 3a8e713...

@bors
Copy link
Contributor

bors commented Jun 4, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 3a8e713 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 4, 2022
@bors bors merged commit 3a8e713 into rust-lang:master Jun 4, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 4, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3a8e713): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
3.1% 4.2% 3
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
2.3% 2.3% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 2.3% 2.3% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@Urgau Urgau deleted the bootstrap-check-cfg-features branch May 5, 2023 16:46
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants