From 7a00434b94991cfed753e9f166f80eb9a10afd97 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 25 Mar 2022 09:12:18 -0400 Subject: [PATCH 1/5] do not use jemalloc on windows not supported, see https://github.com/gnzlbg/jemallocator#documentation and https://github.com/gnzlbg/jemallocator#platform-support --- Cargo.toml | 4 +++- src/lib.rs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2a40a39..431bb55 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,9 +8,11 @@ edition = "2018" [dependencies] getopts = "0.2" anyhow = "1.0" -jemallocator = "0.3.2" libc = "0.2" +[target.'cfg(not(target_env = "msvc"))'.dependencies] +jemallocator = "0.3.2" + [dev-dependencies] tempfile = "3.3.0" diff --git a/src/lib.rs b/src/lib.rs index 676ce34..561ab23 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,7 +13,9 @@ mod task; pub mod trace; pub mod work; +#[cfg(not(target_env = "msvc"))] use jemallocator::Jemalloc; +#[cfg(not(target_env = "msvc"))] #[global_allocator] static GLOBAL: Jemalloc = Jemalloc; From 69c846da9942ae803f37ac96d033704b9ab10881 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 25 Mar 2022 10:19:52 -0400 Subject: [PATCH 2/5] make build on windows --- Cargo.lock | 28 ++++++++++++++++++++++++++-- Cargo.toml | 4 ++++ src/graph.rs | 21 +++++++++++++++++++++ src/progress.rs | 20 ++++++++++++++++++++ src/signal.rs | 2 ++ src/task.rs | 9 +++++++-- src/work.rs | 5 ++++- 7 files changed, 84 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee5f7ae..1a5ba22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -80,6 +80,16 @@ dependencies = [ "libc", ] +[[package]] +name = "kernel32-sys" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d" +dependencies = [ + "winapi 0.2.8", + "winapi-build", +] + [[package]] name = "libc" version = "0.2.112" @@ -93,8 +103,10 @@ dependencies = [ "anyhow", "getopts", "jemallocator", + "kernel32-sys", "libc", "tempfile", + "winapi 0.3.9", ] [[package]] @@ -112,7 +124,7 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" dependencies = [ - "winapi", + "winapi 0.3.9", ] [[package]] @@ -126,7 +138,7 @@ dependencies = [ "libc", "redox_syscall", "remove_dir_all", - "winapi", + "winapi 0.3.9", ] [[package]] @@ -135,6 +147,12 @@ version = "0.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9337591893a19b88d8d87f2cec1e73fad5cdfd10e5a6f349f498ad6ea2ffb1e3" +[[package]] +name = "winapi" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a" + [[package]] name = "winapi" version = "0.3.9" @@ -145,6 +163,12 @@ dependencies = [ "winapi-x86_64-pc-windows-gnu", ] +[[package]] +name = "winapi-build" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc" + [[package]] name = "winapi-i686-pc-windows-gnu" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index 431bb55..0ffd39a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,10 @@ getopts = "0.2" anyhow = "1.0" libc = "0.2" +[target.'cfg(windows)'.dependencies] +kernel32-sys = "0.2.2" +winapi = { version = "0.3.6", features = [ "winuser" ] } + [target.'cfg(not(target_env = "msvc"))'.dependencies] jemallocator = "0.3.2" diff --git a/src/graph.rs b/src/graph.rs index 78c3ed2..0ddc3ad 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -4,8 +4,13 @@ use crate::canon::{canon_path, canon_path_in_place}; use crate::densemap::{self, DenseMap}; use std::collections::HashMap; use std::hash::Hasher; + +#[cfg(unix)] use std::os::unix::fs::MetadataExt; +#[cfg(windows)] +use std::os::windows::fs::MetadataExt; + /// Hash value used to identify a given instance of a Build's execution; /// compared to verify whether a Build is up to date. #[derive(Debug, Copy, Clone, Eq, PartialEq)] @@ -272,10 +277,26 @@ pub enum MTime { Stamp(u32), } +#[cfg(windows)] +fn mtime_from_filetime(mut filetime: u64) -> i64 { + // FILETIME is in 100-nanosecond increments since 1600. Convert to seconds since 2000. + filetime /= 1000000000 / 100; // 100ns -> s. + + // 1600 epoch -> 2000 epoch (subtract 400 years). + use std::convert::TryInto; + (filetime - 12622770400).try_into().unwrap() +} + /// stat() an on-disk path, producing its MTime. pub fn stat(path: &str) -> std::io::Result { + // TODO: Support timestamps with better-than-seconds resolution. + // TODO: On Windows, use FindFirstFileEx()/FindNextFile() to get timestamps per + // directory, for better stat perf. Ok(match std::fs::metadata(path) { + #[cfg(unix)] Ok(meta) => MTime::Stamp(meta.mtime() as u32), + #[cfg(windows)] + Ok(meta) => MTime::Stamp(mtime_from_filetime(meta.last_write_time()) as u32), Err(err) => { if err.kind() == std::io::ErrorKind::NotFound { MTime::Missing diff --git a/src/progress.rs b/src/progress.rs index 867cfa2..f8660c6 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -10,6 +10,7 @@ use crate::graph::BuildId; use crate::work::BuildState; use crate::work::StateCounts; +#[cfg(unix)] #[allow(clippy::uninit_assumed_init)] pub fn get_terminal_cols() -> Option { unsafe { @@ -21,6 +22,25 @@ pub fn get_terminal_cols() -> Option { } } +#[cfg(windows)] +#[allow(clippy::uninit_assumed_init)] +pub fn get_terminal_cols() -> Option { + extern crate winapi; + extern crate kernel32; + use kernel32::{GetConsoleScreenBufferInfo, GetStdHandle}; + let console = unsafe { GetStdHandle(winapi::um::winbase::STD_OUTPUT_HANDLE) }; + if console == winapi::um::handleapi::INVALID_HANDLE_VALUE { + return None; + } + unsafe { + let mut csbi = ::std::mem::MaybeUninit::uninit().assume_init(); + if GetConsoleScreenBufferInfo(console, &mut csbi) == 0 { + return None; + } + Some(csbi.dwSize.X as usize) + } +} + /// Compute the message to display on the console for a given build. pub fn build_message(build: &Build) -> &str { build diff --git a/src/signal.rs b/src/signal.rs index 3f96745..c912933 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -4,10 +4,12 @@ //! and let the parent properly print that progress. This also lets us still //! write out pending debug traces, too. +#[cfg(unix)] extern "C" fn sigint_handler(_sig: libc::c_int) { // Do nothing; SA_RESETHAND should clear the handler. } +#[cfg(unix)] pub fn register_sigint() { // Safety: registering a signal handler is libc unsafe code. unsafe { diff --git a/src/task.rs b/src/task.rs index 80f5a99..aa0cb70 100644 --- a/src/task.rs +++ b/src/task.rs @@ -9,11 +9,15 @@ use crate::depfile; use crate::graph::BuildId; use crate::scanner::Scanner; use anyhow::{anyhow, bail}; -use std::io::Write; -use std::os::unix::process::ExitStatusExt; use std::sync::mpsc; use std::time::{Duration, Instant}; +#[cfg(unix)] +use std::io::Write; + +#[cfg(unix)] +use std::os::unix::process::ExitStatusExt; + pub struct FinishedTask { /// A (faked) "thread id", used to put different finished builds in different /// tracks in a performance trace. @@ -69,6 +73,7 @@ fn run_task(cmdline: &str, depfile: Option<&str>) -> anyhow::Result }; } else { // Command failed. + #[cfg(unix)] if let Some(sig) = cmd.status.signal() { match sig { libc::SIGINT => write!(output, "interrupted").unwrap(), diff --git a/src/work.rs b/src/work.rs index 0293dc6..092e7d9 100644 --- a/src/work.rs +++ b/src/work.rs @@ -5,13 +5,15 @@ use crate::densemap::DenseMap; use crate::graph::*; use crate::progress; use crate::progress::Progress; -use crate::signal; use crate::task; use crate::trace; use std::collections::HashSet; use std::collections::VecDeque; use std::time::Duration; +#[cfg(unix)] +use crate::signal; + /// Build steps go through this sequence of states. #[derive(Clone, Copy, Debug, PartialEq)] pub enum BuildState { @@ -631,6 +633,7 @@ impl<'a> Work<'a> { // Returns a Result for failures, but we must clean up the progress before // returning the result to the caller. fn run_without_cleanup(&mut self) -> anyhow::Result> { + #[cfg(unix)] signal::register_sigint(); let mut tasks_done = 0; while self.build_states.unfinished() { From d2f5440eb49811a76e32639c1b2466e91c601e68 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 25 Mar 2022 13:41:36 -0400 Subject: [PATCH 3/5] win: CreateProcessA first steps Starts subprocesses, but drops their output for now. Basic builds work! --- Cargo.toml | 2 +- src/task.rs | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0ffd39a..3f4af23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ libc = "0.2" [target.'cfg(windows)'.dependencies] kernel32-sys = "0.2.2" -winapi = { version = "0.3.6", features = [ "winuser" ] } +winapi = { version = "0.3.6", features = [ "synchapi" ] } [target.'cfg(not(target_env = "msvc"))'.dependencies] jemallocator = "0.3.2" diff --git a/src/task.rs b/src/task.rs index aa0cb70..ff37eb3 100644 --- a/src/task.rs +++ b/src/task.rs @@ -18,6 +18,9 @@ use std::io::Write; #[cfg(unix)] use std::os::unix::process::ExitStatusExt; +#[cfg(windows)] +extern crate winapi; + pub struct FinishedTask { /// A (faked) "thread id", used to put different finished builds in different /// tracks in a performance trace. @@ -55,6 +58,7 @@ fn read_depfile(path: &str) -> anyhow::Result> { /// Executes a build task as a subprocess. /// Returns an Err() if we failed outside of the process itself. +#[cfg(unix)] fn run_task(cmdline: &str, depfile: Option<&str>) -> anyhow::Result { let mut cmd = std::process::Command::new("/bin/sh") .arg("-c") @@ -73,7 +77,6 @@ fn run_task(cmdline: &str, depfile: Option<&str>) -> anyhow::Result }; } else { // Command failed. - #[cfg(unix)] if let Some(sig) = cmd.status.signal() { match sig { libc::SIGINT => write!(output, "interrupted").unwrap(), @@ -89,6 +92,120 @@ fn run_task(cmdline: &str, depfile: Option<&str>) -> anyhow::Result }) } +#[cfg(windows)] +fn zeroed_startupinfo() -> winapi::um::processthreadsapi::STARTUPINFOA { + winapi::um::processthreadsapi::STARTUPINFOA { + cb: 0, + lpReserved: std::ptr::null_mut(), + lpDesktop: std::ptr::null_mut(), + lpTitle: std::ptr::null_mut(), + dwX: 0, + dwY: 0, + dwXSize: 0, + dwYSize: 0, + dwXCountChars: 0, + dwYCountChars: 0, + dwFillAttribute: 0, + dwFlags: 0, + wShowWindow: 0, + cbReserved2: 0, + lpReserved2: std::ptr::null_mut(), + hStdInput: winapi::um::handleapi::INVALID_HANDLE_VALUE, + hStdOutput: winapi::um::handleapi::INVALID_HANDLE_VALUE, + hStdError: winapi::um::handleapi::INVALID_HANDLE_VALUE, + } +} + +#[cfg(windows)] +fn zeroed_process_information() -> winapi::um::processthreadsapi::PROCESS_INFORMATION { + winapi::um::processthreadsapi::PROCESS_INFORMATION { + hProcess: std::ptr::null_mut(), + hThread: std::ptr::null_mut(), + dwProcessId: 0, + dwThreadId: 0, + } +} + +#[cfg(windows)] +fn run_task(cmdline: &str, depfile: Option<&str>) -> anyhow::Result { + // Don't want to run `cmd /c` since that limits cmd line length to 8192 bytes. + // std::process::Command can't take a string and pass it through to CreateProcess unchanged, + // so call that ourselves. + + // TODO: Set this to just 0 for console pool jobs. + let process_flags = winapi::um::winbase::CREATE_NEW_PROCESS_GROUP; + + let mut startup_info = zeroed_startupinfo(); + startup_info.cb = std::mem::size_of::() as u32; + startup_info.dwFlags = winapi::um::winbase::STARTF_USESTDHANDLES; + + let mut process_info = zeroed_process_information(); + + let mut mut_cmdline = cmdline.to_string() + "\0"; + + let create_process_success = unsafe { + winapi::um::processthreadsapi::CreateProcessA( + std::ptr::null_mut(), + mut_cmdline.as_mut_ptr() as *mut i8, + std::ptr::null_mut(), + std::ptr::null_mut(), + /*inherit handles = */ winapi::shared::ntdef::TRUE.into(), + process_flags, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut startup_info, + &mut process_info, + ) + }; + if create_process_success == 0 { + // TODO: better error? + let error = unsafe { winapi::um::errhandlingapi::GetLastError() }; + bail!("CreateProcessA failed: {}", error); + } + + unsafe { + winapi::um::handleapi::CloseHandle(process_info.hThread); + } + + unsafe { + winapi::um::synchapi::WaitForSingleObject( + process_info.hProcess, + winapi::um::winbase::INFINITE, + ); + } + + let mut exit_code: u32 = 0; + unsafe { + winapi::um::processthreadsapi::GetExitCodeProcess(process_info.hProcess, &mut exit_code); + } + + unsafe { + winapi::um::handleapi::CloseHandle(process_info.hProcess); + } + + let mut output = Vec::new(); + // TODO: Set up pipes so that we can print the process's output. + //output.append(&mut cmd.stdout); + //output.append(&mut cmd.stderr); + let success = exit_code == 0; + + let mut discovered_deps: Option> = None; + if success { + discovered_deps = match depfile { + None => None, + Some(deps) => Some(read_depfile(deps)?), + }; + } else { + // Command failed. + } + + Ok(TaskResult { + success, + output, + discovered_deps, + }) +} + /// Tracks faked "thread ids" -- integers assigned to build tasks to track /// paralllelism in perf trace output. struct ThreadIds { From d8f80cc7aa3c1f7ab2e96f01fe7bf06b7b787213 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 25 Mar 2022 14:16:34 -0400 Subject: [PATCH 4/5] win: Make basic_build and create_subdir pass --- tests/basic.rs | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/tests/basic.rs b/tests/basic.rs index e3605b3..6b11e4e 100644 --- a/tests/basic.rs +++ b/tests/basic.rs @@ -83,16 +83,30 @@ fn empty_file() -> anyhow::Result<()> { Ok(()) } + +#[cfg(unix)] +fn touch_command() -> String { + " +rule touch + command = touch $out +".to_owned() +} + +#[cfg(windows)] +fn touch_command() -> String { + " +rule touch + command = cmd /c type nul > $out +".to_owned() +} + #[test] fn basic_build() -> anyhow::Result<()> { let space = TestSpace::new()?; space.write( "build.ninja", - " -rule touch - command = touch $out -build out: touch in -", + &(touch_command() + "build out: touch in +"), )?; space.write("in", "")?; space.run_expect(&mut n2_command(vec!["out"]))?; @@ -100,18 +114,14 @@ build out: touch in Ok(()) } - #[test] fn create_subdir() -> anyhow::Result<()> { // Run a build rule that needs a subdir to be automatically created. let space = TestSpace::new()?; space.write( "build.ninja", - " -rule touch - command = touch $out -build subdir/out: touch in -", + &(touch_command() + "build subdir/out: touch in +"), )?; space.write("in", "")?; space.run_expect(&mut n2_command(vec!["subdir/out"]))?; From ee18aee825005b964165d2420133359a97632998 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 25 Mar 2022 14:23:42 -0400 Subject: [PATCH 5/5] win: disable generate_build_file() test This test relies heavily on `sh`, which doesn't exist on Windows. --- tests/basic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/basic.rs b/tests/basic.rs index 6b11e4e..5d03414 100644 --- a/tests/basic.rs +++ b/tests/basic.rs @@ -130,6 +130,7 @@ fn create_subdir() -> anyhow::Result<()> { Ok(()) } +#[cfg(unix)] #[test] fn generate_build_file() -> anyhow::Result<()> { // Run a project where a build rule generates the build.ninja.