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

Unquote default process type commands #69

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

runesoerensen
Copy link
Contributor

@runesoerensen runesoerensen commented Mar 21, 2025

This PR simply removes the quotes around process commands from the output produced by bin/release. This is inconsequential for all currently supported uses of the buildpack.

However, the recently adopted CNB changes to support paths with spaces won't work as expected in the following scenario:

  • The CNB_EXEC_ENV config var is set to test.
  • The solution/project filename includes spaces/special characters.

Which results in bin/release output like this:

---
default_process_types:
  test: "dotnet test "solution with spaces.sln""

This is very much an unsupported use case currently, as launch.toml processes used by bin/release never contains double quotes (when targeting the production execution environment), and the argument quoting introduced by #67 only affects launch.toml parsing used in bin/test.

It's of course still worth fixing as the command quoting in bin/release is unnecessary / not required by neither the Procfile nor the Buildpack API specs. It's also quite likely that the CNB will produce launch.toml processes differently in the future, e.g. to support Development execution environments.

@runesoerensen runesoerensen marked this pull request as ready for review March 21, 2025 03:25
@runesoerensen runesoerensen requested a review from a team as a code owner March 21, 2025 03:25
@runesoerensen runesoerensen merged commit e65d53a into main Mar 21, 2025
6 checks passed
@runesoerensen runesoerensen deleted the unquote-default-process-type-commands branch March 21, 2025 23:29
@heroku-linguist heroku-linguist bot mentioned this pull request Mar 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants