Skip to content

Commit

Permalink
will not be added to when is defined and not defined
Browse files Browse the repository at this point in the history
Signed-off-by: WYGIN <[email protected]>
  • Loading branch information
WYGIN committed Oct 3, 2023
1 parent e7902f8 commit cdd9417
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 76 deletions.
20 changes: 12 additions & 8 deletions internal/commands/buildpack_new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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:[email protected]"'
Expand Down
1 change: 0 additions & 1 deletion internal/commands/buildpack_new_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
64 changes: 29 additions & 35 deletions internal/target/parse.go
Original file line number Diff line number Diff line change
@@ -1,101 +1,95 @@
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
}

Check warning on line 39 in internal/target/parse.go

View check run for this annotation

Codecov / codecov/patch

internal/target/parse.go#L38-L39

Added lines #L38 - L39 were not covered by tests
output = dist.Target{
OS: os,
Arch: arch,
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, "/")
}
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) {
Expand Down
50 changes: 42 additions & 8 deletions internal/target/parse_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package target_test

import (
"bytes"
"testing"

"github.com/heroku/color"
Expand All @@ -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"
)

Expand All @@ -19,33 +21,53 @@ 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",
Arch: "arm",
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:[email protected];[email protected]@10.06"})
output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:[email protected];[email protected]@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf))
h.AssertNil(t, err)
h.AssertEq(t, output, []dist.Target{
{
Expand All @@ -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("[email protected]@20.08")
output, err := target.ParseDistro("[email protected]@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("@[email protected]", 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("[email protected]@20.08;[email protected]@10.06")
output, err := target.ParseDistros("[email protected]@20.08;[email protected]@10.06", logging.NewLogWithWriters(&outBuf, &outBuf))
h.AssertEq(t, output, []dist.Distribution{
{
Name: "ubuntu",
Expand All @@ -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)
})
})
}
14 changes: 7 additions & 7 deletions internal/target/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
17 changes: 0 additions & 17 deletions internal/warn/warn.go

This file was deleted.

0 comments on commit cdd9417

Please sign in to comment.