From cdd941717a891708300d4db6e2032bb184f34dec Mon Sep 17 00:00:00 2001 From: WYGIN Date: Tue, 3 Oct 2023 05:54:30 +0000 Subject: [PATCH] will not be added to when is defined and not defined Signed-off-by: WYGIN --- internal/commands/buildpack_new.go | 20 ++++---- internal/commands/buildpack_new_test.go | 1 - internal/target/parse.go | 64 +++++++++++-------------- internal/target/parse_test.go | 50 +++++++++++++++---- internal/target/platform.go | 14 +++--- internal/warn/warn.go | 17 ------- 6 files changed, 90 insertions(+), 76 deletions(-) delete mode 100644 internal/warn/warn.go diff --git a/internal/commands/buildpack_new.go b/internal/commands/buildpack_new.go index 9ee62679f..263cfd375 100644 --- a/internal/commands/buildpack_new.go +++ b/internal/commands/buildpack_new.go @@ -70,12 +70,16 @@ func BuildpackNew(logger logging.Logger, creator BuildpackCreator) *cobra.Comman }) } - targets, warn, err := target.ParseTargets(flags.Targets) - for _, w := range warn.Messages { - logger.Warn(w) - } - if err != nil { - return err + var targets []dist.Target + if len(flags.Targets) == 0 && len(flags.Stacks) == 0 { + targets = []dist.Target{{ + OS: runtime.GOOS, + Arch: runtime.GOARCH, + }} + } else { + if targets, err = target.ParseTargets(flags.Targets, logger); err != nil { + return err + } } if err := creator.NewBuildpack(cmd.Context(), client.NewBuildpackOptions{ @@ -97,9 +101,9 @@ func BuildpackNew(logger logging.Logger, creator BuildpackCreator) *cobra.Comman cmd.Flags().StringVarP(&flags.API, "api", "a", "0.8", "Buildpack API compatibility of the generated buildpack") cmd.Flags().StringVarP(&flags.Path, "path", "p", "", "Path to generate the buildpack") cmd.Flags().StringVarP(&flags.Version, "version", "V", "1.0.0", "Version of the generated buildpack") - cmd.Flags().StringSliceVarP(&flags.Stacks, "stacks", "s", []string{}, "Stack(s) this buildpack will be compatible with"+stringSliceHelp("stack")) + cmd.Flags().StringSliceVarP(&flags.Stacks, "stacks", "s", nil, "Stack(s) this buildpack will be compatible with"+stringSliceHelp("stack")) cmd.Flags().MarkDeprecated("stacks", "prefer `--targets` instead: https://github.com/buildpacks/rfcs/blob/main/text/0096-remove-stacks-mixins.md") - cmd.Flags().StringSliceVarP(&flags.Targets, "targets", "t", []string{runtime.GOOS + "/" + runtime.GOARCH}, + cmd.Flags().StringSliceVarP(&flags.Targets, "targets", "t", nil, `Targets are the list platforms that one targeting, these are generated as part of scaffolding inside buildpack.toml file. one can provide target platforms in format [os][/arch][/variant]:[distroname@osversion@anotherversion];[distroname@osversion] - Base case for two different architectures : '--targets "linux/amd64" --targets "linux/arm64"' - case for distribution version: '--targets "windows/amd64:windows-nano@10.0.19041.1415"' diff --git a/internal/commands/buildpack_new_test.go b/internal/commands/buildpack_new_test.go index b23be1f24..4a988672d 100644 --- a/internal/commands/buildpack_new_test.go +++ b/internal/commands/buildpack_new_test.go @@ -186,7 +186,6 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { ID: "io.buildpacks.stacks.jammy", Mixins: []string{}, }}, - Targets: targets, }).Return(nil).MaxTimes(1) path := filepath.Join(tmpDir, "stacks") diff --git a/internal/target/parse.go b/internal/target/parse.go index 7efc079c7..f5ea95589 100644 --- a/internal/target/parse.go +++ b/internal/target/parse.go @@ -1,46 +1,41 @@ package target import ( - "fmt" "strings" "github.com/pkg/errors" "github.com/buildpacks/pack/internal/style" - "github.com/buildpacks/pack/internal/warn" "github.com/buildpacks/pack/pkg/dist" + "github.com/buildpacks/pack/pkg/logging" ) -func ParseTargets(t []string) (targets []dist.Target, warn warn.Warn, err error) { +func ParseTargets(t []string, logger logging.Logger) (targets []dist.Target, err error) { for _, v := range t { - target, w, err := ParseTarget(v) - warn.AddWarn(w) + target, err := ParseTarget(v, logger) if err != nil { - return nil, warn, err + return nil, err } targets = append(targets, target) } - return targets, warn, nil + return targets, nil } -func ParseTarget(t string) (output dist.Target, warn warn.Warn, err error) { - nonDistro, distros, w, err := getTarget(t) - warn.AddWarn(w) +func ParseTarget(t string, logger logging.Logger) (output dist.Target, err error) { + nonDistro, distros, err := getTarget(t, logger) if v, _ := getSliceAt[string](nonDistro, 0); len(nonDistro) <= 1 && v == "" { - warn.Add(style.Error("os/arch must be defined")) + logger.Warn("os/arch must be defined") } if err != nil { - return output, warn, err + return output, err } - os, arch, variant, w, err := getPlatform(nonDistro) - warn.AddWarn(w) + os, arch, variant, err := getPlatform(nonDistro, logger) if err != nil { - return output, warn, err + return output, err } - v, w, err := ParseDistros(distros) - warn.AddWarn(w) + v, err := ParseDistros(distros, logger) if err != nil { - return output, warn, err + return output, err } output = dist.Target{ OS: os, @@ -48,46 +43,45 @@ func ParseTarget(t string) (output dist.Target, warn warn.Warn, err error) { ArchVariant: variant, Distributions: v, } - return output, warn, err + return output, err } -func ParseDistros(distroSlice string) (distros []dist.Distribution, warn warn.Warn, err error) { +func ParseDistros(distroSlice string, logger logging.Logger) (distros []dist.Distribution, err error) { distro := strings.Split(distroSlice, ";") if l := len(distro); l == 1 && distro[0] == "" { - return nil, warn, err + return nil, err } for _, d := range distro { - v, w, err := ParseDistro(d) - warn.AddWarn(w) + v, err := ParseDistro(d, logger) if err != nil { - return nil, warn, err + return nil, err } distros = append(distros, v) } - return distros, warn, nil + return distros, nil } -func ParseDistro(distroString string) (distro dist.Distribution, warn warn.Warn, err error) { +func ParseDistro(distroString string, logger logging.Logger) (distro dist.Distribution, err error) { d := strings.Split(distroString, "@") if d[0] == "" || len(d) == 0 { - return distro, warn, errors.Errorf("distro's versions %s cannot be specified without distro's name", style.Symbol("@"+strings.Join(d[1:], "@"))) + return distro, errors.Errorf("distro's versions %s cannot be specified without distro's name", style.Symbol("@"+strings.Join(d[1:], "@"))) } - if len(d) <= 2 && d[0] == "" { - warn.Add(fmt.Sprintf("distro with name %s has no specific version!", style.Symbol(d[0]))) + if len(d) <= 2 && (strings.Contains(strings.Join(d[1:], ""), "") || d[1] == "") { + logger.Warnf("distro with name %s has no specific version!", style.Symbol(d[0])) } distro.Name = d[0] distro.Versions = d[1:] - return distro, warn, err + return distro, err } -func getTarget(t string) (nonDistro []string, distros string, warn warn.Warn, err error) { +func getTarget(t string, logger logging.Logger) (nonDistro []string, distros string, err error) { target := strings.Split(t, ":") - if _, err := getSliceAt[string](target, 0); err != nil { - return nonDistro, distros, warn, errors.Errorf("invalid target %s, atleast one of [os][/arch][/archVariant] must be specified", t) + if (len(target) == 1 && target[0] == "") || len(target) == 0 { + return nonDistro, distros, errors.Errorf("invalid target %s, atleast one of [os][/arch][/archVariant] must be specified", t) } if len(target) == 2 && target[0] == "" { v, _ := getSliceAt[string](target, 1) - warn.Add(style.Warn("adding distros %s without [os][/arch][/variant]", v)) + logger.Warn(style.Warn("adding distros %s without [os][/arch][/variant]", v)) } else { i, _ := getSliceAt[string](target, 0) nonDistro = strings.Split(i, "/") @@ -95,7 +89,7 @@ func getTarget(t string) (nonDistro []string, distros string, warn warn.Warn, er if i, err := getSliceAt[string](target, 1); err == nil { distros = i } - return nonDistro, distros, warn, err + return nonDistro, distros, err } func getSliceAt[T interface{}](slice []T, index int) (value T, err error) { diff --git a/internal/target/parse_test.go b/internal/target/parse_test.go index 541263fb9..030f2c772 100644 --- a/internal/target/parse_test.go +++ b/internal/target/parse_test.go @@ -1,6 +1,7 @@ package target_test import ( + "bytes" "testing" "github.com/heroku/color" @@ -9,6 +10,7 @@ import ( "github.com/buildpacks/pack/internal/target" "github.com/buildpacks/pack/pkg/dist" + "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" ) @@ -19,18 +21,22 @@ func TestParseTargets(t *testing.T) { } func testParseTargets(t *testing.T, when spec.G, it spec.S) { + outBuf := bytes.Buffer{} it.Before(func() { + outBuf = bytes.Buffer{} + h.AssertEq(t, outBuf.String(), "") var err error h.AssertNil(t, err) }) when("target#ParseTarget", func() { it("should show a warn when [os][/arch][/variant] is nil", func() { - _, warn, _ := target.ParseTarget(":distro@version") - h.AssertNotNil(t, warn.Messages) + target.ParseTarget(":distro@version", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotEq(t, outBuf.String(), "") }) it("should parse target as expected", func() { - output, _, err := target.ParseTarget("linux/arm/v6") + output, err := target.ParseTarget("linux/arm/v6", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertEq(t, outBuf.String(), "") h.AssertNil(t, err) h.AssertEq(t, output, dist.Target{ OS: "linux", @@ -38,14 +44,30 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { ArchVariant: "v6", }) }) + it("should return an error", func() { + _, err := target.ParseTarget("", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotNil(t, err) + }) + it("should log a warning when only [os] has typo or is unknown", func() { + target.ParseTarget("os/arm/v6", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotEq(t, outBuf.String(), "") + }) + it("should log a warning when only [arch] has typo or is unknown", func() { + target.ParseTarget("darwin/arm/v6", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotEq(t, outBuf.String(), "") + }) + it("should log a warning when only [variant] has typo or is unknown", func() { + target.ParseTarget("linux/arm/unknown", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotEq(t, outBuf.String(), "") + }) }) when("target#ParseTargets", func() { it("should throw an error when atleast one target throws error", func() { - _, _, err := target.ParseTargets([]string{"linux/arm/v6", ":distro@version"}) + _, err := target.ParseTargets([]string{"linux/arm/v6", ":distro@version"}, logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertNotNil(t, err) }) it("should parse targets as expected", func() { - output, _, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:ubuntu@22.04;debian@8.10@10.06"}) + output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:ubuntu@22.04;debian@8.10@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertNil(t, err) h.AssertEq(t, output, []dist.Target{ { @@ -72,17 +94,25 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { }) when("target#ParseDistro", func() { it("should parse distro as expected", func() { - output, _, err := target.ParseDistro("ubuntu@22.04@20.08") + output, err := target.ParseDistro("ubuntu@22.04@20.08", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertEq(t, output, dist.Distribution{ Name: "ubuntu", Versions: []string{"22.04", "20.08"}, }) h.AssertNil(t, err) }) + it("should return an error", func() { + _, err := target.ParseDistro("@22.04@20.08", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotNil(t, err) + }) + it("should warn when distro version is not specified", func() { + target.ParseDistro("ubuntu", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotEq(t, outBuf.String(), "") + }) }) when("target#ParseDistros", func() { it("should parse distros as expected", func() { - output, _, err := target.ParseDistros("ubuntu@22.04@20.08;debian@8.10@10.06") + output, err := target.ParseDistros("ubuntu@22.04@20.08;debian@8.10@10.06", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertEq(t, output, []dist.Distribution{ { Name: "ubuntu", @@ -96,9 +126,13 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("result should be nil", func() { - output, _, err := target.ParseDistros("") + output, err := target.ParseDistros("", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertEq(t, output, []dist.Distribution(nil)) h.AssertNil(t, err) }) + it("should return an error", func() { + _, err := target.ParseDistros(";", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotNil(t, err) + }) }) } diff --git a/internal/target/platform.go b/internal/target/platform.go index 3122d3b78..4d48d6845 100644 --- a/internal/target/platform.go +++ b/internal/target/platform.go @@ -6,26 +6,26 @@ import ( "github.com/pkg/errors" "github.com/buildpacks/pack/internal/style" - "github.com/buildpacks/pack/internal/warn" + "github.com/buildpacks/pack/pkg/logging" ) -func getPlatform(t []string) (os, arch, variant string, warn warn.Warn, err error) { +func getPlatform(t []string, logger logging.Logger) (os, arch, variant string, err error) { os, _ = getSliceAt[string](t, 0) arch, _ = getSliceAt[string](t, 1) variant, _ = getSliceAt[string](t, 2) if !supportsOS(os) && supportsVariant(arch, variant) { - warn.Add(style.Warn("unknown os %s, is this a typo", os)) + logger.Warn(style.Warn("unknown os %s, is this a typo", os)) } if supportsArch(os, arch) && !supportsVariant(arch, variant) { - warn.Add(style.Warn("unknown variant %s", variant)) + logger.Warn(style.Warn("unknown variant %s", variant)) } if supportsOS(os) && !supportsArch(os, arch) && supportsVariant(arch, variant) { - warn.Add(style.Warn("unknown arch %s", arch)) + logger.Warn(style.Warn("unknown arch %s", arch)) } if !SupportsPlatform(os, arch, variant) { - return os, arch, variant, warn, errors.Errorf("unknown target: %s", style.Symbol(strings.Join(t, "/"))) + return os, arch, variant, errors.Errorf("unknown target: %s", style.Symbol(strings.Join(t, "/"))) } - return os, arch, variant, warn, err + return os, arch, variant, err } var supportedOSArchs = map[string][]string{ diff --git a/internal/warn/warn.go b/internal/warn/warn.go deleted file mode 100644 index 07829f0c3..000000000 --- a/internal/warn/warn.go +++ /dev/null @@ -1,17 +0,0 @@ -package warn - -type Warn struct { - Messages []string -} - -func (warn *Warn) Add(message string) { - warn.Messages = append(warn.Messages, message) -} - -func (warn *Warn) AddSlice(messages ...string) { - warn.Messages = append(warn.Messages, messages...) -} - -func (warn *Warn) AddWarn(w Warn) { - warn.Messages = append(warn.Messages, w.Messages...) -}