fix removal of app symlinks

cli/command/app/launch.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"github.com/docker/cli/cli"
@@ -80,20 +79,9 @@ func runLaunch(dir string, options *AppOptions) error {
 
 // launch copies the current environment and set DOCKER_APP_BASE before spawning the app
 func launch(app string, options *AppOptions) error {
-	envs := make(map[string]string)
-
-	// copy the current environment
-	for _, v := range os.Environ() {
-		kv := strings.SplitN(v, "=", 2)
-		envs[kv[0]] = kv[1]
-	}
-
-	envs["DOCKER_APP_BASE"] = options._appBase
-	appPath, err := options.appPath()
+	envs, err := options.makeEnvs()
 	if err != nil {
 		return err
 	}
-	envs["DOCKER_APP_PATH"] = appPath
-
 	return spawn(app, options.launchArgs(), envs, options.detach)
 }

cli/command/app/opts.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -69,8 +70,93 @@ func defaultAppBase() string {
 	return filepath.Join(home, ".docker", "app")
 }
 
+type commonOptions struct {
+	// command line args
+	_args []string
+
+	// docker app base location, fixed once set
+	_appBase string
+}
+
+func (o *commonOptions) setArgs(args []string) {
+	o._args = args
+}
+
+// buildContext returns the build context for building image
+func (o *commonOptions) buildContext() string {
+	if len(o._args) == 0 {
+		return "."
+	}
+	c, _ := splitSemver(o._args[0])
+	return c
+}
+
+func (o *commonOptions) buildVersion() string {
+	if len(o._args) == 0 {
+		return ""
+	}
+	_, v := splitSemver(o._args[0])
+	return v
+}
+
+// appPath returns the app directory under the default app base
+func (o *commonOptions) appPath() (string, error) {
+	if len(o._args) == 0 {
+		return "", errors.New("missing args")
+	}
+	return o.makeAppPath(o._args[0])
+}
+
+// binPath returns the bin directory under the default app base
+func (o *commonOptions) binPath() string {
+	return filepath.Join(o._appBase, "bin")
+}
+
+// pkgPath returns the pkg directory under the default app base
+func (o *commonOptions) pkgPath() string {
+	return filepath.Join(o._appBase, "pkg")
+}
+
+// makeAppPath builds the default app path
+// in the format: appBase/pkg/scheme/host/path
+func (o *commonOptions) makeAppPath(s string) (string, error) {
+	u, err := parseURL(s)
+	if err != nil {
+		return "", err
+	}
+	if u.Path == "" {
+		return "", fmt.Errorf("missing path: %v", u)
+	}
+	return filepath.Join(o._appBase, "pkg", u.Scheme, u.Host, shortenPath(u.Path)), nil
+}
+
+func (o *commonOptions) makeEnvs() (map[string]string, error) {
+	appPath, err := o.appPath()
+	if err != nil {
+		return nil, err
+	}
+
+	envs := make(map[string]string)
+
+	// copy the current environment
+	for _, v := range os.Environ() {
+		kv := strings.SplitN(v, "=", 2)
+		envs[kv[0]] = kv[1]
+	}
+
+	envs["DOCKER_APP_BASE"] = o._appBase
+	envs["DOCKER_APP_PATH"] = appPath
+	envs["VERSION"] = o.buildVersion()
+	envs["HOSTOS"] = runtime.GOOS
+	envs["HOSTARCH"] = runtime.GOARCH
+
+	return envs, nil
+}
+
 // AppOptions holds the options for the `app` subcommands
 type AppOptions struct {
+	commonOptions
+
 	// flags for install
 
 	// path on local host
@@ -103,34 +189,6 @@ type AppOptions struct {
 	runOpts       *container.RunOptions
 	containerOpts *container.ContainerOptions
 	copyOpts      *container.CopyOptions
-
-	// runtime
-	// command line args
-	_args []string
-
-	// docker app base location, fixed once set
-	_appBase string
-}
-
-func (o *AppOptions) setArgs(args []string) {
-	o._args = args
-}
-
-// buildContext returns the build context for building image
-func (o *AppOptions) buildContext() string {
-	if len(o._args) == 0 {
-		return "."
-	}
-	c, _ := splitSemver(o._args[0])
-	return c
-}
-
-func (o *AppOptions) buildVersion() string {
-	if len(o._args) == 0 {
-		return ""
-	}
-	_, v := splitSemver(o._args[0])
-	return v
 }
 
 // runArgs returns the command line args for running the container
@@ -158,19 +216,6 @@ func (o *AppOptions) isDockerAppBase() bool {
 	return strings.HasPrefix(s, o._appBase)
 }
 
-// binPath returns the bin directory under the default app base
-func (o *AppOptions) binPath() string {
-	return filepath.Join(o._appBase, "bin")
-}
-
-// appPath returns the app directory under the default app base
-func (o *AppOptions) appPath() (string, error) {
-	if len(o._args) == 0 {
-		return "", errors.New("missing args")
-	}
-	return makeAppPath(o._appBase, o._args[0])
-}
-
 // cacheDir returns a temp cache directory under the default app base
 // appBase is chosen as the parent directory to avoid issues such as:
 // permission, disk space, renaming across partitions.
@@ -200,7 +245,9 @@ func (o *AppOptions) containerID() (string, error) {
 
 func newAppOptions() *AppOptions {
 	return &AppOptions{
-		_appBase: defaultAppBase(),
+		commonOptions: commonOptions{
+			_appBase: defaultAppBase(),
+		},
 	}
 }
 
@@ -216,41 +263,13 @@ func validateAppOptions(options *AppOptions) error {
 }
 
 type removeOptions struct {
-	_appBase string
-}
-
-// makeAppPath returns the app directory under the default app base
-// appBase/pkg/scheme/host/path
-func (o *removeOptions) makeAppPath(s string) (string, error) {
-	return makeAppPath(o._appBase, s)
-}
-
-// binPath returns the bin directory under the default app base
-func (o *removeOptions) binPath() string {
-	return filepath.Join(o._appBase, "bin")
-}
-
-// pkgPath returns the pkg directory under the default app base
-func (o *removeOptions) pkgPath() string {
-	return filepath.Join(o._appBase, "pkg")
+	commonOptions
 }
 
 func newRemoveOptions() *removeOptions {
 	return &removeOptions{
-		_appBase: defaultAppBase(),
-	}
-}
-
-// makeAppPath builds the default app path
-// in the format: appBase/pkg/scheme/host/path
-func makeAppPath(appBase, s string) (string, error) {
-	u, err := parseURL(s)
-	if err != nil {
-		return "", err
+		commonOptions: commonOptions{
+			_appBase: defaultAppBase(),
+		},
 	}
-	if u.Path == "" {
-		return "", fmt.Errorf("missing path: %v", u)
-	}
-
-	return filepath.Join(appBase, "pkg", u.Scheme, u.Host, u.Path), nil
 }

cli/command/app/remove.go

@@ -43,18 +43,34 @@ func runRemove(dockerCli command.Cli, apps []string, options *removeOptions) err
 	// find all symlinks in binPath to remove later
 	// if they point to the app package
 	targets := make(map[string]string)
+	readlink := func(link string) (string, error) {
+		target, err := os.Readlink(link)
+		if err != nil {
+			return "", err
+		}
+		if !filepath.IsAbs(target) {
+			target = filepath.Join(filepath.Dir(link), target)
+		}
+		abs, err := filepath.Abs(target)
+		if err != nil {
+			return "", err
+		}
+		return abs, nil
+	}
 	if links, err := findLinks(binPath); err == nil {
 		for _, link := range links {
-			if target, err := os.Readlink(link); err == nil {
+			if target, err := readlink(link); err == nil {
 				targets[target] = link
+				fmt.Fprintf(dockerCli.Out(), "found symlink %s -> %s\n", link, target)
 			}
 		}
 	}
 
 	var failed []string
 
 	for _, app := range apps {
-		appPath, err := options.makeAppPath(app)
+		options.setArgs([]string{app})
+		appPath, err := options.appPath()
 		if err != nil {
 			failed = append(failed, app)
 			continue
@@ -63,7 +79,8 @@ func runRemove(dockerCli command.Cli, apps []string, options *removeOptions) err
 		// optionally run uninstall if provided
 		uninstaller := filepath.Join(appPath, uninstallerName)
 		if _, err := os.Stat(uninstaller); err == nil {
-			err := spawn(uninstaller, nil, nil, false)
+			envs, _ := options.makeEnvs()
+			err := spawn(uninstaller, nil, envs, false)
 			if err != nil {
 				fmt.Fprintf(dockerCli.Err(), "%s failed to run: %v\n", uninstaller, err)
 			}
@@ -78,8 +95,11 @@ func runRemove(dockerCli command.Cli, apps []string, options *removeOptions) err
 		fmt.Fprintf(dockerCli.Out(), "app package removed %s\n", appPath)
 
 		// remove symlinks of the app if any
+		owns := func(app, target string) bool {
+			return strings.Contains(target, app)
+		}
 		for target, link := range targets {
-			if strings.Contains(target, appPath) {
+			if owns(appPath, target) {
 				if err := os.Remove(link); err != nil {
 					fmt.Fprintf(dockerCli.Err(), "failed to remove %s: %v\n", link, err)
 				} else {

cli/command/app/remove_test.go

@@ -41,8 +41,10 @@ func TestRunRemove(t *testing.T) {
 
 	createApp := func(name string, args []string) ([]string, error) {
 		o := &AppOptions{
-			_appBase: appBase,
-			_args:    args,
+			commonOptions: commonOptions{
+				_appBase: appBase,
+				_args:    args,
+			},
 		}
 		appPath, err := o.appPath()
 		if err != nil {
@@ -77,7 +79,7 @@ func TestRunRemove(t *testing.T) {
 			expectErr: "",
 		},
 		{
-			name: "a few apps", args: []string{"example.com/org/one", "example.com/org/two", "example.com/org/three"},
+			name: "a few apps", args: []string{"example.com/org/one", "example.com/org/two", "example.com/org/three@v1.2.3"},
 			fakeInstall: func(args []string) []string {
 				var files []string
 				for _, a := range args {

cli/command/app/utils.go

@@ -218,3 +218,12 @@ func removeEmptyPath(root, dir string) error {
 
 	return rm(filepath.Dir(dir))
 }
+
+// shortenPath removes home directory from path to make it shorter
+func shortenPath(path string) string {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return path
+	}
+	return strings.ReplaceAll(path, home+"/", "_")
+}