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

Don't require the static runtime for openblas on Windows #69

Merged
merged 1 commit into from
May 1, 2021
Merged

Don't require the static runtime for openblas on Windows #69

merged 1 commit into from
May 1, 2021

Conversation

ethanhs
Copy link
Contributor

@ethanhs ethanhs commented Apr 14, 2021

As part of shipping openblas-src on Windows as part of a Python package, I would like to link dynamically to the CRT (this greatly reduces binary size). Currently I cannot do so because openblas-src forces linking to a static CRT.

I have made this patch as a workaround. I could also add a feature flag that only on Windows sets the static CRT if you would prefer, otherwise it can be left up to the parent crate what to do in a build.rs.

@termoshtt termoshtt self-requested a review April 24, 2021 05:59
@termoshtt
Copy link
Member

termoshtt commented Apr 24, 2021

I would like to link dynamically to the CRT (this greatly reduces binary size)

It is reasonable. As vcpkg-rs's document says, we have three options and should add one more option for windows-x64-static-md case.

I prefer to add new flag crt-static, which will be ignored when static=false:

static=true static=false (default)
crt-static=true x64-windows-static x64-windows
crt-static=false (default) x64-windows-static-md x64-windows

@termoshtt
Copy link
Member

I prefer to add new flag crt-static, which will be ignored when static=false:

https://doc.rust-lang.org/reference/linkage.html#static-and-dynamic-c-runtimes

RUSTFLAGS='-C target-feature=+crt-static' cargo build --target x86_64-pc-windows-msvc

Looking the reference, there is an example to specify +crt-static in RUSTFLAG for just this case. This must be preferable instead of introducing crt-static flag, which requires me to add new flag to upstream crate e.g. ndarray-linalg.

termoshtt added a commit that referenced this pull request Apr 24, 2021
@termoshtt termoshtt merged commit 543c56d into blas-lapack-rs:master May 1, 2021
@termoshtt
Copy link
Member

Merged through #71 with CI and README update

termoshtt added a commit that referenced this pull request May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants