From 5e28f068ec98c4901aef25829c18756418a6a953 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 7 Oct 2025 13:06:16 +0200 Subject: [PATCH] build.rs: dedicated error for bad encoding in environment variables We should probably not silently treat invalid Unicode the same as "the variable is unset", even though it probably makes no difference in practice. --- build.rs | 18 +++++++++--------- crates/build-helper/src/lib.rs | 19 ++++++++++++++++++- crates/build-man-pages/build.rs | 11 ++++------- crates/gettext-maps/build.rs | 5 +++-- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/build.rs b/build.rs index 257487848..ea757bfd4 100644 --- a/build.rs +++ b/build.rs @@ -1,6 +1,6 @@ #![allow(clippy::uninlined_format_args)] -use fish_build_helper::{fish_build_dir, workspace_root}; +use fish_build_helper::{env_var, fish_build_dir, workspace_root}; use rsconf::Target; use std::env; use std::path::{Path, PathBuf}; @@ -30,9 +30,9 @@ fn main() { ); // Some build info - rsconf::set_env_value("BUILD_TARGET_TRIPLE", &env::var("TARGET").unwrap()); - rsconf::set_env_value("BUILD_HOST_TRIPLE", &env::var("HOST").unwrap()); - rsconf::set_env_value("BUILD_PROFILE", &env::var("PROFILE").unwrap()); + rsconf::set_env_value("BUILD_TARGET_TRIPLE", &env_var("TARGET").unwrap()); + rsconf::set_env_value("BUILD_HOST_TRIPLE", &env_var("HOST").unwrap()); + rsconf::set_env_value("BUILD_PROFILE", &env_var("PROFILE").unwrap()); let version = &get_version(&env::current_dir().unwrap()); // Per https://doc.rust-lang.org/cargo/reference/build-scripts.html#inputs-to-the-build-script, @@ -114,7 +114,7 @@ fn detect_apple(_: &Target) -> bool { fn detect_cygwin(_: &Target) -> bool { // Cygwin target is usually cross-compiled. - std::env::var("CARGO_CFG_TARGET_OS").unwrap() == "cygwin" + env_var("CARGO_CFG_TARGET_OS").unwrap() == "cygwin" } /// Detect if we're being compiled for a BSD-derived OS, allowing targeting code conditionally with @@ -126,7 +126,7 @@ fn detect_cygwin(_: &Target) -> bool { fn detect_bsd(_: &Target) -> bool { // Instead of using `uname`, we can inspect the TARGET env variable set by Cargo. This lets us // support cross-compilation scenarios. - let mut target = std::env::var("TARGET").unwrap(); + let mut target = env_var("TARGET").unwrap(); if !target.chars().all(|c| c.is_ascii_lowercase()) { target = target.to_ascii_lowercase(); } @@ -178,14 +178,14 @@ fn setup_paths() { use unix_path::{Path, PathBuf}; fn get_path(name: &str, default: &str, onvar: &Path) -> PathBuf { - let mut var = PathBuf::from(env::var(name).unwrap_or(default.to_string())); + let mut var = PathBuf::from(env_var(name).unwrap_or(default.to_string())); if var.is_relative() { var = onvar.join(var); } var } - let prefix = PathBuf::from(env::var("PREFIX").unwrap_or("/usr/local".to_string())); + let prefix = PathBuf::from(env_var("PREFIX").unwrap_or("/usr/local".to_string())); rsconf::rebuild_if_env_changed("PREFIX"); rsconf::set_env_value("PREFIX", prefix.to_str().unwrap()); @@ -229,7 +229,7 @@ fn get_version(src_dir: &Path) -> String { use std::fs::read_to_string; use std::process::Command; - if let Ok(var) = std::env::var("FISH_BUILD_VERSION") { + if let Some(var) = env_var("FISH_BUILD_VERSION") { return var; } diff --git a/crates/build-helper/src/lib.rs b/crates/build-helper/src/lib.rs index 0831823c4..df9baf1b1 100644 --- a/crates/build-helper/src/lib.rs +++ b/crates/build-helper/src/lib.rs @@ -1,4 +1,21 @@ -use std::{borrow::Cow, env, path::Path}; +use std::{borrow::Cow, env, os::unix::ffi::OsStrExt, path::Path}; + +pub fn env_var(name: &str) -> Option { + let err = match env::var(name) { + Ok(p) => return Some(p), + Err(err) => err, + }; + use env::VarError::*; + match err { + NotPresent => None, + NotUnicode(os_string) => { + panic!( + "Environment variable {name} is not valid Unicode: {:?}", + os_string.as_bytes() + ) + } + } +} pub fn workspace_root() -> &'static Path { let manifest_dir = Path::new(env!("CARGO_MANIFEST_DIR")); diff --git a/crates/build-man-pages/build.rs b/crates/build-man-pages/build.rs index 0492ea68c..1cb097d04 100644 --- a/crates/build-man-pages/build.rs +++ b/crates/build-man-pages/build.rs @@ -14,12 +14,9 @@ fn main() { #[cfg(not(clippy))] fn build_man(man_dir: &Path) { - use std::{ - env, - process::{Command, Stdio}, - }; + use std::process::{Command, Stdio}; - use fish_build_helper::workspace_root; + use fish_build_helper::{env_var, workspace_root}; let workspace_root = workspace_root(); @@ -47,7 +44,7 @@ fn build_man(man_dir: &Path) { let _ = std::fs::create_dir_all(sec1_str); rsconf::rebuild_if_env_changed("FISH_BUILD_DOCS"); - if env::var("FISH_BUILD_DOCS") == Ok("0".to_string()) { + if env_var("FISH_BUILD_DOCS") == Some("0".to_string()) { rsconf::warn!("Skipping man pages because $FISH_BUILD_DOCS is set to 0"); return; } @@ -64,7 +61,7 @@ fn build_man(man_dir: &Path) { .spawn() { Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - if env::var("FISH_BUILD_DOCS") == Ok("1".to_string()) { + if env_var("FISH_BUILD_DOCS") == Some("1".to_string()) { panic!("Could not find sphinx-build to build man pages.\nInstall sphinx or disable building the docs by setting $FISH_BUILD_DOCS=0."); } rsconf::warn!("Cannot find sphinx-build to build man pages."); diff --git a/crates/gettext-maps/build.rs b/crates/gettext-maps/build.rs index 078cce37d..4e08cf6c9 100644 --- a/crates/gettext-maps/build.rs +++ b/crates/gettext-maps/build.rs @@ -1,10 +1,11 @@ use std::{ - env, ffi::OsStr, path::{Path, PathBuf}, process::{Command, Stdio}, }; +use fish_build_helper::env_var; + fn main() { let cache_dir = PathBuf::from(fish_build_helper::fish_build_dir()).join("fish-localization-map-cache"); @@ -27,7 +28,7 @@ fn embed_localizations(cache_dir: &Path) { std::fs::create_dir_all(cache_dir).unwrap(); let localization_map_path = - Path::new(&env::var("OUT_DIR").unwrap()).join("localization_maps.rs"); + Path::new(&env_var("OUT_DIR").unwrap()).join("localization_maps.rs"); let mut localization_map_file = BufWriter::new(File::create(&localization_map_path).unwrap()); // This will become a map which maps from language identifiers to maps containing localizations