From 6a44dff86a385cddea4abb50242d889bfb77f829 Mon Sep 17 00:00:00 2001
From: Sambhav Kothari <skothari44@bloomberg.net>
Date: Wed, 5 Jan 2022 22:40:39 +0000
Subject: [PATCH 1/3] Use direct=true processes for go buildpack
The go buildpack currently uses direct=false processes. AFAICT,
the buildpack itself, doesn't need to support direct=false process types
as it does not rely on any profile.d scripts or use any environment
variables as arguments for its process type. Direct=false processes
also have an extremely unintuitive/error causing behavior with arg
handling at runtime. For more info see
https://github.com/buildpacks/rfcs/blob/main/text/0093-remove-shell-processes.md#argument-handling
This also prevents divergence in the behavior of the output images
between the tiny stack and the other stacks with runtime arguments being
handled consistently now.
This also leads to performance improvements as direct=true processes
do not need to load bash/go through the shell for startup.
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
---
build.go | 4 ++--
build_test.go | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/build.go b/build.go
index 294e3913..b04e7894 100644
--- a/build.go
+++ b/build.go
@@ -118,7 +118,7 @@ func Build(
processes = append(processes, packit.Process{
Type: filepath.Base(binary),
Command: binary,
- Direct: context.Stack == TinyStackName,
+ Direct: true,
Default: index == 0 && !shouldReload,
})
@@ -126,7 +126,7 @@ func Build(
processes = append(processes, packit.Process{
Type: fmt.Sprintf("reload-%s", filepath.Base(binary)),
Command: fmt.Sprintf("watchexec --restart --watch %s --watch %s '%s'", context.WorkingDir, filepath.Dir(binary), binary),
- Direct: context.Stack == TinyStackName,
+ Direct: true,
Default: index == 0,
})
}
diff --git a/build_test.go b/build_test.go
index 39d4f27e..35e8957e 100644
--- a/build_test.go
+++ b/build_test.go
@@ -140,13 +140,13 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
{
Type: "some-start-command",
Command: "path/some-start-command",
- Direct: false,
+ Direct: true,
Default: true,
},
{
Type: "another-start-command",
Command: "path/another-start-command",
- Direct: false,
+ Direct: true,
},
},
},
@@ -203,23 +203,23 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
{
Type: "some-start-command",
Command: "path/some-start-command",
- Direct: false,
+ Direct: true,
},
{
Type: "reload-some-start-command",
Command: fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/some-start-command'", workingDir),
- Direct: false,
+ Direct: true,
Default: true,
},
{
Type: "another-start-command",
Command: "path/another-start-command",
- Direct: false,
+ Direct: true,
},
{
Type: "reload-another-start-command",
Command: fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/another-start-command'", workingDir),
- Direct: false,
+ Direct: true,
},
},
}))
@@ -347,13 +347,13 @@ launch = true
{
Type: "some-start-command",
Command: "path/some-start-command",
- Direct: false,
+ Direct: true,
Default: true,
},
{
Type: "another-start-command",
Command: "path/another-start-command",
- Direct: false,
+ Direct: true,
},
},
},
From 054c3bf8d4831910806f72a620596898b2b2d6d8 Mon Sep 17 00:00:00 2001
From: Frankie Gallina-Jones <frankieg@vmware.com>
Date: Thu, 13 Jan 2022 11:48:01 -0500
Subject: [PATCH 2/3] fix reloadable processes
---
build.go | 3 ++-
build_test.go | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/build.go b/build.go
index b04e7894..62d8f207 100644
--- a/build.go
+++ b/build.go
@@ -125,7 +125,8 @@ func Build(
if shouldReload {
processes = append(processes, packit.Process{
Type: fmt.Sprintf("reload-%s", filepath.Base(binary)),
- Command: fmt.Sprintf("watchexec --restart --watch %s --watch %s '%s'", context.WorkingDir, filepath.Dir(binary), binary),
+ Command: "/bin/bash",
+ Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch %s '%s'", context.WorkingDir, filepath.Dir(binary), binary)},
Direct: true,
Default: index == 0,
})
diff --git a/build_test.go b/build_test.go
index 35e8957e..2380c40c 100644
--- a/build_test.go
+++ b/build_test.go
@@ -207,7 +207,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
},
{
Type: "reload-some-start-command",
- Command: fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/some-start-command'", workingDir),
+ Command: "/bin/bash",
+ Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/some-start-command'", workingDir)},
Direct: true,
Default: true,
},
@@ -218,7 +219,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
},
{
Type: "reload-another-start-command",
- Command: fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/another-start-command'", workingDir),
+ Command: "/bin/bash",
+ Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/another-start-command'", workingDir)},
Direct: true,
},
},
From f5378a3e4a856d1049eec275d4a106133743ba51 Mon Sep 17 00:00:00 2001
From: Ryan Moran <rmoran@vmware.com>
Date: Tue, 18 Jan 2022 10:15:07 -0800
Subject: [PATCH 3/3] Split watchexec command into args for execve
---
build.go | 5 +++--
build_test.go | 8 ++++----
integration/default_test.go | 7 ++-----
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/build.go b/build.go
index 62d8f207..f5f37b8b 100644
--- a/build.go
+++ b/build.go
@@ -3,6 +3,7 @@ package gobuild
import (
"fmt"
"path/filepath"
+ "strconv"
"time"
"github.com/paketo-buildpacks/packit"
@@ -125,8 +126,8 @@ func Build(
if shouldReload {
processes = append(processes, packit.Process{
Type: fmt.Sprintf("reload-%s", filepath.Base(binary)),
- Command: "/bin/bash",
- Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch %s '%s'", context.WorkingDir, filepath.Dir(binary), binary)},
+ Command: "watchexec",
+ Args: []string{"--restart", "--watch", context.WorkingDir, "--watch", filepath.Dir(binary), strconv.Quote(binary)},
Direct: true,
Default: index == 0,
})
diff --git a/build_test.go b/build_test.go
index 2380c40c..e6dfd694 100644
--- a/build_test.go
+++ b/build_test.go
@@ -207,8 +207,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
},
{
Type: "reload-some-start-command",
- Command: "/bin/bash",
- Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/some-start-command'", workingDir)},
+ Command: "watchexec",
+ Args: []string{"--restart", "--watch", workingDir, "--watch", "path", "\"path/some-start-command\""},
Direct: true,
Default: true,
},
@@ -219,8 +219,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
},
{
Type: "reload-another-start-command",
- Command: "/bin/bash",
- Args: []string{"-c", fmt.Sprintf("watchexec --restart --watch %s --watch path 'path/another-start-command'", workingDir)},
+ Command: "watchexec",
+ Args: []string{"--restart", "--watch", workingDir, "--watch", "path", "\"path/another-start-command\""},
Direct: true,
},
},
diff --git a/integration/default_test.go b/integration/default_test.go
index f38b206d..64689566 100644
--- a/integration/default_test.go
+++ b/integration/default_test.go
@@ -258,11 +258,8 @@ func testDefault(t *testing.T, context spec.G, it spec.S) {
MatchRegexp(` Completed in ([0-9]*(\.[0-9]*)?[a-z]+)+`),
"",
" Assigning launch processes:",
- fmt.Sprintf(" workspace: /layers/%s/targets/bin/workspace",
- strings.ReplaceAll(settings.Buildpack.ID, "/", "_")),
- fmt.Sprintf(" reload-workspace (default): watchexec --restart --watch /workspace --watch /layers/%s/targets/bin '/layers/%s/targets/bin/workspace'",
- strings.ReplaceAll(settings.Buildpack.ID, "/", "_"),
- strings.ReplaceAll(settings.Buildpack.ID, "/", "_")),
+ fmt.Sprintf(" workspace: /layers/%s/targets/bin/workspace", strings.ReplaceAll(settings.Buildpack.ID, "/", "_")),
+ fmt.Sprintf(" reload-workspace (default): watchexec --restart --watch /workspace --watch /layers/%[1]s/targets/bin \"/layers/%[1]s/targets/bin/workspace\"", strings.ReplaceAll(settings.Buildpack.ID, "/", "_")),
))
noReloadContainer, err = docker.Container.Run.