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

Move package os to be configured via package.toml #926

Merged

Conversation

aemengo
Copy link
Contributor

@aemengo aemengo commented Oct 27, 2020

Summary

The following PR is intended as an addendum to #840, which enables pack package-buildpack for windows. It removes the recently added --os parameter in favor a platform.os declaration in one's package.toml . This new declaration is optional: defaulting to linux when omitted, and thus required for specifying windows. The intention is a more consistent (and backward compatible) user experience.

Output

$ cat package.toml
[buildpack]
uri = "simple-layers-buildpack.tgz"

[platform]
os = "windows"

$ pack package-buildpack /tmp/package.tgz -f file -c package.toml
Successfully created package /tmp/package.tgz

Documentation

Should this change be documented?

[x] Yes, see buildpacks/docs#242

Related

cc: @micahyoung @TisVictress @ameyer-pivotal

@aemengo aemengo force-pushed the feature/package-buildpack-os-configuration branch from b3ff0d8 to 34f7d8c Compare October 28, 2020 15:46
Anthony Emengo added 2 commits October 28, 2020 12:06
@aemengo aemengo force-pushed the feature/package-buildpack-os-configuration branch from 34f7d8c to c3e7e8d Compare October 28, 2020 16:06
- Validate only ['linux', 'windows'] is allowed in platform.os of package.toml
- Validate correct platform.os for DOCKER_OS

Signed-off-by: Anthony Emengo <[email protected]>
@aemengo aemengo marked this pull request as ready for review October 28, 2020 20:21
@aemengo aemengo requested a review from a team as a code owner October 28, 2020 20:21
Copy link
Member

@micahyoung micahyoung left a comment

Choose a reason for hiding this comment

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

LGTM. Checks all the boxes for me, based on the Slack conversation: https://buildpacks.slack.com/archives/CD61YAG69/p1603485561046900

@jromero jromero added this to the 0.15.0 milestone Oct 29, 2020
@jromero jromero added experimental Issue or PR refers to an experimental feature. type/enhancement Issue that requests a new feature or improvement. labels Oct 29, 2020
@jromero jromero changed the title Feature/package buildpack os configuration Move package os to be configured via package.toml Oct 29, 2020
@jromero jromero merged commit 7d52ed9 into buildpacks:main Oct 29, 2020
@jromero
Copy link
Member

jromero commented Oct 29, 2020

💯 @aemengo. Thanks for driving this through!

micahyoung pushed a commit to micahyoung/pipeline-builder that referenced this pull request Nov 11, 2020
- Pack experimental feature: buildpacks/pack#926

Signed-off-by: Micah Young <[email protected]>
@aemengo aemengo deleted the feature/package-buildpack-os-configuration branch May 20, 2021 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issue or PR refers to an experimental feature. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants