Riscv64linux Test fixes #80839

tblah · 2021-01-09T16:07:15Z

Get tests passing again using the riscv64gc-unknown-linux-gnu docker image.

Test with

src/ci/docker/run.sh riscv64gc-linux

linkcheck

Linkcheck tests that interdocument links in the documentation are correct. Some interdocument links go between rustc and tools (such as rustdoc and cargo). When cross compiling, rustc is built for the host while some tools are built for the target. This goes for the documentation too. Because of this, links in the rustc documentation reffering to cargo or rustdoc documentation look broken.

This issue is worked around by disabling linkcheck for cross compilation builds.

run-make tests

#78911 seems to happen because --target was not passed to rustc, but the target linker was specified, causing the target linker to be called with options intended for the host.

Resolves #78911

In a separate issue, issue-36710 was trying to run a binary built for the target on the host system. This will not work for any platform using remote-test-server/client (such as riscv64). I don't know of a way of skipping those platforms specifically, so I set this test to skip only on riscv64 for now.

rust-highfive · 2021-01-09T16:07:18Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

src/test/run-make/incr-prev-body-beyond-eof/Makefile

src/bootstrap/test.rs

src/test/run-make/incr-prev-body-beyond-eof/Makefile

JohnCSimon · 2021-02-08T00:13:14Z

Ping from triage:
@tblah - can you please post your status on this PR?

tblah · 2021-02-08T09:21:49Z

@JohnCSimon ready for review

tblah · 2021-02-17T09:29:31Z

@Mark-Simulacrum @nagisa ping

Mark-Simulacrum · 2021-02-17T12:47:37Z

For linkcheck, I'm sort of still uncertain - I would expect us to build docs for both rustc and cargo (for example) for a given platform, not just one of them, and as such for links to work as expected. Does one of them perhaps need a host-only annotation?

Mark-Simulacrum · 2021-02-20T17:26:47Z

Ok, so after looking into the code a bit, it sounds like the problem is that rustc and rustdoc internal documentation (i.e., rustdoc run on those crates) is built for host platforms only, but other documentation (e.g., cargo, reference, etc. books) are built for targets too.

I don't think disabling linkcheck is the right fix - you have genuinely broken links in the generated docs. Rather, it seems like the problem is that building the default documentation (i.e., x.py doc) isn't generating working documentation with host platforms not included in the targets. I'm not sure what exactly to do about this -- the user's intent with x.py doc --host x86_64-unknown-linux-gnu --target riscv64gc-unknown-linux-gnu is not precisely clear, but currently does build sort of "what is asked for":

a set of docs for the host parts
a set of docs for target parts

This would quite clearly lead to link failures from the host to the target.

I can see several strategies for fixing this:

disable link checking in such configurations (this PR)
build target docs for the host parts if we're linking to them (i.e., add ensures(...) for them) manually
change link checking to run on targets, rather than hosts

I'm personally not even sure that we should do any of these 3, but of them, I feel like option 2 is the most appealing from a "building working documentation" policy -- it is also unfortunately going to be very error prone over time if we're not testing this kind of configuration as links get added. Option 1 is definitely the easiest -- I'd be ok landing it with a panic instead of silently passing, and recommending that folks that explicitly are OK with broken documentation pass --exclude src/tools/linkchecker to their x.py test invocation. Do you think that's reasonable?

tblah · 2021-02-20T19:29:00Z

@Mark-Simulacrum thanks for taking a look.

The x.py invocation is for the riscv tests dockerfile:

python3 ../x.py --stage 2 test --target riscv64gc-unknown-linux-gnu

Are you saying the intent of that is unclear? What should it be? It looks like armhf-gnu uses --host='' --target arm-unknown-linux-gnueabihf. I'll try this to see if it fixes this specific usecase.

Thanks for the possible solutions to the general problem. I'll see what I can get working this weekend.

Mark-Simulacrum · 2021-02-20T19:44:41Z

Yeah, currently the test invocation is testing sort of an unusual combination- the host is specified implicitly as the build triple, but the target list doesn't include the host. That's at least plausibly not intended. It's possible just specifying host to be empty (i.e. only check the target bits, not the compiler for example) is what's actually intended.

