Skip to content

add nasm support for msvc #87

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

Merged
merged 7 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ jobs:
rust: stable-x86_64-msvc
os: windows-latest
crt_static: yes
- target: x86_64-pc-windows-msvc
rust: stable-x86_64-msvc
os: windows-latest
nasm_exe: installed

steps:
- uses: actions/checkout@v1
Expand All @@ -90,18 +94,26 @@ jobs:
- name: Use strawberry perl
if: startsWith(matrix.os, 'windows')
run: echo OPENSSL_SRC_PERL=C:/Strawberry/perl/bin/perl >> $GITHUB_ENV
- run: |
- name: Run tests (not Windows)
if: "!startsWith(matrix.os, 'windows')"
run: |
set -e
cargo generate-lockfile
./ci/run-docker.sh ${{ matrix.target }}
if: "!startsWith(matrix.os, 'windows')"
name: Run tests (not Windows)
- run: |
- name: Download nasm.exe (Windows)
if: matrix.nasm_exe == 'installed'
run: |
WINNASMVERSION='2.15.05'
curl -O https://www.nasm.us/pub/nasm/releasebuilds/${WINNASMVERSION}/win64/nasm-${WINNASMVERSION}-win64.zip
unzip nasm-${WINNASMVERSION}-win64.zip
echo "$GITHUB_WORKSPACE\\nasm-${WINNASMVERSION}" >> $GITHUB_PATH
echo "OPENSSL_RUST_USE_NASM=1" >> $GITHUB_ENV
- name: Run tests (Windows)
if: startsWith(matrix.os, 'windows')
run: |
cargo test --manifest-path testcrate/Cargo.toml --target ${{ matrix.target }} -vv
cargo test --manifest-path testcrate/Cargo.toml --target ${{ matrix.target }} --release -vv
cargo run --release --target ${{ matrix.target }} --manifest-path testcrate/Cargo.toml --features package -vv
if: startsWith(matrix.os, 'windows')
name: Run tests (Windows)

rustfmt:
name: Rustfmt
Expand Down
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@ This project is licensed under either of

at your option.

### Windows MSVC Assembly
Building OpenSSL for `windows-msvc` targets, users can choose whether to enable
assembly language routines, which requires [nasm](https://www.nasm.us/).
The build process will automatically detect whether `nasm.exe` is installed in
PATH. If found, the assembly language routines will be enabled (in other words,
the `no-asm` option will NOT be configured).
You can manipulate this behavior by setting the `OPENSSL_RUST_USE_NASM` environment
variable:
* `1`: Force enable the assembly language routines. (panic if `nasm.exe` is not
availible.)
* `0`: Force disable the assembly language routines even if the `nasm.exe` can be
found in PATH.
* not set: Let the build process automatically detect whether `nasm.exe` is
installed. If found, enable. If not, disable.
However, this environment variable does not take effects on non-windows platforms.

### Contribution

Unless you explicitly state otherwise, any contribution intentionally submitted
Expand Down
62 changes: 59 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,53 @@ impl Build {
}
}

#[cfg(windows)]
fn check_env_var(&self, var_name: &str) -> Option<bool> {
env::var_os(var_name).map(|s| {
if s == "1" {
// a message to stdout, let user know asm is force enabled
println!(
"{}: nasm.exe is force enabled by the \
'OPENSSL_RUST_USE_NASM' env var.",
env!("CARGO_PKG_NAME")
);
true
} else if s == "0" {
// a message to stdout, let user know asm is force disabled
println!(
"{}: nasm.exe is force disabled by the \
'OPENSSL_RUST_USE_NASM' env var.",
env!("CARGO_PKG_NAME")
);
false
} else {
panic!(
"The environment variable {} is set to an unacceptable value: {:?}",
var_name, s
);
}
})
}

#[cfg(windows)]
fn is_nasm_ready(&self) -> bool {
self.check_env_var("OPENSSL_RUST_USE_NASM")
.unwrap_or_else(|| {
// On Windows, use cmd `where` command to check if nasm is installed
let wherenasm = Command::new("cmd")
.args(&["/C", "where nasm"])
.output()
.expect("Failed to execute `cmd`.");
wherenasm.status.success()
})
}

#[cfg(not(windows))]
Copy link
Owner

Choose a reason for hiding this comment

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

I think the #[cfg] here can be avoided since nothing is windows-specific in that it only compiles on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think these bothers. I think this notices us that we don't consider running nasm.exe on a non-windows os. (Even though nasm has non-windows version. However, according to OpenSSL's docs, it doesn't need nasm to build on non-windows platforms.)

fn is_nasm_ready(&self) -> bool {
// We assume that nobody would run nasm.exe on a non-windows system.
false
}

pub fn build(&mut self) -> Artifacts {
let target = &self.target.as_ref().expect("TARGET dir not set")[..];
let host = &self.host.as_ref().expect("HOST dir not set")[..];
Expand Down Expand Up @@ -147,9 +194,18 @@ impl Build {
}

if target.contains("msvc") {
// On MSVC we need nasm.exe to compile the assembly files, but let's
// just pessimistically assume for now that's not available.
configure.arg("no-asm");
// On MSVC we need nasm.exe to compile the assembly files.
// ASM compiling will be enabled if nasm.exe is installed, unless
// the environment variable `OPENSSL_RUST_USE_NASM` is set.
if self.is_nasm_ready() {
// a message to stdout, let user know asm is enabled
println!(
"{}: Enable the assembly language routines in building OpenSSL.",
env!("CARGO_PKG_NAME")
);
} else {
configure.arg("no-asm");
}
}

let os = match target {
Expand Down