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 setting environment variables using the build image #230

Merged
merged 2 commits into from
Nov 13, 2022

Conversation

sambhav
Copy link
Member

@sambhav sambhav commented Aug 29, 2022

Signed-off-by: Sambhav Kothari [email protected]

Readable

Reference Implementation

@buildpack-bot
Copy link
Member

buildpack-bot commented Aug 29, 2022

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

Comment on lines +55 to +71
Platform Enviroment varaible: `FOO=value`
Build config: `FOO=another-value`
Final value: `FOO=value`
Copy link
Member

@natalieparellano natalieparellano Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to include a buildpack-provided value in this example? If I'm not mistaken, the order of application is:
buildpack -> platform (behavior: override) -> builder (behavior: configurable, with default default)

I recall that changing the order of application would be a larger breaking change - but what about changing the platform behavior so that it is:
buildpack -> platform (behavior: configurable, with default default) -> builder (behavior: configurable, with default default)

Platforms such as pack could add .override to env var filenames, to keep the same behavior. In the case of "special" env vars such as PATH where the contents are always prepended, platforms could add .prepend to env var filenames.

Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be really happy if we could make the behavior of the platform env directory more consistent with the builder env directory. That said, if that is too big of a change to tackle right now, I'm also fine to wait until some later date to improve the env API.

@sambhav
Copy link
Member Author

sambhav commented Nov 3, 2022

@natalieparellano the platform directory change seems like something that could be done independently. I am wary of scope creep on this RFC and gathering consensus again on the new set of changes. Would it be okay if we merge this RFC in first (given that we already have a draft PR implemention in lifecycle) and then have another RFC to make platform behaviour more consistent with the rest of the env API?

@natalieparellano
Copy link
Member

@samj1912 I am good with that 🙂 ...if there's general consensus that the platform change IS something we'd like to do, I'd be happy to draft the RFC.

@natalieparellano
Copy link
Member

Should we move this to "voting" with a close date?

@sambhav
Copy link
Member Author

sambhav commented Nov 3, 2022

Moving to voting with a merge date of Nov 10.

@sambhav
Copy link
Member Author

sambhav commented Nov 13, 2022

/queue-issue buildpacks/lifecycle "Allow setting environment variables using the build image"
/queue-issue buildpacks/spec "Allow setting environment variables using the build image"
/queue-issue buildpacks/docs "Allow setting environment variables using the build image"

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.

8 participants