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

@@ -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;
     }
 

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<String> {
+    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"));

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.");

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