Merge pull request #240 from mnottale/split-merge-inplace

split, merge: Handle in-place conversion, move out of experimental.

Author: mnottale

cmd/docker-app/merge.go

@@ -1,29 +1,66 @@
 package main
 
 import (
+	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
+	"strings"
 
 	"github.com/docker/app/internal"
 	"github.com/docker/app/internal/packager"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
 var mergeOutputFile string
 
+// Check appname directory for extra files and return them
+func extraFiles(appname string) ([]string, error) {
+	files, err := ioutil.ReadDir(appname)
+	if err != nil {
+		return nil, err
+	}
+	var res []string
+	for _, f := range files {
+		hit := false
+		for _, afn := range internal.FileNames {
+			if afn == f.Name() {
+				hit = true
+				break
+			}
+		}
+		if !hit {
+			res = append(res, f.Name())
+		}
+	}
+	return res, nil
+}
+
 func mergeCmd(dockerCli command.Cli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "merge [<app-name>] [-o output_dir]",
+		Use:   "merge [<app-name>] [-o output_file]",
 		Short: "Merge the application as a single file multi-document YAML",
 		Args:  cli.RequiresMaxArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			appname, cleanup, err := packager.Extract(firstOrEmpty(args))
+			extractedApp, err := packager.ExtractWithOrigin(firstOrEmpty(args))
 			if err != nil {
 				return err
 			}
-			defer cleanup()
+			defer extractedApp.Cleanup()
+			inPlace := mergeOutputFile == ""
+			if inPlace {
+				extra, err := extraFiles(extractedApp.AppName)
+				if err != nil {
+					return errors.Wrap(err, "error scanning application directory")
+				}
+				if len(extra) != 0 {
+					return fmt.Errorf("refusing to overwrite %s: extra files would be deleted: %s", extractedApp.OriginalAppName, strings.Join(extra, ","))
+				}
+				mergeOutputFile = extractedApp.OriginalAppName + ".tmp"
+			}
 			var target io.Writer
 			if mergeOutputFile == "-" {
 				target = dockerCli.Out()
@@ -32,13 +69,25 @@ func mergeCmd(dockerCli command.Cli) *cobra.Command {
 				if err != nil {
 					return err
 				}
-				defer target.(io.WriteCloser).Close()
 			}
-			return packager.Merge(appname, target)
+			if err := packager.Merge(extractedApp.AppName, target); err != nil {
+				return err
+			}
+			if mergeOutputFile != "-" {
+				// Need to close for the Rename to work on windows.
+				target.(io.WriteCloser).Close()
+			}
+			if inPlace {
+				if err := os.RemoveAll(extractedApp.OriginalAppName); err != nil {
+					return errors.Wrap(err, "failed to erase previous application")
+				}
+				if err := os.Rename(mergeOutputFile, extractedApp.OriginalAppName); err != nil {
+					return errors.Wrap(err, "failed to rename new application")
+				}
+			}
+			return nil
 		},
 	}
-	if internal.Experimental == "on" {
-		cmd.Flags().StringVarP(&mergeOutputFile, "output", "o", "-", "Output file (default: stdout)")
-	}
+	cmd.Flags().StringVarP(&mergeOutputFile, "output", "o", "", "Output file (default: in-place)")
 	return cmd
 }

cmd/docker-app/root.go

@@ -38,20 +38,20 @@ func addCommands(cmd *cobra.Command, dockerCli *command.DockerCli) {
 		initCmd(),
 		inspectCmd(dockerCli),
 		lsCmd(),
+		mergeCmd(dockerCli),
 		pushCmd(),
 		renderCmd(dockerCli),
 		saveCmd(dockerCli),
+		splitCmd(),
 		versionCmd(dockerCli),
 	)
 	if internal.Experimental == "on" {
 		cmd.AddCommand(
 			imageAddCmd(),
 			imageLoadCmd(),
 			loadCmd(),
-			mergeCmd(dockerCli),
 			packCmd(dockerCli),
 			pullCmd(),
-			splitCmd(),
 			unpackCmd(),
 		)
 	}

cmd/docker-app/split.go