tblah · 2021-02-21T18:03:34Z

I've updated the MR adding --host='' to the riscv dockerfile and following strategy 1 with the panic!.

Thanks for your help @Mark-Simulacrum

tblah · 2021-02-21T19:36:48Z

The most recent failure was because the x64_64-gnu-llvm-9 Dockerfile also left the host to implicitly be different to the target (in one instance, other invocations do specify --host=''). I am not sure if that was intentional or a bug. 9b23df1 fixes it. We hadn't seen the linkcheck issue from this invocation because it is limited to ui tests.

I've checked the other Dockerfiles for the same issue and they look okay.

camelid · 2021-03-12T03:52:57Z

Ping from triage: @tblah do you want a new review? If so, please post a comment including @rustbot label: +S-waiting-on-review -S-waiting-on-author to indicate that this PR is waiting on review. Thanks!

tblah · 2021-03-12T15:46:25Z

@camelid thanks

@rustbot label: +S-waiting-on-review -S-waiting-on-author

bors · 2021-03-17T23:39:45Z

💔 Test failed - checks-actions

bors · 2021-03-25T16:14:49Z

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

Resolves rust-lang#78911 The target's linker was used but rustc wasn't told to build for that target (instead defaulting to the host). This led to the host instead of the target getting tested and to the linker getting inappropriate arguments.

The test assumes it can run target binaries on the host. This not true for riscv64 CI (or for other platforms using remote-test-server).

When we cross compile, some things (and their documentation) are built for the host (e.g. rustc), while others (and their documentation) are built for the target. This generated documentation will have broken links between documentation for different platforms e.g. between rustc and cargo.

