Merge pull request #391 from mnottale/split-merge-fix

split/merge: Fix loading from image/URL with an empty target.

Author: silvin-lubecki

cmd/docker-app/merge.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/docker/app/internal"
 	"github.com/docker/app/internal/packager"
+	"github.com/docker/app/types"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/pkg/errors"
@@ -39,6 +40,25 @@ func extraFiles(appname string) ([]string, error) {
 	return res, nil
 }
 
+//handleInPlace returns the operation target path and if it's in-place
+func handleInPlace(app *types.App) (string, bool) {
+	if app.Source == types.AppSourceURL || app.Source == types.AppSourceImage {
+		return internal.DirNameFromAppName(app.Name), false
+	}
+	return app.Path + ".tmp", true
+}
+
+// removeAndRename removes target and rename source into target
+func removeAndRename(source, target string) error {
+	if err := os.RemoveAll(target); err != nil {
+		return errors.Wrap(err, "failed to erase previous application")
+	}
+	if err := os.Rename(source, target); err != nil {
+		return errors.Wrap(err, "failed to rename new application")
+	}
+	return nil
+}
+
 func mergeCmd(dockerCli command.Cli) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "merge [<app-name>] [-o output_file]",
@@ -50,7 +70,10 @@ func mergeCmd(dockerCli command.Cli) *cobra.Command {
 				return err
 			}
 			defer extractedApp.Cleanup()
-			inPlace := mergeOutputFile == ""
+			inPlace := false
+			if mergeOutputFile == "" {
+				mergeOutputFile, inPlace = handleInPlace(extractedApp)
+			}
 			if inPlace {
 				extra, err := extraFiles(extractedApp.Path)
 				if err != nil {
@@ -59,7 +82,6 @@ func mergeCmd(dockerCli command.Cli) *cobra.Command {
 				if len(extra) != 0 {
 					return fmt.Errorf("refusing to overwrite %s: extra files would be deleted: %s", extractedApp.Path, strings.Join(extra, ","))
 				}
-				mergeOutputFile = extractedApp.Path + ".tmp"
 			}
 			var target io.Writer
 			if mergeOutputFile == "-" {
@@ -78,12 +100,7 @@ func mergeCmd(dockerCli command.Cli) *cobra.Command {
 				target.(io.WriteCloser).Close()
 			}
 			if inPlace {
-				if err := os.RemoveAll(extractedApp.Path); err != nil {
-					return errors.Wrap(err, "failed to erase previous application")
-				}
-				if err := os.Rename(mergeOutputFile, extractedApp.Path); err != nil {
-					return errors.Wrap(err, "failed to rename new application")
-				}
+				return removeAndRename(mergeOutputFile, extractedApp.Path)
 			}
 			return nil
 		},

cmd/docker-app/split.go

@@ -1,11 +1,8 @@
 package main
 
 import (
-	"os"
-
 	"github.com/docker/app/internal/packager"
 	"github.com/docker/cli/cli"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -22,20 +19,15 @@ func splitCmd() *cobra.Command {
 				return err
 			}
 			defer extractedApp.Cleanup()
-			inPlace := splitOutputDir == ""
-			if inPlace {
-				splitOutputDir = extractedApp.Path + ".tmp"
+			inPlace := false
+			if splitOutputDir == "" {
+				splitOutputDir, inPlace = handleInPlace(extractedApp)
 			}
 			if err := packager.Split(extractedApp, splitOutputDir); err != nil {
 				return err
 			}
 			if inPlace {
-				if err := os.RemoveAll(extractedApp.Path); err != nil {
-					return errors.Wrap(err, "failed to erase previous application directory")
-				}
-				if err := os.Rename(splitOutputDir, extractedApp.Path); err != nil {
-					return errors.Wrap(err, "failed to rename new application directory")
-				}
+				return removeAndRename(splitOutputDir, extractedApp.Path)
 			}
 			return nil
 		},

internal/packager/extract.go

@@ -113,15 +113,18 @@ func Extract(name string, ops ...func(*types.App) error) (*types.App, error) {
 		// URL or docker image
 		u, err := url.Parse(name)
 		if err == nil && (u.Scheme == "http" || u.Scheme == "https") {
+			ops = append(ops, types.WithSource(types.AppSourceURL))
 			return loader.LoadFromURL(name, ops...)
 		}
 		// look for a docker image
+		ops = append(ops, types.WithSource(types.AppSourceImage))
 		return extractImage(name, ops...)
 	}
 	if s.IsDir() {
 		// directory: already decompressed
 		appOpts := append(ops,
 			types.WithPath(appname),
+			types.WithSource(types.AppSourceSplit),
 		)
 		return loader.LoadFromDirectory(appname, appOpts...)
 	}
@@ -133,7 +136,9 @@ func Extract(name string, ops ...func(*types.App) error) (*types.App, error) {
 			return nil, err
 		}
 		defer f.Close()
+		ops = append(ops, types.WithSource(types.AppSourceMerged))
 		return loader.LoadFromSingleFile(appname, f, ops...)
 	}
+	app.Source = types.AppSourceArchive
 	return app, nil
 }

types/types.go

@@ -16,11 +16,28 @@ import (
 // SingleFileSeparator is the separator used in single-file app
 const SingleFileSeparator = "\n---\n"
 
+// AppSourceKind represents what format the app was in when read
+type AppSourceKind int
+
+const (
+	// AppSourceSplit represents an Application in multiple file format
+	AppSourceSplit AppSourceKind = iota
+	// AppSourceMerged represents an Application in single file format
+	AppSourceMerged
+	// AppSourceImage represents an Application pulled from an image
+	AppSourceImage
+	// AppSourceURL represents an Application fetched from an URL
+	AppSourceURL
+	// AppSourceArchive represents an Application in an archive format
+	AppSourceArchive
+)
+
 // App represents an app
 type App struct {
 	Name    string
 	Path    string
 	Cleanup func()
+	Source  AppSourceKind
 
 	composesContent [][]byte
 	settingsContent [][]byte
@@ -149,6 +166,14 @@ func WithCleanup(f func()) func(*App) error {
 	}
 }
 
+// WithSource sets the source of the app
+func WithSource(source AppSourceKind) func(*App) error {
+	return func(app *App) error {
+		app.Source = source
+		return nil
+	}
+}
+
 // WithSettingsFiles adds the specified settings files to the app
 func WithSettingsFiles(files ...string) func(*App) error {
 	return settingsLoader(func() ([][]byte, error) { return readFiles(files...) })