Added support for inline buildpacks #1166

jkutner · 2021-05-07T14:52:54Z

Summary

This is an implementation of Inline Buildpacks described in RFC-0048.

Output

Give an app with a project.toml containing:

[project]
id = "my-app"

[[build.buildpacks]]
id = "my/inline"

  [build.buildpacks.script]
  api = "0.4"
  inline = "echo 'hello!'"

Before

$ pack build inline-test
...
ERROR: failed to build: invalid buildpack string my/inline@

After

$ pack build inline-test
...
===> DETECTING
my/inline 0.0.0
===> ANALYZING
===> RESTORING
===> BUILDING
hello!
...
Successfully built image inline-test

Documentation

Should this change be documented?
- Yes, see Added documentation for inline buildpacks docs#365
- No

codecov · 2021-05-07T14:57:12Z

Codecov Report

Merging #1166 (63df5b4) into main (f3d6be6) will decrease coverage by 0.08%.
The diff coverage is 64.45%.

❗ Current head 63df5b4 differs from pull request most recent head 0671626. Consider uploading reports for the commit 0671626 to get more accurate results

@@            Coverage Diff             @@
##             main    #1166      +/-   ##
==========================================
- Coverage   80.96%   80.89%   -0.07%     
==========================================
  Files         136      136              
  Lines        8316     8369      +53     
==========================================
+ Hits         6732     6769      +37     
- Misses       1158     1167       +9     
- Partials      426      433       +7

Flag	Coverage Δ
os_linux	`80.51% <66.16%> (-0.01%)`	⬇️
os_macos	`78.04% <66.16%> (+0.01%)`	⬆️
os_windows	`80.80% <64.45%> (-0.07%)`	⬇️
unit	`80.51% <66.16%> (-0.06%)`	⬇️

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

new_buildpack.go

jromero · 2021-05-08T03:26:10Z

build.go

+				if err != nil {
+					return nil, nil, errors.Wrap(err, "Could not create temporary inline buildpack")
+				}
+				println(pathToInlineBuildpack)


What is this? 🤔

code gnomes put that there

dwillist · 2021-05-11T15:22:58Z

build.go

