From ce2b0cb9505486291a83e4e238e053b3fca4d025 Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Tue, 25 Apr 2023 14:00:53 +0200 Subject: [PATCH] Improve Env::get, add Env::get_string_lossy --- CHANGELOG.md | 8 ++++++++ libcnb/src/env.rs | 20 +++++++++++++++++--- libcnb/src/layer/handling.rs | 4 ++-- libcnb/src/layer_env.rs | 24 ++++++++++++------------ libcnb/src/platform.rs | 4 ++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 808753d5..d60b8784 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ separate changelogs for each crate were used. If you need to refer to these old ## [Unreleased] +### Changed + +- `Env::get` now returns `Option<&OsString>` instead of `Option`. This is more in line with expectations users have when dealing with a collection type. This is a breaking change, compile errors can be fixed by adding a [`Option::cloned`](https://doc.rust-lang.org/std/option/enum.Option.html#method.cloned-1) call after `Env::get` to get the old behaviour. In some cases, cloning might not be necessary, slightly improving the code that uses `Env::get`. ([#565](https://github.com/heroku/libcnb.rs/pull/565)) + +### Added + +- `Env::get_string_lossy` as a convenience method to work with environment variables directly. Getting a value out of an `Env` and treating its contents as unicode is a common case. Using this new method can simplify buildpack code. ([#565](https://github.com/heroku/libcnb.rs/pull/565)) + ## [0.11.5] 2023-02-07 ### Changed diff --git a/libcnb/src/env.rs b/libcnb/src/env.rs index 66d6c5ee..52ed77f0 100644 --- a/libcnb/src/env.rs +++ b/libcnb/src/env.rs @@ -56,10 +56,24 @@ impl Env { self } - /// Returns a cloned value corresponding to the given key. + /// Returns the value corresponding to the given key. #[must_use] - pub fn get(&self, key: impl AsRef) -> Option { - self.inner.get(key.as_ref()).cloned() + pub fn get(&self, key: impl AsRef) -> Option<&OsString> { + self.inner.get(key.as_ref()) + } + + /// Returns the value corresponding to the given key, interpreted as Unicode data. + /// + /// Any non-Unicode sequences are replaced with + /// [`U+FFFD REPLACEMENT CHARACTER`][U+FFFD]. + /// + /// [U+FFFD]: std::char::REPLACEMENT_CHARACTER + /// + /// See [`OsStr::to_string_lossy`] for more details. + #[must_use] + pub fn get_string_lossy(&self, key: impl AsRef) -> Option { + self.get(key) + .map(|os_string| os_string.to_string_lossy().to_string()) } /// Returns true if the environment contains a value for the specified key. diff --git a/libcnb/src/layer/handling.rs b/libcnb/src/layer/handling.rs index e4cf2d79..127536a0 100644 --- a/libcnb/src/layer/handling.rs +++ b/libcnb/src/layer/handling.rs @@ -857,13 +857,13 @@ mod tests { let applied_layer_env = layer_data.env.apply_to_empty(Scope::Build); assert_eq!( - applied_layer_env.get("PATH"), + applied_layer_env.get("PATH").cloned(), Some(layer_dir.join("bin").into()) ); assert_eq!( applied_layer_env.get("CUSTOM_ENV"), - Some(OsString::from("CUSTOM_ENV_VALUE")) + Some(&OsString::from("CUSTOM_ENV_VALUE")) ); } diff --git a/libcnb/src/layer_env.rs b/libcnb/src/layer_env.rs index 6ab969a3..db4fca7c 100644 --- a/libcnb/src/layer_env.rs +++ b/libcnb/src/layer_env.rs @@ -75,7 +75,7 @@ use std::path::Path; /// let layer_env = LayerEnv::read_from_layer_dir(&layer_dir).unwrap(); /// let env = layer_env.apply_to_empty(Scope::Launch); /// -/// assert_eq!(env.get("PATH").unwrap(), layer_dir.join("bin")); +/// assert_eq!(env.get("PATH").cloned().unwrap(), layer_dir.join("bin")); /// assert_eq!(env.get("CPATH"), None); // None, because CPATH is only added during build /// ``` #[derive(Eq, PartialEq, Debug, Default, Clone)] @@ -276,8 +276,8 @@ impl LayerEnv { /// let layer_env = LayerEnv::read_from_layer_dir(&layer_dir).unwrap(); /// let env = layer_env.apply_to_empty(Scope::Launch); /// - /// assert_eq!(env.get("PATH").unwrap(), layer_dir.join("bin")); - /// assert_eq!(env.get("ZERO_WING").unwrap(), "ALL_YOUR_BASE_ARE_BELONG_TO_US"); + /// assert_eq!(env.get("PATH").cloned().unwrap(), layer_dir.join("bin")); + /// assert_eq!(env.get("ZERO_WING").cloned().unwrap(), "ALL_YOUR_BASE_ARE_BELONG_TO_US"); /// ``` pub fn read_from_layer_dir(layer_dir: impl AsRef) -> Result { let mut result_layer_env = Self::new(); @@ -438,7 +438,7 @@ impl LayerEnvDelta { } } ModificationBehavior::Append => { - let mut previous_value = result_env.get(name).unwrap_or_default(); + let mut previous_value = result_env.get(name).cloned().unwrap_or_default(); if previous_value.len() > 0 { previous_value.push(self.delimiter_for(name)); @@ -449,7 +449,7 @@ impl LayerEnvDelta { result_env.insert(name, previous_value); } ModificationBehavior::Prepend => { - let previous_value = result_env.get(name).unwrap_or_default(); + let previous_value = result_env.get(name).cloned().unwrap_or_default(); let mut new_value = OsString::new(); new_value.push(value); @@ -876,8 +876,8 @@ mod tests { let layer_env = LayerEnv::read_from_layer_dir(layer_dir).unwrap(); let env = layer_env.apply_to_empty(Scope::Launch); - assert_eq!(env.get("PATH").unwrap(), layer_dir.join("bin")); - assert_eq!(env.get("LD_LIBRARY_PATH").unwrap(), layer_dir.join("lib")); + assert_eq!(env.get("PATH").unwrap(), &layer_dir.join("bin")); + assert_eq!(env.get("LD_LIBRARY_PATH").unwrap(), &layer_dir.join("lib")); assert_eq!(env.get("LIBRARY_PATH"), None); assert_eq!(env.get("CPATH"), None); assert_eq!(env.get("PKG_CONFIG_PATH"), None); @@ -897,13 +897,13 @@ mod tests { let layer_env = LayerEnv::read_from_layer_dir(layer_dir).unwrap(); let env = layer_env.apply_to_empty(Scope::Build); - assert_eq!(env.get("PATH").unwrap(), layer_dir.join("bin")); - assert_eq!(env.get("LD_LIBRARY_PATH").unwrap(), layer_dir.join("lib")); - assert_eq!(env.get("LIBRARY_PATH").unwrap(), layer_dir.join("lib")); - assert_eq!(env.get("CPATH").unwrap(), layer_dir.join("include")); + assert_eq!(env.get("PATH").unwrap(), &layer_dir.join("bin")); + assert_eq!(env.get("LD_LIBRARY_PATH").unwrap(), &layer_dir.join("lib")); + assert_eq!(env.get("LIBRARY_PATH").unwrap(), &layer_dir.join("lib")); + assert_eq!(env.get("CPATH").unwrap(), &layer_dir.join("include")); assert_eq!( env.get("PKG_CONFIG_PATH").unwrap(), - layer_dir.join("pkgconfig") + &layer_dir.join("pkgconfig") ); } diff --git a/libcnb/src/platform.rs b/libcnb/src/platform.rs index 19760e4d..9efaeb55 100644 --- a/libcnb/src/platform.rs +++ b/libcnb/src/platform.rs @@ -77,8 +77,8 @@ mod tests { fs::write(env_dir.join("HELLO"), "World!").unwrap(); let env = read_platform_env(tmpdir.path()).unwrap(); - assert_eq!(env.get("FOO"), Some(OsString::from("BAR"))); - assert_eq!(env.get("HELLO"), Some(OsString::from("World!"))); + assert_eq!(env.get("FOO"), Some(&OsString::from("BAR"))); + assert_eq!(env.get("HELLO"), Some(&OsString::from("World!"))); } #[test]