tests: Port `symbol-mangling-hashed` to rmake.rs #135768

jieyouxu · 2025-01-20T13:38:44Z

This PR supersedes #128567 and is co-authored with @lolbinarycat.

Summary

This PR ports tests/run-make/symbol-mangling-hashed to rmake.rs. Notable differences when compared to the Makefile version includes:

It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too.
The test uses object crate to be more precise in the filtering, and avoids relying on parsing the human-readable nm output for some nm in the given environment (which isn't really a thing on msvc anyway, and llvm-nm doesn't handle msvc dylibs AFAICT).
Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI.

Review advice

Best reviewed commit-by-commit.
I'm not super sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look.

try-job: x86_64-msvc-1
try-job: i686-msvc-1
try-job: i686-mingw
try-job: x86_64-mingw-1
try-job: x86_64-apple-1
try-job: aarch64-apple
try-job: test-various

jieyouxu · 2025-01-20T13:39:19Z

(Sorry Mark, this isn't ready for review yet)

jieyouxu · 2025-01-20T13:39:27Z

@bors try

bors · 2025-01-20T13:40:39Z

⌛ Trying commit 10841e5 with merge cd8beab...

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

jieyouxu · 2025-01-20T13:44:05Z

@bors try

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

bors · 2025-01-20T13:45:15Z

⌛ Trying commit 10841e5 with merge a5e58b6...

tests/run-make/symbol-mangling-hashed/rmake.rs

jieyouxu · 2025-01-20T15:49:08Z

@bors try

bors · 2025-01-20T15:50:20Z

⌛ Trying commit 897a145 with merge 06b5854...

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

bors · 2025-01-20T17:04:42Z

💔 Test failed - checks-actions

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

jieyouxu · 2025-01-28T13:50:46Z

@bors try

bors · 2025-01-28T13:52:02Z

⌛ Trying commit 2aa5888 with merge 840317a...

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

Noratrieb

Okay, I think that's it. I took a look at some stuff and I think I've figured it out now.
We should drop the last part of the test, fix the comment on the symbols helper and use different identifiers and then this should be good.

src/tools/run-make-support/src/symbols.rs

tests/run-make/symbol-mangling-hashed/rmake.rs

bors · 2025-01-28T16:35:20Z

☀️ Try build successful - checks-actions
Build commit: 840317a (840317a990493de24cf139e520ebc5459b787864)

jieyouxu · 2025-01-29T05:32:19Z

Changes since last review:

Fixed symbol helper comment.
Dropped symbol checks on binary.
Changed hello fn names to be different between the various input rlib/dylib crates.

jieyouxu · 2025-01-29T05:32:37Z

@bors try

…hed, r=<try> tests: Port `symbol-mangling-hashed` to rmake.rs Part of rust-lang#121876. This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.` ### Summary This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes: - It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too. - The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT). - Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI. ### Review advice - Best reviewed commit-by-commit. - I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look. --- try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: i686-mingw try-job: x86_64-mingw-1 try-job: x86_64-apple-1 try-job: aarch64-apple try-job: test-various

bors · 2025-01-29T05:33:48Z

⌛ Trying commit ebdcf01 with merge 1395e24...

bors · 2025-01-29T08:14:59Z

☀️ Try build successful - checks-actions
Build commit: 1395e24 (1395e249256ef3527df6193bb9a86b7f9649ae3f)

jieyouxu · 2025-01-29T08:18:05Z

@rustbot ready

Noratrieb

gamer

…ic` helpers to rustc Co-authored-by: binarycat <[email protected]>

- `dynamic_symbol_names` - `text_section_global_dynamic_symbol_names` - `global_undefined_dynamic_symbol_names` Also add some missing `#[track_caller]` attributes. Co-authored-by: binarycat <[email protected]>

- Use `object` based test logic instead of processing `nm` human-readable textual output. - Try to expand test coverage to not be limited to only linux + x86_64. Co-authored-by: binarycat <[email protected]>

jieyouxu · 2025-01-31T04:32:35Z

Rebased to fix tidy conflict.
@bors r=Noratrieb rollup=iffy (might bounce)

bors · 2025-01-31T04:32:37Z

📌 Commit 9734ebb has been approved by Noratrieb

It is now in the queue for this repository.

rustbot assigned Mark-Simulacrum Jan 20, 2025

This comment was marked as off-topic.

Sign in to view

jieyouxu unassigned Mark-Simulacrum Jan 20, 2025

jieyouxu mentioned this pull request Jan 20, 2025

Migrate symbol-mangling-hashed to rmake.rs #128567

Closed

jieyouxu mentioned this pull request Jan 20, 2025

Tracking Issue for porting run-make tests to use Rust #121876

Open

This comment was marked as resolved.

Sign in to view

jieyouxu 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 20, 2025

jieyouxu commented Jan 20, 2025

View reviewed changes

tests/run-make/symbol-mangling-hashed/rmake.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

jieyouxu force-pushed the migrate-symbol-mangling-hashed branch from 10841e5 to 897a145 Compare January 20, 2025 15:39

This comment has been minimized.

Sign in to view

jieyouxu force-pushed the migrate-symbol-mangling-hashed branch from 897a145 to edf0248 Compare January 21, 2025 04:52

rustbot 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 28, 2025

This comment was marked as outdated.

Sign in to view

Noratrieb requested changes Jan 28, 2025

View reviewed changes

src/tools/run-make-support/src/symbols.rs Outdated Show resolved Hide resolved

tests/run-make/symbol-mangling-hashed/rmake.rs Outdated Show resolved Hide resolved

tests/run-make/symbol-mangling-hashed/rmake.rs Outdated Show resolved Hide resolved

jieyouxu force-pushed the migrate-symbol-mangling-hashed branch from 2aa5888 to ebdcf01 Compare January 29, 2025 05:28

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 Jan 29, 2025

Noratrieb approved these changes Jan 30, 2025

View reviewed changes

jieyouxu and others added 5 commits January 31, 2025 04:29

run-make-support: add -Csymbol-mangling-version and `-Cprefer-dynam…

09f6848

…ic` helpers to rustc Co-authored-by: binarycat <[email protected]>

run-make-support: collapse re-export

17e0fc6

run-make-support: add is_windows_gnu helper

c15c970

run-make-support: add object-based symbol helpers

0bc1b4f

- `dynamic_symbol_names` - `text_section_global_dynamic_symbol_names` - `global_undefined_dynamic_symbol_names` Also add some missing `#[track_caller]` attributes. Co-authored-by: binarycat <[email protected]>

tests: port symbol-mangling-hashed to rmake.rs

9734ebb

- Use `object` based test logic instead of processing `nm` human-readable textual output. - Try to expand test coverage to not be limited to only linux + x86_64. Co-authored-by: binarycat <[email protected]>

jieyouxu force-pushed the migrate-symbol-mangling-hashed branch from ebdcf01 to 9734ebb Compare January 31, 2025 04:31

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 Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Port `symbol-mangling-hashed` to rmake.rs #135768

tests: Port `symbol-mangling-hashed` to rmake.rs #135768

jieyouxu commented Jan 20, 2025 •

edited

Loading

This comment was marked as off-topic.

jieyouxu commented Jan 20, 2025

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment was marked as resolved.

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment has been minimized.

This comment was marked as outdated.

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment has been minimized.

bors commented Jan 20, 2025

This comment was marked as outdated.

This comment was marked as outdated.

jieyouxu commented Jan 28, 2025

bors commented Jan 28, 2025

Noratrieb left a comment

bors commented Jan 28, 2025

jieyouxu commented Jan 29, 2025

jieyouxu commented Jan 29, 2025

bors commented Jan 29, 2025

bors commented Jan 29, 2025

jieyouxu commented Jan 29, 2025

Noratrieb left a comment

jieyouxu commented Jan 31, 2025

bors commented Jan 31, 2025

tests: Port symbol-mangling-hashed to rmake.rs #135768

Are you sure you want to change the base?

tests: Port symbol-mangling-hashed to rmake.rs #135768

Conversation

jieyouxu commented Jan 20, 2025 • edited Loading

Summary

Review advice

This comment was marked as off-topic.

jieyouxu commented Jan 20, 2025

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment was marked as resolved.

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment has been minimized.

This comment was marked as outdated.

jieyouxu commented Jan 20, 2025

bors commented Jan 20, 2025

This comment has been minimized.

bors commented Jan 20, 2025

This comment was marked as outdated.

This comment was marked as outdated.

jieyouxu commented Jan 28, 2025

bors commented Jan 28, 2025

Noratrieb left a comment

Choose a reason for hiding this comment

bors commented Jan 28, 2025

jieyouxu commented Jan 29, 2025

jieyouxu commented Jan 29, 2025

bors commented Jan 29, 2025

bors commented Jan 29, 2025

jieyouxu commented Jan 29, 2025

Noratrieb left a comment

Choose a reason for hiding this comment

jieyouxu commented Jan 31, 2025

bors commented Jan 31, 2025

tests: Port `symbol-mangling-hashed` to rmake.rs #135768

tests: Port `symbol-mangling-hashed` to rmake.rs #135768

jieyouxu commented Jan 20, 2025 •

edited

Loading