Skip to content

Commit

Permalink
Include symlinks when packing buildpacks
Browse files Browse the repository at this point in the history
Symlinks were not previous managed correctly when packing buildpacks
using `jam`. This meant that for buildpacks that use the single-binary
pattern for their executables, those buildpacks were packaging 3
binaries (`bin/build`, `bin/detect`, and `bin/run`). This lead to larger
buildpack tarballs instead of smaller ones.

This fix handles that symlinking correctly so that buildpacks can
benefit from the reduced number of executables included in the tarball.
  • Loading branch information
Ryan Moran authored and joshzarrabi committed May 28, 2020
1 parent 9900e31 commit 7632cd4
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 130 deletions.
52 changes: 3 additions & 49 deletions cargo/directory_duplicator.go
Original file line number Diff line number Diff line change
@@ -1,59 +1,13 @@
package cargo

import (
"fmt"
"io"
"os"
"path/filepath"
)
import "github.com/paketo-buildpacks/packit/fs"

type DirectoryDuplicator struct{}

func NewDirectoryDuplicator() DirectoryDuplicator {
return DirectoryDuplicator{}
}

func (d DirectoryDuplicator) Duplicate(source, sink string) error {
return filepath.Walk(source, func(path string, info os.FileInfo, err error) error {
if err != nil {
return fmt.Errorf("source dir does not exist: %s", err)
}

relPath, err := filepath.Rel(source, path)
if err != nil {
return fmt.Errorf("failed to get relative path: %s", err)
}

destPath := filepath.Join(sink, relPath)
if info.IsDir() {
err := os.MkdirAll(destPath, info.Mode())
if err != nil {
return fmt.Errorf("duplicate error creating dir: %s", err)
}
} else if os.ModeType&info.Mode() == 0 {
src, err := os.Open(path)
if err != nil {
return fmt.Errorf("opening source file failed: %s", err)
}
defer src.Close()

srcInfo, err := src.Stat()
if err != nil {
return fmt.Errorf("unable to stat source file: %s", err)
}

dst, err := os.OpenFile(destPath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, srcInfo.Mode())
if err != nil {
return fmt.Errorf("duplicate error creating file: %s", err)
}
defer dst.Close()

_, err = io.Copy(dst, src)
if err != nil {
return fmt.Errorf("copy dst to src failed: %s", err)
}

}
return nil
})
func (d DirectoryDuplicator) Duplicate(source, destination string) error {
return fs.Copy(source, destination)
}
50 changes: 11 additions & 39 deletions cargo/directory_duplicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func testDirectoryDuplicator(t *testing.T, context spec.G, it spec.S) {

Expect(os.MkdirAll(filepath.Join(sourceDir, "some-dir"), os.ModePerm)).To(Succeed())
Expect(ioutil.WriteFile(filepath.Join(sourceDir, "some-dir", "other-file"), []byte("other content"), 0755)).To(Succeed())
Expect(os.Symlink(filepath.Join(sourceDir, "some-dir", "other-file"), filepath.Join(sourceDir, "some-dir", "link"))).To(Succeed())

destDir, err = ioutil.TempDir("", "dest")
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -75,14 +76,22 @@ func testDirectoryDuplicator(t *testing.T, context spec.G, it spec.S) {
Expect(info.Mode()).To(Equal(os.FileMode(0755)))

Expect(file.Close()).To(Succeed())

info, err = os.Lstat(filepath.Join(destDir, "some-dir", "link"))
Expect(err).NotTo(HaveOccurred())
Expect(info.Mode() & os.ModeSymlink).To(Equal(os.ModeSymlink))

path, err := os.Readlink(filepath.Join(destDir, "some-dir", "link"))
Expect(err).NotTo(HaveOccurred())
Expect(path).To(Equal(filepath.Join(destDir, "some-dir", "other-file")))
})
})

context("failure cases", func() {
context("source dir does not exist", func() {
it("fails", func() {
err := directoryDup.Duplicate("does-not-exist", destDir)
Expect(err).To(MatchError(ContainSubstring("source dir does not exist: ")))
Expect(err).To(MatchError(ContainSubstring("no such file or directory")))
})
})

Expand All @@ -97,44 +106,7 @@ func testDirectoryDuplicator(t *testing.T, context spec.G, it spec.S) {

it("fails", func() {
err := directoryDup.Duplicate(sourceDir, destDir)
Expect(err).To(MatchError(ContainSubstring("opening source file failed:")))
})
})

context("when destination directory bad permissions", func() {
context("when creating dir", func() {
it.Before(func() {
Expect(os.Chmod(destDir, 0000)).To(Succeed())
})

it.After(func() {
Expect(os.Chmod(destDir, os.ModePerm)).To(Succeed())
})

it("fails", func() {
err := directoryDup.Duplicate(sourceDir, destDir)
Expect(err).To(MatchError(ContainSubstring("duplicate error creating dir:")))
Expect(err).To(MatchError(ContainSubstring("permission denied")))
})
})

context("when creating file", func() {
var dirPath string

it.Before(func() {
dirPath = filepath.Join(destDir, "some-dir")
Expect(os.MkdirAll(dirPath, 0000)).To(Succeed())
})

it.After(func() {
Expect(os.Chmod(dirPath, os.ModePerm)).To(Succeed())
})

it("fails", func() {
err := directoryDup.Duplicate(sourceDir, destDir)
Expect(err).To(MatchError(ContainSubstring("duplicate error creating file:")))
Expect(err).To(MatchError(ContainSubstring("permission denied")))
})
Expect(err).To(MatchError(ContainSubstring("permission denied")))
})
})
})
Expand Down
32 changes: 24 additions & 8 deletions cargo/file_bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"
"time"
)

Expand All @@ -15,6 +16,7 @@ type File struct {

Name string
Info os.FileInfo
Link string
}

type FileInfo struct {
Expand Down Expand Up @@ -81,18 +83,32 @@ func (b FileBundler) Bundle(root string, paths []string, config Config) ([]File,
file.Info = NewFileInfo("buildpack.toml", buf.Len(), 0644, time.Now())

default:
fd, err := os.Open(filepath.Join(root, path))
if err != nil {
return nil, fmt.Errorf("error opening included file: %s", err)
}

info, err := fd.Stat()
var err error
file.Info, err = os.Lstat(filepath.Join(root, path))
if err != nil {
return nil, fmt.Errorf("error stating included file: %s", err)
}

file.ReadCloser = fd
file.Info = info
if file.Info.Mode()&os.ModeType != 0 {
link, err := os.Readlink(filepath.Join(root, path))
if err != nil {
return nil, fmt.Errorf("error readlinking included file: %s", err)
}

if !strings.HasPrefix(link, string(filepath.Separator)) {
link = filepath.Clean(filepath.Join(root, link))
}

file.Link, err = filepath.Rel(root, link)
if err != nil {
return nil, fmt.Errorf("error finding relative link path: %s", err)
}
} else {
file.ReadCloser, err = os.Open(filepath.Join(root, path))
if err != nil {
return nil, fmt.Errorf("error opening included file: %s", err)
}
}
}

files = append(files, file)
Expand Down
26 changes: 18 additions & 8 deletions cargo/file_bundler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func testFileBundler(t *testing.T, context spec.G, it spec.S) {

context("Bundle", func() {
it("returns a list of cargo files", func() {
files, err := fileBundler.Bundle(filepath.Join("jam", "testdata", "example-cnb"), []string{"bin/build", "bin/detect", "buildpack.toml"}, cargo.Config{
files, err := fileBundler.Bundle(filepath.Join("jam", "testdata", "example-cnb"), []string{"bin/build", "bin/detect", "bin/link", "buildpack.toml"}, cargo.Config{
API: "0.2",
Buildpack: cargo.ConfigBuildpack{
ID: "other-buildpack-id",
Expand All @@ -37,18 +37,20 @@ func testFileBundler(t *testing.T, context spec.G, it spec.S) {
IncludeFiles: []string{
"bin/build",
"bin/detect",
"bin/link",
"buildpack.toml",
},
PrePackage: "some-pre-package-script.sh",
},
})
Expect(err).NotTo(HaveOccurred())

Expect(files).To(HaveLen(3))
Expect(files).To(HaveLen(4))

Expect(files[0].Name).To(Equal("bin/build"))
Expect(files[0].Info.Size()).To(Equal(int64(14)))
Expect(files[0].Info.Mode()).To(Equal(os.FileMode(0755)))
Expect(files[0].Link).To(Equal(""))

content, err := ioutil.ReadAll(files[0])
Expect(err).NotTo(HaveOccurred())
Expand All @@ -57,16 +59,24 @@ func testFileBundler(t *testing.T, context spec.G, it spec.S) {
Expect(files[1].Name).To(Equal("bin/detect"))
Expect(files[1].Info.Size()).To(Equal(int64(15)))
Expect(files[1].Info.Mode()).To(Equal(os.FileMode(0755)))
Expect(files[1].Link).To(Equal(""))

content, err = ioutil.ReadAll(files[1])
Expect(err).NotTo(HaveOccurred())
Expect(string(content)).To(Equal("detect-contents"))

Expect(files[2].Name).To(Equal("buildpack.toml"))
Expect(files[2].Info.Size()).To(Equal(int64(244)))
Expect(files[2].Info.Mode()).To(Equal(os.FileMode(0644)))
Expect(files[2].Name).To(Equal("bin/link"))
Expect(files[2].Info.Size()).To(Equal(int64(7)))
Expect(files[2].Info.Mode() & os.ModeSymlink).To(Equal(os.ModeSymlink))
Expect(files[2].Link).To(Equal("build"))
Expect(files[2].ReadCloser).To(BeNil())

content, err = ioutil.ReadAll(files[2])
Expect(files[3].Name).To(Equal("buildpack.toml"))
Expect(files[3].Info.Size()).To(Equal(int64(256)))
Expect(files[3].Info.Mode()).To(Equal(os.FileMode(0644)))
Expect(files[3].Link).To(Equal(""))

content, err = ioutil.ReadAll(files[3])
Expect(err).NotTo(HaveOccurred())
Expect(string(content)).To(MatchTOML(`api = "0.2"
[buildpack]
Expand All @@ -75,15 +85,15 @@ name = "other-buildpack-name"
version = "other-buildpack-version"
[metadata]
include_files = ["bin/build", "bin/detect", "buildpack.toml"]
include_files = ["bin/build", "bin/detect", "bin/link", "buildpack.toml"]
pre_package = "some-pre-package-script.sh"`))
})

context("error cases", func() {
context("when included file does not exist", func() {
it("fails", func() {
_, err := fileBundler.Bundle(filepath.Join("jam", "testdata", "example-cnb"), []string{"bin/fake/build", "bin/detect", "buildpack.toml"}, cargo.Config{})
Expect(err).To(MatchError(ContainSubstring("error opening included file:")))
Expect(err).To(MatchError(ContainSubstring("error stating included file:")))
})
})
})
Expand Down
12 changes: 9 additions & 3 deletions cargo/jam/pack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ func testPack(t *testing.T, context spec.G, it spec.S) {
})

context("when packaging an implementation buildpack", func() {

it.Before(func() {
err := cargo.NewDirectoryDuplicator().Duplicate(filepath.Join("testdata", "example-cnb"), buildpackDir)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -136,6 +135,7 @@ func testPack(t *testing.T, context spec.G, it spec.S) {
Expect(session.Out).To(gbytes.Say(fmt.Sprintf(" Building tarball: %s", filepath.Join(tmpDir, "output.tgz"))))
Expect(session.Out).To(gbytes.Say(" bin/build"))
Expect(session.Out).To(gbytes.Say(" bin/detect"))
Expect(session.Out).To(gbytes.Say(" bin/link"))
Expect(session.Out).To(gbytes.Say(" buildpack.toml"))
Expect(session.Out).To(gbytes.Say(" generated-file"))

Expand All @@ -161,7 +161,7 @@ func testPack(t *testing.T, context spec.G, it spec.S) {
homepage = "some-homepage-link"
[metadata]
include_files = ["bin/build", "bin/detect", "buildpack.toml", "generated-file"]
include_files = ["bin/build", "bin/detect", "bin/link", "buildpack.toml", "generated-file"]
pre_package = "./scripts/build.sh"
[metadata.default-versions]
some-dependency = "some-default-version"
Expand Down Expand Up @@ -202,6 +202,12 @@ func testPack(t *testing.T, context spec.G, it spec.S) {
Expect(hdr.Uname).To(Equal(userName))
Expect(hdr.Gname).To(Equal(groupName))

_, hdr, err = ExtractFile(file, "bin/link")
Expect(err).NotTo(HaveOccurred())
Expect(hdr.Linkname).To(Equal("bin/build"))
Expect(hdr.Uname).To(Equal(userName))
Expect(hdr.Gname).To(Equal(groupName))

contents, hdr, err = ExtractFile(file, "generated-file")
Expect(err).NotTo(HaveOccurred())
Expect(string(contents)).To(Equal("hello\n"))
Expand Down Expand Up @@ -283,7 +289,7 @@ func testPack(t *testing.T, context spec.G, it spec.S) {
homepage = "some-homepage-link"
[metadata]
include_files = ["bin/build", "bin/detect", "buildpack.toml", "generated-file", "dependencies/f058c8bf6b65b829e200ef5c2d22fde0ee65b96c1fbd1b88869be133aafab64a"]
include_files = ["bin/build", "bin/detect", "bin/link", "buildpack.toml", "generated-file", "dependencies/f058c8bf6b65b829e200ef5c2d22fde0ee65b96c1fbd1b88869be133aafab64a"]
pre_package = "./scripts/build.sh"
[metadata.default-versions]
some-dependency = "some-default-version"
Expand Down
1 change: 1 addition & 0 deletions cargo/jam/testdata/example-cnb/bin/link
2 changes: 1 addition & 1 deletion cargo/jam/testdata/example-cnb/buildpack.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ api = "0.2"
homepage = "some-homepage-link"

[metadata]
include_files = ["bin/build", "bin/detect", "buildpack.toml", "generated-file"]
include_files = ["bin/build", "bin/detect", "bin/link", "buildpack.toml", "generated-file"]
pre_package = "./scripts/build.sh"
[metadata.default-versions]
some-dependency = "some-default-version"
Expand Down
3 changes: 2 additions & 1 deletion cargo/tar_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ func (b TarBuilder) Build(path string, files []File) error {

for _, file := range files {
b.logger.Subprocess(file.Name)
hdr, err := tar.FileInfoHeader(file.Info, file.Name)

hdr, err := tar.FileInfoHeader(file.Info, file.Link)
if err != nil {
return fmt.Errorf("failed to create header for file %q: %w", file.Name, err)
}
Expand Down
14 changes: 13 additions & 1 deletion cargo/tar_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func testTarBuilder(t *testing.T, context spec.G, it spec.S) {

context("Build", func() {
context("given a destination and a list of files", func() {
it("constructs a tar ball", func() {
it("constructs a tarball", func() {
err := builder.Build(tempFile, []cargo.File{
{
Name: "buildpack.toml",
Expand All @@ -62,12 +62,18 @@ func testTarBuilder(t *testing.T, context spec.G, it spec.S) {
Info: cargo.NewFileInfo("detect", len("detect-contents"), 0755, time.Now()),
ReadCloser: ioutil.NopCloser(strings.NewReader("detect-contents")),
},
{
Name: "bin/link",
Info: cargo.NewFileInfo("link", len("./build"), os.ModeSymlink|0755, time.Now()),
Link: "./build",
},
})
Expect(err).NotTo(HaveOccurred())

Expect(output.String()).To(ContainSubstring(fmt.Sprintf("Building tarball: %s", tempFile)))
Expect(output.String()).To(ContainSubstring("bin/build"))
Expect(output.String()).To(ContainSubstring("bin/detect"))
Expect(output.String()).To(ContainSubstring("bin/link"))
Expect(output.String()).To(ContainSubstring("buildpack.toml"))

file, err := os.Open(tempFile)
Expand All @@ -93,6 +99,12 @@ func testTarBuilder(t *testing.T, context spec.G, it spec.S) {
Expect(err).NotTo(HaveOccurred())
Expect(string(contents)).To(Equal("detect-contents"))
Expect(hdr.Mode).To(Equal(int64(0755)))

_, hdr, err = ExtractFile(file, "bin/link")
Expect(err).NotTo(HaveOccurred())
Expect(hdr.Typeflag).To(Equal(byte(tar.TypeSymlink)))
Expect(hdr.Linkname).To(Equal("./build"))
Expect(hdr.Mode).To(Equal(int64(0755)))
})
})

Expand Down
Loading

0 comments on commit 7632cd4

Please sign in to comment.