Skip to content

Commit 3c177dc

Browse files
author
John Howard
committed
Windows: Docker build starting to work
Signed-off-by: John Howard <[email protected]>
1 parent ba9db62 commit 3c177dc

16 files changed

Lines changed: 265 additions & 159 deletions

‎builder/dispatchers.go‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ package builder
1010
import (
1111
"fmt"
1212
"io/ioutil"
13+
"path"
1314
"path/filepath"
1415
"regexp"
1516
"runtime"
@@ -39,9 +40,6 @@ func nullDispatch(b *Builder, args []string, attributes map[string]bool, origina
3940
// in the dockerfile available from the next statement on via ${foo}.
4041
//
4142
func env(b *Builder, args []string, attributes map[string]bool, original string) error {
42-
if runtime.GOOS == "windows" {
43-
return fmt.Errorf("ENV is not supported on Windows.")
44-
}
4543
if len(args) == 0 {
4644
return fmt.Errorf("ENV requires at least one argument")
4745
}
@@ -269,12 +267,39 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
269267
return err
270268
}
271269

270+
// Note that workdir passed comes from the Dockerfile. Hence it is in
271+
// Linux format using forward-slashes, even on Windows. However,
272+
// b.Config.WorkingDir is in platform-specific notation (in other words
273+
// on Windows will use `\`
272274
workdir := args[0]
273275

274-
if !filepath.IsAbs(workdir) {
275-
workdir = filepath.Join("/", b.Config.WorkingDir, workdir)
276+
isAbs := false
277+
if runtime.GOOS == "windows" {
278+
// Alternate processing for Windows here is necessary as we can't call
279+
// filepath.IsAbs(workDir) as that would verify Windows style paths,
280+
// along with drive-letters (eg c:\pathto\file.txt). We (arguably
281+
// correctly or not) check for both forward and back slashes as this
282+
// is what the 1.4.2 GoLang implementation of IsAbs() does in the
283+
// isSlash() function.
284+
isAbs = workdir[0] == '\\' || workdir[0] == '/'
285+
} else {
286+
isAbs = filepath.IsAbs(workdir)
287+
}
288+
289+
if !isAbs {
290+
current := b.Config.WorkingDir
291+
if runtime.GOOS == "windows" {
292+
// Convert to Linux format before join
293+
current = strings.Replace(current, "\\", "/", -1)
294+
}
295+
// Must use path.Join so works correctly on Windows, not filepath
296+
workdir = path.Join("/", current, workdir)
276297
}
277298

299+
// Convert to platform specific format
300+
if runtime.GOOS == "windows" {
301+
workdir = strings.Replace(workdir, "/", "\\", -1)
302+
}
278303
b.Config.WorkingDir = workdir
279304

280305
return b.commit("", b.Config.Cmd, fmt.Sprintf("WORKDIR %v", workdir))

‎builder/internals.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,8 @@ func (b *Builder) processImageFrom(img *imagepkg.Image) error {
489489
b.Config = img.Config
490490
}
491491

492-
if len(b.Config.Env) == 0 {
492+
// The default path will be blank on Windows (set by HCS)
493+
if len(b.Config.Env) == 0 && daemon.DefaultPathEnv != "" {
493494
b.Config.Env = append(b.Config.Env, "PATH="+daemon.DefaultPathEnv)
494495
}
495496

‎daemon/container.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,11 @@ func (container *Container) LogEvent(action string) {
201201
// symlinking to a different path) between using this method and using the
202202
// path. See symlink.FollowSymlinkInScope for more details.
203203
func (container *Container) GetResourcePath(path string) (string, error) {
204-
cleanPath := filepath.Join("/", path)
205-
return symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs)
204+
// IMPORTANT - These are paths on the OS where the daemon is running, hence
205+
// any filepath operations must be done in an OS agnostic way.
206+
cleanPath := filepath.Join(string(os.PathSeparator), path)
207+
r, e := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs)
208+
return r, e
206209
}
207210

208211
// Evaluates `path` in the scope of the container's root, with proper path
@@ -218,7 +221,9 @@ func (container *Container) GetResourcePath(path string) (string, error) {
218221
// symlinking to a different path) between using this method and using the
219222
// path. See symlink.FollowSymlinkInScope for more details.
220223
func (container *Container) GetRootResourcePath(path string) (string, error) {
221-
cleanPath := filepath.Join("/", path)
224+
// IMPORTANT - These are paths on the OS where the daemon is running, hence
225+
// any filepath operations must be done in an OS agnostic way.
226+
cleanPath := filepath.Join(string(os.PathSeparator), path)
222227
return symlink.FollowSymlinkInScope(filepath.Join(container.root, cleanPath), container.root)
223228
}
224229

‎daemon/container_windows.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import (
1010
"github.com/docker/docker/pkg/archive"
1111
)
1212

13-
// TODO Windows. A reasonable default at the moment.
14-
const DefaultPathEnv = `c:\windows\system32;c:\windows\system32\WindowsPowerShell\v1.0`
13+
// This is deliberately empty on Windows as the default path will be set by
14+
// the container. Docker has no context of what the default path should be.
15+
const DefaultPathEnv = ""
1516

1617
type Container struct {
1718
CommonContainer
@@ -48,7 +49,8 @@ func (container *Container) setupLinkedContainers() ([]string, error) {
4849
}
4950

5051
func (container *Container) createDaemonEnvironment(linkedEnv []string) []string {
51-
return nil
52+
// On Windows, nothing to link. Just return the container environment.
53+
return container.Config.Env
5254
}
5355

5456
func (container *Container) initializeNetworking() error {

‎daemon/volumes.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ func (daemon *Daemon) registerMountPoints(container *Container, hostConfig *runc
258258
return nil
259259
}
260260

261+
// TODO Windows. Factor out as not relevant (as Windows daemon support not in pre-1.7)
261262
// verifyVolumesInfo ports volumes configured for the containers pre docker 1.7.
262263
// It reads the container configuration and creates valid mount points for the old volumes.
263264
func (daemon *Daemon) verifyVolumesInfo(container *Container) error {

‎pkg/archive/archive.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
452452
}
453453
seen[relFilePath] = true
454454

455+
// TODO Windows: Verify if this needs to be os.Pathseparator
455456
// Rename the base resource
456457
if options.Name != "" && filePath == srcPath+"/"+filepath.Base(relFilePath) {
457458
renamedRelFilePath = relFilePath
@@ -503,7 +504,8 @@ loop:
503504
}
504505

505506
// Normalize name, for safety and for a simple is-root check
506-
// This keeps "../" as-is, but normalizes "/../" to "/"
507+
// This keeps "../" as-is, but normalizes "/../" to "/". Or Windows:
508+
// This keeps "..\" as-is, but normalizes "\..\" to "\".
507509
hdr.Name = filepath.Clean(hdr.Name)
508510

509511
for _, exclude := range options.ExcludePatterns {
@@ -512,7 +514,10 @@ loop:
512514
}
513515
}
514516

515-
if !strings.HasSuffix(hdr.Name, "/") {
517+
// After calling filepath.Clean(hdr.Name) above, hdr.Name will now be in
518+
// the filepath format for the OS on which the daemon is running. Hence
519+
// the check for a slash-suffix MUST be done in an OS-agnostic way.
520+
if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) {
516521
// Not the root directory, ensure that the parent directory exists
517522
parent := filepath.Dir(hdr.Name)
518523
parentPath := filepath.Join(dest, parent)
@@ -529,7 +534,7 @@ loop:
529534
if err != nil {
530535
return err
531536
}
532-
if strings.HasPrefix(rel, "../") {
537+
if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
533538
return breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest))
534539
}
535540

@@ -658,10 +663,13 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
658663
if err != nil {
659664
return err
660665
}
666+
661667
if srcSt.IsDir() {
662668
return fmt.Errorf("Can't copy a directory")
663669
}
664-
// Clean up the trailing slash
670+
671+
// Clean up the trailing slash. This must be done in an operating
672+
// system specific manner.
665673
if dst[len(dst)-1] == os.PathSeparator {
666674
dst = filepath.Join(dst, filepath.Base(src))
667675
}
@@ -709,8 +717,10 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
709717
// for a single file. It copies a regular file from path `src` to
710718
// path `dst`, and preserves all its metadata.
711719
//
712-
// If `dst` ends with a trailing slash '/', the final destination path
713-
// will be `dst/base(src)`.
720+
// Destination handling is in an operating specific manner depending
721+
// where the daemon is running. If `dst` ends with a trailing slash
722+
// the final destination path will be `dst/base(src)` (Linux) or
723+
// `dst\base(src)` (Windows).
714724
func CopyFileWithTar(src, dst string) (err error) {
715725
return defaultArchiver.CopyFileWithTar(src, dst)
716726
}

‎pkg/archive/changes.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,17 @@ func Changes(layers []string, rw string) ([]Change, error) {
8484
if err != nil {
8585
return err
8686
}
87-
path = filepath.Join("/", path)
87+
88+
// As this runs on the daemon side, file paths are OS specific.
89+
path = filepath.Join(string(os.PathSeparator), path)
8890

8991
// Skip root
90-
if path == "/" {
92+
if path == string(os.PathSeparator) {
9193
return nil
9294
}
9395

9496
// Skip AUFS metadata
95-
if matched, err := filepath.Match("/.wh..wh.*", path); err != nil || matched {
97+
if matched, err := filepath.Match(string(os.PathSeparator)+".wh..wh.*", path); err != nil || matched {
9698
return err
9799
}
98100

@@ -169,12 +171,13 @@ type FileInfo struct {
169171
}
170172

171173
func (root *FileInfo) LookUp(path string) *FileInfo {
174+
// As this runs on the daemon side, file paths are OS specific.
172175
parent := root
173-
if path == "/" {
176+
if path == string(os.PathSeparator) {
174177
return root
175178
}
176179

177-
pathElements := strings.Split(path, "/")
180+
pathElements := strings.Split(path, string(os.PathSeparator))
178181
for _, elem := range pathElements {
179182
if elem != "" {
180183
child := parent.children[elem]
@@ -189,7 +192,8 @@ func (root *FileInfo) LookUp(path string) *FileInfo {
189192

190193
func (info *FileInfo) path() string {
191194
if info.parent == nil {
192-
return "/"
195+
// As this runs on the daemon side, file paths are OS specific.
196+
return string(os.PathSeparator)
193197
}
194198
return filepath.Join(info.parent.path(), info.name)
195199
}
@@ -257,7 +261,8 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
257261

258262
// If there were changes inside this directory, we need to add it, even if the directory
259263
// itself wasn't changed. This is needed to properly save and restore filesystem permissions.
260-
if len(*changes) > sizeAtEntry && info.isDir() && !info.added && info.path() != "/" {
264+
// As this runs on the daemon side, file paths are OS specific.
265+
if len(*changes) > sizeAtEntry && info.isDir() && !info.added && info.path() != string(os.PathSeparator) {
261266
change := Change{
262267
Path: info.path(),
263268
Kind: ChangeModify,
@@ -279,8 +284,9 @@ func (info *FileInfo) Changes(oldInfo *FileInfo) []Change {
279284
}
280285

281286
func newRootFileInfo() *FileInfo {
287+
// As this runs on the daemon side, file paths are OS specific.
282288
root := &FileInfo{
283-
name: "/",
289+
name: string(os.PathSeparator),
284290
children: make(map[string]*FileInfo),
285291
}
286292
return root

‎pkg/archive/changes_other.go‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"fmt"
77
"os"
88
"path/filepath"
9+
"runtime"
10+
"strings"
911

1012
"github.com/docker/docker/pkg/system"
1113
)
@@ -48,9 +50,20 @@ func collectFileInfo(sourceDir string) (*FileInfo, error) {
4850
if err != nil {
4951
return err
5052
}
51-
relPath = filepath.Join("/", relPath)
5253

53-
if relPath == "/" {
54+
// As this runs on the daemon side, file paths are OS specific.
55+
relPath = filepath.Join(string(os.PathSeparator), relPath)
56+
57+
// See https://github.com/golang/go/issues/9168 - bug in filepath.Join.
58+
// Temporary workaround. If the returned path starts with two backslashes,
59+
// trim it down to a single backslash. Only relevant on Windows.
60+
if runtime.GOOS == "windows" {
61+
if strings.HasPrefix(relPath, `\\`) {
62+
relPath = relPath[1:]
63+
}
64+
}
65+
66+
if relPath == string(os.PathSeparator) {
5467
return nil
5568
}
5669

‎pkg/archive/diff.go‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import (
77
"io/ioutil"
88
"os"
99
"path/filepath"
10+
"runtime"
1011
"strings"
1112
"syscall"
1213

14+
"github.com/Sirupsen/logrus"
1315
"github.com/docker/docker/pkg/pools"
1416
"github.com/docker/docker/pkg/system"
1517
)
@@ -40,12 +42,35 @@ func UnpackLayer(dest string, layer ArchiveReader) (size int64, err error) {
4042
// Normalize name, for safety and for a simple is-root check
4143
hdr.Name = filepath.Clean(hdr.Name)
4244

43-
if !strings.HasSuffix(hdr.Name, "/") {
45+
// Windows does not support filenames with colons in them. Ignore
46+
// these files. This is not a problem though (although it might
47+
// appear that it is). Let's suppose a client is running docker pull.
48+
// The daemon it points to is Windows. Would it make sense for the
49+
// client to be doing a docker pull Ubuntu for example (which has files
50+
// with colons in the name under /usr/share/man/man3)? No, absolutely
51+
// not as it would really only make sense that they were pulling a
52+
// Windows image. However, for development, it is necessary to be able
53+
// to pull Linux images which are in the repository.
54+
//
55+
// TODO Windows. Once the registry is aware of what images are Windows-
56+
// specific or Linux-specific, this warning should be changed to an error
57+
// to cater for the situation where someone does manage to upload a Linux
58+
// image but have it tagged as Windows inadvertantly.
59+
if runtime.GOOS == "windows" {
60+
if strings.Contains(hdr.Name, ":") {
61+
logrus.Warnf("Windows: Ignoring %s (is this a Linux image?)", hdr.Name)
62+
continue
63+
}
64+
}
65+
66+
// Note as these operations are platform specific, so must the slash be.
67+
if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) {
4468
// Not the root directory, ensure that the parent directory exists.
4569
// This happened in some tests where an image had a tarfile without any
4670
// parent directories.
4771
parent := filepath.Dir(hdr.Name)
4872
parentPath := filepath.Join(dest, parent)
73+
4974
if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
5075
err = system.MkdirAll(parentPath, 0600)
5176
if err != nil {
@@ -74,13 +99,14 @@ func UnpackLayer(dest string, layer ArchiveReader) (size int64, err error) {
7499
}
75100
continue
76101
}
77-
78102
path := filepath.Join(dest, hdr.Name)
79103
rel, err := filepath.Rel(dest, path)
80104
if err != nil {
81105
return 0, err
82106
}
83-
if strings.HasPrefix(rel, "../") {
107+
108+
// Note as these operations are platform specific, so must the slash be.
109+
if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
84110
return 0, breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest))
85111
}
86112
base := filepath.Base(path)

0 commit comments

Comments
 (0)