Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow participating buildpacks to opt-out of Node.js build scripts #928

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
80 changes: 80 additions & 0 deletions Cargo.lock

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

4 changes: 4 additions & 0 deletions buildpacks/nodejs-npm-install/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

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([#928](https://github.com/heroku/buildpacks-nodejs/pull/928))

## [3.2.14] - 2024-09-24

- No changes.
Expand Down
36 changes: 27 additions & 9 deletions buildpacks/nodejs-npm-install/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,37 @@ added that executes `npm start`.

## Build Plan

### Provides

| Name | Description |
|----------------------|---------------------------------------------------------------------------------------------------------------------------------------|
| `node_modules` | Allows other buildpacks to depend on the Node modules provided by this buildpack. |
| `node_build_scripts` | Allows other buildpacks to depend on the [build script execution](#step-3-execute-build-scripts) behavior provided by this buildpack. |

### Requires

| Name | Description |
|----------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `node` | To execute `npm` a [Node.js][Node.js] runtime is required. It can be provided by the [`heroku/nodejs-engine`][heroku/nodejs-engine] buildpack. |
| `npm` | To install node modules, the [npm][npm] package manager is required. It can be provided by either the [`heroku/nodejs-engine`][heroku/nodejs-engine] or [`heroku/nodejs-npm-engine`][heroku/nodejs-npm-engine] buildpacks. |
| `node_modules` | This is not a strict requirement of the buildpack. Requiring `node_modules` ensures that this buildpack can be used even when no other buildpack requires `node_modules`. |
| Name | Description |
|----------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `node` | To execute `npm` a [Node.js][Node.js] runtime is required. It can be provided by the [`heroku/nodejs-engine`][heroku/nodejs-engine] buildpack. |
| `npm` | To install node modules, the [npm][npm] package manager is required. It can be provided by either the [`heroku/nodejs-engine`][heroku/nodejs-engine] or [`heroku/nodejs-npm-engine`][heroku/nodejs-npm-engine] buildpacks. |
| `node_modules` | This is not a strict requirement of the buildpack. Requiring `node_modules` ensures that this buildpack can be used even when no other buildpack requires `node_modules`. |
| `node_build_scripts` | This is not a strict requirement of the buildpack. Requiring `node_build_scripts` ensures that this buildpack will perform [build script execution](#step-3-execute-build-scripts) even when no other buildpack requires `node_build_scripts`. | |

#### Build Plan Metadata Schemas

### Provides
##### `node_build_scripts`

* `enabled` ([boolean][toml_type_boolean], optional)

###### Example

```toml
[[requires]]
name = "node_build_scripts"

| Name | Description |
|----------------|-----------------------------------------------------------------------------------|
| `node_modules` | Allows other buildpacks to depend on the Node modules provided by this buildpack. |
[requires.metadata]
enabled = false # this will prevent build scripts from running
```

## License

Expand All @@ -63,3 +80,4 @@ See [LICENSE](../../LICENSE) file.
[npm]: https://docs.npmjs.com/
[heroku/nodejs-engine]: ../nodejs-engine/README.md
[heroku/nodejs-npm-engine]: ../nodejs-npm-engine/README.md
[toml_type_boolean]: https://toml.io/en/v1.0.0#boolean
27 changes: 27 additions & 0 deletions buildpacks/nodejs-npm-install/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use commons::output::fmt;
use commons::output::fmt::DEBUG_INFO;
use fun_run::CmdError;
use heroku_nodejs_utils::application;
use heroku_nodejs_utils::buildplan::{
NodeBuildScriptsMetadataError, NODE_BUILD_SCRIPTS_BUILD_PLAN_NAME,
};
use heroku_nodejs_utils::package_json::PackageJsonError;
use indoc::formatdoc;
use std::fmt::Display;
Expand All @@ -27,6 +30,7 @@ pub(crate) enum NpmInstallBuildpackError {
NpmSetCacheDir(CmdError),
NpmVersion(npm::VersionError),
PackageJson(PackageJsonError),
NodeBuildScriptsMetadata(NodeBuildScriptsMetadataError),
}

pub(crate) fn on_error(error: libcnb::Error<NpmInstallBuildpackError>) {
Expand All @@ -44,13 +48,36 @@ fn on_buildpack_error(error: NpmInstallBuildpackError, logger: Box<dyn StartedLo
NpmInstallBuildpackError::Application(e) => on_application_error(&e, logger),
NpmInstallBuildpackError::BuildScript(e) => on_build_script_error(&e, logger),
NpmInstallBuildpackError::Detect(e) => on_detect_error(&e, logger),
NpmInstallBuildpackError::NodeBuildScriptsMetadata(e) => {
on_node_build_scripts_metadata_error(e, logger);
}
NpmInstallBuildpackError::NpmInstall(e) => on_npm_install_error(&e, logger),
NpmInstallBuildpackError::NpmSetCacheDir(e) => on_set_cache_dir_error(&e, logger),
NpmInstallBuildpackError::NpmVersion(e) => on_npm_version_error(e, logger),
NpmInstallBuildpackError::PackageJson(e) => on_package_json_error(e, logger),
}
}

fn on_node_build_scripts_metadata_error(
error: NodeBuildScriptsMetadataError,
logger: Box<dyn StartedLogger>,
) {
let NodeBuildScriptsMetadataError::InvalidEnabledValue(value) = error;
let value_type = value.type_str();
logger.announce().error(&formatdoc! { "
A participating buildpack has set invalid `[requires.metadata]` for the build plan \
named `{NODE_BUILD_SCRIPTS_BUILD_PLAN_NAME}`.

Expected metadata format:
[requires.metadata]
enabled = <bool>

But was:
[requires.metadata]
enabled = <{value_type}>
"});
}

fn on_package_json_error(error: PackageJsonError, logger: Box<dyn StartedLogger>) {
match error {
PackageJsonError::AccessError(e) => {
Expand Down
44 changes: 32 additions & 12 deletions buildpacks/nodejs-npm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ use commons::output::section_log::{log_step, log_step_stream};
use commons::output::warn_later::WarnGuard;
use fun_run::{CommandWithName, NamedOutput};
use heroku_nodejs_utils::application;
use heroku_nodejs_utils::buildplan::{
read_node_build_scripts_metadata, NodeBuildScriptsMetadata, NODE_BUILD_SCRIPTS_BUILD_PLAN_NAME,
};
use heroku_nodejs_utils::package_json::PackageJson;
use heroku_nodejs_utils::package_manager::PackageManager;
use heroku_nodejs_utils::vrs::Version;
Expand Down Expand Up @@ -53,9 +56,11 @@ impl Buildpack for NpmInstallBuildpack {
.build_plan(
BuildPlanBuilder::new()
.provides("node_modules")
.provides(NODE_BUILD_SCRIPTS_BUILD_PLAN_NAME)
.requires("node")
.requires("npm")
.requires("node_modules")
.requires("node")
.requires(NODE_BUILD_SCRIPTS_BUILD_PLAN_NAME)
.build(),
)
.build()
Expand All @@ -74,6 +79,8 @@ impl Buildpack for NpmInstallBuildpack {
let app_dir = &context.app_dir;
let package_json = PackageJson::read(app_dir.join("package.json"))
.map_err(NpmInstallBuildpackError::PackageJson)?;
let node_build_scripts_metadata = read_node_build_scripts_metadata(&context.buildpack_plan)
.map_err(NpmInstallBuildpackError::NodeBuildScriptsMetadata)?;

run_application_checks(app_dir, &warn_later)?;

Expand All @@ -84,7 +91,12 @@ impl Buildpack for NpmInstallBuildpack {
let logger = section.end_section();

let section = logger.section("Running scripts");
run_build_scripts(&package_json, &env, section.as_ref())?;
run_build_scripts(
&package_json,
&node_build_scripts_metadata,
&env,
section.as_ref(),
)?;
let logger = section.end_section();

let section = logger.section("Configuring default processes");
Expand Down Expand Up @@ -154,6 +166,7 @@ fn run_npm_install(

fn run_build_scripts(
package_json: &PackageJson,
node_build_scripts_metadata: &NodeBuildScriptsMetadata,
env: &Env,
_section_logger: &dyn SectionLogger,
) -> Result<(), NpmInstallBuildpackError> {
Expand All @@ -162,16 +175,23 @@ fn run_build_scripts(
log_step("No build scripts found");
} else {
for script in build_scripts {
let mut npm_run = npm::RunScript { env, script }.into_command();
log_step_stream(
format!("Running {}", fmt::command(npm_run.name())),
|stream| {
npm_run
.stream_output(stream.io(), stream.io())
.and_then(NamedOutput::nonzero_captured)
.map_err(NpmInstallBuildpackError::BuildScript)
},
)?;
if let Some(false) = node_build_scripts_metadata.enabled {
log_step(format!(
"Not running {} as it was disabled by a participating buildpack",
fmt::value(script)
));
} else {
let mut npm_run = npm::RunScript { env, script }.into_command();
log_step_stream(
format!("Running {}", fmt::command(npm_run.name())),
|stream| {
npm_run
.stream_output(stream.io(), stream.io())
.and_then(NamedOutput::nonzero_captured)
.map_err(NpmInstallBuildpackError::BuildScript)
},
)?;
}
}
};
Ok(())
Expand Down
Loading