Skip to content

Commit

Permalink
Always add the Pip dependencies layer bin directory to PATH (#232)
Browse files Browse the repository at this point in the history
Previously the `bin` directory would only be added to `PATH` if it
existed (due to it being set by `lifecycle`), which is only the case
if one of the installed dependencies has an entry point script, which
causes Pip to create the `bin` directory and place a wrapper script
inside it for that package's CLI command.

This meant:
1. We had to suppress Pip's warning during install (since Pip isn't
  to know that the `PATH` will be correctly set later)
2. If an app has no dependencies with an entry-point (so the app
  image doesn't have a `bin` directory), then at run-time tries to
  pip install a new package that does have an entry point (eg when
  debugging), then that new script won't be on `PATH`.

As such, we now add the `bin` directory to PATH explicitly, instead
of relying on `lifecycle` automatic addition.
  • Loading branch information
edmorley authored Jul 29, 2024
1 parent 53b705a commit 5944bf6
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Stopped manually creating a `src` directory inside the Pip dependencies layer. Pip will create the directory itself if needed (when there are editable VCS dependencies). ([#228](https://github.com/heroku/buildpacks-python/pull/228))
- Stopped setting `CPATH` and `PKG_CONFIG_PATH` at launch time. ([#231](https://github.com/heroku/buildpacks-python/pull/231))
- The `bin` directory in the Pip dependencies layer is now always added to `PATH` instead of only when an installed dependency has an entry point script. ([#232](https://github.com/heroku/buildpacks-python/pull/232))

## [0.12.1] - 2024-07-15

Expand Down
24 changes: 19 additions & 5 deletions src/layers/pip_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ impl Layer for PipDependenciesLayer<'_> {
"--cache-dir",
&self.pip_cache_dir.to_string_lossy(),
"--no-input",
// Prevent warning about the `bin/` directory not being on `PATH`, since it
// will be added automatically by libcnb/lifecycle later.
"--no-warn-script-location",
"--progress",
"off",
// Install dependencies into the user `site-packages` directory (set by `PYTHONUSERBASE`),
Expand Down Expand Up @@ -110,6 +107,16 @@ impl Layer for PipDependenciesLayer<'_> {
/// Environment variables that will be set by this layer.
fn generate_layer_env(layer_path: &Path) -> LayerEnv {
LayerEnv::new()
// We set `PATH` explicitly, since lifecycle will only add the bin directory to `PATH` if it
// exists - and we want to support the scenario of installing a debugging package with CLI at
// run-time, when none of the dependencies installed at build-time had an entrypoint script.
.chainable_insert(
Scope::All,
ModificationBehavior::Prepend,
"PATH",
layer_path.join("bin"),
)
.chainable_insert(Scope::All, ModificationBehavior::Delimiter, "PATH", ":")
// `PYTHONUSERBASE` overrides the default user base directory, which is used by Python to
// compute the path of the user `site-packages` directory:
// https://docs.python.org/3/using/cmdline.html#envvar-PYTHONUSERBASE
Expand Down Expand Up @@ -149,17 +156,24 @@ mod tests {
#[test]
fn pip_dependencies_layer_env() {
let mut base_env = Env::new();
base_env.insert("PATH", "/base");
base_env.insert("PYTHONUSERBASE", "this-should-be-overridden");

let layer_env = generate_layer_env(Path::new("/layer-dir"));

assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Build, &base_env)),
[("PYTHONUSERBASE", "/layer-dir")]
[
("PATH", "/layer-dir/bin:/base"),
("PYTHONUSERBASE", "/layer-dir"),
]
);
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Launch, &base_env)),
[("PYTHONUSERBASE", "/layer-dir")]
[
("PATH", "/layer-dir/bin:/base"),
("PYTHONUSERBASE", "/layer-dir"),
]
);
}
}
2 changes: 1 addition & 1 deletion tests/pip_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn pip_basic_install_and_cache_reuse() {
&formatdoc! {"
LANG=C.UTF-8
LD_LIBRARY_PATH=/layers/heroku_python/python/lib:/layers/heroku_python/dependencies/lib
PATH=/layers/heroku_python/python/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PATH=/layers/heroku_python/dependencies/bin:/layers/heroku_python/python/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PIP_DISABLE_PIP_VERSION_CHECK=1
PYTHONHOME=/layers/heroku_python/python
PYTHONUNBUFFERED=1
Expand Down

0 comments on commit 5944bf6

Please sign in to comment.