COPY command #40
COPY command #40
Conversation
priyawadhwa
force-pushed
the
copy-cmd
branch
6 times, most recently
from
0bb35a9 to
f15b63a
Compare
pkg/util/command_util.go
Outdated
| // ContainsWildcards returns true if any entry in paths contains wildcards | ||
| func ContainsWildcards(paths []string) bool { | ||
| for _, path := range paths { | ||
| for i := 0; i < len(path); i++ { |
There was a problem hiding this comment.
You can probably use indexFunc here instead of a for loop:
https://golang.org/pkg/strings/#IndexFunc
pkg/util/command_util.go
Outdated
| srcs := srcsAndDest[:len(srcsAndDest)-1] | ||
| dest := srcsAndDest[len(srcsAndDest)-1] | ||
| // If destination is a directory, return nil | ||
| if IsDestDir(dest) { |
There was a problem hiding this comment.
This makes it seem like it's ok to copy no source files if the destination is a directory. Is that actually OK?
There was a problem hiding this comment.
whoops, nope, fixed!
pkg/util/fs_util.go
Outdated
| baseDir := filepath.Dir(path) | ||
| if _, err := os.Stat(baseDir); os.IsNotExist(err) { | ||
| logrus.Debugf("baseDir %s for file %s does not exist. Creating.", baseDir, path) | ||
| if err := os.MkdirAll(baseDir, perm); err != nil { |
There was a problem hiding this comment.
It looks like the perm passed in applies to the directory, not the file. Is that intentional?
There was a problem hiding this comment.
fixed as well, it should be applied to just the file
pkg/util/command_util.go
Outdated
| // If source is a dir: | ||
| // Assume dest is also a dir, and copy to dest/relpath | ||
| // If dest is not an absolute filepath, add /cwd to the beginning | ||
| func RelativeFilepath(filename, srcName, dest, cwd, buildcontext string) (string, error) { |
There was a problem hiding this comment.
This name is a bit weird since it returns absolute paths right?
pkg/util/command_util.go
Outdated
| return strings.HasSuffix(path, "/") | ||
| } | ||
|
|
||
| func IsAbsoluteFilepath(path string) bool { |
There was a problem hiding this comment.
pkg/commands/copy.go
Outdated
| } else { | ||
| // ... Else, we want to copy over a file | ||
| logrus.Infof("Copying file %s to %s", file, destPath) | ||
| contents, err := ioutil.ReadFile(filepath.Join(c.buildcontext, file)) |
There was a problem hiding this comment.
The files are all local, right? You might be able to use an os.Copy, instead of buffering the file into memory then writing it back.
pkg/commands/copy.go
Outdated
| // FilesToSnapshot should return an empty array if still nil; no files were changed | ||
| func (c *CopyCommand) FilesToSnapshot() []string { | ||
| if c.snapshotFiles == nil { | ||
| return []string{} |
There was a problem hiding this comment.
You might just be able to return c.snapshotFiles, for most things an empty slice is the same as nil.
pkg/util/fs_util.go
Outdated
| // FilepathExists returns true if the path exists | ||
| func FilepathExists(path string) bool { | ||
| _, err := os.Stat(path) | ||
| return (err == nil) |
There was a problem hiding this comment.
I think this should technically be
!os.IsNotExist(err)
stat could fail for another reason.
pkg/util/fs_util.go
Outdated
| } | ||
|
|
||
| // CreateFile creates a file at path with contents specified | ||
| func CreateFile(path string, contents []byte, perm os.FileMode) error { |
There was a problem hiding this comment.
I think you can replace some of this function with iotuil.WriteFile, but you might not need it after switching to os.Copy above.
There was a problem hiding this comment.
I ended up replacing with ioutil.WriteFile since the directories still need to be created if they don't already exist
There was a problem hiding this comment.
Just kidding, ended up using io.Copy and passing in a Reader instead of contents :)
priyawadhwa
added
the
area/dockerfile-command
#38
The COPY command copies from sources to a specified destination. The sources are relative to a directory (which I call buildcontext in the command), and which should already be unpacked by the time the command is called.