From dd09b7d77a4e12a1fa7c993a4552c615b0560e89 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Mon, 17 May 2021 22:33:10 +0300 Subject: [PATCH] simplify nobuild/force/build logic Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/pkg_build.go | 29 ++++++++++++++++++--------- src/cmd/linuxkit/pkglib/build.go | 10 +-------- src/cmd/linuxkit/pkglib/build_test.go | 6 +++--- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/cmd/linuxkit/pkg_build.go b/src/cmd/linuxkit/pkg_build.go index 657bab092..d6fda1029 100644 --- a/src/cmd/linuxkit/pkg_build.go +++ b/src/cmd/linuxkit/pkg_build.go @@ -33,24 +33,31 @@ func pkgBuildPush(args []string, withPush bool) { flags.PrintDefaults() } - force := flags.Bool("force", false, "Force rebuild") + force := flags.Bool("force", false, "Force rebuild even if image is in local cache") docker := flags.Bool("docker", false, "Store the built image in the docker image cache instead of the default linuxkit cache") platforms := flags.String("platforms", "", "Which platforms to build for, defaults to all of those for which the package can be built") skipPlatforms := flags.String("skip-platforms", "", "Platforms that should be skipped, even if present in build.yml") builders := flags.String("builders", "", "Which builders to use for which platforms, e.g. linux/arm64=docker-context-arm64, overrides defaults and environment variables, see https://github.com/linuxkit/linuxkit/blob/master/docs/packages.md#Providing-native-builder-nodes") buildCacheDir := flags.String("cache", defaultLinuxkitCache(), "Directory for storing built image, incompatible with --docker") + // some logic clarification: + // pkg build - always builds unless is in cache + // pkg build --force - always builds even if is in cache + // pkg push - always builds unless is in cache + // pkg push --force - always builds even if is in cache + // pkg push --nobuild - skips build; if not in cache, fails + // pkg push --nobuild --force - nonsensical + var ( - release *string - nobuild, manifest, image *bool - imageRef = false + release *string + nobuild, manifest *bool + nobuildRef = false ) - image = &imageRef + nobuild = &nobuildRef if withPush { release = flags.String("release", "", "Release the given version") - nobuild = flags.Bool("nobuild", false, "Skip the build") + nobuild = flags.Bool("nobuild", false, "Skip building the image before pushing, conflicts with -force") manifest = flags.Bool("manifest", true, "Create and push multi-arch manifest") - image = flags.Bool("image", true, "Build and push image for the current platform") } pkgs, err := pkglib.NewFromCLI(flags, args...) @@ -59,10 +66,12 @@ func pkgBuildPush(args []string, withPush bool) { os.Exit(1) } - var opts []pkglib.BuildOpt - if *image { - opts = append(opts, pkglib.WithBuildImage()) + if *nobuild && *force { + fmt.Fprint(os.Stderr, "flags -force and -nobuild conflict") + os.Exit(1) } + + var opts []pkglib.BuildOpt if *force { opts = append(opts, pkglib.WithBuildForce()) } diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go index f5da82180..5eb6161aa 100644 --- a/src/cmd/linuxkit/pkglib/build.go +++ b/src/cmd/linuxkit/pkglib/build.go @@ -68,14 +68,6 @@ func WithBuildPush() BuildOpt { } } -// WithBuildImage builds the image -func WithBuildImage() BuildOpt { - return func(bo *buildOpts) error { - bo.image = true - return nil - } -} - // WithBuildManifest creates a multi-arch manifest for the image func WithBuildManifest() BuildOpt { return func(bo *buildOpts) error { @@ -231,7 +223,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { } } - if bo.image && !skipBuild { + if !skipBuild { fmt.Fprintf(writer, "building %s\n", ref) var ( args []string diff --git a/src/cmd/linuxkit/pkglib/build_test.go b/src/cmd/linuxkit/pkglib/build_test.go index 8d83ea461..34356613f 100644 --- a/src/cmd/linuxkit/pkglib/build_test.go +++ b/src/cmd/linuxkit/pkglib/build_test.go @@ -303,9 +303,9 @@ func TestBuild(t *testing.T) { {"no build cache", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64"}, commitHash: "HEAD"}, nil, []string{"amd64"}, &dockerMocker{supportBuildKit: false}, &cacheMocker{}, "must provide linuxkit build cache"}, {"unsupported buildkit", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir)}, []string{"amd64"}, &dockerMocker{supportBuildKit: false}, &cacheMocker{}, "buildkit not supported, check docker version"}, {"load docker without local platform", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir), WithBuildTargetDockerCache()}, []string{nonLocal}, &dockerMocker{supportBuildKit: false}, &cacheMocker{}, "must build for local platform"}, - {"amd64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir), WithBuildImage()}, []string{"amd64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, - {"arm64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir), WithBuildImage()}, []string{"arm64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, - {"amd64 and arm64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir), WithBuildImage()}, []string{"amd64", "arm64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, + {"amd64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir)}, []string{"amd64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, + {"arm64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir)}, []string{"arm64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, + {"amd64 and arm64", Pkg{org: "foo", image: "bar", hash: "abc", arches: []string{"amd64", "arm64"}, commitHash: "HEAD"}, []BuildOpt{WithBuildCacheDir(cacheDir)}, []string{"amd64", "arm64"}, &dockerMocker{supportBuildKit: true, enableBuild: true}, &cacheMocker{enableImagePull: false, enableImageLoad: true, enableIndexWrite: true}, ""}, } for _, tt := range tests { t.Run(tt.msg, func(t *testing.T) {