Skip to content

run-make: investigate if we can make tests with linkage less error-prone #128821

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

Open
jieyouxu opened this issue Aug 8, 2024 · 0 comments
Open
Labels
A-compiletest Area: The compiletest test runner A-linkage Area: linking into static, shared libraries and binaries A-run-make Area: port run-make Makefiles to rmake.rs C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status O-windows-msvc Toolchain: MSVC, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Aug 8, 2024

In #128807 we found out that for fmt-write-bloat we needed to do (on Windows):

- #[link(name = "c")]
+ #[cfg_attr(not(target_env = "msvc"), link(name = "c"))]
+ #[cfg_attr(all(target_env = "msvc", target_feature = "crt-static"), link(name = "libcmt"))]
+ #[cfg_attr(all(target_env = "msvc", not(target_feature = "crt-static")), link(name = "msvcrt"))]
extern "C" {}

in order for the test to not be ignore-windows-msvc.

The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works.

It also can't just be #[cfg_attr(target_env = "msvc", link(name = "libcmt"))] simply, because that would fail with

warning LNK4098: defaultlib 'msvcrt' conflicts with use of other libs

We should find a way to make this less tricky / error-prone (because this is very non-obvious).

@ChrisDenton said

I do think this logic should ideally either be in run-make-support or better yet compiletest could have something like a NO_STD_EXTRA_ARGS variable that any test can use. Or at least I think that's better than individual tests needing to figure it out. But I'm not entirely sure how best to approach that.

@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-linkage Area: linking into static, shared libraries and binaries T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. O-windows-msvc Toolchain: MSVC, Operating system: Windows A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs labels Aug 8, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 8, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 8, 2024
@jieyouxu jieyouxu moved this to Backlog in Oxidizing run-make tests Aug 8, 2024
@jieyouxu jieyouxu added E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-linkage Area: linking into static, shared libraries and binaries A-run-make Area: port run-make Makefiles to rmake.rs C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status O-windows-msvc Toolchain: MSVC, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Backlog
Development

No branches or pull requests

2 participants