Merge pull request #10625 from duglin/FixHelpHomeSlash

Fix for help when $HOME is /

integration-cli/docker_cli_help_test.go

@@ -1,7 +1,9 @@
 package main
 
 import (
+	"os"
 	"os/exec"
+	"runtime"
 	"strings"
 	"testing"
 	"unicode"
@@ -9,62 +11,44 @@ import (
 	"github.com/docker/docker/pkg/homedir"
 )
 
-func TestMainHelpWidth(t *testing.T) {
+func TestHelpWidth(t *testing.T) {
 	// Make sure main help text fits within 80 chars and that
-	// on non-windows system we use ~ when possible (to shorten things)
+	// on non-windows system we use ~ when possible (to shorten things).
+	// Test for HOME set to its default value and set to "/" on linux
+	// Yes on windows setting up an array and looping (right now) isn't
+	// necessary because we just have one value, but we'll need the
+	// array/loop on linux so we might as well set it up so that we can
+	// test any number of home dirs later on and all we need to do is
+	// modify the array - the rest of the testing infrastructure should work
+	homes := []string{homedir.Get()}
 
-	home := homedir.Get()
-	helpCmd := exec.Command(dockerBinary, "help")
-	out, ec, err := runCommandWithOutput(helpCmd)
-	if err != nil || ec != 0 {
-		t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec)
+	// Non-Windows machines need to test for this special case of $HOME
+	if runtime.GOOS != "windows" {
+		homes = append(homes, "/")
 	}
-	lines := strings.Split(out, "\n")
-	for _, line := range lines {
-		if len(line) > 80 {
-			t.Fatalf("Line is too long(%d chars):\n%s", len(line), line)
 
-		}
-		if home != "" && strings.Contains(line, home) {
-			t.Fatalf("Line should use '%q' instead of %q:\n%s", homedir.GetShortcutString(), home, line)
-		}
-	}
-	logDone("help - verify main width")
-}
+	homeKey := homedir.Key()
+	baseEnvs := os.Environ()
 
-func TestCmdHelpWidth(t *testing.T) {
-	// Make sure main help text fits within 80 chars and that
-	// on non-windows system we use ~ when possible (to shorten things)
-
-	home := homedir.Get()
-	// Pull the list of commands from the "Commands:" section of docker help
-	helpCmd := exec.Command(dockerBinary, "help")
-	out, ec, err := runCommandWithOutput(helpCmd)
-	if err != nil || ec != 0 {
-		t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec)
-	}
-	i := strings.Index(out, "Commands:")
-	if i < 0 {
-		t.Fatalf("Missing 'Commands:' in:\n%s", out)
-	}
-
-	// Grab all chars starting at "Commands:"
-	// Skip first line, its "Commands:"
-	count := 0
-	cmds := ""
-	for _, command := range strings.Split(out[i:], "\n")[1:] {
-		// Stop on blank line or non-idented line
-		if command == "" || !unicode.IsSpace(rune(command[0])) {
+	// Remove HOME env var from list so we can add a new value later.
+	for i, env := range baseEnvs {
+		if strings.HasPrefix(env, homeKey+"=") {
+			baseEnvs = append(baseEnvs[:i], baseEnvs[i+1:]...)
 			break
 		}
+	}
 
-		// Grab just the first word of each line
-		command = strings.Split(strings.TrimSpace(command), " ")[0]
+	for _, home := range homes {
+		// Dup baseEnvs and add our new HOME value
+		newEnvs := make([]string, len(baseEnvs)+1)
+		copy(newEnvs, baseEnvs)
+		newEnvs[len(newEnvs)-1] = homeKey + "=" + home
 
-		count++
-		cmds = cmds + "\n" + command
+		scanForHome := runtime.GOOS != "windows" && home != "/"
 
-		helpCmd := exec.Command(dockerBinary, command, "--help")
+		// Check main help text to make sure its not over 80 chars
+		helpCmd := exec.Command(dockerBinary, "help")
+		helpCmd.Env = newEnvs
 		out, ec, err := runCommandWithOutput(helpCmd)
 		if err != nil || ec != 0 {
 			t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec)
@@ -72,19 +56,75 @@ func TestCmdHelpWidth(t *testing.T) {
 		lines := strings.Split(out, "\n")
 		for _, line := range lines {
 			if len(line) > 80 {
-				t.Fatalf("Help for %q is too long(%d chars):\n%s", command, len(line), line)
+				t.Fatalf("Line is too long(%d chars):\n%s", len(line), line)
 			}
-			if home != "" && strings.Contains(line, home) {
-				t.Fatalf("Help for %q should use home shortcut instead of %q on:\n%s", command, home, line)
+			if scanForHome && strings.Contains(line, `=`+home) {
+				t.Fatalf("Line should use '%q' instead of %q:\n%s", homedir.GetShortcutString(), home, line)
+			}
+			if runtime.GOOS != "windows" {
+				i := strings.Index(line, homedir.GetShortcutString())
+				if i >= 0 && i != len(line)-1 && line[i+1] != '/' {
+					t.Fatalf("Main help should not have used home shortcut:\n%s", line)
+				}
 			}
 		}
-	}
 
-	expected := 39
-	if count != expected {
-		t.Fatalf("Wrong # of commands (%d), it should be: %d\nThe list:\n%s",
-			len(cmds), expected, cmds)
+		// Make sure each cmd's help text fits within 80 chars and that
+		// on non-windows system we use ~ when possible (to shorten things).
+		// Pull the list of commands from the "Commands:" section of docker help
+		helpCmd = exec.Command(dockerBinary, "help")
+		helpCmd.Env = newEnvs
+		out, ec, err = runCommandWithOutput(helpCmd)
+		if err != nil || ec != 0 {
+			t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec)
+		}
+		i := strings.Index(out, "Commands:")
+		if i < 0 {
+			t.Fatalf("Missing 'Commands:' in:\n%s", out)
+		}
+
+		// Grab all chars starting at "Commands:"
+		// Skip first line, its "Commands:"
+		cmds := []string{}
+		for _, cmd := range strings.Split(out[i:], "\n")[1:] {
+			// Stop on blank line or non-idented line
+			if cmd == "" || !unicode.IsSpace(rune(cmd[0])) {
+				break
+			}
+
+			// Grab just the first word of each line
+			cmd = strings.Split(strings.TrimSpace(cmd), " ")[0]
+			cmds = append(cmds, cmd)
+
+			helpCmd := exec.Command(dockerBinary, cmd, "--help")
+			helpCmd.Env = newEnvs
+			out, ec, err := runCommandWithOutput(helpCmd)
+			if err != nil || ec != 0 {
+				t.Fatalf("Error on %q help: %s\nexit code:%d", cmd, out, ec)
+			}
+			lines := strings.Split(out, "\n")
+			for _, line := range lines {
+				if len(line) > 80 {
+					t.Fatalf("Help for %q is too long(%d chars):\n%s", cmd,
+						len(line), line)
+				}
+				if scanForHome && strings.Contains(line, `"`+home) {
+					t.Fatalf("Help for %q should use ~ instead of %q on:\n%s",
+						cmd, home, line)
+				}
+				i := strings.Index(line, "~")
+				if i >= 0 && i != len(line)-1 && line[i+1] != '/' {
+					t.Fatalf("Help for %q should not have used ~:\n%s", cmd, line)
+				}
+			}
+		}
+
+		expected := 39
+		if len(cmds) != expected {
+			t.Fatalf("Wrong # of cmds(%d), it should be: %d\nThe list:\n%q",
+				len(cmds), expected, cmds)
+		}
 	}
 
-	logDone("help - cmd widths")
+	logDone("help - widths")
 }

pkg/homedir/homedir.go

@@ -5,14 +5,20 @@ import (
 	"runtime"
 )
 
+// Key returns the env var name for the user's home dir based on
+// the platform being run on
+func Key() string {
+	if runtime.GOOS == "windows" {
+		return "USERPROFILE"
+	}
+	return "HOME"
+}
+
 // Get returns the home directory of the current user with the help of
 // environment variables depending on the target operating system.
 // Returned path should be used with "path/filepath" to form new paths.
 func Get() string {
-	if runtime.GOOS == "windows" {
-		return os.Getenv("USERPROFILE")
-	}
-	return os.Getenv("HOME")
+	return os.Getenv(Key())
 }
 
 // GetShortcutString returns the string that is shortcut to user's home directory

pkg/mflag/flag.go

@@ -86,6 +86,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -505,7 +506,16 @@ func Set(name, value string) error {
 // otherwise, the default values of all defined flags in the set.
 func (f *FlagSet) PrintDefaults() {
 	writer := tabwriter.NewWriter(f.Out(), 20, 1, 3, ' ', 0)
-	home := homedir.Get()
+	var home string
+	if runtime.GOOS != "windows" {
+		// Only do this on non-windows systems
+		home = homedir.Get()
+
+		// Don't substitute when HOME is /
+		if home == "/" {
+			home = ""
+		}
+	}
 	f.VisitAll(func(flag *Flag) {
 		format := "  -%s=%s"
 		names := []string{}