Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: explicit parents override implicit #166

Merged
merged 5 commits into from
Nov 15, 2024
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
11 changes: 6 additions & 5 deletions internal/deb/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,12 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
}
// Create the entry itself.
createOptions := &fsutil.CreateOptions{
Path: filepath.Join(options.TargetDir, targetPath),
Mode: tarHeader.FileInfo().Mode(),
Data: pathReader,
Link: tarHeader.Linkname,
MakeParents: true,
Path: filepath.Join(options.TargetDir, targetPath),
Mode: tarHeader.FileInfo().Mode(),
Data: pathReader,
Link: tarHeader.Linkname,
MakeParents: true,
OverrideMode: true,
letFunny marked this conversation as resolved.
Show resolved Hide resolved
}
err := options.Create(extractInfos, createOptions)
if err != nil {
Expand Down
27 changes: 24 additions & 3 deletions internal/deb/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package deb_test

import (
"bytes"
"os"
"path"
"path/filepath"
"sort"
"strings"
Expand All @@ -17,7 +19,7 @@ type extractTest struct {
summary string
pkgdata []byte
options deb.ExtractOptions
hackopt func(o *deb.ExtractOptions)
hackopt func(c *C, o *deb.ExtractOptions)
result map[string]string
// paths which the extractor did not create explicitly.
notCreated []string
Expand Down Expand Up @@ -98,7 +100,7 @@ var extractTests = []extractTest{{
"/dir/several/levels/deep/file": "file 0644 6bc26dff",
"/other-dir/": "dir 0755",
},
hackopt: func(o *deb.ExtractOptions) {
hackopt: func(c *C, o *deb.ExtractOptions) {
o.Create = nil
},
}, {
Expand Down Expand Up @@ -352,6 +354,25 @@ var extractTests = []extractTest{{
},
},
error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`,
}, {
summary: "Explicit extraction overrides existing file",
pkgdata: testutil.PackageData["test-package"],
options: deb.ExtractOptions{
Extract: map[string][]deb.ExtractInfo{
"/dir/": []deb.ExtractInfo{{
Path: "/dir/",
Mode: 0777,
}},
},
},
hackopt: func(c *C, o *deb.ExtractOptions) {
err := os.Mkdir(path.Join(o.TargetDir, "/dir"), 0666)
c.Assert(err, IsNil)
},
result: map[string]string{
"/dir/": "dir 0777",
},
notCreated: []string{},
}}

func (s *S) TestExtract(c *C) {
Expand All @@ -374,7 +395,7 @@ func (s *S) TestExtract(c *C) {
}

if test.hackopt != nil {
test.hackopt(&options)
test.hackopt(c, &options)
}

err := deb.Extract(bytes.NewBuffer(test.pkgdata), &options)
Expand Down
14 changes: 13 additions & 1 deletion internal/fsutil/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ type CreateOptions struct {
// If MakeParents is true, missing parent directories of Path are
// created with permissions 0755.
MakeParents bool
// If OverrideMode is true and entry already exists, update the mode. Does
// not affect symlinks.
OverrideMode bool
}

type Entry struct {
Expand Down Expand Up @@ -65,9 +68,18 @@ func Create(options *CreateOptions) (*Entry, error) {
if err != nil {
return nil, err
}
mode := s.Mode()
if o.OverrideMode && mode != o.Mode && o.Mode&fs.ModeSymlink == 0 {
err := os.Chmod(o.Path, o.Mode)
if err != nil {
return nil, err
}
mode = o.Mode
}

entry := &Entry{
Path: o.Path,
Mode: s.Mode(),
Mode: mode,
SHA256: hash,
Size: rp.size,
Link: o.Link,
Expand Down
87 changes: 87 additions & 0 deletions internal/fsutil/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,93 @@ var createTests = []createTest{{
// mode is not updated.
"/foo": "file 0666 d67e2e94",
},
}, {
options: fsutil.CreateOptions{
Path: "foo",
Mode: fs.ModeDir | 0775,
OverrideMode: true,
},
hackdir: func(c *C, dir string) {
c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil)
},
result: map[string]string{
// mode is updated.
"/foo/": "dir 0775",
},
}, {
options: fsutil.CreateOptions{
Path: "foo",
Mode: 0775,
Data: bytes.NewBufferString("whatever"),
OverrideMode: true,
},
hackdir: func(c *C, dir string) {
err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
c.Assert(err, IsNil)
},
result: map[string]string{
// mode is updated.
"/foo": "file 0775 85738f8f",
},
}, {
options: fsutil.CreateOptions{
Path: "foo",
Link: "./bar",
Mode: 0666 | fs.ModeSymlink,
},
hackdir: func(c *C, dir string) {
err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
c.Assert(err, IsNil)
},
result: map[string]string{
"/foo": "symlink ./bar",
},
letFunny marked this conversation as resolved.
Show resolved Hide resolved
letFunny marked this conversation as resolved.
Show resolved Hide resolved
}, {
options: fsutil.CreateOptions{
Path: "foo",
Link: "./bar",
Mode: 0776 | fs.ModeSymlink,
OverrideMode: true,
},
hackdir: func(c *C, dir string) {
err := os.WriteFile(filepath.Join(dir, "bar"), []byte("data"), 0666)
c.Assert(err, IsNil)
err = os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
c.Assert(err, IsNil)
},
result: map[string]string{
"/foo": "symlink ./bar",
// mode is not updated.
"/bar": "file 0666 3a6eb079",
},
}, {
options: fsutil.CreateOptions{
Path: "bar",
// Existing link with different target.
Link: "other",
Mode: 0666 | fs.ModeSymlink,
},
hackdir: func(c *C, dir string) {
err := os.Symlink("foo", filepath.Join(dir, "bar"))
c.Assert(err, IsNil)
},
result: map[string]string{
"/bar": "symlink other",
},
}, {
options: fsutil.CreateOptions{
Path: "bar",
// Existing link with same target.
Link: "foo",
Mode: 0666 | fs.ModeSymlink,
},
hackdir: func(c *C, dir string) {
err := os.Symlink("foo", filepath.Join(dir, "bar"))
c.Assert(err, IsNil)
},
result: map[string]string{
"/bar": "symlink foo",
},
}}

func (s *S) TestCreate(c *C) {
Expand Down
44 changes: 30 additions & 14 deletions internal/slicer/slicer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,43 +321,59 @@ var slicerTests = []slicerTest{{
}, {
summary: "Install two packages, explicit path has preference over implicit parent",
slices: []setup.SliceKey{
{"implicit-parent", "myslice"},
{"explicit-dir", "myslice"}},
{"a-implicit-parent", "myslice"},
{"b-explicit-dir", "myslice"},
{"c-implicit-parent", "myslice"}},
pkgs: []*testutil.TestPackage{{
Name: "implicit-parent",
Name: "a-implicit-parent",
Data: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./dir/"),
testutil.Reg(0644, "./dir/file", "random"),
testutil.Reg(0644, "./dir/file-1", "random"),
}),
}, {
Name: "explicit-dir",
Name: "b-explicit-dir",
Data: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(01777, "./dir/"),
}),
}, {
Name: "c-implicit-parent",
Data: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./dir/"),
testutil.Reg(0644, "./dir/file-2", "random"),
}),
}},
release: map[string]string{
"slices/mydir/implicit-parent.yaml": `
package: implicit-parent
"slices/mydir/a-implicit-parent.yaml": `
package: a-implicit-parent
slices:
myslice:
contents:
/dir/file:
/dir/file-1:
`,
"slices/mydir/explicit-dir.yaml": `
package: explicit-dir
"slices/mydir/b-explicit-dir.yaml": `
package: b-explicit-dir
slices:
myslice:
contents:
/dir/:
`,
"slices/mydir/c-implicit-parent.yaml": `
package: c-implicit-parent
slices:
myslice:
contents:
/dir/file-2:
`,
},
filesystem: map[string]string{
"/dir/": "dir 01777",
"/dir/file": "file 0644 a441b15f",
"/dir/": "dir 01777",
"/dir/file-1": "file 0644 a441b15f",
"/dir/file-2": "file 0644 a441b15f",
},
manifestPaths: map[string]string{
"/dir/": "dir 01777 {explicit-dir_myslice}",
"/dir/file": "file 0644 a441b15f {implicit-parent_myslice}",
"/dir/": "dir 01777 {b-explicit-dir_myslice}",
"/dir/file-1": "file 0644 a441b15f {a-implicit-parent_myslice}",
"/dir/file-2": "file 0644 a441b15f {c-implicit-parent_myslice}",
},
}, {
summary: "Valid same file in two slices in different packages",
Expand Down
Loading