Skip to content

Adding Fuchsia compiler testing script, docs #103842

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

andrewpollack
Copy link
Member

Adding Fuchsia compiler testing script and related docs updates

r? @tmandry

cc. @djkoloski

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 1, 2022
@andrewpollack andrewpollack force-pushed the add-fuchsia-test-script branch 2 times, most recently from f4e6f3d to 9faaa5e Compare November 1, 2022 17:50
@andrewpollack andrewpollack force-pushed the add-fuchsia-test-script branch from 9faaa5e to cabffb7 Compare November 8, 2022 16:32
@andrewpollack andrewpollack requested a review from tmandry November 8, 2022 16:32
test src/test/ui \
--target x86_64-fuchsia \
--run=always --jobs 1 \
--test-args --target-rustcflags -L \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? As I recall, --test-args takes a single arg or a space separated list of args. But you would have to escape it to get the second behavior, e.g.

--test-args "--target-rustcflags -L"
--test-args "--target-rustcflags ${SDK_PATH}/arch/{x64|arm64}/sysroot/lib

What's going on here is you're passing an arg to pass to compiletest which is then specifying an arg to pass to rustc. So compiletest should see something like

--target-rustcflags -L --target-rustcflags ${SDK_PATH}/arch/{x64,arm64}/sysroot/lib

while rustc sees

-L ${SDK_PATH}/arch/{x64,arm64}/sysroot/lib

It's pretty gross :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, and is the only way to pass multiple --test-args in our current CI on Fuchsia. When using --test-args "", the double-quotes confuse the arg parsing and this is the only workaround

We could make upstream have a nicer-looking double-quote, but this will bring it out of line with our CI on Fuchsia

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood on the double quotes not working, but without them I would think you need

Suggested change
--test-args --target-rustcflags -L \
--test-args --target-rustcflags \
--test-args -L

If this somehow works I'm interested to know how, not that that needs to block this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no -- I apologize! I completely misunderstood what you were saying -- yes, you are right on the need to add an additional --test-args. Creating a PR to fix now

@andrewpollack andrewpollack force-pushed the add-fuchsia-test-script branch from cabffb7 to e1636b8 Compare November 11, 2022 18:44
@tmandry
Copy link
Member

tmandry commented Nov 14, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 14, 2022

📌 Commit e1636b8 has been approved by tmandry

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 Nov 14, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2022
…iaskrgr

Rollup of 13 pull requests

Successful merges:

 - rust-lang#103842 (Adding Fuchsia compiler testing script, docs)
 - rust-lang#104354 (Remove leading newlines from `NonZero*` doc examples)
 - rust-lang#104372 (Update compiler-builtins)
 - rust-lang#104380 (rustdoc: remove unused CSS `code { opacity: 1 }`)
 - rust-lang#104381 (Remove dead NoneError diagnostic handling)
 - rust-lang#104383 (Remove unused symbols and diagnostic items)
 - rust-lang#104391 (Deriving cleanups)
 - rust-lang#104403 (Specify language of code comment to generate document)
 - rust-lang#104404 (Fix missing minification for static files)
 - rust-lang#104413 ([llvm-wrapper] adapt for LLVM API change)
 - rust-lang#104415 (rustdoc: fix corner case in search keyboard commands)
 - rust-lang#104422 (Fix suggest associated call syntax)
 - rust-lang#104426 (Add test for rust-lang#102154)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 98be657 into rust-lang:master Nov 15, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 15, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2022
…-docs, r=tmandry

Fuchsia test suite script fix

Fixing error from rust-lang#103842 (comment)

r? `@tmandry`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2022
…-docs, r=tmandry

Fuchsia test suite script fix

Fixing error from rust-lang#103842 (comment)

r? ``@tmandry``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2022
…-docs, r=tmandry

Fuchsia test suite script fix

Fixing error from rust-lang#103842 (comment)

r? ```@tmandry```
@tmandry tmandry added the O-fuchsia Operating system: Fuchsia label May 24, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc O-fuchsia Operating system: Fuchsia S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants