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

Add cargo:rustc-link-arg to pass custom linker arguments #6298

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@lu-zero

lu-zero commented Nov 9, 2018

It is useful to produce correct cdylibs on platforms such as Linux and
MacOS.

Groundwork to address #5045 in the future.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 12, 2018

Thanks for the PR! I'm personally a bit wary to add this though without more protections and/or safeguards. In addition to the soft feature freeze this is a feature we have intentionally not added historically as it can be tricky to do so.

I would personally prefer at least that this takes the route of a more formal feature proposal such as a discussion on internals or a mini-rfc.

@lu-zero

This comment has been minimized.

lu-zero commented Nov 12, 2018

If you have time to guide me on that I'm more than willing to :)

Keep in mind that we have an env key that already let you pass those values, but it does to all the rustc invocations and my actual need is to pass the right linker flags so the soname is set for cdylibs.

We can restrict it to just cdylibs or just make cargo come up with the os-specific incantations at least for the tier1 platforms.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 18, 2018

@bors

This comment has been minimized.

Contributor

bors commented Nov 18, 2018

☔️ The latest upstream changes (presumably #6328) made this pull request unmergeable. Please resolve the merge conflicts.

lu-zero added some commits Nov 9, 2018

Add cargo:rustc-link-arg to pass custom linker arguments
It is useful to produce correct cdylibs on platforms such as Linux and
MacOS.

@lu-zero lu-zero force-pushed the lu-zero:rustc-link-args branch from fd3e037 to 104b861 Nov 18, 2018

@lu-zero

This comment has been minimized.

lu-zero commented Nov 18, 2018

Here an attempt on restricting, not sure where is the best place to error out if somebody mixes cdylib and dylib.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 21, 2018

Thanks, although I think there's still a number of open questions here:

  • Currently this is ignoring link-args on non-cdylib crates (silently), is that the behavior that we want?
  • What happens if there's mutliple crate types, where do the link args go?
  • Do we want to read this from .cargo/config? Is there a use case for that?
@lu-zero

This comment has been minimized.

lu-zero commented Nov 21, 2018

Thanks, although I think there's still a number of open questions here:

* Currently this is ignoring link-args on non-cdylib crates (silently), is that the behavior that we want?

The use-case I'm focusing is to pair it with a build.rs providing the link lines. So non-cdylib or non-cdylib+staticlib could be enjoy an hard error.

* What happens if there's mutliple crate types, where do the link args go?

from what I'm seeing for the cdylib+staticlib case it works as expected even if rustc is called only once.

* Do we want to read this from `.cargo/config`? Is there a use case for that?

For my usecase there isn't.

On the other hand this can be reused by cargo-deb/rpm/ebuild later for their distro-specific purposes (e.g. custom rpaths), but I'd rather not conflate it.

@bors

This comment has been minimized.

Contributor

bors commented Nov 28, 2018

☔️ The latest upstream changes (presumably #6352) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment