Allow pack buildpack package to create buildpackages without package.… #1103

sambhav · 2021-03-08T12:40:51Z

…toml for component buildpacks

Summary

This change allows users to create buildpackages without a package.toml

It follows the behavior specified in #1082

i.e.

If no package.toml is specified -

it uses the current dir as the buildpack dir

if --path is specified it uses that dir

if both are specified it returns an error saying that their usage is incompatible together

Output

Before

pack buildpack package --help                                                                                                                                                                                             
buildpack package allows users to package (a) buildpack(s) into OCI format, which can then to be hosted in image repositories. You can also package a number of buildpacks together, to enable easier distribution of a set of buildpacks. Packaged buildpacks can be used as inputs to `pack build` (using the `--buildpack` flag), and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.

Usage:
  pack buildpack package <name> --config <config-path> [flags]

Examples:
pack buildpack package my-buildpack --config ./package.toml

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -c, --config string               Path to package TOML config (required)
  -f, --format string               Format to save package as ("image" or "file")
  -h, --help                        Help for 'package'
      --publish                     Publish to registry (applies to "--format=image" only)
      --pull-policy string          Pull policy to use. Accepted values are always, never, and if-not-present. The default is always

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

After

pack buildpack package --help    
buildpack package allows users to package (a) buildpack(s) into OCI format, which can then to be hosted in image repositories. You can also package a number of buildpacks together, to enable easier distribution of a set of buildpacks. Packaged buildpacks can be used as inputs to `pack build` (using the `--buildpack` flag), and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.

Usage:
  pack buildpack package <name> --config <config-path> [flags]

Examples:
pack buildpack package my-buildpack --config ./package.toml

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -c, --config string               Path to package TOML config
  -f, --format string               Format to save package as ("image" or "file")
  -h, --help                        Help for 'package'
  -p, --path string                 Path to the Buildpack that needs to be packaged
      --publish                     Publish to registry (applies to "--format=image" only)
      --pull-policy string          Pull policy to use. Accepted values are always, never, and if-not-present. The default is always

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

Documentation

Should this change be documented?
- Yes, see Document the fact that pack can build buildpacks without package.toml for certain cases docs#321
- No

Codecov Report

Merging #1103 (6213950) into main (6198c8b) will decrease coverage by 0.54%.
The diff coverage is 77.78%.

@@            Coverage Diff             @@
##             main    #1103      +/-   ##
==========================================
- Coverage   80.53%   79.99%   -0.53%     
==========================================
  Files         136      136              
  Lines        8233     6002    -2231     
==========================================
- Hits         6630     4801    -1829     
+ Misses       1174      767     -407     
- Partials      429      434       +5

