Skip to content
This repository has been archived by the owner on May 4, 2021. It is now read-only.

Commit

Permalink
Revert "Make ADD untar tar.gz files (#309)" (#311)
Browse files Browse the repository at this point in the history
This reverts commit f4c2bf0.
  • Loading branch information
Yiran Wang authored Mar 12, 2020
1 parent f4c2bf0 commit d94239b
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 159 deletions.
10 changes: 3 additions & 7 deletions lib/builder/step/add_copy_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,12 @@ type addCopyStep struct {
toPath string
chown string
preserveOwner bool
isAdd bool
}

// newAddCopyStep returns a BuildStep from given arguments.
func newAddCopyStep(
directive Directive, args, chown, fromStage string,
fromPaths []string, toPath string, commit, preserveOwner, isAdd bool) (*addCopyStep, error) {
fromPaths []string, toPath string, commit, preserveOwner bool) (*addCopyStep, error) {

toPath = strings.Trim(toPath, "\"'")
for i := range fromPaths {
Expand All @@ -81,7 +80,6 @@ func newAddCopyStep(
toPath: toPath,
chown: chown,
preserveOwner: preserveOwner,
isAdd: isAdd,
}, nil
}

Expand Down Expand Up @@ -138,16 +136,14 @@ func (s *addCopyStep) Execute(ctx *context.BuildContext, modifyFS bool) (err err

internal := s.fromStage != ""
blacklist := append(pathutils.DefaultBlacklist, ctx.ImageStore.RootDir)
copyOp, err := snapshot.NewCopyOperation(relPaths, sourceRoot, s.workingDir, s.toPath, s.chown,
s.preserveOwner, internal, blacklist, s.isAdd)
copyOp, err := snapshot.NewCopyOperation(
relPaths, sourceRoot, s.workingDir, s.toPath, s.chown, blacklist, internal, s.preserveOwner)
if err != nil {
return fmt.Errorf("invalid copy operation: %s", err)
}

ctx.CopyOps = append(ctx.CopyOps, copyOp)
if modifyFS {
// Note: We still want to keep this copyop in the list, just in case there
// is no RUN afterwards, and layer needs to be created from copyops.
return copyOp.Execute()
}
return nil
Expand Down
8 changes: 4 additions & 4 deletions lib/builder/step/add_copy_step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ func TestContextDirs(t *testing.T) {
require := require.New(t)

srcs := []string{}
ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false)
ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false)
require.NoError(err)
stage, paths := ac.ContextDirs()
require.Equal("", stage)
require.Len(paths, 0)

srcs = []string{"src"}
ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false)
ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false)
require.NoError(err)
stage, paths = ac.ContextDirs()
require.Equal("", stage)
require.Len(paths, 0)

srcs = []string{"src"}
ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false, false)
ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false)
require.NoError(err)
stage, paths = ac.ContextDirs()
require.Equal("stage", stage)
Expand All @@ -48,7 +48,7 @@ func TestContextDirs(t *testing.T) {
func TestTrimmingPaths(t *testing.T) {
require := require.New(t)

ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false, false)
ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false)
require.NoError(err)

require.Equal("/from/path", ac.fromPaths[0])
Expand Down
3 changes: 1 addition & 2 deletions lib/builder/step/add_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ type AddStep struct {

// NewAddStep creates a new AddStep
func NewAddStep(args, chown string, fromPaths []string, toPath string, commit, preserverOwner bool) (*AddStep, error) {
s, err := newAddCopyStep(
Add, args, chown, "", fromPaths, toPath, commit, preserverOwner, true)
s, err := newAddCopyStep(Add, args, chown, "", fromPaths, toPath, commit, preserverOwner)
if err != nil {
return nil, fmt.Errorf("new add/copy step: %s", err)
}
Expand Down
3 changes: 1 addition & 2 deletions lib/builder/step/copy_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func NewCopyStep(
args, chown, fromStage string, fromPaths []string, toPath string, commit, preserveOwner bool,
) (*CopyStep, error) {

s, err := newAddCopyStep(
Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner, false)
s, err := newAddCopyStep(Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner)
if err != nil {
return nil, fmt.Errorf("new add/copy step: %s", err)
}
Expand Down
44 changes: 7 additions & 37 deletions lib/snapshot/copy_op.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (

"github.com/uber/makisu/lib/fileio"
"github.com/uber/makisu/lib/pathutils"
"github.com/uber/makisu/lib/tario"
"github.com/uber/makisu/lib/utils"
)

// CopyOperation defines a copy operation that occurred to generate a layer from.
type CopyOperation struct {
srcRoot string
srcs []string
Expand All @@ -34,20 +34,16 @@ type CopyOperation struct {
gid int
preserveOwner bool

// Indicates if the copy op is used for copying from previous stages.
// Blacklist is ignored in that case.
internal bool
blacklist []string

// Set to true if the copy op was created by ADD step.
// Some functionalities are only available to ADD.
fromAdd bool
// Indicates if the copy op is used for copying from previous stages.
internal bool
}

// NewCopyOperation initializes and validates a CopyOperation. Use "internal" to
// specify if the copy op is used for copying from previous stages.
func NewCopyOperation(srcs []string, srcRoot, workDir, dst, chown string,
preserveOwner, internal bool, blacklist []string, fromAdd bool) (*CopyOperation, error) {
func NewCopyOperation(
srcs []string, srcRoot, workDir, dst, chown string,
blacklist []string, internal, preserveOwner bool) (*CopyOperation, error) {

if err := checkCopyParams(srcs, workDir, dst); err != nil {
return nil, fmt.Errorf("check copy param: %s", err)
Expand All @@ -74,7 +70,6 @@ func NewCopyOperation(srcs []string, srcRoot, workDir, dst, chown string,
preserveOwner: preserveOwner,
blacklist: blacklist,
internal: internal,
fromAdd: fromAdd,
}, nil
}

Expand All @@ -91,32 +86,6 @@ func (c *CopyOperation) Execute() error {
if err != nil {
return fmt.Errorf("lstat %s: %s", src, err)
}

if strings.HasSuffix(src, ".tar.gz") && c.fromAdd {
// Special feature for ADD - Extract tar.gz into dst directory.
// If dst is an existing directory, untar.
// If dst doesn't exist, create it with root.
// If dst exists and is not a directory, fail.
reader, err := os.Open(src)
if err != nil {
return fmt.Errorf("open tar file: %s", err)
}
defer reader.Close()

if err := os.MkdirAll(c.dst, 0755); err != nil {
return fmt.Errorf("create/validate untar dst: %s", err)
}

gzipReader, err := tario.NewGzipReader(reader)
if err != nil {
return fmt.Errorf("unzip gz %s: %s", src, err)
}
if err := tario.Untar(gzipReader, c.dst); err != nil {
return fmt.Errorf("untar %s to dst %s: %s", src, c.dst, err)
}
return nil
}

var copier fileio.Copier
if c.internal {
copier = fileio.NewInternalCopier()
Expand Down Expand Up @@ -146,6 +115,7 @@ func (c *CopyOperation) Execute() error {
return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err)
}
}

} else {
// File to file
if c.preserveOwner {
Expand Down
Loading

0 comments on commit d94239b

Please sign in to comment.