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

Make the pack build warn that the positional argument will not be treated as the source directory path #2256

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Make the pack build warn that the positional argument will not be treated as the source directory path #2256

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0a5cf9c
Make the 'pack build' command warn that the positional argument will …
hhiroshell Sep 2, 2024
7066e52
Add a test case for when the 'pack build' is called with the '--path'…
hhiroshell Sep 5, 2024
66982d0
Improve descriptions for some test cases.
hhiroshell Sep 5, 2024
cee65f2
Make it warns only when a local path with the same string as the spec…
hhiroshell Sep 17, 2024
59a0abf
rephrase the test description
hhiroshell Sep 17, 2024
569e992
avoid using os.MkdirTemp() to prevent test failures on macOS
hhiroshell Sep 17, 2024
98c32ed
Merge branch 'main' into fix-pack-2231
jjbustamante Sep 27, 2024
3aee96e
Use an existing directory instead of creating and deleting directorie…
hhiroshell Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/commands/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import (
"strings"
"time"

"github.com/buildpacks/pack/pkg/cache"

"github.com/google/go-containerregistry/pkg/name"
"github.com/pkg/errors"
"github.com/spf13/cobra"

"github.com/buildpacks/pack/internal/config"
"github.com/buildpacks/pack/internal/style"
"github.com/buildpacks/pack/pkg/cache"
"github.com/buildpacks/pack/pkg/client"
"github.com/buildpacks/pack/pkg/image"
"github.com/buildpacks/pack/pkg/logging"
Expand Down Expand Up @@ -328,6 +327,11 @@ func validateBuildFlags(flags *BuildFlags, cfg config.Config, inputImageRef clie
return client.NewExperimentError("Exporting to OCI layout is currently experimental.")
}

if !inputImageRef.Layout() && flags.AppPath == "" {
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
logger.Warnf("You are building an image named '%s'. If you mean it as an app directory path, run 'pack build <args> --path %s'",
inputImageRef.Name(), inputImageRef.Name())
}

return nil
}

Expand Down
41 changes: 35 additions & 6 deletions internal/commands/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ import (
"github.com/sclevine/spec/report"
"github.com/spf13/cobra"

"github.com/buildpacks/pack/internal/paths"

"github.com/buildpacks/pack/internal/commands"
"github.com/buildpacks/pack/internal/commands/testmocks"
"github.com/buildpacks/pack/internal/config"
"github.com/buildpacks/pack/internal/paths"
"github.com/buildpacks/pack/pkg/client"
"github.com/buildpacks/pack/pkg/image"
"github.com/buildpacks/pack/pkg/logging"
Expand Down Expand Up @@ -70,13 +69,15 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) {
})

when("a builder and image are set", func() {
it("builds an image with a builder", func() {
it("builds an image with a builder"+
"and warns that the positional argument will not be treated as the source path", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithImage("my-builder", "image")).
Return(nil)

command.SetArgs([]string{"--builder", "my-builder", "image"})
h.AssertNil(t, command.Execute())
h.AssertContains(t, outBuf.String(), "Warning: You are building an image named 'image'. If you mean it as an app directory path, run 'pack build <args> --path image'")
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
})

it("builds an image with a builder short command arg", func() {
Expand Down Expand Up @@ -931,6 +932,19 @@ builder = "my-builder"
})
})

when("path to app dir or zip-formatted file is provided", func() {
it("builds with the specified path"+
"and doesn't warn that the positional argument will not be treated as the source path", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithPath("my-source")).
Return(nil)

command.SetArgs([]string{"image", "--builder", "my-builder", "--path", "my-source"})
h.AssertNil(t, command.Execute())
h.AssertNotContainsMatch(t, outBuf.String(), `Warning: You are building an image named '([^']+)'\. If you mean it as an app directory path, run 'pack build <args> --path ([^']+)'`)
})
})

when("export to OCI layout is expected but experimental isn't set in the config", func() {
it("errors with a descriptive message", func() {
command.SetArgs([]string{"oci:image", "--builder", "my-builder"})
Expand Down Expand Up @@ -959,7 +973,8 @@ builder = "my-builder"
})

when("path to save the image is provided", func() {
it("build is called with oci layout configuration", func() {
it("builds with oci layout configuration"+
"and it doesn't warn that the positional argument will not be treated as the source path", func() {
sparse = false
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithLayoutConfig("image", previousImage, sparse, layoutDir)).
Expand All @@ -968,11 +983,13 @@ builder = "my-builder"
command.SetArgs([]string{"oci:image", "--builder", "my-builder"})
err := command.Execute()
h.AssertNil(t, err)
h.AssertNotContainsMatch(t, outBuf.String(), `Warning: You are building an image named '([^']+)'\. If you mean it as an app directory path, run 'pack build <args> --path ([^']+)'`)
})
})

when("previous-image flag is provided", func() {
it("build is called with oci layout configuration", func() {
it("builds with oci layout configuration"+
"and it doesn't warn that the positional argument will not be treated as the source path", func() {
sparse = false
previousImage = "my-previous-image"
mockClient.EXPECT().
Expand All @@ -982,11 +999,13 @@ builder = "my-builder"
command.SetArgs([]string{"oci:image", "--previous-image", "oci:my-previous-image", "--builder", "my-builder"})
err := command.Execute()
h.AssertNil(t, err)
h.AssertNotContainsMatch(t, outBuf.String(), `Warning: You are building an image named '([^']+)'\. If you mean it as an app directory path, run 'pack build <args> --path ([^']+)'`)
})
})

when("-sparse flag is provided", func() {
it("build is called with oci layout configuration and sparse true", func() {
it("build with oci layout configuration and sparse true"+
"and it doesn't warn that the positional argument will not be treated as the source path", func() {
sparse = true
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithLayoutConfig("image", previousImage, sparse, layoutDir)).
Expand All @@ -995,6 +1014,7 @@ builder = "my-builder"
command.SetArgs([]string{"oci:image", "--sparse", "--builder", "my-builder"})
err := command.Execute()
h.AssertNil(t, err)
h.AssertNotContainsMatch(t, outBuf.String(), `Warning: You are building an image named '([^']+)'\. If you mean it as an app directory path, run 'pack build <args> --path ([^']+)'`)
})
})
})
Expand Down Expand Up @@ -1175,6 +1195,15 @@ func EqBuildOptionsWithDateTime(t *time.Time) interface{} {
}
}

func EqBuildOptionsWithPath(path string) interface{} {
return buildOptionsMatcher{
description: fmt.Sprintf("AppPath=%s", path),
equals: func(o client.BuildOptions) bool {
return o.AppPath == path
},
}
}

func EqBuildOptionsWithLayoutConfig(image, previousImage string, sparse bool, layoutDir string) interface{} {
return buildOptionsMatcher{
description: fmt.Sprintf("image=%s, previous-image=%s, sparse=%t, layout-dir=%s", image, previousImage, sparse, layoutDir),
Expand Down
Loading