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

ignore-cross-compile on optimization-remarks-dir-pgo test #114958

Merged
merged 1 commit into from
Aug 19, 2023

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Aug 18, 2023

We noticed this on our upstream pull on ferrocene a week ago as it was failing our CI. The test attempts to run the produced binary which won't work when cross compiling.

@rustbot
Copy link
Collaborator

rustbot commented Aug 18, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2023
@lqd
Copy link
Member

lqd commented Aug 18, 2023

The other PGO tests don't need this directive, so there should be something more subtle going on.

Maybe running the produced binary should be done with a $(call RUN,foo) || exit 1 like the other tests. I don't know what it does in cross-compiling cases, but it's the most obvious difference.

@lqd
Copy link
Member

lqd commented Aug 18, 2023

I'll take a look, r? lqd

@rustbot rustbot assigned lqd and unassigned Mark-Simulacrum Aug 18, 2023
@lqd
Copy link
Member

lqd commented Aug 18, 2023

The other PGO tests don't need this directive, so there should be something more subtle going on

Discussing with @Veykril, the subtlety was that these tests are already ignored in ferrocene's CI. This is just the first PGO test being added after the initial set was ignored.

This makes sense, as running the binary is not particularly conducive to cross-compilation anyways. We should also do this on the other tests (and ferrocene can then remove their ignores), but there's no need to do so in this PR.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 18, 2023

📌 Commit fb148f6 has been approved by lqd

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 Aug 18, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 18, 2023
…-pgo, r=lqd

`ignore-cross-compile` on `optimization-remarks-dir-pgo` test

We noticed this on our upstream pull on ferrocene a week ago as it was failing our CI. The test attempts to run the produced binary which won't work when cross compiling.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#114953 (Add myself back to review rotation)
 - rust-lang#114958 (`ignore-cross-compile` on `optimization-remarks-dir-pgo` test)
 - rust-lang#114971 (Add doc aliases for trigonometry and other f32,f64 methods.)
 - rust-lang#114972 (Add a test to check that inline const is in required_consts)
 - rust-lang#114977 (Add `modulo` and `mod` as doc aliases for `rem_euclid`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2b13128 into rust-lang:master Aug 19, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 19, 2023
@pietroalbini pietroalbini deleted the optimization-remarks-dir-pgo branch August 28, 2023 08:11
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 30, 2023
`ignore-cross-compile` remaining tests that run binaries

Follow up to rust-lang#114958
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2023
`ignore-cross-compile` remaining tests that run binaries

Follow up to rust-lang#114958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants