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

too early in phases to detect if BP_LAUNCHPOINT is available #588

Open
till opened this issue Sep 20, 2024 · 10 comments
Open

too early in phases to detect if BP_LAUNCHPOINT is available #588

till opened this issue Sep 20, 2024 · 10 comments

Comments

@till
Copy link

till commented Sep 20, 2024

The BP_LAUNCHPOINT variable only seems to work with checked in code, not with code produced by a build step.

Expected Behavior

I am using this build pack in the scope of a node build (for a nuxt app), and expect that I can set BP_LAUNCHPOINT=./to/a/generatedfile. But it seems to disregard my setting and ignore it.

Current Behavior

[detector] ======== Output: paketo-buildpacks/node-start@2.1.1 ========
[detector] expected value derived from BP_LAUNCHPOINT [./.output/server/index.mjs] to be an existing file
[detector] err:  paketo-buildpacks/node-start@2.1.1 (1)
[detector] ======== Output: paketo-buildpacks/node-start@2.1.1 ========
[detector] expected value derived from BP_LAUNCHPOINT [./.output/server/index.mjs] to be an existing file
[detector] err:  paketo-buildpacks/node-start@2.1.1 (1)
[detector] 4 of 10 buildpacks participating
[detector] paketo-buildpacks/ca-certificates 3.8.5
[detector] paketo-buildpacks/node-engine     4.1.11
[detector] paketo-buildpacks/npm-install     1.4.11
[detector] paketo-buildpacks/node-run-script 1.0.28

Possible Solution

Define node .output/foo/bar/etc as a npm run start command.

Steps to Reproduce

Create a nuxt app and run pack.

Motivations

Requires different/additional config which I am trying to avoid.

@mhdawson
Copy link
Member

The documentation does mention that it has to exist, although it could be more specific about that versus mentioning in the comments ->

e.g. If `BP_LAUNCHPOINT=./src/launchpoint.js`, the buildpack will verify that

@till is it possible to include a dummy empty file as ./to/a/generatedfile as part of the application and then overwrite later on in the build process?

@till
Copy link
Author

till commented Sep 24, 2024

I suppose it's possible when you force commit it into the git ignored location.

I was more thinking, maybe this buildpack should be later in the chain?

@mhdawson
Copy link
Member

mhdawson commented Sep 30, 2024

@till which buildpack is it in your case that is generating the file? These are the only ones after node-start in the list (https://github.com/paketo-buildpacks/nodejs/blob/main/buildpack.toml and I would not expect any of those to be involved:

[[order.group]]
id = "paketo-buildpacks/npm-start"
optional = true
version = "2.0.12"

[[order.group]]
id = "paketo-buildpacks/procfile"
optional = true
version = "5.9.3"

[[order.group]]
id = "paketo-buildpacks/environment-variables"
optional = true
version = "4.7.3"

[[order.group]]
id = "paketo-buildpacks/image-labels"
optional = true
version = "4.7.3"

@till
Copy link
Author

till commented Sep 30, 2024

Maybe for additional context: we are trying to enable builds with as little configuration as possible.

We have an analyzer for different app types and set BP_NODE_RUN_SCRIPT in our pipeline to invoke the npm run build command. The analyzer does a little pre-processing to ensure the command is available before it kicks off the build.

Then that command produces the desired file.

@mhdawson
Copy link
Member

@till that does help. While the node-run-script is before node-start in the buildpack.toml the failing check occurs during the detect phase while the script specified by BP_NODE_RUN_SCRIPT will be run in the build phase. Since all buildpacks run their detect phase before any buildpacks run the build phase no change in ordering is going to help.

The current detection logic is in https://github.com/paketo-buildpacks/libnodejs/blob/1868fa9c2bcedb19f1386291c46153c190d5b261/find_node_application.go#L11.

I could see that the way it current works where it errors out during the detect and it effectively means its ignored is not always what people would expect. On the other hand the documentation is clear that it validates that the file exists.

I'm thinking that maybe adding a BP_LAUNCHPOINT_FORCE which would be documented as follows:

BP_LAUNCHPOINT_FORCE

The BP_LAUNCHPOINT_FORCE environment variable may be used to specify a file for the start command that is not included in the above set. It is similar to BP_LAUNCHPOINT except that it does not check if the file specified exists at detect time. BP_LAUNCHPOINT_FORCE takes precedence over BP_LAUNCHPOINT and does not check if the file exists during the detect phase. If the file specified does not exist when node-start runs the build phase, the build process will fail.

@mhdawson
Copy link
Member

@till any feedback on the possible addition?

@till
Copy link
Author

till commented Oct 10, 2024

Force is a good idea, I suppose the other requires hackery with the build task and BP_KEEP_FILES?

I was looking around if there's anything else that wouldn't break BC, but I don't see it.

@mhdawson
Copy link
Member

@till thanks for confirming. One last question, any interest in submitting a PR to add BP_LAUNCHPOINT_FORCE ? I can add it to my backlog but not sure when I'd get to it. I would prioritize reviewing your PRs if you have an interest.

It would require an update in paketobuildpacks/libnodejs to add an option (ideally with a default) to control if the file must exist or not, and then an update to the node-start builpack that would use that option to not require the file to exist during detect, but require that it exist during the build phase.

@till
Copy link
Author

till commented Oct 11, 2024

I'll take a look

@mhdawson
Copy link
Member

@till many thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants