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

adapt native-link-modifier-bundle test to use llvm-nm #98573

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

krasimirgg
Copy link
Contributor

No functional changes intended.

This updates the test case to use llvm-nm as an alternative to #98424.

This fixes a test failure over at the experimental build of rustc with HEAD LLVM:
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/11144#01814d0f-a46a-4c19-91cf-41e720edb6f9/684-691.

The issue is that this test uses the system nm, which may not be recent
enough to understand the bitcode produced by rustc when compiled against HEAD LLVM.

Similar to what we did for another test in #94023.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2022
@krasimirgg krasimirgg marked this pull request as ready for review June 27, 2022 16:53
@krasimirgg krasimirgg requested a review from petrochenkov June 28, 2022 14:19
@nikic
Copy link
Contributor

nikic commented Jun 28, 2022

Can you please squash the commits?

@krasimirgg
Copy link
Contributor Author

Can you please squash the commits?

done

@nikic
Copy link
Contributor

nikic commented Jun 29, 2022

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jun 29, 2022

📌 Commit 26b48fc8c5d8aa8c9d2fa1936f5c79fb26ed10c7 has been approved by nikic

@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 29, 2022
@krasimirgg
Copy link
Contributor Author

📌 Commit 26b48fc has been approved by nikic

This message looks wrong: after squashing, the correct commit of this PR should be c1ed82f. Not sure if I did something wrong?

@nikic
Copy link
Contributor

nikic commented Jun 29, 2022

@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 Jun 29, 2022
@nikic
Copy link
Contributor

nikic commented Jun 29, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jun 29, 2022

📌 Commit 26b48fc8c5d8aa8c9d2fa1936f5c79fb26ed10c7 has been approved by nikic

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 29, 2022
@nikic nikic closed this Jun 29, 2022
@nikic nikic reopened this Jun 29, 2022
@nikic
Copy link
Contributor

nikic commented Jun 29, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jun 29, 2022

📌 Commit c1ed82f5f28d13072894157bed630eece6fc74d9 has been approved by nikic

@nikic
Copy link
Contributor

nikic commented Jun 29, 2022

Well, no idea what went wrong, but closing and reopening helped :)

@bors
Copy link
Contributor

bors commented Jul 3, 2022

⌛ Testing commit c1ed82f5f28d13072894157bed630eece6fc74d9 with merge 88dd358e884c13ea8b9d82b0108af381ab05d291...

@bors
Copy link
Contributor

bors commented Jul 3, 2022

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 3, 2022
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 3, 2022
@rust-log-analyzer

This comment has been minimized.

@mati865
Copy link
Contributor

mati865 commented Jul 3, 2022

/bin/sh: line 1: D:arustrustbuildi686-pc-windows-gnullvmbuildbin/llvm-nm: No such file or directory

Path to llvm-nm must have had mix of forward and backward slashes.

@krasimirgg
Copy link
Contributor Author

Path to llvm-nm must have had mix of forward and backward slashes.

I'll try escaping this differently. Not sure how to re-try the i686-mingw-1 checks.

@nikic
Copy link
Contributor

nikic commented Jul 4, 2022

Seems to use the same spelling as other LLVM tools in tests now, so let's hope that works...

@bors r+

@bors
Copy link
Contributor

bors commented Jul 4, 2022

📌 Commit aa4c8f6 has been approved by nikic

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

bors commented Jul 4, 2022

⌛ Testing commit aa4c8f6 with merge 17581a7...

@bors
Copy link
Contributor

bors commented Jul 4, 2022

☀️ Test successful - checks-actions
Approved by: nikic
Pushing 17581a7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 4, 2022
@bors bors merged commit 17581a7 into rust-lang:master Jul 4, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 4, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (17581a7): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
4.7% 4.7% 1
Improvements 🎉
(primary)
-0.2% -0.2% 1
Improvements 🎉
(secondary)
-2.5% -2.5% 1
All 😿🎉 (primary) -0.2% -0.2% 1

Cycles

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

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. number of relevant changes

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.

10 participants