Hide --output command, require a tag

Also apply recommendations from code review

Signed-off-by: Nicolas De Loof <[email protected]>

Author: ndeloof

e2e/build_test.go

@@ -21,7 +21,7 @@ func TestBuild(t *testing.T) {
 		dir := fs.NewDir(t, "test-name")
 		defer dir.Remove()
 		f := dir.Join("bundle.json")
-		cmd.Command = dockerCli.Command("app", "build", path.Join(testDir, "single"), "--output", f)
+		cmd.Command = dockerCli.Command("app", "build", path.Join(testDir, "single"), "a-simple-tag", "--output", f)
 		icmd.RunCmd(cmd).Assert(t, icmd.Success)
 
 		data, err := ioutil.ReadFile(f)

e2e/commands_test.go

@@ -75,7 +75,7 @@ func TestRenderFormatters(t *testing.T) {
 		cmd := info.configuredCmd
 
 		appPath := filepath.Join("testdata", "simple", "simple.dockerapp")
-		cmd.Command = dockerCli.Command("app", "build", appPath)
+		cmd.Command = dockerCli.Command("app", "build", appPath, "a-simple-tag")
 		icmd.RunCmd(cmd).Assert(t, icmd.Success)
 
 		cmd.Command = dockerCli.Command("app", "render", "--formatter", "json", appPath)

e2e/images_test.go

@@ -12,11 +12,11 @@ import (
 
 func insertBundles(t *testing.T, cmd icmd.Cmd, info dindSwarmAndRegistryInfo) {
 	// Push an application so that we can later pull it by digest
-	cmd.Command = dockerCli.Command("app", "build", "--tag", info.registryAddress+"/c-myapp", filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
+	cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "push-pull", "push-pull.dockerapp"), info.registryAddress+"/c-myapp")
 	icmd.RunCmd(cmd).Assert(t, icmd.Success)
-	cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "--tag", "b-simple-app")
+	cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "b-simple-app")
 	icmd.RunCmd(cmd).Assert(t, icmd.Success)
-	cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "--tag", "a-simple-app")
+	cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "a-simple-app")
 	icmd.RunCmd(cmd).Assert(t, icmd.Success)
 }
 
@@ -86,7 +86,7 @@ func TestImageTag(t *testing.T) {
 		}
 
 		// given a first available image
-		cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "--tag", "a-simple-app")
+		cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "simple", "simple.dockerapp"), "a-simple-app")
 		icmd.RunCmd(cmd).Assert(t, icmd.Success)
 
 		singleImageExpectation := `APP IMAGE           APP NAME
@@ -175,7 +175,7 @@ c-simple-app:latest simple
 `)
 
 		// given a new application
