Update buildpack spec for RFC-0042 #98
Conversation
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
buildpack.md
Outdated
|
|
||
| ##### Default | ||
|
|
||
| If the environment variable file name ends in `.default`, then the value of the environment variable MUST only be the file contents if the environment variable is empty. | ||
| For that environment variable value, | ||
| - Earlier buildpacks' environment default variable file contents MUST override later buildpacks' environment variable file contents. | ||
| - For default environment variable file contents originating from the same buildpack, file contents that are earlier (when sorted alphabetically ascending by associated layer name) MUST override file contents that are later. | ||
| - Default environment variable file contents originating in the same layer MUST be sorted such that file contents in `<layers>/<layer>/env/` override file contents in `<layers>/<layer>/env.build/` or `<layers>/<layer>/env.launch/`. | ||
| - **Default environment variable file contents originating in the same layer MUST be sorted such that file contents in `<layers>/<layer>/env/` override file contents in `<layers>/<layer>/env.build/` or `<layers>/<layer>/env.launch/` which override file contents in `<layers>/<layer>/env.launch/<process>/`.** |
There was a problem hiding this comment.
I think this one is backwards (both here and in the current spec). Wouldn't we want defaults from the more specific case to override the more general case?
There was a problem hiding this comment.
I agree. The whole idea is to set something only if nothing is already set. It seems to me that a single layer attempting to set the default would be expecting the most specific default to win.
There was a problem hiding this comment.
I originally thought this was inaccurate. After talking with @sclevine I realize it is accurate. If we wanted to change this behavior for default env vars I think that would be separate conversation and doesn't need to happen as part of this PR.
There was a problem hiding this comment.
@ekcasey is there a tl;dr from your discussion with Stephen?
There was a problem hiding this comment.
TLDR: This is how it works today. Setting multiple defaults is a bit of a weird edge case anyways. It would be slightly complicated to change b/c of the current implementation (the current behavior is easy/natural given the order in which we process env dirs). But we certainly could change it if we were motivated.
I feel strongly that any discussion of changing this behavior should be decoupled from the process-specific env vars discussion.
|
@jkutner I repointed this PR at the |
nebhale
changed the base branch from
buildpack/0.3
to
extensions/buildpack-registry/0.1
nebhale
changed the base branch from
extensions/buildpack-registry/0.1
to
buildpack/0.3
Signed-off-by: Joe Kutner <jpkutner@gmail.com> Co-authored-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
|
@jabrown85 can you take a second look at this and confirm it matches the implementation you're working on? |
Include process type specific paths when processing env vars during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Include process type specific paths when processing env vars during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
|
@jkutner @jabrown85 Seems like RFC-0042 includes process-specific profile scripts in addition to env vars. Are you going to make a separate PR for that or should it be added to this one? |
|
@ekcasey good question. What would you rather me do? I don't mind adding another commit here or another PR. |
Include process type specific paths when processing env vars during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Include process type specific paths when processing env vars during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Either is fine with me. Given the title of the PR I lean towards just adding another commit. |
Include process type specific paths when processing profile.d scripts during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
Signed-off-by: Jesse Brown <jabrown85@gmail.com> Signed-off-by: Joe Kutner <jpkutner@gmail.com>
Include process type specific paths when processing profile.d scripts during launch. Spec: buildpacks/spec#98 Signed-off-by: Jesse Brown <jabrown85@gmail.com>
This change adds support for Process-specific environment variables and profile.d scripts. [RFC 42][buildpacks/spec#98][buildpacks/lifecycle#327] Signed-off-by: Ben Hale <bhale@vmware.com>
Signed-off-by: Joe Kutner <jpkutner@gmail.com> Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>
Fixes #90