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.

Sign up for GitHub

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

Jump to bottom

RFC: Buildpack configuration for distribution #15

Merged
merged 3 commits into from
Aug 7, 2019
Merged

RFC: Buildpack configuration for distribution #15

merged 3 commits into from
Aug 7, 2019

Conversation

jkutner
Copy link
Member

@jkutner jkutner commented Jul 24, 2019

@jkutner
Added RFC for Buildpack configuration in buildpack.toml
6f094b3 
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@jkutner jkutner requested review from hone, nebhale and sclevine July 24, 2019 15:12
@jkutner jkutner marked this pull request as ready for review July 24, 2019 15:12
@hone hone self-assigned this Jul 25, 2019
@hone hone force-pushed the buildpack-config-for-dist branch from 4da94c9 to 9c4143c Compare July 25, 2019 22:59
@hone
Updates from the discussions from the WG meeting
b1d3ac6 
* remove dependencies table
* move buildpack entries to the top level: stacks, order, metadata
* add additional build plan groupings using `[[or]]` key

Signed-off-by: Terence Lee <hone02@gmail.com>
@hone hone force-pushed the buildpack-config-for-dist branch from 9c4143c to b1d3ac6 Compare July 25, 2019 23:03
@sclevine
Copy link
Member

sclevine commented Jul 26, 2019 •
edited
Loading

After discussion at the WG meeting today, I would prefer the inner buildpack.toml file to have a flag or different top-level key (e.g., [module] instead of [buildpack]), so that inner buildpacks can't accidentally be packaged individually. Otherwise, it isn't possible for two or more buildpacks to reliably share files (such as large dependencies). I would also prefer inner buildpacks not to have a version field, and instead inherit the version field of the parent buildpack.

If we do intend for inner buildpacks to be packaged individually, then I think we should drop monorepo support (including path in order) entirely from the spec, and then write our own individual tools to make buildpack monorepos easier. For example, you could write a packaging tool that translated path to an ID and version. I don't prefer this option, because I think it's useful for buildpacks to be able to share files.

That said, I'm approving this so that we can move forward. I don't think we can reach complete consensus on this.

sclevine
sclevine reviewed Jul 26, 2019
View reviewed changes
text/0000-buildpack-config-for-dist.md
[motivation]: #motivation

In the approval of the [Distribution Spec](https://github.com/buildpack/rfcs/blob/master/text/0007-spec-distribution.md),
we landed on a [compromise that no one was really happy with](https://github.com/buildpack/rfcs/pull/11#issuecomment-510483638).
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this characterization. I liked the flexibility provided by the original proposal, and the drawbacks mentioned below are not significant for my personal use cases / preferences.

@sclevine sclevine mentioned this pull request Jul 26, 2019
Merged
@jkutner
Remove path from groups in buildpack distribution
0e6ef96 
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@jkutner
Copy link
Member Author

jkutner commented Aug 2, 2019

I've removed the path key from the group table, which means we're eschewing support for monorepo buildpacks for now.

@sclevine
Copy link
Member

sclevine commented Aug 4, 2019

Spec PR updated accordingly: buildpacks/spec#53

@jkutner jkutner merged commit 0e6ef96 into master Aug 7, 2019
@jkutner jkutner deleted the buildpack-config-for-dist branch August 9, 2019 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sclevine sclevine sclevine approved these changes

@nebhale nebhale nebhale approved these changes

@hone hone hone approved these changes

Assignees

@hone hone

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkutner @sclevine @hone @nebhale