From 4482e9d86aeeacfe929eb19f0bb96a6339dabd62 Mon Sep 17 00:00:00 2001 From: Alexander Petric Date: Tue, 8 Jul 2025 02:52:44 -0400 Subject: [PATCH] fix: correct paths and no symlink for windows (go) (#6139) * fix: correct paths and no symlink for windows (go) * fix: improve Windows Go executor compatibility - Fix path handling inconsistency - use consistent double backslashes - Replace hardcoded Windows paths with dynamic system path detection - Add missing env_clear() call for mod init command - Create helper functions to reduce Windows environment setup duplication - Use SYSTEMDRIVE and TMP/TEMP environment variables for better compatibility Co-authored-by: Alexander Petric --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- backend/windmill-worker/src/go_executor.rs | 138 +++++++++++++++++++-- 1 file changed, 129 insertions(+), 9 deletions(-) diff --git a/backend/windmill-worker/src/go_executor.rs b/backend/windmill-worker/src/go_executor.rs index c5fe55184a..88b8863884 100644 --- a/backend/windmill-worker/src/go_executor.rs +++ b/backend/windmill-worker/src/go_executor.rs @@ -28,6 +28,51 @@ use crate::{ }; use windmill_common::client::AuthedClient; +#[cfg(windows)] +use crate::SYSTEM_ROOT; + +#[cfg(windows)] +fn get_windows_tmp_dir() -> String { + std::env::var("TMP") + .or_else(|_| std::env::var("TEMP")) + .unwrap_or_else(|_| { + let system_drive = std::env::var("SYSTEMDRIVE").unwrap_or_else(|_| "C:".to_string()); + format!("{}\\tmp", system_drive) + }) +} + +#[cfg(windows)] +fn get_windows_program_files() -> String { + std::env::var("ProgramFiles").unwrap_or_else(|_| { + let system_drive = std::env::var("SYSTEMDRIVE").unwrap_or_else(|_| "C:".to_string()); + format!("{}\\Program Files", system_drive) + }) +} + +#[cfg(windows)] +fn windows_gopath() -> String { + let tmp_dir = get_windows_tmp_dir(); + GO_CACHE_DIR.replace("/tmp", &tmp_dir).replace("/", r"\\") +} + +#[cfg(windows)] +fn set_windows_env_vars(cmd: &mut Command) { + cmd.env("SystemRoot", SYSTEM_ROOT.as_str()) + .env("TMP", get_windows_tmp_dir()) + .env("USERPROFILE", crate::USERPROFILE_ENV.as_str()) + .env( + "APPDATA", + std::env::var("APPDATA") + .unwrap_or_else(|_| format!("{}\\AppData\\Roaming", HOME_ENV.as_str())), + ) + .env("ProgramFiles", get_windows_program_files()) + .env( + "LOCALAPPDATA", + std::env::var("LOCALAPPDATA") + .unwrap_or_else(|_| format!("{}\\AppData\\Local", HOME_ENV.as_str())), + ); +} + const GO_REQ_SPLITTER: &str = "//go.sum\n"; const NSJAIL_CONFIG_RUN_GO_CONTENT: &str = include_str!("../nsjail/run.go.config.proto"); @@ -195,7 +240,16 @@ func Run(req Req) (interface{{}}, error){{ .env_clear() .env("PATH", PATH_ENV.as_str()) .env("BASE_INTERNAL_URL", base_internal_url) - .env("GOPATH", GO_CACHE_DIR) + .env("GOPATH", { + #[cfg(unix)] + { + GO_CACHE_DIR + } + #[cfg(windows)] + { + windows_gopath() + } + }) .env("HOME", HOME_ENV.as_str()) .envs(PROXY_ENVS.clone()) .args(vec!["build", "main.go"]) @@ -203,7 +257,7 @@ func Run(req Req) (interface{{}}, error){{ .stderr(Stdio::piped()); #[cfg(windows)] - build_go_cmd.env("USERPROFILE", crate::USERPROFILE_ENV.as_str()); + set_windows_env_vars(&mut build_go_cmd); let build_go_process = start_child_process(build_go_cmd, GO_PATH.as_str()).await?; handle_child( @@ -223,10 +277,28 @@ func Run(req Req) (interface{{}}, error){{ ) .await?; + #[cfg(unix)] + let executable_path = format!("{job_dir}/main"); + #[cfg(windows)] + let executable_path = format!("{job_dir}/main.exe"); + + // Set executable permissions on Windows + #[cfg(windows)] + { + use std::fs; + if let Ok(metadata) = fs::metadata(&executable_path) { + let mut permissions = metadata.permissions(); + permissions.set_readonly(false); + // On Windows, we need to ensure the file is not marked as read-only + // and has appropriate permissions for execution + let _ = fs::set_permissions(&executable_path, permissions); + } + } + match save_cache( &bin_path, &format!("{GO_OBJECT_STORE_PREFIX}{hash}"), - &format!("{job_dir}/main"), + &executable_path, false, ) .await @@ -239,11 +311,20 @@ func Run(req Req) (interface{{}}, error){{ Ok(logs) => logs, } } else { + #[cfg(unix)] let target = format!("{job_dir}/main"); + #[cfg(windows)] + let target = format!("{job_dir}/main.exe"); + #[cfg(unix)] let symlink = std::os::unix::fs::symlink(&bin_path, &target); #[cfg(windows)] - let symlink = std::os::windows::fs::symlink_dir(&bin_path, &target); + let symlink = { + // On Windows, copy the file instead of creating a symlink + // because symlinks might not work correctly for executables + use std::fs; + fs::copy(&bin_path, &target).map(|_| ()) + }; symlink.map_err(|e| { Error::ExecutionErr(format!( @@ -284,8 +365,16 @@ func Run(req Req) (interface{{}}, error){{ .stderr(Stdio::piped()); start_child_process(nsjail_cmd, NSJAIL_PATH.as_str()).await? } else { + #[cfg(unix)] let compiled_executable_name = "./main"; - let mut run_go = Command::new(compiled_executable_name); + #[cfg(windows)] + let compiled_executable_name = format!("{}/main.exe", job_dir); + + #[cfg(unix)] + let mut run_go = Command::new(&compiled_executable_name); + #[cfg(windows)] + let mut run_go = Command::new(&compiled_executable_name); + run_go .current_dir(job_dir) .env_clear() @@ -294,7 +383,16 @@ func Run(req Req) (interface{{}}, error){{ .env("PATH", PATH_ENV.as_str()) .env("TZ", TZ_ENV.as_str()) .env("BASE_INTERNAL_URL", base_internal_url) - .env("GOPATH", GO_CACHE_DIR) + .env("GOPATH", { + #[cfg(unix)] + { + GO_CACHE_DIR + } + #[cfg(windows)] + { + windows_gopath() + } + }) .env("HOME", HOME_ENV.as_str()); if let Some(ref goprivate) = *GOPRIVATE { @@ -305,10 +403,10 @@ func Run(req Req) (interface{{}}, error){{ } #[cfg(windows)] - run_go.env("USERPROFILE", crate::USERPROFILE_ENV.as_str()); + set_windows_env_vars(&mut run_go); run_go.stdout(Stdio::piped()).stderr(Stdio::piped()); - start_child_process(run_go, compiled_executable_name).await? + start_child_process(run_go, &compiled_executable_name).await? }; handle_child( &job.id, @@ -380,9 +478,18 @@ pub async fn install_go_dependencies( let mut child_cmd = Command::new(GO_PATH.as_str()); child_cmd .current_dir(job_dir) + .env_clear() .args(vec!["mod", "init", "mymod"]) .stdout(Stdio::piped()) .stderr(Stdio::piped()); + + #[cfg(windows)] + child_cmd.env("GOPATH", windows_gopath()); + #[cfg(unix)] + child_cmd.env("GOPATH", GO_CACHE_DIR); + + #[cfg(windows)] + set_windows_env_vars(&mut child_cmd); let child_process = start_child_process(child_cmd, GO_PATH.as_str()).await?; handle_child( @@ -458,10 +565,23 @@ pub async fn install_go_dependencies( let mut child_cmd = Command::new(GO_PATH.as_str()); child_cmd .current_dir(job_dir) - .env("GOPATH", GO_CACHE_DIR) + .env_clear() + .env("GOPATH", { + #[cfg(unix)] + { + GO_CACHE_DIR + } + #[cfg(windows)] + { + windows_gopath() + } + }) .args(vec!["mod", mod_command]) .stdout(Stdio::piped()) .stderr(Stdio::piped()); + + #[cfg(windows)] + set_windows_env_vars(&mut child_cmd); let child_process = start_child_process(child_cmd, GO_PATH.as_str()).await?; handle_child(