beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #22462 from Microsoft/jjh/22181unittests

Browse source 
Windows: Reduce CLI time, move some to unit tests
This commit is contained in:
Sebastiaan van Stijn 2016-05-13 00:27:29 +02:00
commit 9de21de453
9 changed files with 248 additions and 260 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
builder/dockerfile/dispatchers.go
View file

@ -9,8 +9,6 @@ package dockerfile
import (
"fmt"
"os"
"path/filepath"
"regexp"
"runtime"
"sort"
@ -20,7 +18,6 @@ import (
"github.com/docker/docker/api"
"github.com/docker/docker/builder"
"github.com/docker/docker/pkg/signal"
"github.com/docker/docker/pkg/system"
runconfigopts "github.com/docker/docker/runconfig/opts"
"github.com/docker/engine-api/types/container"
"github.com/docker/engine-api/types/strslice"
@ -257,45 +254,17 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
return errExactlyOneArgument("WORKDIR")
}
if err := b.flags.Parse(); err != nil {
err := b.flags.Parse()
if err != nil {
return err
}
// This is from the Dockerfile and will not necessarily be in platform
// specific semantics, hence ensure it is converted.
workdir := filepath.FromSlash(args[0])
current := filepath.FromSlash(b.runConfig.WorkingDir)
if runtime.GOOS == "windows" {
// Windows is a little more complicated than Linux. This code ensures
// we end up with a workdir which is consistent in terms of platform
// semantics. This means C:\somefolder, specifically in the format:
// UPPERCASEDriveLetter-Colon-Backslash-FolderName. We are already
// guaranteed that `current`, if set, is consistent. This allows us to
// cope correctly with any of the following in a Dockerfile:
// WORKDIR a --> C:\a
// WORKDIR c:\\foo --> C:\foo
// WORKDIR \\foo --> C:\foo
// WORKDIR /foo --> C:\foo
// WORKDIR c:\\foo \ WORKDIR bar --> C:\foo --> C:\foo\bar
// WORKDIR C:/foo \ WORKDIR bar --> C:\foo --> C:\foo\bar
// WORKDIR C:/foo \ WORKDIR \\bar --> C:\foo --> C:\bar
// WORKDIR /foo \ WORKDIR c:/bar --> C:\foo --> C:\bar
if len(current) == 0 || system.IsAbs(workdir) {
if (workdir[0] == os.PathSeparator) ||
(len(workdir) > 1 && string(workdir[1]) != ":") ||
(len(workdir) == 1) {
workdir = filepath.Join(`C:\`, workdir)
}
} else {
workdir = filepath.Join(current, workdir)
}
workdir = strings.ToUpper(string(workdir[0])) + workdir[1:] // Upper-case drive letter
} else {
if !filepath.IsAbs(workdir) {
workdir = filepath.Join(string(os.PathSeparator), current, workdir)
}
b.runConfig.WorkingDir, err = normaliseWorkdir(b.runConfig.WorkingDir, args[0])
if err != nil {
return err
}
b.runConfig.WorkingDir = workdir
return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", workdir))
}

23
builder/dockerfile/dispatchers_unix.go Normal file
View file

@ -0,0 +1,23 @@
// +build !windows
package dockerfile
import (
"fmt"
"os"
"path/filepath"
)
// normaliseWorkdir normalises a user requested working directory in a
// platform sematically consistent way.
func normaliseWorkdir(current string, requested string) (string, error) {
if requested == "" {
return "", fmt.Errorf("cannot normalise nothing")
}
current = filepath.FromSlash(current)
requested = filepath.FromSlash(requested)
if !filepath.IsAbs(requested) {
return filepath.Join(string(os.PathSeparator), current, requested), nil
}
return requested, nil
}

45
builder/dockerfile/dispatchers_windows.go Normal file
View file

@ -0,0 +1,45 @@
package dockerfile
import (
"fmt"
"os"
"path/filepath"
"strings"
"github.com/docker/docker/pkg/system"
)
// normaliseWorkdir normalises a user requested working directory in a
// platform sematically consistent way.
func normaliseWorkdir(current string, requested string) (string, error) {
if requested == "" {
return "", fmt.Errorf("cannot normalise nothing")
}
current = filepath.FromSlash(current)
requested = filepath.FromSlash(requested)
// Target semantics is C:\somefolder, specifically in the format:
// UPPERCASEDriveLetter-Colon-Backslash-FolderName. We are already
// guaranteed that `current`, if set, is consistent. This allows us to
// cope correctly with any of the following in a Dockerfile:
// WORKDIR a --> C:\a
// WORKDIR c:\\foo --> C:\foo
// WORKDIR \\foo --> C:\foo
// WORKDIR /foo --> C:\foo
// WORKDIR c:\\foo \ WORKDIR bar --> C:\foo --> C:\foo\bar
// WORKDIR C:/foo \ WORKDIR bar --> C:\foo --> C:\foo\bar
// WORKDIR C:/foo \ WORKDIR \\bar --> C:\foo --> C:\bar
// WORKDIR /foo \ WORKDIR c:/bar --> C:\foo --> C:\bar
if len(current) == 0 || system.IsAbs(requested) {
if (requested[0] == os.PathSeparator) ||
(len(requested) > 1 && string(requested[1]) != ":") ||
(len(requested) == 1) {
requested = filepath.Join(`C:\`, requested)
}
} else {
requested = filepath.Join(current, requested)
}
// Upper-case drive letter
return (strings.ToUpper(string(requested[0])) + requested[1:]), nil
}

34
builder/dockerfile/dispatchers_windows_test.go Normal file
View file

@ -0,0 +1,34 @@
// +build windows
package dockerfile
import "testing"
func TestNormaliseWorkdir(t *testing.T) {
tests := []struct{ current, requested, expected, etext string }{
{``, ``, ``, `cannot normalise nothing`},
{``, `a`, `C:\a`, ``},
{``, `c:\foo`, `C:\foo`, ``},
{``, `\foo`, `C:\foo`, ``},
{``, `/foo`, `C:\foo`, ``},
{``, `C:/foo`, `C:\foo`, ``},
{`C:\foo`, `bar`, `C:\foo\bar`, ``},
{`C:\foo`, `/bar`, `C:\bar`, ``},
{`C:\foo`, `\bar`, `C:\bar`, ``},
}
for _, i := range tests {
r, e := normaliseWorkdir(i.current, i.requested)
if i.etext != "" && e == nil {
t.Fatalf("TestNormaliseWorkingDir Expected error %s", i.etext)
}
if i.etext != "" && e.Error() != i.etext {
t.Fatalf("TestNormaliseWorkingDir Expected error %s, got %s", i.etext, e.Error())
}
if r != i.expected {
t.Fatalf("TestNormaliseWorkingDir Expected %s for %s %s", i.expected, i.current, i.requested)
}
}
}

56
builder/dockerfile/internals.go
View file

@ -200,60 +200,8 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
// Twiddle the destination when its a relative path - meaning, make it
// relative to the WORKINGDIR
endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator))
if runtime.GOOS == "windows" {
// On Windows, this is more complicated. We are guaranteed that the
// WorkingDir is already platform consistent meaning in the format
// UPPERCASEDriveLetter-Colon-Backslash-Foldername. However, Windows
// for now also has the limitation that ADD/COPY can only be done to
// the C: (system) drive, not any drives that might be present as a
// result of bind mounts.
//
// So... if the path specified is Linux-style absolute (/foo or \\foo),
// we assume it is the system drive. If it is a Windows-style absolute
// (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we
// strip any configured working directories drive letter so that it
// can be subsequently legitimately converted to a Windows volume-style
// pathname.
// Not a typo - filepath.IsAbs, not system.IsAbs on this next check as
// we only want to validate where the DriveColon part has been supplied.
if filepath.IsAbs(dest) {
if strings.ToUpper(string(dest[0])) != "C" {
return fmt.Errorf("Windows does not support %s with a destinations not on the system drive (C:)", cmdName)
}
dest = dest[2:] // Strip the drive letter
}
// Cannot handle relative where WorkingDir is not the system drive.
if len(b.runConfig.WorkingDir) > 0 {
if !system.IsAbs(b.runConfig.WorkingDir[2:]) {
return fmt.Errorf("Current WorkingDir %s is not platform consistent", b.runConfig.WorkingDir)
}
if !system.IsAbs(dest) {
if string(b.runConfig.WorkingDir[0]) != "C" {
return fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName)
}
dest = filepath.Join(string(os.PathSeparator), b.runConfig.WorkingDir[2:], dest)
// Make sure we preserve any trailing slash
if endsInSlash {
dest += string(os.PathSeparator)
}
}
}
} else {
if !system.IsAbs(dest) {
dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(b.runConfig.WorkingDir), dest)
// Make sure we preserve any trailing slash
if endsInSlash {
dest += string(os.PathSeparator)
}
}
if dest, err = normaliseDest(cmdName, b.runConfig.WorkingDir, dest); err != nil {
return err
}
for _, info := range infos {

26
builder/dockerfile/internals_unix.go Normal file
View file

@ -0,0 +1,26 @@
// +build !windows
package dockerfile
import (
"os"
"path/filepath"
"strings"
"github.com/docker/docker/pkg/system"
)
// normaliseDest normalises the destination of a COPY/ADD command in a
// platform semantically consistent way.
func normaliseDest(cmdName, workingDir, requested string) (string, error) {
dest := filepath.FromSlash(requested)
endsInSlash := strings.HasSuffix(requested, string(os.PathSeparator))
if !system.IsAbs(requested) {
dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(workingDir), dest)
// Make sure we preserve any trailing slash
if endsInSlash {
dest += string(os.PathSeparator)
}
}
return dest, nil
}

56
builder/dockerfile/internals_windows.go Normal file
View file

@ -0,0 +1,56 @@
package dockerfile
import (
"fmt"
"os"
"path/filepath"
"strings"
"github.com/docker/docker/pkg/system"
)
// normaliseDest normalises the destination of a COPY/ADD command in a
// platform semantically consistent way.
func normaliseDest(cmdName, workingDir, requested string) (string, error) {
dest := filepath.FromSlash(requested)
endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator))
// We are guaranteed that the working directory is already consistent,
// However, Windows also has, for now, the limitation that ADD/COPY can
// only be done to the system drive, not any drives that might be present
// as a result of a bind mount.
//
// So... if the path requested is Linux-style absolute (/foo or \\foo),
// we assume it is the system drive. If it is a Windows-style absolute
// (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we
// strip any configured working directories drive letter so that it
// can be subsequently legitimately converted to a Windows volume-style
// pathname.
// Not a typo - filepath.IsAbs, not system.IsAbs on this next check as
// we only want to validate where the DriveColon part has been supplied.
if filepath.IsAbs(dest) {
if strings.ToUpper(string(dest[0])) != "C" {
return "", fmt.Errorf("Windows does not support %s with a destinations not on the system drive (C:)", cmdName)
}
dest = dest[2:] // Strip the drive letter
}
// Cannot handle relative where WorkingDir is not the system drive.
if len(workingDir) > 0 {
if ((len(workingDir) > 1) && !system.IsAbs(workingDir[2:])) || (len(workingDir) == 1) {
return "", fmt.Errorf("Current WorkingDir %s is not platform consistent", workingDir)
}
if !system.IsAbs(dest) {
if string(workingDir[0]) != "C" {
return "", fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName)
}
dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest)
// Make sure we preserve any trailing slash
if endsInSlash {
dest += string(os.PathSeparator)
}
}
}
return dest, nil
}

51
builder/dockerfile/internals_windows_test.go Normal file
View file

@ -0,0 +1,51 @@
// +build windows
package dockerfile
import "testing"
func TestNormaliseDest(t *testing.T) {
tests := []struct{ current, requested, expected, etext string }{
{``, `D:\`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`},
{``, `e:/`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`},
{`invalid`, `./c1`, ``, `Current WorkingDir invalid is not platform consistent`},
{`C:`, ``, ``, `Current WorkingDir C: is not platform consistent`},
{`C`, ``, ``, `Current WorkingDir C is not platform consistent`},
{`D:\`, `.`, ``, "Windows does not support TEST with relative paths when WORKDIR is not the system drive"},
{``, `D`, `D`, ``},
{``, `./a1`, `.\a1`, ``},
{``, `.\b1`, `.\b1`, ``},
{``, `/`, `\`, ``},
{``, `\`, `\`, ``},
{``, `c:/`, `\`, ``},
{``, `c:\`, `\`, ``},
{``, `.`, `.`, ``},
{`C:\wdd`, `./a1`, `\wdd\a1`, ``},
{`C:\wde`, `.\b1`, `\wde\b1`, ``},
{`C:\wdf`, `/`, `\`, ``},
{`C:\wdg`, `\`, `\`, ``},
{`C:\wdh`, `c:/`, `\`, ``},
{`C:\wdi`, `c:\`, `\`, ``},
{`C:\wdj`, `.`, `\wdj`, ``},
{`C:\wdk`, `foo/bar`, `\wdk\foo\bar`, ``},
{`C:\wdl`, `foo\bar`, `\wdl\foo\bar`, ``},
{`C:\wdm`, `foo/bar/`, `\wdm\foo\bar\`, ``},
{`C:\wdn`, `foo\bar/`, `\wdn\foo\bar\`, ``},
}
for _, i := range tests {
got, err := normaliseDest("TEST", i.current, i.requested)
if err != nil && i.etext == "" {
t.Fatalf("TestNormaliseDest Got unexpected error %q for %s %s. ", err.Error(), i.current, i.requested)
}
if i.etext != "" && ((err == nil) || (err != nil && err.Error() != i.etext)) {
if err == nil {
t.Fatalf("TestNormaliseDest Expected an error for %s %s but didn't get one", i.current, i.requested)
} else {
t.Fatalf("TestNormaliseDest Wrong error text for %s %s - %s", i.current, i.requested, err.Error())
}
}
if i.etext == "" && got != i.expected {
t.Fatalf("TestNormaliseDest Expected %q for %q and %q. Got %q", i.expected, i.current, i.requested, got)
}
}
}

176
integration-cli/docker_cli_build_test.go
View file

@ -2089,33 +2089,16 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) {
}
}
// #22181 Regression test. Validates combinations of supported
// WORKDIR dockerfile directives in Windows and non-Windows semantics.
// #22181 Regression test. Single end-to-end test of using
// Windows semantics. Most path handling verifications are in unit tests
func (s *DockerSuite) TestBuildWindowsWorkdirProcessing(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowsworkdirprocessing"
_, err := buildImage(name,
`FROM busybox
WORKDIR a
RUN sh -c "[ "$PWD" = "C:/a" ]"
WORKDIR c:\\foo
RUN sh -c "[ "$PWD" = "C:/foo" ]"
WORKDIR \\foo
RUN sh -c "[ "$PWD" = "C:/foo" ]"
WORKDIR /foo
RUN sh -c "[ "$PWD" = "C:/foo" ]"
WORKDIR C:/foo
WORKDIR C:\\foo
WORKDIR bar
RUN sh -c "[ "$PWD" = "C:/foo/bar" ]"
WORKDIR c:/foo
WORKDIR bar
RUN sh -c "[ "$PWD" = "C:/foo/bar" ]"
WORKDIR c:/foo
WORKDIR \\bar
RUN sh -c "[ "$PWD" = "C:/bar" ]"
WORKDIR /foo
WORKDIR c:\\bar
RUN sh -c "[ "$PWD" = "C:/bar" ]"
`,
true)
if err != nil {
@ -2123,8 +2106,8 @@ func (s *DockerSuite) TestBuildWindowsWorkdirProcessing(c *check.C) {
}
}
// #22181 Regression test. Validates combinations of supported
// COPY dockerfile directives in Windows and non-Windows semantics.
// #22181 Regression test. Most paths handling verifications are in unit test.
// One functional test for end-to-end
func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowsaddcopypathprocessing"
@ -2134,50 +2117,7 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
// by docker on the Windows platform.
dockerfile := `
FROM busybox
# First cases with no workdir, all end up in the root directory of the system drive
COPY a1 ./
ADD a2 ./
RUN sh -c "[ $(cat c:/a1) = 'helloa1' ]"
RUN sh -c "[ $(cat c:/a2) = 'worlda2' ]"
COPY b1 /
ADD b2 /
RUN sh -c "[ $(cat c:/b1) = 'hellob1' ]"
RUN sh -c "[ $(cat c:/b2) = 'worldb2' ]"
COPY c1 c:/
ADD c2 c:/
RUN sh -c "[ $(cat c:/c1) = 'helloc1' ]"
RUN sh -c "[ $(cat c:/c2) = 'worldc2' ]"
COPY d1 c:/
ADD d2 c:/
RUN sh -c "[ $(cat c:/d1) = 'hellod1' ]"
RUN sh -c "[ $(cat c:/d2) = 'worldd2' ]"
COPY e1 .
ADD e2 .
RUN sh -c "[ $(cat c:/e1) = 'helloe1' ]"
RUN sh -c "[ $(cat c:/e2) = 'worlde2' ]"
# Now with a workdir
WORKDIR c:\\wa12
COPY wa1 ./
ADD wa2 ./
RUN sh -c "[ $(cat c:/wa12/wa1) = 'hellowa1' ]"
RUN sh -c "[ $(cat c:/wa12/wa2) = 'worldwa2' ]"
# No trailing slash on COPY/ADD, Linux-style path.
# Results in dir being changed to a file
WORKDIR /wb1
COPY wb1 .
WORKDIR /wb2
ADD wb2 .
WORKDIR c:/
RUN sh -c "[ $(cat c:/wb1) = 'hellowb1' ]"
RUN sh -c "[ $(cat c:/wb2) = 'worldwb2' ]"
# No trailing slash on COPY/ADD, Windows-style path.
# No trailing slash on COPY/ADD
# Results in dir being changed to a file
WORKDIR /wc1
COPY wc1 c:/wc1
@ -2196,20 +2136,6 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
RUN sh -c "[ $(cat c:/wd2/wd2) = 'worldwd2' ]"
`
ctx, err := fakeContext(dockerfile, map[string]string{
"a1": "helloa1",
"a2": "worlda2",
"b1": "hellob1",
"b2": "worldb2",
"c1": "helloc1",
"c2": "worldc2",
"d1": "hellod1",
"d2": "worldd2",
"e1": "helloe1",
"e2": "worlde2",
"wa1": "hellowa1",
"wa2": "worldwa2",
"wb1": "hellowb1",
"wb2": "worldwb2",
"wc1": "hellowc1",
"wc2": "worldwc2",
"wd1": "hellowd1",
@ -2225,96 +2151,6 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
}
}
// #22181 Regression test.
func (s *DockerSuite) TestBuildWindowsCopyFailsNonSystemDrive(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowscopyfailsnonsystemdrive"
dockerfile := `
FROM busybox
cOpY foo d:/
`
ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
if err != nil {
c.Fatal(err)
}
defer ctx.Close()
_, err = buildImageFromContext(name, ctx, false)
if err == nil {
c.Fatal(err)
}
if !strings.Contains(err.Error(), "Windows does not support COPY with a destinations not on the system drive (C:)") {
c.Fatal(err)
}
}
// #22181 Regression test.
func (s *DockerSuite) TestBuildWindowsCopyFailsWorkdirNonSystemDrive(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowscopyfailsworkdirsystemdrive"
dockerfile := `
FROM busybox
WORKDIR d:/
cOpY foo .
`
ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
if err != nil {
c.Fatal(err)
}
defer ctx.Close()
_, err = buildImageFromContext(name, ctx, false)
if err == nil {
c.Fatal(err)
}
if !strings.Contains(err.Error(), "Windows does not support COPY with relative paths when WORKDIR is not the system drive") {
c.Fatal(err)
}
}
// #22181 Regression test.
func (s *DockerSuite) TestBuildWindowsAddFailsNonSystemDrive(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowsaddfailsnonsystemdrive"
dockerfile := `
FROM busybox
AdD foo d:/
`
ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
if err != nil {
c.Fatal(err)
}
defer ctx.Close()
_, err = buildImageFromContext(name, ctx, false)
if err == nil {
c.Fatal(err)
}
if !strings.Contains(err.Error(), "Windows does not support ADD with a destinations not on the system drive (C:)") {
c.Fatal(err)
}
}
// #22181 Regression test.
func (s *DockerSuite) TestBuildWindowsAddFailsWorkdirNonSystemDrive(c *check.C) {
testRequires(c, DaemonIsWindows)
name := "testbuildwindowsaddfailsworkdirsystemdrive"
dockerfile := `
FROM busybox
WORKDIR d:/
AdD foo .
`
ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
if err != nil {
c.Fatal(err)
}
defer ctx.Close()
_, err = buildImageFromContext(name, ctx, false)
if err == nil {
c.Fatal(err)
}
if !strings.Contains(err.Error(), "Windows does not support ADD with relative paths when WORKDIR is not the system drive") {
c.Fatal(err)
}
}
func (s *DockerSuite) TestBuildWorkdirWithEnvVariables(c *check.C) {
name := "testbuildworkdirwithenvvariables"