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

Move rustdoc run-make-fulldeps tests to run-make #83775

Closed
wants to merge 1 commit into from
Closed

Move rustdoc run-make-fulldeps tests to run-make #83775

wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 2, 2021

This cuts the time to run the tests in half, because they don't require
building a stage 2 compiler. They were all added to fulldeps before #82802 because rustdoc wasn't available in run-make tests.

This doesn't change coverage tests, which will be changed soon in a
separate PR (#83755 (comment)).

This also changes some of the -include directives, see #83773 for what's going on there.

r? @petrochenkov

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Apr 2, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2021
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 2, 2021

📌 Commit b46f615 has been approved by petrochenkov

@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 Apr 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 2, 2021
@Dylan-DPC
Rollup merge of rust-lang#83775 - jyn514:run-make, r=petrochenkov
cfe69ad 
Move rustdoc run-make-fulldeps tests to run-make

This cuts the time to run the tests in half, because they don't require
building a stage 2 compiler. They were all added to fulldeps before rust-lang#82802 because rustdoc wasn't available in run-make tests.

This doesn't change coverage tests, which will be changed soon in a
separate PR (rust-lang#83755 (comment)).

This also changes some of the `-include` directives, see rust-lang#83773 for what's going on there.

r? `@petrochenkov`
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Apr 2, 2021
Closed
@Dylan-DPC-zz
Copy link

failed in rollup

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 4, 2021

@petrochenkov do you understand this test failure?

---- [run-make] run-make/exit-code stdout ----
error: make failed
status: exit status: 2
------------------------------------------
LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code  -Clinker='arm-none-eabi-gcc' success.rs; [ $? -eq 0 ]
------------------------------------------
stderr:
------------------------------------------
------------------------------------------
error: linking with `arm-none-eabi-gcc` failed: exit status: 1
  |
  = note: "arm-none-eabi-gcc" "-m64" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-Wl,--as-needed" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" # lots more args
  = note: arm-none-eabi-gcc: error: unrecognized command line option '-m64'

My only thought is that different args are being passed depending on the stage? I looked for -m64 in the repo but everything I saw was in compiler/rustc_target, I don't know why it would be affected by my change.

@petrochenkov petrochenkov 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 Apr 5, 2021
@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 5, 2021 •
edited
Loading 

LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/exit-code/exit-code  -Clinker='arm-none-eabi-gcc' success.rs

Looks like we are building for x86_64 Linux (that's where -m64 comes from), but passing arm-none-eabi-gcc as a linker, which is obviously not right.
It's also very suspicious that stage2/bin/rustc is used as the compiler.

It may be possible that the run-make suite is entirely broken, and that's the reason why everyone uses run-make-fulldeps instead.

@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 Apr 5, 2021
@bors
Copy link
Contributor

bors commented Apr 5, 2021

☔ The latest upstream changes (presumably #83880) made this pull request unmergeable. Please resolve the merge conflicts.

@jyn514
Move rustdoc run-make-fulldeps tests to run-make
ed8ac0c 
This cuts the time to run the tests in half, because they don't require
building a stage 2 compiler.

This doesn't change coverage tests, which will be changed soon in a
separate PR.
@jyn514
Copy link
Member Author

jyn514 commented Apr 16, 2021

I'm still not sure exactly what's going on, but it's related to cross-compiling - I can reproduce locally with x.py test src/test/run-make --target thumbv6m-none-eabi (after installing qemu-system-arm and gcc-arm-none-eabi). The exact command that fails is this monster:

LD_LIBRARY_PATH="/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/run-make/issue-38237/issue-38237:/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage1/lib:/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0/lib" '/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage1/bin/rustc' --out-dir /home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/run-make/issue-38237/issue-38237 -L /home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/run-make/issue-38237/issue-38237 -Clinker='arm-none-eabi-gcc' foo.rs

Maybe passing -Clinker=arm-gcc is incorrect here? I don't know whether run-make tests are supposed to depend on the target or not.

I think stage2 is unrelated - I used --stage 1 and it ran with stage1/rustc and gave the same error. Probably it's related to how the CI pipeline passes --stage 2 for all tests.

I meant to see what the command was for run-make-fulldeps (in particular, to see whether it passed -Clinker=arm-gcc or not), but my computer crashed and I didn't want to lose the part of the comment I'd written up.

@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2021
@jyn514 jyn514 mentioned this pull request May 9, 2021
Closed
@bstrie
Copy link
Contributor

bstrie commented May 19, 2021

If there's a deeper problem with run-make, should we open an issue to track that and close this for the time being?

@jyn514
Copy link
Member Author

jyn514 commented May 19, 2021

@bstrie I don't understand what's going on well enough to know if it's an issue with run-make or not, and if so, what it is.

@bors
Copy link
Contributor

bors commented May 26, 2021

☔ The latest upstream changes (presumably #85711) made this pull request unmergeable. Please resolve the merge conflicts.

@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 11, 2021
@JohnCSimon
Copy link
Member

ping from triage:
@jyn514 - fyi: this PR has sat idle for over a month

@jyn514
Copy link
Member Author

jyn514 commented Jun 27, 2021

I will not have time to work on this in the foreseeable future.

@jyn514 jyn514 closed this Jun 27, 2021
@jyn514 jyn514 deleted the run-make branch June 27, 2021 22:54
@jyn514 jyn514 mentioned this pull request Aug 27, 2021
Merged
@jyn514 jyn514 mentioned this pull request Nov 11, 2021
Merged
@jyn514 jyn514 mentioned this pull request Mar 30, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2023
@bors
Auto merge of rust-lang#109770 - jyn514:run-make-fulldeps, r=Mark-Sim…
3328913 
…ulacrum

Move almost all run-make-fulldeps tests to run-make

They pass fine, and this avoids having to build the compiler twice.

There are few enough tests left that I think it should be possible to get rid of this test suite altogether, but I expect this PR to fail at least a few times in bors and want to get it merged before tackling further changes. cc rust-lang#83775

Fixes rust-lang#66085. Fixes rust-lang#83773.
oli-obk pushed a commit to oli-obk/miri that referenced this pull request Apr 4, 2023
@bors
Auto merge of #109770 - jyn514:run-make-fulldeps, r=Mark-Simulacrum
68e3894 
Move almost all run-make-fulldeps tests to run-make

They pass fine, and this avoids having to build the compiler twice.

There are few enough tests left that I think it should be possible to get rid of this test suite altogether, but I expect this PR to fail at least a few times in bors and want to get it merged before tackling further changes. cc rust-lang/rust#83775

Fixes rust-lang/rust#66085. Fixes rust-lang/rust#83773.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jyn514 @petrochenkov @bors @Dylan-DPC-zz @bstrie @JohnCSimon @crlf0710 @rust-highfive