From 7a38b6f1641e833d7bf1970bdb4071d1bfd30482 Mon Sep 17 00:00:00 2001 From: nzbr Date: Fri, 3 May 2024 05:21:13 +0200 Subject: [PATCH 1/6] fix(shell-wrapper): propagate login shell status --- utils/src/shell_wrapper.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index 7d54427e..a61c9429 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -47,8 +47,27 @@ fn real_main() -> anyhow::Result<()> { } } + let shell_exe = &shell + .file_name() + .ok_or(anyhow!("when trying to get the shell's filename"))? + .to_str() + .ok_or(anyhow!( + "when trying to convert the shell's filename to a string" + ))? + .to_string(); + + let arg0 = if env::args() + .next() + .map(|arg| arg.starts_with('-')) + .unwrap_or(false) + { + "-".to_string() + shell_exe.as_str() // prepend "-" to the shell's arg0 to make it a login shell + } else { + shell_exe.clone() + }; + Err(anyhow!(Command::new(&shell) - .arg0(shell) + .arg0(arg0) .args(env::args_os().skip(1)) .exec()) .context("when trying to exec the wrapped shell")) From 82db35670c61ae8a17a04c617004abb21a5144bb Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 8 May 2024 04:45:08 +0200 Subject: [PATCH 2/6] log errors to file --- utils/src/shell_wrapper.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index a61c9429..9d4a1d16 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -77,5 +77,12 @@ fn main() { let result = real_main(); env::set_var("RUST_BACKTRACE", "1"); - eprintln!("[shell-wrapper] Error: {:?}", result.unwrap_err()); + let err = result.unwrap_err(); + + eprintln!("[shell-wrapper] Error: {:?}", &err); + + // Write the result to /tmp/shell-wrapper_crash_.log + let pid = std::process::id(); + let output_file = format!("/tmp/shell-wrapper_crash_{}.log", pid); + std::fs::write(output_file, format!("{:?}", &err)).expect("Failed to write crash log to file"); } From 18a8a5684283db334f96348dc89b07a34a1484b9 Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 8 May 2024 04:46:01 +0200 Subject: [PATCH 3/6] print a warning if /etc/set-environment is missing --- utils/src/shell_wrapper.rs | 52 ++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index 9d4a1d16..7ecd6847 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -18,32 +18,36 @@ fn real_main() -> anyhow::Result<()> { // Skip if environment was already set if env::var_os("__NIXOS_SET_ENVIRONMENT_DONE") != Some("1".into()) { - // Load the environment from /etc/set-environment - let output = Command::new(env!("NIXOS_WSL_SH")) - .args(&[ - "-c", - &format!(". /etc/set-environment && {} -0", env!("NIXOS_WSL_ENV")), - ]) - .output() - .context("when reading /etc/set-environment")?; + if !std::path::Path::new("/etc/set-environment").exists() { + eprintln!("[shell-wrapper] Warning: /etc/set-environment does not exist"); + } else { + // Load the environment from /etc/set-environment + let output = Command::new(env!("NIXOS_WSL_SH")) + .args(&[ + "-c", + &format!(". /etc/set-environment && {} -0", env!("NIXOS_WSL_ENV")), + ]) + .output() + .context("when reading /etc/set-environment")?; - // Parse the output - let output_string = - String::from_utf8(output.stdout).context("when decoding the output of env")?; - let env = output_string - .split('\0') - .filter(|entry| !entry.is_empty()) - .map(|entry| { - entry - .split_once("=") - .ok_or(anyhow!("invalid env entry: {}", entry)) - }) - .collect::, _>>() - .context("when parsing the output of env")?; + // Parse the output + let output_string = + String::from_utf8(output.stdout).context("when decoding the output of env")?; + let env = output_string + .split('\0') + .filter(|entry| !entry.is_empty()) + .map(|entry| { + entry + .split_once("=") + .ok_or(anyhow!("invalid env entry: {}", entry)) + }) + .collect::, _>>() + .context("when parsing the output of env")?; - // Apply the environment variables - for &(key, val) in &env { - env::set_var(key, val); + // Apply the environment variables + for &(key, val) in &env { + env::set_var(key, val); + } } } From 473eda6b6dcd3a78b68300df6122b09f6b358c81 Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 8 May 2024 05:31:28 +0200 Subject: [PATCH 4/6] skip sourcing /etc/set-environment when SIGCHLD is ignored --- utils/src/shell_wrapper.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index 7ecd6847..a5485aae 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -1,4 +1,6 @@ use anyhow::{anyhow, Context}; +use nix::libc::{sigaction, PT_NULL, SIGCHLD, SIG_IGN}; +use std::mem::MaybeUninit; use std::os::unix::process::CommandExt; use std::process::Command; use std::{env, fs::read_link}; @@ -18,9 +20,23 @@ fn real_main() -> anyhow::Result<()> { // Skip if environment was already set if env::var_os("__NIXOS_SET_ENVIRONMENT_DONE") != Some("1".into()) { - if !std::path::Path::new("/etc/set-environment").exists() { - eprintln!("[shell-wrapper] Warning: /etc/set-environment does not exist"); - } else { + || -> anyhow::Result<()> { + if !std::path::Path::new("/etc/set-environment").exists() { + eprintln!("[shell-wrapper] Warning: /etc/set-environment does not exist"); + return Ok(()); + } + + unsafe { + // WSL starts a single shell under login to make sure that a logind session exists. + // That shell is started with SIGCHLD ignored + // If it is, we are probably that shell and can just skip setting the environment + let mut act: sigaction = MaybeUninit::zeroed().assume_init(); + sigaction(SIGCHLD, PT_NULL as *const sigaction, &mut act); + if act.sa_sigaction == SIG_IGN { + return Ok(()); + } + } + // Load the environment from /etc/set-environment let output = Command::new(env!("NIXOS_WSL_SH")) .args(&[ @@ -48,7 +64,9 @@ fn real_main() -> anyhow::Result<()> { for &(key, val) in &env { env::set_var(key, val); } - } + + Ok(()) + }()?; } let shell_exe = &shell From 41700b8a70ba681abc036b2d175835be6dc7af1d Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 8 May 2024 16:45:20 +0200 Subject: [PATCH 5/6] log to journal instead of file --- utils/Cargo.lock | 49 ++++++++++++++++++++++++++++++++++++++ utils/Cargo.toml | 1 + utils/src/shell_wrapper.rs | 22 ++++++++++++----- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/utils/Cargo.lock b/utils/Cargo.lock index 671d7e0a..5516a333 100644 --- a/utils/Cargo.lock +++ b/utils/Cargo.lock @@ -167,6 +167,16 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "errno" +version = "0.3.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245" +dependencies = [ + "libc", + "windows-sys 0.52.0", +] + [[package]] name = "gimli" version = "0.28.1" @@ -212,11 +222,20 @@ version = "0.2.153" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" +[[package]] +name = "linux-raw-sys" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c" + [[package]] name = "log" version = "0.4.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" +dependencies = [ + "value-bag", +] [[package]] name = "memchr" @@ -257,6 +276,7 @@ dependencies = [ "kernlog", "log", "nix", + "systemd-journal-logger", ] [[package]] @@ -298,6 +318,19 @@ version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" +[[package]] +name = "rustix" +version = "0.38.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.52.0", +] + [[package]] name = "strsim" version = "0.10.0" @@ -315,6 +348,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "systemd-journal-logger" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5f3848dd723f2a54ac1d96da793b32923b52de8dfcced8722516dac312a5b2a" +dependencies = [ + "log", + "rustix", +] + [[package]] name = "unicode-ident" version = "1.0.12" @@ -327,6 +370,12 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" +[[package]] +name = "value-bag" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a84c137d37ab0142f0f2ddfe332651fdbf252e7b7dbb4e67b6c1f1b2e925101" + [[package]] name = "windows-sys" version = "0.48.0" diff --git a/utils/Cargo.toml b/utils/Cargo.toml index 2efc8f48..78ef8589 100644 --- a/utils/Cargo.toml +++ b/utils/Cargo.toml @@ -10,6 +10,7 @@ anyhow = { version = "1.0.82", features = ["backtrace"] } nix = { version = "0.28.0", features = ["mount", "process"] } log = "0.4.21" kernlog = "0.3.1" +systemd-journal-logger = "2.1.1" # pinned to prevent running over Rust 1.69 clap = { version = "<4.4", features = ["derive"] } diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index a5485aae..9255bfd8 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -1,9 +1,11 @@ use anyhow::{anyhow, Context}; +use log::{error, info, warn, LevelFilter}; use nix::libc::{sigaction, PT_NULL, SIGCHLD, SIG_IGN}; use std::mem::MaybeUninit; use std::os::unix::process::CommandExt; use std::process::Command; use std::{env, fs::read_link}; +use systemd_journal_logger::JournalLog; fn real_main() -> anyhow::Result<()> { let exe = read_link("/proc/self/exe").context("when locating the wrapper binary")?; @@ -22,7 +24,7 @@ fn real_main() -> anyhow::Result<()> { if env::var_os("__NIXOS_SET_ENVIRONMENT_DONE") != Some("1".into()) { || -> anyhow::Result<()> { if !std::path::Path::new("/etc/set-environment").exists() { - eprintln!("[shell-wrapper] Warning: /etc/set-environment does not exist"); + warn!("/etc/set-environment does not exist"); return Ok(()); } @@ -33,6 +35,7 @@ fn real_main() -> anyhow::Result<()> { let mut act: sigaction = MaybeUninit::zeroed().assume_init(); sigaction(SIGCHLD, PT_NULL as *const sigaction, &mut act); if act.sa_sigaction == SIG_IGN { + info!("SIGCHLD is ignored, skipping setting environment"); return Ok(()); } } @@ -96,15 +99,22 @@ fn real_main() -> anyhow::Result<()> { } fn main() { + JournalLog::new() + .context("When initializing journal logger") + .unwrap() + .with_syslog_identifier("shell-wrapper".to_string()) + .install() + .unwrap(); + + log::set_max_level(LevelFilter::Info); + let result = real_main(); env::set_var("RUST_BACKTRACE", "1"); let err = result.unwrap_err(); - eprintln!("[shell-wrapper] Error: {:?}", &err); + eprintln!("{:?}", &err); - // Write the result to /tmp/shell-wrapper_crash_.log - let pid = std::process::id(); - let output_file = format!("/tmp/shell-wrapper_crash_{}.log", pid); - std::fs::write(output_file, format!("{:?}", &err)).expect("Failed to write crash log to file"); + // Log the error to the journal + error!("{:?}", &err); } From 721c68495f50a24d9a20d2ee666284ae82feaaf4 Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 8 May 2024 16:50:32 +0200 Subject: [PATCH 6/6] add comment about sigaction --- utils/src/shell_wrapper.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/src/shell_wrapper.rs b/utils/src/shell_wrapper.rs index 9255bfd8..7875535e 100644 --- a/utils/src/shell_wrapper.rs +++ b/utils/src/shell_wrapper.rs @@ -32,6 +32,7 @@ fn real_main() -> anyhow::Result<()> { // WSL starts a single shell under login to make sure that a logind session exists. // That shell is started with SIGCHLD ignored // If it is, we are probably that shell and can just skip setting the environment + // sigaction from libc is used here, because the wrapped version from the nix crate does not accept null let mut act: sigaction = MaybeUninit::zeroed().assume_init(); sigaction(SIGCHLD, PT_NULL as *const sigaction, &mut act); if act.sa_sigaction == SIG_IGN {