Flag	Coverage Δ
os_linux	`79.99% <77.78%> (+0.03%)`	⬆️
os_macos	`77.48% <77.78%> (-0.03%)`	⬇️
os_windows	`100.00% <ø> (+19.56%)`	⬆️
unit	`79.99% <77.78%> (-<0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

dwillist · 2021-03-08T17:43:22Z

Acceptance Notes

Setup

used the following buildpacks

node-engine tarball buildpack.tar.gz
unzip this file then run pack buildpack package test-node-engine --buildpack /path/to/unarchived/buildpack to create the test-node-engine buildpack
paketobuildpacks/yarn:0.1.2
paketobuildpacks/yarn-install:0.2.3
paketobuildpacks/yarn-start:0.0.4

Build this sample app using the above buildpacks.

Output

a successfully built image 👍

Conclusion

This looks great!

jromero · 2021-03-08T20:11:56Z

internal/commands/buildpack_package.go

 	cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`)
 	cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish to registry (applies to "--format=image" only)`)
 	cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always")
+	cmd.Flags().StringVarP(&flags.Buildpack, "buildpack", "b", "", "Path to the Buildpack that needs to be packaged. If config is specified, the Buildpack URI is overridden to this value.")


Some questions about why this flag is necessary.

Could we not just rename config to path and be able to provide a directory, buildpack.toml, or package.toml file?

I'm also concerned by it's behaviour "if config is specified, the buildpack URI is overriden". What's the use case in mind?

Don't think I had a particular use case in mind. I tried to replicate the behavior that was specified in the issue. Should buildpack and config be mutually exclusive?

Also re:path, are there use cases where we would want to specify a path to the buildpack.toml instead of the buildpack dir? And for distinguishing the use case, should we just provide a path to a folder in general? if that contains a package.toml that would be picked, otherwise it would treat it like a folder with buildpack.toml, would this behavior work?

Don't think I had a particular use case in mind. I tried to replicate the behavior that was specified in the issue. Should buildpack and config be mutually exclusive?

Ah, I see "If --buildpack and --config are provided, --buildpack should take precendence over buildpack.uri in package.toml.". @ekcasey what's the thought behind that?

Would supporting multiple reference types using one flag support the desired use cases (while keeping a sensible UX)?

I'm thinking of pack build ... --path where path can be a path to a jar file that then gets treated as source code.

In this particular case --path may be:

a buildpack directory (in that directory we can search for a package.toml first, then a buildpack.toml)

a package.toml file

a buildpack.toml file

If we do end up deciding to go with --path, then I would think we would deprecate --config/-c which feels sort of wrong as well. Thoughts?

Related: #932

I would prefer to keep it to a directory only instead of accepting paths to specific toml files. That way the input to the path config variable is consistent. It should then be upto pack to determine how to package the given directory. If it contains package.toml use that. If it contains buildpack.toml that is valid and can be standalone, it should be able to figure out the default package.toml (which is the behavior in this PR). And by default this path variable will be the current dir. This will make it consistent with something like pack build. Thoughts?

The reason why I proposed supporting pointing to files is because I don't think we should restrict what those files should be named. Could there be use cases for multiple flavors of a package.toml?

package.stack-a.toml - packaging information specific for stack-a.

package.stack-b.toml - packaging information specific for stack-b.

My example is pretty edgy but I think it's a principle of not forcing a filename that is important to me. Whether that's a shared sentiment or we feel is worth the trade off is up for debate.

Stepping back a little bit the alternative isn't bad either.

Let me try to summarize the options:

Option 1: --path for all

Support --path to point to any of the following:

a buildpack directory

a package.toml-formatted file

a buildpack.toml file

When a directory is provided, search for package.toml, if not found fallback to buildpack.toml (Use a default package config for a buildpackage #932 behaviour)

Deprecate --config flag

Option 2: --config for project.toml, --path for buildpack dir

Support --config to point to a specific package.toml-formatted file

Support --path to point to a specific buildpack directory

Make --config and --path mutually exclusive

When --path is provided, search for package.toml, if not found fallback to buildpack.toml (Use a default package config for a buildpackage #932 behaviour)

That makes sense, I never realized that package.toml was not a special file per the spec. I have a couple of questions WRT Option 1 - how will we detect if the given path is a package.toml file or not if it can be named something other than package.toml? If that is a usecase we wish to support then it might be easy to provide support for option #2. Thoughts?

The buildpack.uri field in package.toml also supports a tarball buildpack file. Can we support that as well?

Based on our conversation during Sub-Team Sync, we are circling around Option 2 as the prefered method to eliminate the added complexity needed to detect project.toml vs buildpack.toml.

@ryanmoran, that sounds like a great idea. We'd probably want to punt that into a separate following issue though.

Should be all done now - the flag has been renamed to --path and it has been made mutually exclusive to --config

…toml for component buildpacks Signed-off-by: Sambhav Kothari <[email protected]>

jromero · 2021-03-11T15:54:06Z

✅ Accepted

❯ pack-dev buildpack package --help
buildpack package allows users to package (a) buildpack(s) into OCI format, which can then to be hosted in image repositories. You can also package a number of buildpacks together, to enable easier distribution of a set of buildpacks. Packaged buildpacks can be used as inputs to `pack build` (using the `--buildpack` flag), and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.

Usage:
  pack buildpack package <name> --config <config-path> [flags]

Examples:
pack buildpack package my-buildpack --config ./package.toml

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -c, --config string               Path to package TOML config
  -f, --format string               Format to save package as ("image" or "file")
  -h, --help                        Help for 'package'
  -p, --path string                 Path to the Buildpack that needs to be packaged
      --publish                     Publish to registry (applies to "--format=image" only)
      --pull-policy string          Pull policy to use. Accepted values are always, never, and if-not-present. The default is always

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

samples on  main [$!] on ☁️ [email protected]
❯ pack-dev buildpack package my-package -c packages/hello-universe/
ERROR: reading config: decoding toml: read packages/hello-universe/: is a directory

samples on  main [$!] on ☁️ [email protected]
❯ pack-dev buildpack package my-package -c packages/hello-universe/package.toml
Successfully created package my-package

samples on  main [$!] on ☁️ [email protected]
❯ pack-dev buildpack package my-buildpack -p buildpacks/hello-world/
Successfully created package my-buildpack

samples on  main [$!] on ☁️ [email protected]
❯ cat buildpacks/hello-universe/buildpack.toml
# Buildpack API version
api = "0.2"

# Buildpack ID and metadata
[buildpack]
id = "samples/hello-universe"
version = "0.0.1"
name = "Hello Universe Buildpack"
homepage = "https://github.com/buildpacks/samples/tree/main/buildpacks/hello-universe"

# Order used for detection
[[order]]
[[order.group]]
id = "samples/hello-world"
version = "0.0.1"

[[order.group]]
id = "samples/hello-moon"
version = "0.0.1"⏎

samples on  main [$!] on ☁️ [email protected]
❯ pack-dev buildpack package my-buildpack -p buildpacks/hello-universe/
ERROR: saving image: buildpack samples/[email protected] references buildpack samples/[email protected] which is not present

samples on  main [$!] on ☁️ [email protected]
❯ pack-dev buildpack package my-buildpack -p buildpacks/hello-universe/ -c packages/hello-universe/package.toml
ERROR: --config and --path cannot be used together. Please specify the relative path to the Buildpack directory in the package config file.

sambhav requested a review from a team as a code owner March 8, 2021 12:40

github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Mar 8, 2021

github-actions bot added this to the 0.18.0 milestone Mar 8, 2021

sambhav force-pushed the package branch from e49c416 to 62a337e Compare March 8, 2021 12:41

sambhav mentioned this pull request Mar 8, 2021

Document the fact that pack can build buildpacks without package.toml for certain cases buildpacks/docs#321

Closed

4 tasks

jromero reviewed Mar 8, 2021

View reviewed changes

aemengo approved these changes Mar 9, 2021

View reviewed changes

sambhav force-pushed the package branch from 62a337e to 488126c Compare March 10, 2021 17:36

Allow pack buildpack package to create buildpackages without package.…

1aaead1

…toml for component buildpacks Signed-off-by: Sambhav Kothari <[email protected]>

sambhav force-pushed the package branch from 488126c to 1aaead1 Compare March 10, 2021 17:39

github-actions bot added the type/chore Issue that requests non-user facing changes. label Mar 10, 2021

Merge branch 'main' into package

6213950

jromero approved these changes Mar 11, 2021

View reviewed changes

jromero merged commit 8759cb6 into buildpacks:main Mar 11, 2021

jromero mentioned this pull request Mar 11, 2021

pack buildpack package shouldn't require package.toml in all circumstances #1082

Open

1 task

sambhav deleted the package branch March 11, 2021 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow pack buildpack package to create buildpackages without package.… #1103

Allow pack buildpack package to create buildpackages without package.… #1103

sambhav commented Mar 8, 2021 •

edited

Loading

codecov bot commented Mar 8, 2021 •

edited

Loading

dwillist commented Mar 8, 2021

jromero Mar 8, 2021

sambhav Mar 8, 2021

jromero Mar 8, 2021

sambhav Mar 8, 2021 •

edited

Loading

jromero Mar 9, 2021 •

edited

Loading

sambhav Mar 9, 2021

ryanmoran Mar 10, 2021

jromero Mar 10, 2021

sambhav Mar 10, 2021

jromero commented Mar 11, 2021

Allow pack buildpack package to create buildpackages without package.… #1103

Allow pack buildpack package to create buildpackages without package.… #1103

Conversation

sambhav commented Mar 8, 2021 • edited Loading

Summary

Output

Before

After

Documentation

Related

codecov bot commented Mar 8, 2021 • edited Loading

Codecov Report

dwillist commented Mar 8, 2021

Acceptance Notes

Setup

Output

Conclusion

jromero Mar 8, 2021

Choose a reason for hiding this comment

sambhav Mar 8, 2021

Choose a reason for hiding this comment

jromero Mar 8, 2021

Choose a reason for hiding this comment

sambhav Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

jromero Mar 9, 2021 • edited Loading

Choose a reason for hiding this comment

Option 1: --path for all

Option 2: --config for project.toml, --path for buildpack dir

sambhav Mar 9, 2021

Choose a reason for hiding this comment

ryanmoran Mar 10, 2021

Choose a reason for hiding this comment

jromero Mar 10, 2021

Choose a reason for hiding this comment

sambhav Mar 10, 2021

Choose a reason for hiding this comment

jromero commented Mar 11, 2021

✅ Accepted

sambhav commented Mar 8, 2021 •

edited

Loading

codecov bot commented Mar 8, 2021 •

edited

Loading

sambhav Mar 8, 2021 •

edited

Loading

jromero Mar 9, 2021 •

edited

Loading

Option 1: `--path` for all

Option 2: `--config` for `project.toml`, `--path` for buildpack dir