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 static-nobundle test to use llvm-nm #94023

Merged
merged 2 commits into from
Feb 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2060,6 +2060,7 @@ impl Step for RustDev {
"llvm-bcanalyzer",
"llvm-cov",
"llvm-dwp",
"llvm-nm",
] {
tarball.add_file(src_bindir.join(exe(bin, target)), "bin", 0o755);
}
Expand Down
6 changes: 4 additions & 2 deletions src/test/run-make-fulldeps/static-nobundle/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ all: $(call NATIVE_STATICLIB,aaa)
$(RUSTC) bbb.rs --crate-type=rlib

# Check that bbb does NOT contain the definition of `native_func`
nm $(TMPDIR)/libbbb.rlib | $(CGREP) -ve "T _*native_func"
nm $(TMPDIR)/libbbb.rlib | $(CGREP) -e "U _*native_func"
# We're using the llvm-nm instead of the system nm to ensure it
# is compatible with the LLVM bitcode generated by rustc.
"$(LLVM_BIN_DIR)/llvm-nm" $(TMPDIR)/libbbb.rlib | $(CGREP) -ve "T _*native_func"
"$(LLVM_BIN_DIR)/llvm-nm" $(TMPDIR)/libbbb.rlib | $(CGREP) -e "U _*native_func"
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the test when the llvm backend isn't used as in that case none of the llvm tools are built. While currently only the llvm backend is tested on CI, this may change in the future. All previous cases of llvm tools are actually llvm specific tests. This one is not.

Copy link
Member

Choose a reason for hiding this comment

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

That's a good point, but I think for now I'm inclined to leave this test be. IIRC, @nikic suggested somewhere (Zulip?) we should be renaming the section in the rlib files such that the failure this is fixing doesn't happen.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. run-make-fulldeps currently isn't being tested in cg_clif's CI anyway.


# Check that aaa gets linked (either as `-l aaa` or `aaa.lib`) when building ccc.
$(RUSTC) ccc.rs -C prefer-dynamic --crate-type=dylib --print link-args | $(CGREP) -e '-l[" ]*aaa|aaa\.lib'
Expand Down