Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Cleanup of non-deterministic files in Node.js, npm, and pnpm layers to prevent unnecessary invalidation of runtime layers for export. ([#1274](https://github.com/heroku/buildpacks-nodejs/pull/1274))

## [5.3.5] - 2026-01-27

### Added
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ toml = "0.9"
toml_edit = "0.23"
tracing = "0.1"
yaml-rust2 = "0.10"
walkdir = "2.5.0"

[dev-dependencies]
insta = "1"
Expand Down
9 changes: 5 additions & 4 deletions crates/test_support/src/snapshot_filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ pub(super) fn create_snapshot_filters() -> Vec<(String, String)> {

// TODO: investigate why the `...native_modules_are_recompiled...`-based tests (mostly the pnpm version)
// occassionally switch from Adding layer 'heroku/nodejs:xyz' to Reusing layer 'heroku/nodejs:xyz'
filters.push((
r"(?:Adding|Reusing) layer 'heroku/nodejs:virtual",
"<add_or_reusing> layer 'heroku/nodejs:virtual",
));
// TEMPORARILY DISABLED FOR INVESTIGATION
// filters.push((
// r"(?:Adding|Reusing) layer 'heroku/nodejs:virtual",
// "<add_or_reusing> layer 'heroku/nodejs:virtual",
// ));

filters
.into_iter()
Expand Down
36 changes: 36 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use crate::NodeJsBuildpack;
use crate::layer_cleanup::LayerCleanupTarget;
use std::cell::RefCell;
use std::ops::Deref;

/// Wrapper around libcnb `BuildContext` that tracks layers needing cleanup
/// of non-deterministic build artifacts (Python bytecode, Makefiles)
pub(crate) struct NodeJsBuildContext {
inner: libcnb::build::BuildContext<NodeJsBuildpack>,
cleanup_registry: RefCell<Vec<LayerCleanupTarget>>,
}

impl NodeJsBuildContext {
pub(crate) fn new(inner: libcnb::build::BuildContext<NodeJsBuildpack>) -> Self {
Self {
inner,
cleanup_registry: RefCell::new(Vec::new()),
}
}

pub(crate) fn register_layer_for_cleanup(&self, target: LayerCleanupTarget) {
self.cleanup_registry.borrow_mut().push(target);
}

pub(crate) fn layers_to_cleanup(&self) -> Vec<LayerCleanupTarget> {
self.cleanup_registry.borrow().iter().cloned().collect()
}
}

impl Deref for NodeJsBuildContext {
type Target = libcnb::build::BuildContext<NodeJsBuildpack>;

fn deref(&self) -> &Self::Target {
&self.inner
}
}
179 changes: 179 additions & 0 deletions src/layer_cleanup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
use bullet_stream::global::print;
use std::fs;
use std::path::{Path, PathBuf};
use walkdir::WalkDir;

#[derive(Debug, Clone)]
pub(crate) enum LayerKind {
/// Node.js distribution layer (contains npm with node-gyp)
Dist,
/// Custom npm version layer (contains npm with node-gyp)
NpmEngine,
/// pnpm distribution layer (contains pnpm with node-gyp)
Pnpm,
/// pnpm virtual store layer (contains native module builds with Makefiles)
Virtual,
}

#[derive(Debug, Clone)]
pub(crate) struct LayerCleanupTarget {
pub(crate) path: PathBuf,
pub(crate) kind: LayerKind,
}

/// Remove Python bytecode cache files (__pycache__/*.pyc) from node-gyp directories
/// These files are generated during native module compilation and are non-deterministic
fn remove_python_bytecode_cache(base_path: &Path) -> Result<usize, std::io::Error> {
let mut removed_count = 0;

// Look for node-gyp/gyp/pylib directory containing Python source
let pylib_path = base_path.join("node_modules/node-gyp/gyp/pylib");

if !pylib_path.exists() {
return Ok(0);
}

// Walk the pylib tree looking for __pycache__ directories
for entry in WalkDir::new(&pylib_path)
.into_iter()
.filter_map(Result::ok)
.filter(|e| e.file_type().is_dir() && e.file_name() == "__pycache__")
{
// Remove the entire __pycache__ directory
fs::remove_dir_all(entry.path())?;
removed_count += 1;
}

Ok(removed_count)
}

/// Remove Makefile and *.mk files from native module build directories
/// These files have non-deterministic dependency ordering causing layer invalidation
fn remove_build_makefiles(base_path: &Path) -> Result<usize, std::io::Error> {
let mut removed_count = 0;

// Walk directory tree looking for build/Makefile patterns
for entry in WalkDir::new(base_path)
.into_iter()
.filter_map(Result::ok)
.filter(|e| {
if !e.file_type().is_file() {
return false;
}

// Check if this is a Makefile or .mk file in a build/ directory
let path = e.path();
if let Some(parent) = path.parent()
&& parent.file_name() == Some(std::ffi::OsStr::new("build"))
&& let Some(filename) = path.file_name()
{
return filename.to_string_lossy() == "Makefile";
}

false
})
{
fs::remove_file(entry.path())?;
removed_count += 1;
}

Ok(removed_count)
}

/// Clean up non-deterministic build artifacts from a layer
pub(crate) fn cleanup_layer(target: &LayerCleanupTarget) -> Result<(), std::io::Error> {
let path = &target.path;

if !path.exists() {
// Layer doesn't exist, nothing to clean
return Ok(());
}

match target.kind {
LayerKind::Dist => {
// npm dist layer: contains bundled npm with node-gyp
// Clean Python bytecode from: dist/lib/node_modules/npm/node_modules/node-gyp/
print::bullet("Cleaning up Node.js installation layer");
let npm_modules = path.join("lib/node_modules/npm");
if npm_modules.exists() {
let removed = remove_python_bytecode_cache(&npm_modules)?;
if removed > 0 {
print::sub_bullet(format!("Removed {removed} __pycache__ directories"));
}
}
}
LayerKind::NpmEngine => {
// npm_engine layer: custom npm version installed via npm registry
// Clean Python bytecode from: npm_engine/node_modules/node-gyp/
print::bullet("Cleaning up npm installation layer");
let removed = remove_python_bytecode_cache(path)?;
if removed > 0 {
print::sub_bullet(format!("Removed {removed} __pycache__ directories"));
}
}
LayerKind::Pnpm => {
// pnpm layer: contains pnpm distribution with node-gyp
// Clean Python bytecode from: pnpm/dist/node_modules/node-gyp/
print::bullet("Cleaning up pnpm installation layer");
let pnpm_dist = path.join("dist");
if pnpm_dist.exists() {
let removed = remove_python_bytecode_cache(&pnpm_dist)?;
if removed > 0 {
print::sub_bullet(format!("Removed {removed} __pycache__ directories"));
}
}
}
LayerKind::Virtual => {
// pnpm virtual store: contains symlinked packages with native module builds
// Clean Makefiles from: virtual/store/*/node_modules/*/build/
print::bullet("Cleaning up pnpm virtual store layer");
let removed = remove_build_makefiles(path)?;
if removed > 0 {
print::sub_bullet(format!("Removed {removed} Makefile artifacts"));
}
}
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use std::fs;
use tempfile::TempDir;

#[test]
fn test_remove_python_bytecode_cache() {
let temp = TempDir::new().unwrap();
let base = temp.path();

// Create directory structure with __pycache__
let pycache_path = base.join("node_modules/node-gyp/gyp/pylib/gyp/__pycache__");
fs::create_dir_all(&pycache_path).unwrap();
fs::write(pycache_path.join("test.cpython-312.pyc"), b"bytecode").unwrap();

let removed = remove_python_bytecode_cache(base).unwrap();

assert_eq!(removed, 1);
assert!(!pycache_path.exists());
}

#[test]
fn test_remove_build_makefiles() {
let temp = TempDir::new().unwrap();
let base = temp.path();

// Create build directory with Makefile
let build_dir = base.join("node_modules/some-package/build");
fs::create_dir_all(&build_dir).unwrap();
fs::write(build_dir.join("Makefile"), b"makefile content").unwrap();
fs::write(build_dir.join("output.o"), b"binary").unwrap(); // Should not be removed

let removed = remove_build_makefiles(base).unwrap();

assert_eq!(removed, 1); // Makefile
assert!(!build_dir.join("Makefile").exists());
assert!(build_dir.join("output.o").exists()); // Not a makefile
}
}
21 changes: 19 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::buildpack_config::{ConfigValue, ConfigValueSource};
use crate::context::NodeJsBuildContext;
use crate::layer_cleanup::cleanup_layer;
use crate::o11y::*;
use crate::package_manager::InstalledPackageManager;
use crate::utils::error_handling::{ErrorMessage, on_framework_error};
Expand All @@ -16,6 +18,8 @@ use libcnb_test as _;
use toml::Table;

mod buildpack_config;
mod context;
mod layer_cleanup;
mod o11y;
mod package_json;
mod package_manager;
Expand All @@ -25,7 +29,7 @@ mod runtimes;
mod utils;

type BuildpackDetectContext = libcnb::detect::DetectContext<NodeJsBuildpack>;
type BuildpackBuildContext = libcnb::build::BuildContext<NodeJsBuildpack>;
type BuildpackBuildContext = NodeJsBuildContext;
type BuildpackError = libcnb::Error<ErrorMessage>;
type BuildpackResult<T> = Result<T, BuildpackError>;

Expand Down Expand Up @@ -68,8 +72,11 @@ impl libcnb::Buildpack for NodeJsBuildpack {
#[allow(clippy::too_many_lines)]
fn build(
&self,
context: BuildpackBuildContext,
context: libcnb::build::BuildContext<NodeJsBuildpack>,
) -> libcnb::Result<libcnb::build::BuildResult, ErrorMessage> {
// Wrap the context to track layers needing cleanup
let context = NodeJsBuildContext::new(context);

let buildpack_start = print::buildpack(
context
.buildpack_descriptor
Expand Down Expand Up @@ -215,6 +222,16 @@ impl libcnb::Buildpack for NodeJsBuildpack {
}
}

// Clean up non-deterministic build artifacts from registered layers
let layers_to_cleanup = context.layers_to_cleanup();
if !layers_to_cleanup.is_empty() {
for layer_to_cleanup in layers_to_cleanup {
if let Err(e) = cleanup_layer(&layer_to_cleanup) {
print::sub_bullet(format!("- Error during cleanup: {e}"));
}
}
}

print::all_done(&Some(buildpack_start));

build_result_builder.store(store).build()
Expand Down
14 changes: 11 additions & 3 deletions src/package_managers/npm.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::layer_cleanup::{LayerCleanupTarget, LayerKind};
use crate::utils::error_handling::ErrorType::Internal;
use crate::utils::error_handling::{
ErrorMessage, ErrorType, SuggestRetryBuild, SuggestSubmitIssue, error_message,
Expand Down Expand Up @@ -69,14 +70,21 @@ pub(crate) fn install_npm(
npm_packument: &npm_registry::PackagePackument,
node_version: &Version,
) -> BuildpackResult<()> {
npm_registry::install_package_layer(
let npm_engine_layer_path = npm_registry::install_package_layer(
layer_name!("npm_engine"),
context,
env,
npm_packument,
node_version,
)
.map_err(Into::into)
)?;

// Register npm_engine layer for cleanup of non-deterministic Python bytecode
context.register_layer_for_cleanup(LayerCleanupTarget {
path: npm_engine_layer_path,
kind: LayerKind::NpmEngine,
});

Ok(())
}

pub(crate) fn install_npm_dependencies(
Expand Down
20 changes: 17 additions & 3 deletions src/package_managers/pnpm.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::layer_cleanup::{LayerCleanupTarget, LayerKind};
use crate::utils::error_handling::{
ErrorMessage, ErrorType, SuggestRetryBuild, SuggestSubmitIssue, error_message, file_value,
};
Expand Down Expand Up @@ -38,14 +39,21 @@ pub(crate) fn install_pnpm(
pnpm_packument: &PackagePackument,
node_version: &Version,
) -> BuildpackResult<()> {
utils::npm_registry::install_package_layer(
let pnpm_layer_path = utils::npm_registry::install_package_layer(
layer_name!("pnpm"),
context,
env,
pnpm_packument,
node_version,
)
.map_err(Into::into)
)?;

// Register pnpm layer for cleanup of non-deterministic Python bytecode
context.register_layer_for_cleanup(LayerCleanupTarget {
path: pnpm_layer_path,
kind: LayerKind::Pnpm,
});

Ok(())
}

pub(crate) fn install_dependencies(
Expand Down Expand Up @@ -182,6 +190,12 @@ fn create_virtual_store_directory(context: &BuildpackBuildContext) -> BuildpackR
Err(create_node_modules_symlink_error(&error))?;
}

// Register virtual layer for cleanup of non-deterministic Makefiles
context.register_layer_for_cleanup(LayerCleanupTarget {
path: pnpm_virtual_store_layer.path().clone(),
kind: LayerKind::Virtual,
});

Ok(virtual_store_dir)
}

Expand Down
Loading
Loading