+		shell = "/bin/sh"
+	}
+
+	binBuild := fmt.Sprintf(`#!%s


#! looks like it could cause problems on windows machines. And might be useful to have an integration test with one of these inline buildpacks just to show things do work in both envs.

do you mean windows as the host machine, or windows containers (or both)?

I'm thinking windows containers. #! will just comment out the first line.

Should we have a different default shell for windows containers?

Should we have a different default shell for windows containers?

Technically, Windows doesn't need an explicit shell chosen, nor a shebang line, instead it needs an appropriate file suffix (detect.bat / build.bat) since cmd.Start() will automatically call the all .bat files with cmd.exe. If the suffix is .ps1, they would be called with powershell (if it's present).

Unfortunately though, the #! will be treated as invalid syntax, not as a comment (which are :: or REM in .bat files)

If it's possible to detect the container/daemon OS here, I think it's fine to assume the syntax is .bat when Windows. If it's not possible to detect, I'd also feel ok with a special case like shell = "cmd.exe" triggering the Windows behavior (add .bat suffixes, remove shebang).

Yeah, that would work. I wrote a hybrid buildpack that does just that.

I'll think about it. that would mean writing either an invalid build or build.bat in every case.

I've updated this to create the hybrid buildpack (both build and build.bat)

@jkutner I think that we would still need to remove the shebang in the windows case. https://github.com/buildpacks/pack/pull/1166/files#r634567908

@dfreilich fixed in 81c66ff

jkutner · 2021-06-24T19:33:50Z

@jromero most of the codecov warning are from error cases where I'm not checking some error from the file system (like creating a file). How should I handle that?

dfreilich · 2021-06-24T19:57:09Z

@jkutner We disregard codecov/patch in most cases, as long as there are appropriate tests.

dfreilich · 2021-06-24T20:06:53Z

build.go

+		shell = "/bin/sh"
+	}
+
+	binBuild := fmt.Sprintf(`#!%s


@jkutner I think that we would still need to remove the shebang in the windows case. https://github.com/buildpacks/pack/pull/1166/files#r634567908

build_test.go

new_buildpack.go

dfreilich

Looks great for me! The UA you have in the initial post is perfect :D

Can someone with access to Windows do UA for that? @micahyoung ?

aemengo · 2021-07-02T14:58:07Z

Looks great for me! The UA you have in the initial post is perfect :D

Can someone with access to Windows do UA for that? @micahyoung ?

I'll try to get this done.

EDIT:

Looks great to me! 👍🏾

pr-1166-acceptance-windows.mov

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

…to accept URI buildpacks Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Signed-off-by: Joe Kutner <jpkutner@gmail.com> Co-authored-by: David Freilich <freilich.david@gmail.com>

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

jkutner requested a review from a team as a code owner May 7, 2021 14:52

github-actions bot added this to the 0.19.0 milestone May 7, 2021

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

sonatype-lift bot reviewed May 7, 2021

View reviewed changes

new_buildpack.go Outdated Show resolved Hide resolved

jkutner mentioned this pull request May 7, 2021

Added documentation for inline buildpacks buildpacks/docs#365

Merged

jromero reviewed May 8, 2021

View reviewed changes

jkutner requested a review from jromero May 8, 2021 13:55

dwillist suggested changes May 11, 2021

View reviewed changes

dfreilich modified the milestones: 0.19.0, 0.20.0 Jun 15, 2021

jkutner requested a review from dwillist June 24, 2021 19:32

dfreilich reviewed Jun 24, 2021

View reviewed changes

jkutner requested review from dfreilich and micahyoung June 24, 2021 21:14

dfreilich approved these changes Jul 2, 2021

View reviewed changes

aemengo approved these changes Jul 9, 2021

View reviewed changes

jkutner added 10 commits July 11, 2021 22:48

Added support for inline buildpacks based on RFC-0048

eb712a4

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Fix case statement in processBuildpacks for buildpacks in descriptor …

bda18e5

…to accept URI buildpacks Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Added negative tests for inline buildpacks

8fb54ee

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Add a test for project.toml with buildpack URI

a9de084

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Added test for inline buildpack with id and version

4c45cee

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Fix test for inline ignores script if there is an id and version

8ea9c82

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Check return value of createBuildpackTOML

6d53649

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Remove extraneous println

fdc9d33

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Use /bin/sh as default shebang for inline buildpacks

65d8425

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

Add hybrid inline buildpack support. Create bat files for windows

3bb5802

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

jkutner and others added 2 commits July 11, 2021 22:48

Update build_test.go

48a5610

Signed-off-by: Joe Kutner <jpkutner@gmail.com> Co-authored-by: David Freilich <freilich.david@gmail.com>

Remove shebang from windows bet file for inline buildpack

7f12ca8

Signed-off-by: Joe Kutner <jpkutner@gmail.com>

dfreilich force-pushed the jkutner/rfc-0048 branch from 947b8f4 to 7f12ca8 Compare July 11, 2021 19:49

dfreilich merged commit c942b25 into main Jul 11, 2021

dfreilich deleted the jkutner/rfc-0048 branch July 11, 2021 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for inline buildpacks #1166

Added support for inline buildpacks #1166

jkutner commented May 7, 2021 •

edited

codecov bot commented May 7, 2021 •

edited

jromero May 8, 2021

jkutner May 8, 2021

dwillist May 11, 2021

jkutner May 12, 2021

dwillist May 14, 2021 •

edited

jkutner May 15, 2021

micahyoung May 18, 2021 •

edited

micahyoung May 18, 2021

jkutner May 18, 2021

jkutner Jun 24, 2021

dfreilich Jun 24, 2021

jkutner Jun 24, 2021

jkutner commented Jun 24, 2021

dfreilich commented Jun 24, 2021

dfreilich Jun 24, 2021

dfreilich left a comment

aemengo commented Jul 2, 2021 •

edited

Added support for inline buildpacks #1166

Added support for inline buildpacks #1166

Conversation

jkutner commented May 7, 2021 • edited

Summary

Output

Before

After

Documentation

codecov bot commented May 7, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dwillist May 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

micahyoung May 18, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkutner commented Jun 24, 2021

dfreilich commented Jun 24, 2021

Choose a reason for hiding this comment

dfreilich left a comment

Choose a reason for hiding this comment

aemengo commented Jul 2, 2021 • edited

jkutner commented May 7, 2021 •

edited

codecov bot commented May 7, 2021 •

edited

dwillist May 14, 2021 •

edited

micahyoung May 18, 2021 •

edited

aemengo commented Jul 2, 2021 •

edited