@@ -1,30 +1,45 @@
 package main
 
 import (
-	"github.com/docker/app/internal"
+	"os"
+
 	"github.com/docker/app/internal/packager"
 	"github.com/docker/cli/cli"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
 var splitOutputDir string
 
 func splitCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "split [<app-name>] [-o output_dir]",
+		Use:   "split [<app-name>] [-o output]",
 		Short: "Split a single-file application into multiple files",
 		Args:  cli.RequiresMaxArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			appname, cleanup, err := packager.Extract(firstOrEmpty(args))
+			extractedApp, err := packager.ExtractWithOrigin(firstOrEmpty(args))
 			if err != nil {
 				return err
 			}
-			defer cleanup()
-			return packager.Split(appname, splitOutputDir)
+			defer extractedApp.Cleanup()
+			inPlace := splitOutputDir == ""
+			if inPlace {
+				splitOutputDir = extractedApp.OriginalAppName + ".tmp"
+			}
+			if err := packager.Split(extractedApp.AppName, splitOutputDir); err != nil {
+				return err
+			}
+			if inPlace {
+				if err := os.RemoveAll(extractedApp.OriginalAppName); err != nil {
+					return errors.Wrap(err, "failed to erase previous application directory")
+				}
+				if err := os.Rename(splitOutputDir, extractedApp.OriginalAppName); err != nil {
+					return errors.Wrap(err, "failed to rename new application directory")
+				}
+			}
+			return nil
 		},
 	}
-	if internal.Experimental == "on" {
-		cmd.Flags().StringVarP(&splitOutputDir, "output", "o", ".", "Output directory")
-	}
+	cmd.Flags().StringVarP(&splitOutputDir, "output", "o", "", "Output application directory (default: in-place)")
 	return cmd
 }

e2e/binary_test.go

@@ -303,19 +303,19 @@ func TestHelmInvalidStackVersionBinary(t *testing.T) {
 }
 
 func TestSplitMergeBinary(t *testing.T) {
-	dockerApp, hasExperimental := getBinary(t)
-	if !hasExperimental {
-		t.Skip("experimental mode needed for this test")
-	}
+	dockerApp, _ := getBinary(t)
 	app := "render/envvariables"
 	assertCommand(t, dockerApp, "merge", app, "-o", "remerged.dockerapp")
 	defer os.Remove("remerged.dockerapp")
 	// test that inspect works on single-file
 	assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "remerged")
 	// split it