-		cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "push-pull", "push-pull.dockerapp"), "--tag", "push-pull")
+		cmd.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "push-pull", "push-pull.dockerapp"), "push-pull")
 		icmd.RunCmd(cmd).Assert(t, icmd.Success)
 		expectImageListOutput(t, cmd, `APP IMAGE           APP NAME
 a-simple-app:0.1    simple

e2e/pushpull_test.go

@@ -194,7 +194,7 @@ func TestPushInstallBundle(t *testing.T) {
 		bundleFile := tmpDir.Join("bundle.json")
 
 		// render the app to a bundle, we use the app from the push pull test above.
-		cmd.Command = dockerCli.Command("app", "build", "-o", bundleFile, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
+		cmd.Command = dockerCli.Command("app", "build", "-o", bundleFile, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"), "a-simple-app")
 		icmd.RunCmd(cmd).Assert(t, icmd.Success)
 
 		// push it and install to check it is available
@@ -242,7 +242,7 @@ func TestPushInstallBundle(t *testing.T) {
 			cmdIsolatedStore.Env = append(cmdIsolatedStore.Env, "DOCKER_CONTEXT=swarm-context")
 
 			// bundle the app again but this time with a tag to store it into the bundle store
-			cmdIsolatedStore.Command = dockerCli.Command("app", "build", "--tag", ref2, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
+			cmdIsolatedStore.Command = dockerCli.Command("app", "build", filepath.Join("testdata", "push-pull", "push-pull.dockerapp"), ref2)
 			icmd.RunCmd(cmdIsolatedStore).Assert(t, icmd.Success)
 			// Push the app without tagging it explicitly
 			cmdIsolatedStore.Command = dockerCli.Command("app", "push", ref2)

internal/commands/build/build.go

@@ -41,11 +41,12 @@ type buildOptions struct {
 func Cmd(dockerCli command.Cli) *cobra.Command {
 	var opts buildOptions
 	cmd := &cobra.Command{
-		Use:     "build [APPLICATION]",
+		Use:     "build [APPLICATION] [TAG]",
 		Short:   "Build service images for the application",
 		Example: `$ docker app build myapp.dockerapp`,
-		Args:    cli.ExactArgs(1),
+		Args:    cli.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			opts.tag = args[1]
 			tag, err := runBuild(dockerCli, args[0], opts)
 			if err == nil {
 				fmt.Printf("Successfully build %s\n", tag.String())
@@ -58,8 +59,10 @@ func Cmd(dockerCli command.Cli) *cobra.Command {
 	flags.BoolVar(&opts.noCache, "no-cache", false, "Do not use cache when building the image")
 	flags.StringVar(&opts.progress, "progress", "auto", "Set type of progress output (auto, plain, tty). Use plain to show container output")
 	flags.BoolVar(&opts.pull, "pull", false, "Always attempt to pull a newer version of the image")
+
+	// For diagnostic and testing only
 	flags.StringVarP(&opts.out, "output", "o", "", "Dump generated bundle into a file")
-	flags.StringVarP(&opts.tag, "tag", "t", "", "Name and optionally a tag in the 'name:tag' format")
+	flags.MarkHidden("output") //nolint:errcheck
 
 	return cmd
 }
@@ -70,6 +73,11 @@ func runBuild(dockerCli command.Cli, application string, opt buildOptions) (refe
 		return nil, err
 	}
 
+	if opt.tag == "" {
+		// FIXME temporary, until we get support for Digest in bundleStore and other commands
+		return nil, fmt.Errorf("A tag is required to run docker app build")
+	}
+
 	var ref reference.Named
 	ref, err = packager.GetNamedTagged(opt.tag)
 	if err != nil {
@@ -82,17 +90,12 @@ func runBuild(dockerCli command.Cli, application string, opt buildOptions) (refe
 	}
 	defer app.Cleanup()
 
-	bundle, err := packager.MakeBundleFromApp(dockerCli, app, nil)
-	if err != nil {
-		return nil, err
-	}
-
 	buildopts, err := parseCompose(app, opt)
 	if err != nil {
 		return nil, err
 	}
 
-	buildopts["invocation-image"], err = createInvocationImageBuildOptions(dockerCli, app)
+	buildopts["com.docker.app.invocation-image"], err = createInvocationImageBuildOptions(dockerCli, app)
 	if err != nil {
 		return nil, err
 	}
@@ -114,13 +117,17 @@ func runBuild(dockerCli command.Cli, application string, opt buildOptions) (refe
 
 	pw := progress.NewPrinter(ctx, os.Stderr, opt.progress)
 
-	// We rely on buildx "docker" builder integrated in docker engine, so don't nee a DockerAPI here
+	// We rely on buildx "docker" builder integrated in docker engine, so don't need a DockerAPI here
 	resp, err := build.Build(ctx, driverInfo, buildopts, nil, dockerCli.ConfigFile(), pw)
 	if err != nil {
 		return nil, err
 	}
+	fmt.Fprintln(dockerCli.Out(), "Successfully built service images") //nolint:errcheck
 
-	fmt.Println("Successfully built service images")
+	bundle, err := packager.MakeBundleFromApp(dockerCli, app, nil)
+	if err != nil {
+		return nil, err
+	}
 	updateBundle(bundle, resp)
 
 	if ref == nil {
@@ -140,7 +147,7 @@ func checkMinimalEngineVersion(dockerCli command.Cli) error {
 	if err != nil {
 		return err
 	}
-	majorVersion, err := strconv.Atoi(info.ServerVersion[:strings.IndexRune(info.ServerVersion, '.')])
+	majorVersion, err := strconv.Atoi(strings.SplitN(info.ServerVersion, ".", 2)[0])
 	if err != nil {
 		return err
 	}
@@ -154,7 +161,7 @@ func updateBundle(bundle *bundle.Bundle, resp map[string]*client.SolveResponse)
 	debugSolveResponses(resp)
 	for service, r := range resp {
 		digest := r.ExporterResponse["containerimage.digest"]
-		if service == "invocation-image" {
+		if service == "com.docker.app.invocation-image" {
 			bundle.InvocationImages[0].Digest = digest
 		} else {
 			image := bundle.Images[service]
@@ -175,21 +182,11 @@ func persistBundle(opt buildOptions, bndl *bundle.Bundle, ref reference.Named) e
 		}
 		if opt.out == "-" {
 			_, err = os.Stdout.Write(b)
-			if err != nil {
-				return err
-			}
-		} else {
-			err = ioutil.WriteFile(opt.out, b, 0644)
-			if err != nil {
-				return err
-			}
-		}
-	} else {
-		if err := packager.PersistInBundleStore(ref, bndl); err != nil {
 			return err
 		}
+		return ioutil.WriteFile(opt.out, b, 0644)
 	}
-	return nil
+	return packager.PersistInBundleStore(ref, bndl)
 }
 
 func createInvocationImageBuildOptions(dockerCli command.Cli, app *types.App) (build.Options, error) {

internal/commands/build/compose.go

@@ -3,7 +3,6 @@ package build
 import (
 	"fmt"
 	"path"
-	"reflect"
 
 	"github.com/docker/app/types"
 	"github.com/docker/buildx/build"
@@ -14,7 +13,6 @@ import (
 // parseCompose do parse app compose file and extract buildx Options
 // We don't rely on bake's ReadTargets + TargetsToBuildOpt here as we have to skip environment variable interpolation
 func parseCompose(app *types.App, options buildOptions) (map[string]build.Options, error) {
-	// Fixme can have > 1 composes ?
 	parsed, err := loader.ParseYAML(app.Composes()[0])
 	if err != nil {
 		return nil, err
@@ -25,12 +23,13 @@ func parseCompose(app *types.App, options buildOptions) (map[string]build.Option
 		return nil, fmt.Errorf("Failed to parse compose file: %s", err)
 	}
 
-	var zeroBuildConfig ImageBuildConfig
 	opts := map[string]build.Options{}
 	for _, service := range services {
-		if reflect.DeepEqual(service.Build, zeroBuildConfig) {
+		if service.Build == nil {
 			continue
 		}
+		// FIXME docker app init should update relative paths
+		// compose file has been copied to x.dockerapp, so the relative path to build context get broken
 		contextPath := path.Join(app.Path, "..", service.Build.Context)
 		if service.Build.Dockerfile == "" {
 			service.Build.Dockerfile = "Dockerfile"

internal/commands/build/compose_test.go

@@ -14,7 +14,6 @@ func Test_parseCompose(t *testing.T) {
 		name    string
 		service string
 		want    build.Options
-		wantErr bool
 	}{
 		{
 			name:    "simple",
@@ -58,17 +57,14 @@ func Test_parseCompose(t *testing.T) {
 			assert.NilError(t, err)
 
 			got, err := parseCompose(app, buildOptions{})
-			if (err != nil) != tt.wantErr {
-				t.Errorf("parseCompose() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+			assert.NilError(t, err)
 			if _, ok := got["dontwant"]; ok {
 				t.Errorf("parseCompose() should have excluded 'dontwant' service")
 				return
 			}
 			opt, ok := got[tt.service]
 			if !ok {
-				t.Errorf("parseCompose() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("parseCompose() error = %s not converted into a build.Options", tt.service)
 				return
 			}
 			if !reflect.DeepEqual(opt, tt.want) {

internal/commands/build/types.go

@@ -14,7 +14,7 @@ import (
 type ServiceConfig struct {
 	Name string `yaml:"-" json:"-"`
 
-	Build ImageBuildConfig
+	Build *ImageBuildConfig
 }
 
 type ImageBuildConfig struct {

internal/commands/root.go

@@ -6,9 +6,8 @@ import (
 	"io/ioutil"
 	"os"
 
-	"github.com/docker/app/internal/commands/build"
-
 	"github.com/docker/app/internal"
+	"github.com/docker/app/internal/commands/build"
 	"github.com/docker/app/internal/commands/image"
 	"github.com/docker/app/internal/store"
 	"github.com/docker/cli/cli/command"

internal/version.go

@@ -9,7 +9,7 @@ import (
 
 var (
 	// Version is the git tag that this was built from.
-	Version = "unknown"
+	Version = "v0.8.0-169-gbcdf883109"
 	// GitCommit is the commit that this was built from.
 	GitCommit = "unknown"
 	// BuildTime is the time at which the binary was built.