Skip to content

Commit 719422d

Browse files
committed
Change default PATH delimiter
The default PATH delimiter is `` (empty string) rather than the OS delimiter. This has caused many issues and heartache buildpacks/spec#285. The upstream spec does not state the delimiter that should be used when none is provided. This change suggests that we should default to `:` for unix and `;` for windows. This can be seen as a dry-run for the spec proposal change buildpacks/rfcs#326. If this change can be made with little to no reported problems, we can send that information upstream. Other suggestions have been floated #534. I suggest that we prioritize the most common case, which for Heroku buildpacks is using the OS path separator.
1 parent b8d6835 commit 719422d

2 files changed

Lines changed: 15 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
## [Unreleased]
1111

12+
### Changed
13+
14+
- `libcnb`
15+
- The default delimiter for PATH based `LayerEnv` operations is now colon (`:`) for unix and semi-colon (`;`) for windows rather than an empty string. Developers relying on this behavior should use an explicit empty delimiter.
16+
This value is subject to change if the [upstream spec changes the default value](https://github.com/buildpacks/spec/issues/285). ([#](https://github.com/heroku/libcnb.rs/pull/))
1217

1318
## [0.29.0] - 2025-05-02
1419

libcnb/src/layer_env.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use std::path::Path;
3434
/// static set of environment variables, see [`Env`].
3535
///
3636
/// To apply a `LayerEnv` delta to a given `Env`, use [`LayerEnv::apply`] like so:
37+
///
3738
/// ```
3839
/// use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope};
3940
/// use libcnb::Env;
@@ -47,7 +48,7 @@ use std::path::Path;
4748
/// env.insert("VAR2", "previous-value");
4849
///
4950
/// let modified_env = layer_env.apply(Scope::Build, &env);
50-
/// assert_eq!(modified_env.get("VAR").unwrap(), "foobar");
51+
/// assert_eq!(modified_env.get("VAR").unwrap(), "foo:bar");
5152
/// assert_eq!(modified_env.get("VAR2").unwrap(), "previous-value");
5253
/// ```
5354
///
@@ -132,7 +133,7 @@ impl LayerEnv {
132133
/// env.insert("VAR2", "previous-value");
133134
///
134135
/// let modified_env = layer_env.apply(Scope::Build, &env);
135-
/// assert_eq!(modified_env.get("VAR").unwrap(), "foobar");
136+
/// assert_eq!(modified_env.get("VAR").unwrap(), "foo:bar");
136137
/// assert_eq!(modified_env.get("VAR2").unwrap(), "previous-value");
137138
/// ```
138139
#[must_use]
@@ -482,7 +483,7 @@ impl LayerEnvDelta {
482483
self.entries
483484
.get(&(ModificationBehavior::Delimiter, key.into()))
484485
.cloned()
485-
.unwrap_or_default()
486+
.unwrap_or(OsString::from(PATH_LIST_SEPARATOR))
486487
}
487488

488489
fn read_from_env_dir(path: impl AsRef<Path>) -> Result<Self, std::io::Error> {
@@ -614,8 +615,11 @@ mod tests {
614615

615616
use super::LayerEnvDelta;
616617

617-
/// Direct port of a test from the reference lifecycle implementation:
618+
/// Port of a test from the reference lifecycle implementation:
618619
/// See: <https://github.com/buildpacks/lifecycle/blob/a7428a55c2a14d8a37e84285b95dc63192e3264e/env/env_test.go#L105-L154>
620+
///
621+
/// Modifications:
622+
/// - Default delimiter is now based on OS instead of an empty string
619623
#[test]
620624
fn reference_impl_env_files_have_a_suffix_it_performs_the_matching_action() {
621625
let temp_dir = tempdir().unwrap();
@@ -660,7 +664,7 @@ mod tests {
660664

661665
assert_eq!(
662666
vec![
663-
("VAR_APPEND", "value-append-origvalue-append"),
667+
("VAR_APPEND", "value-append-orig:value-append"),
664668
(
665669
"VAR_APPEND_DELIM",
666670
"value-append-delim-orig[]value-append-delim"
@@ -671,7 +675,7 @@ mod tests {
671675
("VAR_DEFAULT_NEW", "value-default"),
672676
("VAR_OVERRIDE", "value-override"),
673677
("VAR_OVERRIDE_NEW", "value-override"),
674-
("VAR_PREPEND", "value-prependvalue-prepend-orig"),
678+
("VAR_PREPEND", "value-prepend:value-prepend-orig"),
675679
(
676680
"VAR_PREPEND_DELIM",
677681
"value-prepend-delim[]value-prepend-delim-orig"

0 commit comments

Comments
 (0)