-	assertCommand(t, dockerApp, "split", "remerged", "-o", "splitted.dockerapp")
-	defer os.RemoveAll("splitted.dockerapp")
-	assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "splitted")
+	assertCommand(t, dockerApp, "split", "remerged", "-o", "split.dockerapp")
+	defer os.RemoveAll("split.dockerapp")
+	assertCommandOutput(t, "envvariables-inspect.golden", dockerApp, "inspect", "split")
+	// test inplace
+	assertCommand(t, dockerApp, "merge", "split")
+	assertCommand(t, dockerApp, "split", "split")
 }
 
 func TestImageBinary(t *testing.T) {

internal/packager/extract.go

@@ -14,6 +14,13 @@ import (
 	"github.com/pkg/errors"
 )
 
+// ExtractedApp represents a potentially extracted application package
+type ExtractedApp struct {
+	OriginalAppName string
+	AppName         string
+	Cleanup         func()
+}
+
 var (
 	noop = func() {}
 )
@@ -47,7 +54,7 @@ func findApp() (string, error) {
 }
 
 // extractImage extracts a docker application in a docker image to a temporary directory
-func extractImage(appname string) (string, func(), error) {
+func extractImage(appname string) (ExtractedApp, error) {
 	var imagename string
 	if strings.Contains(appname, ":") {
 		nametag := strings.Split(appname, ":")
@@ -65,42 +72,49 @@ func extractImage(appname string) (string, func(), error) {
 	}
 	tempDir, err := ioutil.TempDir("", "dockerapp")
 	if err != nil {
-		return "", noop, errors.Wrap(err, "failed to create temporary directory")
+		return ExtractedApp{}, errors.Wrap(err, "failed to create temporary directory")
 	}
 	defer os.RemoveAll(tempDir)
 	err = Load(imagename, tempDir)
 	if err != nil {
 		if !strings.Contains(imagename, "/") {
-			return "", noop, fmt.Errorf("could not locate application in either filesystem or docker image")
+			return ExtractedApp{}, fmt.Errorf("could not locate application in either filesystem or docker image")
 		}
 		// Try to pull it
 		cmd := exec.Command("docker", "pull", imagename)
 		if err := cmd.Run(); err != nil {
-			return "", noop, fmt.Errorf("could not locate application in filesystem, docker image or registry")
+			return ExtractedApp{}, fmt.Errorf("could not locate application in filesystem, docker image or registry")
 		}
 		if err := Load(imagename, tempDir); err != nil {
-			return "", noop, errors.Wrap(err, "failed to load pulled image")
+			return ExtractedApp{}, errors.Wrap(err, "failed to load pulled image")
 		}
 	}
 	// this gave us a compressed app, run through extract again
-	return Extract(filepath.Join(tempDir, appname))
+	appname, cleanup, err := Extract(filepath.Join(tempDir, appname))
+	return ExtractedApp{"", appname, cleanup}, err
+}
+
+// Extract extracts the app content if it's an archive or single-file
+func Extract(appname string) (string, func(), error) {
+	extracted, err := ExtractWithOrigin(appname)
+	return extracted.AppName, extracted.Cleanup, err
 }
 
-// Extract extracts the app content if argument is an archive, or does nothing if a dir.
-// It returns effective app name, and cleanup function
+// ExtractWithOrigin extracts the app content if argument is an archive, or does nothing if a dir.
+// It returns source file, effective app name, and cleanup function
 // If appname is empty, it looks into cwd, and all subdirs for a single matching .dockerapp
 // If nothing is found, it looks for an image and loads it
-func Extract(appname string) (string, func(), error) {
+func ExtractWithOrigin(appname string) (ExtractedApp, error) {
 	if appname == "" {
 		var err error
 		if appname, err = findApp(); err != nil {
-			return "", nil, err
+			return ExtractedApp{}, err
 		}
 	}
 	if appname == "." {
 		var err error
 		if appname, err = os.Getwd(); err != nil {
-			return "", nil, errors.Wrap(err, "cannot resolve current working directory")
+			return ExtractedApp{}, errors.Wrap(err, "cannot resolve current working directory")
 		}
 	}
 	originalAppname := appname
@@ -118,12 +132,12 @@ func Extract(appname string) (string, func(), error) {
 	}
 	if s.IsDir() {
 		// directory: already decompressed
-		return appname, noop, nil
+		return ExtractedApp{appname, appname, noop}, nil
 	}
 	// not a dir: single-file or a tarball package, extract that in a temp dir
 	tempDir, err := ioutil.TempDir("", "dockerapp")
 	if err != nil {
-		return "", noop, errors.Wrap(err, "failed to create temporary directory")
+		return ExtractedApp{}, errors.Wrap(err, "failed to create temporary directory")
 	}
 	defer func() {
 		if err != nil {
@@ -132,16 +146,16 @@ func Extract(appname string) (string, func(), error) {
 	}()
 	appDir := filepath.Join(tempDir, filepath.Base(appname))
 	if err = os.Mkdir(appDir, 0755); err != nil {
-		return "", noop, errors.Wrap(err, "failed to create application in temporary directory")
+		return ExtractedApp{}, errors.Wrap(err, "failed to create application in temporary directory")
 	}
 	if err = extract(appname, appDir); err == nil {
-		return appDir, func() { os.RemoveAll(tempDir) }, nil
+		return ExtractedApp{appname, appDir, func() { os.RemoveAll(tempDir) }}, nil
 	}
 	if err = extractSingleFile(appname, appDir); err != nil {
-		return "", noop, err
+		return ExtractedApp{}, err
 	}
 	// not a tarball, single-file then
-	return appDir, func() { os.RemoveAll(tempDir) }, nil
+	return ExtractedApp{appname, appDir, func() { os.RemoveAll(tempDir) }}, nil
 }
 
 func extractSingleFile(appname, appDir string) error {
@@ -177,6 +191,7 @@ func extract(appname, outputDir string) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to open application package")
 	}
+	defer f.Close()
 	tarReader := tar.NewReader(f)
 	outputDir = outputDir + "/"
 	for {