The tests issue-36710 and incr-prev-body-beyond-eof were changed in a previous commit so that the correct target was passed to rustc (previously rustc was building for the host not for the specific target). Since that change it turns out that these platforms never worked (they only appeared to work because rustc was actually building for the host architecture). The wasm architectures fall over trying to build the C++ file in issue-36710. They look for clang (which isn't installed in the test-various docker container). If clang is installed, they can't find a wasm c++ standard library to link to. nvtptx64-nvidia-cuda fails in rustc saying it can't find std. The rust platforms support page says that std is supported on cuda so this is surprising. dist-i586-gnu-i586-i686-musl can't find the C++ compiler. There is only a musl-gcc and no musl-g++ in /musl-i586/bin/. The Docker image probably needs tweaking.

tblah · 2021-03-29T07:33:32Z

In the new version I have rebased on top of the new changes and solved the test failure for armhf-gnu.

@rustbot label: +S-waiting-on-review -S-waiting-on-author

Mark-Simulacrum · 2021-03-29T14:15:51Z

@bors r+

bors · 2021-03-29T14:15:53Z

📌 Commit 1fa48cf has been approved by Mark-Simulacrum

bors · 2021-03-29T16:48:49Z

⌛ Testing commit 1fa48cf with merge 2917eda...

bors · 2021-03-29T19:44:22Z

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

jethrogb · 2021-03-30T10:24:21Z

See #83661

rust-highfive assigned Mark-Simulacrum Jan 9, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 9, 2021

nagisa reviewed Jan 9, 2021

View reviewed changes

src/test/run-make/incr-prev-body-beyond-eof/Makefile Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Jan 9, 2021

View reviewed changes

src/bootstrap/test.rs Show resolved Hide resolved

src/test/run-make/incr-prev-body-beyond-eof/Makefile Outdated Show resolved Hide resolved

Mark-Simulacrum 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 Jan 9, 2021

tblah changed the title ~~Riscv64linux Test fixes~~ WIP: Riscv64linux Test fixes Jan 9, 2021

tblah force-pushed the riscv64linux_links branch from 25ccd2e to 00930aa Compare January 11, 2021 20:12

tblah changed the title ~~WIP: Riscv64linux Test fixes~~ Riscv64linux Test fixes Jan 11, 2021

Mark-Simulacrum 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 Feb 17, 2021

Mark-Simulacrum 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 Feb 20, 2021

tblah force-pushed the riscv64linux_links branch from 00930aa to 5419f82 Compare February 21, 2021 18:01

This comment has been minimized.

Sign in to view

tblah force-pushed the riscv64linux_links branch from 5419f82 to b71573b Compare February 21, 2021 18:40

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 12, 2021

This comment has been minimized.

Sign in to view

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 Mar 17, 2021

Mark-Simulacrum 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 Mar 27, 2021

tblah added 7 commits March 28, 2021 16:48

run-make: Specify --target to rustc

a143d6d

Resolves rust-lang#78911 The target's linker was used but rustc wasn't told to build for that target (instead defaulting to the host). This led to the host instead of the target getting tested and to the linker getting inappropriate arguments.

run-make: skip issue-36710 on riscv64 and armhf

d9dc5d8

The test assumes it can run target binaries on the host. This not true for riscv64 CI (or for other platforms using remote-test-server).

ci: docker: riscv64gc: specify host explicitly

8b40dd1

ci: docker: x86_64: specify host explicitly

980961e

test: run-make: flag tests which won't work in no-std environments

e7505fb

tblah force-pushed the riscv64linux_links branch from 4dbf822 to 1fa48cf Compare March 29, 2021 07:31

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 Mar 29, 2021

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 Mar 29, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 29, 2021

bors merged commit 2917eda into rust-lang:master Mar 29, 2021

rustbot added this to the 1.53.0 milestone Mar 29, 2021

Goirad mentioned this pull request Mar 29, 2021

Ineffective --exclude argument on ./x.py test #83661

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Riscv64linux Test fixes #80839

Riscv64linux Test fixes #80839

tblah commented Jan 9, 2021 •

edited by Mark-Simulacrum

Loading

rust-highfive commented Jan 9, 2021

JohnCSimon commented Feb 8, 2021

tblah commented Feb 8, 2021

tblah commented Feb 17, 2021

Mark-Simulacrum commented Feb 17, 2021

Mark-Simulacrum commented Feb 20, 2021

tblah commented Feb 20, 2021

Mark-Simulacrum commented Feb 20, 2021

tblah commented Feb 21, 2021

This comment has been minimized.

This comment has been minimized.

tblah commented Feb 21, 2021

camelid commented Mar 12, 2021

tblah commented Mar 12, 2021

This comment has been minimized.

bors commented Mar 17, 2021

bors commented Mar 25, 2021

tblah commented Mar 29, 2021 •

edited

Loading

Mark-Simulacrum commented Mar 29, 2021

bors commented Mar 29, 2021

bors commented Mar 29, 2021

bors commented Mar 29, 2021

jethrogb commented Mar 30, 2021

Riscv64linux Test fixes #80839

Riscv64linux Test fixes #80839

Conversation

tblah commented Jan 9, 2021 • edited by Mark-Simulacrum Loading

linkcheck

run-make tests

rust-highfive commented Jan 9, 2021

JohnCSimon commented Feb 8, 2021

tblah commented Feb 8, 2021

tblah commented Feb 17, 2021

Mark-Simulacrum commented Feb 17, 2021

Mark-Simulacrum commented Feb 20, 2021

tblah commented Feb 20, 2021

Mark-Simulacrum commented Feb 20, 2021

tblah commented Feb 21, 2021

This comment has been minimized.

This comment has been minimized.

tblah commented Feb 21, 2021

camelid commented Mar 12, 2021

tblah commented Mar 12, 2021

This comment has been minimized.

bors commented Mar 17, 2021

bors commented Mar 25, 2021

tblah commented Mar 29, 2021 • edited Loading

Mark-Simulacrum commented Mar 29, 2021

bors commented Mar 29, 2021

bors commented Mar 29, 2021

bors commented Mar 29, 2021

jethrogb commented Mar 30, 2021

tblah commented Jan 9, 2021 •

edited by Mark-Simulacrum

Loading

tblah commented Mar 29, 2021 •

edited

Loading