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

Platform-specific linker option ignored when specified using cfg syntax #12472

Closed
tgnottingham opened this issue Aug 9, 2023 · 4 comments · Fixed by #12535
Closed

Platform-specific linker option ignored when specified using cfg syntax #12472

tgnottingham opened this issue Aug 9, 2023 · 4 comments · Fixed by #12535
Assignees
Labels
A-configuration Area: cargo config files and env vars A-linkage Area: linker issues, dylib, cdylib, shared libraries, so C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@tgnottingham
Copy link

tgnottingham commented Aug 9, 2023

Problem

When the following is placed in .cargo/config.toml, the linker option is respected (on an x86_64 Linux system):

[target.x86_64-unknown-linux-gnu]
linker = "clang"
rustflags = ["-C", "link-arg=-fuse-ld=lld"]

However, when the cfg syntax is used, as in the following, the linker option seems to be ignored (-C linker=clang isn't visible in cargo -vv output):

[target.'cfg(unix)']
linker = "clang"
rustflags = ["-C", "link-arg=-fuse-ld=lld"]

In either case, the rustflags option is respected.

Is the linker option being ignored here a bug?

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.71.1 (7f1d04c00 2023-07-29)
release: 1.71.1
commit-hash: 7f1d04c0053083b98fa50b69b6f56e339b0556a8
commit-date: 2023-07-29
host: x86_64-unknown-linux-gnu
libgit2: 1.6.4 (sys:0.17.1 vendored)
libcurl: 8.0.1-DEV (sys:0.4.61+curl-8.0.1 vendored ssl:OpenSSL/1.1.1t)
ssl: OpenSSL 1.1.1t  7 Feb 2023
os: Ubuntu 22.04 (jammy) [64-bit]
@tgnottingham tgnottingham added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Aug 9, 2023
@weihanglo
Copy link
Member

weihanglo commented Aug 13, 2023

This section of [target] config lists all possible configurations and doesn't include target.<cfg>'.linker and target.<cfg>.<links>.

Delved into the history, it seems that just nobody requested for that.

However, the mechanisms of plain target-triple and cfg expression are different. May need some efforts to implement those configs. My guess is that they'll behave the same as target.<cfg>.runner: <triple> takes precedence over them, and errors out when multiple <cfg> of them found.

@ehuss the wise, was there any reason not supporting linker and link overridden with cfg expression? As I can tell there is a test verifying the "ignored" behavior:

#[cargo_test]
fn cfg_ignored_fields() {
// Test for some ignored fields in [target.'cfg()'] tables.

@weihanglo weihanglo added A-linkage Area: linker issues, dylib, cdylib, shared libraries, so A-configuration Area: cargo config files and env vars S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Aug 13, 2023
@ehuss
Copy link
Contributor

ehuss commented Aug 13, 2023

I don't think there is a specific reason other than nobody needed it. If we add it, I suppose it should follow the same logic as runner.

@weihanglo
Copy link
Member

Thanks for quick reply!

Here are pointers for people wanting to jump in on adding target.<cfg>.linker feature:

  • Basically as Eric said, it should follow the same logic as runner. Here is how runner find matched config:
    fn target_runner(
    bcx: &BuildContext<'_, '_>,
    kind: CompileKind,
    ) -> CargoResult<Option<(PathBuf, Vec<String>)>> {
  • A new linker field is required to be added to TargetCfgConfig:
    pub struct TargetCfgConfig {
  • TargetConfig.linker is accessible through the field itself, as well as via BuildContext::linker. We should consolidate them into one place. By copying runner's logic, perhaps adding a target_links field to Compilation struct?

@rustbot label -S-needs-info +S-accepted +E-easy

Assumed it is not too hard, though I might be wron.

@rustbot rustbot added E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels Aug 13, 2023
@hi-rustin
Copy link
Member

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars A-linkage Area: linker issues, dylib, cdylib, shared libraries, so C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants