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

Add support for MSVC #70

Merged
merged 1 commit into from Jul 18, 2018
Merged

Add support for MSVC #70

merged 1 commit into from Jul 18, 2018

Conversation

ferjm
Copy link
Contributor

@ferjm ferjm commented Jul 18, 2018

Fixes #59

@ferjm ferjm mentioned this pull request Jul 18, 2018
6 tasks
@ferjm
Copy link
Contributor Author

ferjm commented Jul 18, 2018

I tested this building glib-sys and it seems to work properly.

src/lib.rs Outdated
@@ -410,6 +393,10 @@ impl Config {
if self.is_static(name) {
cmd.arg("--static");
}
let target = env::var("TARGET").unwrap_or_else(|_| String::new());
if target.contains("msvc") {
Copy link
Collaborator

@sdroege sdroege Jul 18, 2018

Choose a reason for hiding this comment

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

Why not

if let Some(target) = env::var("TARGET") {
    if target.contains("msvc") {
        cmd.arg("--msvc-syntax");
    }
}

or something with match and guards for the contains?
One level of indentation more, but doesn't have the empty string, which seems a bit ugly to me

@sdroege
Copy link
Collaborator

sdroege commented Jul 18, 2018

Looks good to me and should work, at least better than what we have currently. As @nirbheek said in #59, --msvc-syntax had some issues in the past in some corner cases but we can worry about that when it happens.

@alexcrichton alexcrichton merged commit 2a0011d into rust-lang:master Jul 18, 2018
@alexcrichton
Copy link
Member

Thanks!

@sdroege
Copy link
Collaborator

sdroege commented Jul 18, 2018

@alexcrichton Thanks! Can you prepare a new release with that change?

@alexcrichton
Copy link
Member

Sure thing, done!

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.

None yet

3 participants