Skip to content

Commit

Permalink
Merge pull request #2256 from hhiroshell/fix-pack-2231
Browse files Browse the repository at this point in the history
Make the `pack build` warn that the positional argument will not be treated as the source directory path
  • Loading branch information
natalieparellano authored Oct 25, 2024
2 parents 4ffdb5e + 3aee96e commit 270eb1e
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
8 changes: 6 additions & 2 deletions internal/commands/build.go
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 _, err := os.Stat(inputImageRef.Name()); err == nil && flags.AppPath == "" {
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
49 changes: 47 additions & 2 deletions internal/commands/build_test.go
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 @@ -931,6 +930,43 @@ builder = "my-builder"
})
})

when("path to app dir or zip-formatted file is provided", func() {
it("builds with the specified 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())
})
})

when("a local path with the same string as the specified image name exists", func() {
when("an app path is specified", func() {
it("doesn't warn that the positional argument will not be treated as the source path", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithImage("my-builder", "testdata")).
Return(nil)

command.SetArgs([]string{"testdata", "--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("no app path is specified", func() {
it("warns that the positional argument will not be treated as the source path", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithImage("my-builder", "testdata")).
Return(nil)

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

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 @@ -1175,6 +1211,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

1 comment on commit 270eb1e

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 270eb1e Previous: 4ffdb5e Ratio
BenchmarkBuild/with_Trusted_Builder 2469540178 ns/op 1217111400 ns/op 2.03

This comment was automatically generated by workflow using github-action-benchmark.

CC: @buildpacks/platform-maintainers

Please sign in to comment.