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

Remove the dummy dependency hack and use extra-ling-args instead. #330

Closed
wants to merge 1 commit into from

Conversation

LaurentMazare
Copy link
Owner

This is only available in cargo 1.50 and in nightly mode at the moment. This can be tried out via:

cargo +nightly run --example basics -Zextra-link-arg --verbose

@LaurentMazare
Copy link
Owner Author

This should be stable in the upcoming 1.56 Rust release. rust-lang/cargo#9557

@riesha
Copy link

riesha commented Jun 27, 2022

any update on this?

@oovm
Copy link
Contributor

oovm commented Mar 7, 2023

This feature should be able to merge, any update?

@LaurentMazare
Copy link
Owner Author

Sadly things have changed a bit since this PR was crafted and now the flags are only used for the crate specifying them and not by dependent crates (or at least that's my understanding). This means that the change here doesn't work, I've put in #642 a similar change that specifies the flags at the tch level rather than torch-sys, so this should work for binaries, examples, and tests from the tch crate but not for crates that use tch. Such crates will also need to specify the flag whereas with the current hack there is no need for any custom configuration on dependent crates.
It's a bit unclear what the best thing to do is here so we should probably think about it a bit more.

@LaurentMazare
Copy link
Owner Author

Closing this one as it doesn't seem possible to pass flags to the final linking step from an intermediary crate library (see this comment). The current best practice is for final crates to have a build.rs that set the proper flags, e.g. in diffusers-rs.

Related issues: cargo#5077. wip for supporting as-needed rust#99424.

@bharrisau
Copy link

bharrisau commented Aug 30, 2023

FYI - I have the following in my project that seems to work on nightly for my needs with pytorch/TensorRT.

#![feature(native_link_modifiers_as_needed)]
#[link(name = "torchtrt_runtime", kind = "dylib", modifiers = "-as-needed")]
extern {}

Once the feature is stabilised you might be able to println!("cargo:rustc-link-lib=dylib:-as-needed=torch_cuda"). I couldn't work out how to use unstable features from cargo build.rs. I guess you could just gate the #[link()] behind a #[cfg(tch-cuda)] and use println!("cargo:rustc-cfg=tch-cuda") from the build script.

@LaurentMazare
Copy link
Owner Author

Thanks for pointing that out @bharrisau that could indeed be super helpful in removing the dummy cuda dependency and providing a less hacky linking setup. My previous post on this thread has a link to the stabilization tracking issue for this but sadly it seems a bit stale a the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants