Skip to content

Commit

Permalink
Skip default process registration when Procfile is present (#985)
Browse files Browse the repository at this point in the history
* Skip default process registration when Procfile is present

This change treats the presence of a `Procfile` as the "source of truth" for how processes should be registered and will skip the default process registration that was previously performed for `npm`, `yarn`, and `pnpm`.

Fixes #908

Related to heroku/buildpacks-go#319

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md
  • Loading branch information
colincasey authored Dec 13, 2024
1 parent 6343a71 commit ef95c45
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 6 deletions.
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]

### Changed

- Default processes will no longer be registered if a Procfile is present. ([#985](https://github.com/heroku/buildpacks-nodejs/pull/985))

## [3.3.5] - 2024-12-11

- No changes.
Expand Down
8 changes: 6 additions & 2 deletions buildpacks/nodejs-npm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl Buildpack for NpmInstallBuildpack {
let logger = section.end_section();

let section = logger.section("Configuring default processes");
let build_result = configure_default_processes(&package_json, section.as_ref());
let build_result = configure_default_processes(&context, &package_json, section.as_ref());
let logger = section.end_section();

configure_npm_runtime_env(&context)?;
Expand Down Expand Up @@ -198,10 +198,14 @@ fn run_build_scripts(
}

fn configure_default_processes(
context: &BuildContext<NpmInstallBuildpack>,
package_json: &PackageJson,
_section_logger: &dyn SectionLogger,
) -> Result<BuildResult, libcnb::Error<NpmInstallBuildpackError>> {
if package_json.has_start_script() {
if context.app_dir.join("Procfile").exists() {
log_step("Skipping default web process (Procfile detected)");
BuildResultBuilder::new().build()
} else if package_json.has_start_script() {
log_step(format!(
"Adding default web process for {}",
fmt::value("npm start")
Expand Down
19 changes: 19 additions & 0 deletions buildpacks/nodejs-npm-install/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,25 @@ fn test_skip_build_scripts_from_buildplan() {
);
}

#[test]
#[ignore]
fn test_default_web_process_registration_is_skipped_if_procfile_exists() {
nodejs_integration_test_with_config(
"./fixtures/npm-project",
|config| {
config.app_dir_preprocessor(|app_dir| {
std::fs::File::create(app_dir.join("Procfile")).unwrap();
});
},
|ctx| {
assert_contains!(
ctx.pack_stdout,
"Skipping default web process (Procfile detected)"
);
},
);
}

fn add_lockfile_entry(app_dir: &Path, package_name: &str, lockfile_entry: serde_json::Value) {
update_json_file(&app_dir.join("package-lock.json"), |json| {
let dependencies = json["dependencies"].as_object_mut().unwrap();
Expand Down
4 changes: 4 additions & 0 deletions buildpacks/nodejs-pnpm-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]

### Changed

- Default processes will no longer be registered if a Procfile is present. ([#985](https://github.com/heroku/buildpacks-nodejs/pull/985))

## [3.3.5] - 2024-12-11

- No changes.
Expand Down
6 changes: 5 additions & 1 deletion buildpacks/nodejs-pnpm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ impl Buildpack for PnpmInstallBuildpack {
}

let result_builder = BuildResultBuilder::new().store(Store { metadata });
if pkg_json.has_start_script() {

if context.app_dir.join("Procfile").exists() {
log_info("Skipping default web process (Procfile detected)");
result_builder.build()
} else if pkg_json.has_start_script() {
result_builder
.launch(
LaunchBuilder::new()
Expand Down
21 changes: 20 additions & 1 deletion buildpacks/nodejs-pnpm-install/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use libcnb::data::buildpack_id;
use libcnb_test::{assert_contains, assert_empty, BuildpackReference};
use test_support::{
add_build_script, assert_web_response, custom_buildpack, integration_test_with_config,
nodejs_integration_test,
nodejs_integration_test, nodejs_integration_test_with_config,
};

#[test]
Expand Down Expand Up @@ -215,3 +215,22 @@ fn test_skip_build_scripts_from_buildplan() {
],
);
}

#[test]
#[ignore = "integration test"]
fn test_default_web_process_registration_is_skipped_if_procfile_exists() {
nodejs_integration_test_with_config(
"./fixtures/pnpm-9",
|config| {
config.app_dir_preprocessor(|app_dir| {
std::fs::File::create(app_dir.join("Procfile")).unwrap();
});
},
|ctx| {
assert_contains!(
ctx.pack_stdout,
"Skipping default web process (Procfile detected)"
);
},
);
}
4 changes: 4 additions & 0 deletions buildpacks/nodejs-yarn/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]

### Changed

- Default processes will no longer be registered if a Procfile is present. ([#985](https://github.com/heroku/buildpacks-nodejs/pull/985))

## [3.3.5] - 2024-12-11

- No changes.
Expand Down
5 changes: 4 additions & 1 deletion buildpacks/nodejs-yarn/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ impl Buildpack for YarnBuildpack {
}
}

if pkg_json.has_start_script() {
if context.app_dir.join("Procfile").exists() {
log_info("Skipping default web process (Procfile detected)");
BuildResultBuilder::new().build()
} else if pkg_json.has_start_script() {
BuildResultBuilder::new()
.launch(
LaunchBuilder::new()
Expand Down
21 changes: 20 additions & 1 deletion buildpacks/nodejs-yarn/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use libcnb::data::buildpack_id;
use libcnb_test::{assert_contains, assert_not_contains, BuildpackReference};
use test_support::{
add_build_script, assert_web_response, custom_buildpack, integration_test_with_config,
nodejs_integration_test,
nodejs_integration_test, nodejs_integration_test_with_config,
};

#[test]
Expand Down Expand Up @@ -214,3 +214,22 @@ fn test_skip_build_scripts_from_buildplan() {
],
);
}

#[test]
#[ignore = "integration test"]
fn test_default_web_process_registration_is_skipped_if_procfile_exists() {
nodejs_integration_test_with_config(
"./fixtures/yarn-project",
|config| {
config.app_dir_preprocessor(|app_dir| {
std::fs::File::create(app_dir.join("Procfile")).unwrap();
});
},
|ctx| {
assert_contains!(
ctx.pack_stdout,
"Skipping default web process (Procfile detected)"
);
},
);
}

0 comments on commit ef95c45

Please sign in to comment.