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

The missing lib #2710

Merged
merged 1 commit into from
May 19, 2016
Merged

The missing lib #2710

merged 1 commit into from
May 19, 2016

Conversation

sbeckeriv
Copy link
Contributor

Dearest Reviewer,

This pull request allows the value of project.links to be passed into
the build script via an environment variable. This closes #1220 . I have
added a test that makes sure that the environment variable is set. Also
note I am using CARGO_LIB not LIB because it appears to be used for
windows in appveyor. I think CARGO_LIB fits better.

I have also updated the documentation to reflect the new variable.

Thanks!
Becker

@rust-highfive
Copy link

r? @wycats

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -109,6 +109,10 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
.env("PROFILE", if cx.build_config.release {"release"} else {"debug"})
.env("HOST", &cx.config.rustc_info().host);

if unit.pkg.manifest().links().is_some(){
p.env("CARGO_LIB", unit.pkg.manifest().links().as_ref().unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

This might be more naturally written with:

if let Some(links) = unit.pkg.manifest().links() {
    p.env("...", links);
}

@alexcrichton
Copy link
Member

Thanks @sbeckeriv! The only question I'd have is the name of this env var. CARGO_LIB seems a "bit too sweet" in terms of being short and perhaps want to be reserved for something else. I wonder if this could be called CARGO_MANIFEST_LINKS to be clear as to what it is?

@sbeckeriv
Copy link
Contributor Author

Thanks! I am still learning to use things like if let! I also updated the name.

Dearest Reviewer,

This pull request allows the value of project.links to be passed into
the build script via an environment variable. This closes rust-lang#1220 . I have
added a test that makes sure that the environment variable is set. Also
note I am using CARGO_LIB not LIB because it appears to be used for
windows in appveyor. I think CARGO_LIB fits better.

I have also updated the documentation to reflect the new variable.

Thanks!
Becker

[Updated]
Github comments moved to CARGO_MANIFEST_LINKS
Fix if statement
@alexcrichton
Copy link
Member

@bors: r+ 388028e

Thanks!

@bors
Copy link
Collaborator

bors commented May 18, 2016

⌛ Testing commit 388028e with merge fb03dad...

bors added a commit that referenced this pull request May 18, 2016
The missing lib

Dearest Reviewer,

This pull request allows the value of project.links to be passed into
the build script via an environment variable. This closes #1220 . I have
added a test that makes sure that the environment variable is set. Also
note I am using CARGO_LIB not LIB because it appears to be used for
windows in appveyor. I think CARGO_LIB fits better.

I have also updated the documentation to reflect the new variable.

Thanks!
Becker
@bors
Copy link
Collaborator

bors commented May 19, 2016

@bors bors merged commit 388028e into rust-lang:master May 19, 2016
@sbeckeriv sbeckeriv deleted the the-missing-lib branch August 1, 2016 19:50
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.

Pass the value of project.links into the build script
5 participants