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

Revert "Make ADD untar tar.gz files (#309)" #311

Merged
merged 1 commit into from
Mar 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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