Skip to content

Commit

Permalink
Merge pull request #1 from rebornplusplus/ROCKS-1471/final-pass
Browse files Browse the repository at this point in the history 
fix: final pass from our pair programming
  • Loading branch information
@zhijie-yang
zhijie-yang authored Nov 29, 2024
2 parents ab9fa08 + 33a06ab commit 76e8281
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 92 deletions.
30 changes: 16 additions & 14 deletions internal/deb/extract.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ type ExtractInfo struct {
}

type HardLinkInfo struct {
// The target path of the hard link logged in the tarball header.
TargetPath string
// Info of the hard links to extract, which has the link target TargetPath.
// The path of the hard link recorded in the tarball header.
LinkPath string
// Info of the hard links to extract, which has the link target LinkPath.
ExtractInfos []ExtractInfo
}

Expand Down Expand Up @@ -151,6 +151,8 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
}
}

// Store the hard links we cannot extract in the first iteration over the
// tarball.
pendingHardLinks := make(map[string][]HardLinkInfo)

// When creating a file we will iterate through its parent directories and
Expand Down Expand Up @@ -284,7 +286,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
// to extract later.
basePath := sanitizeTarSourcePath(tarHeader.Linkname)
info := HardLinkInfo{
TargetPath: targetPath,
LinkPath: targetPath,
ExtractInfos: extractInfos,
}
pendingHardLinks[basePath] = append(pendingHardLinks[basePath], info)
Expand All @@ -310,7 +312,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
tarReader := tar.NewReader(dataReader)
extractHardLinkOptions := &extractHardLinkOptions{
extractOptions: options,
links: pendingHardLinks,
pendingLinks: pendingHardLinks,
tarReader: tarReader,
}
err = extractHardLinks(extractHardLinkOptions)
Expand All @@ -337,7 +339,7 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {

type extractHardLinkOptions struct {
extractOptions *ExtractOptions
links map[string][]HardLinkInfo
pendingLinks map[string][]HardLinkInfo
tarReader *tar.Reader
}

Expand All @@ -356,16 +358,16 @@ func extractHardLinks(opts *extractHardLinkOptions) error {
continue
}

links, ok := opts.links[sourcePath]
links, ok := opts.pendingLinks[sourcePath]
if !ok || len(links) == 0 {
delete(opts.links, sourcePath)
delete(opts.pendingLinks, sourcePath)
continue
}

// Since the hard link target file was not extracted in the first pass,
// we extract the first hard link as a regular file. For the rest of
// the hard link entries, we link those paths to first file extracted.
targetPath := links[0].TargetPath
targetPath := links[0].LinkPath
extractPath := filepath.Join(opts.extractOptions.TargetDir, targetPath)
// Write the content for the first file in the hard link group
createOptions := &fsutil.CreateOptions{
Expand All @@ -381,7 +383,7 @@ func extractHardLinks(opts *extractHardLinkOptions) error {
// Create the remaining hard links.
for _, link := range links[1:] {
createOptions := &fsutil.CreateOptions{
Path: filepath.Join(opts.extractOptions.TargetDir, link.TargetPath),
Path: filepath.Join(opts.extractOptions.TargetDir, link.LinkPath),
Mode: tarHeader.FileInfo().Mode(),
// Link to the first file extracted for the hard links.
Link: extractPath,
Expand All @@ -391,16 +393,16 @@ func extractHardLinks(opts *extractHardLinkOptions) error {
return err
}
}
delete(opts.links, sourcePath)
delete(opts.pendingLinks, sourcePath)
}

// If there are pending links, that means the link targets do not come from
// this package.
if len(opts.links) > 0 {
if len(opts.pendingLinks) > 0 {
var sLinks []string
for target, links := range opts.links {
for target, links := range opts.pendingLinks {
for _, link := range links {
sLinks = append(sLinks, link.TargetPath+" -> "+target)
sLinks = append(sLinks, link.LinkPath+" -> "+target)
}
}
if len(sLinks) == 1 {
Expand Down
12 changes: 6 additions & 6 deletions internal/fsutil/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var createTests = []createTest{{
Path: "foo/bar",
Mode: fs.ModeDir | 0775,
},
error: `mkdir \/[^ ]*\/foo/bar: no such file or directory`,
error: `mkdir /[^ ]*/foo/bar: no such file or directory`,
}, {
summary: "Re-creating an existing directory keeps the original mode",
options: fsutil.CreateOptions{
Expand Down Expand Up @@ -129,7 +129,7 @@ var createTests = []createTest{{
hackopt: func(c *C, dir string, opts *fsutil.CreateOptions) {
opts.Link = filepath.Join(dir, opts.Link)
},
error: `link \/[^ ]*\/missing-file \/[^ ]*\/hardlink: no such file or directory`,
error: `link /[^ ]*/missing-file /[^ ]*/hardlink: no such file or directory`,
}, {
summary: "Hard link is not created if it already exists",
options: fsutil.CreateOptions{
Expand Down Expand Up @@ -160,7 +160,7 @@ var createTests = []createTest{{
c.Assert(os.WriteFile(filepath.Join(dir, "hardlink"), []byte("data"), 0644), IsNil)
opts.Link = filepath.Join(dir, opts.Link)
},
error: `link \/[^ ]*\/file \/[^ ]*\/hardlink: file exists`,
error: `link /[^ ]*/file /[^ ]*/hardlink: file exists`,
}, {
options: fsutil.CreateOptions{
Path: "foo",
Expand Down Expand Up @@ -281,9 +281,9 @@ func (s *S) TestCreate(c *C) {

// [fsutil.Create] does not return information about parent directories
// created implicitly. We only check for the requested path.
if entry.Link != "" && entry.Mode&fs.ModeSymlink == 0 {
// Entry is a hard link. We should test it differently to ensure
// that it produces a hard link indeed.
if entry.LinkType == fsutil.TypeHardLink {
// We should test hard link entries differently to ensure that it
// produces a hard link indeed.
pathInfo, err := os.Lstat(entry.Path)
c.Assert(err, IsNil)
linkInfo, err := os.Lstat(entry.Link)
Expand Down
3 changes: 2 additions & 1 deletion internal/manifest/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ func fastValidate(options *WriteOptions) (err error) {
})
e0 := entries[0]
for _, e := range entries[1:] {
if e.Link != e0.Link || e.Mode != e0.Mode || e.SHA256 != e0.SHA256 || e.Size != e0.Size {
if e.Link != e0.Link || e.Mode != e0.Mode || e.SHA256 != e0.SHA256 ||
e.Size != e0.Size || e.FinalSHA256 != e0.FinalSHA256 {
return fmt.Errorf("hard linked paths %q and %q have diverging contents", e0.Path, e.Path)
}
}
Expand Down
8 changes: 6 additions & 2 deletions internal/manifest/manifest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ var readManifestTests = []struct {
{"kind":"content","slice":"pkg1_manifest","path":"/manifest/manifest.wall"}
{"kind":"content","slice":"pkg1_myslice","path":"/dir/file"}
{"kind":"content","slice":"pkg1_myslice","path":"/dir/foo/bar/"}
{"kind":"content","slice":"pkg1_myslice","path":"/dir/hardlink/file"}
{"kind":"content","slice":"pkg1_myslice","path":"/dir/link/file"}
{"kind":"content","slice":"pkg2_myotherslice","path":"/dir/foo/bar/"}
{"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"}
{"kind":"package","name":"pkg2","version":"v2","sha256":"hash2","arch":"arch2"}
{"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21}
{"kind":"path","path":"/dir/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21,"hard_link_id":1}
{"kind":"path","path":"/dir/foo/bar/","mode":"01777","slices":["pkg2_myotherslice","pkg1_myslice"]}
{"kind":"path","path":"/dir/hardlink/file","mode":"0644","slices":["pkg1_myslice"],"sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","final_sha256":"8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6","size":21,"hard_link_id":1}
{"kind":"path","path":"/dir/link/file","mode":"0644","slices":["pkg1_myslice"],"link":"/dir/file"}
{"kind":"path","path":"/manifest/manifest.wall","mode":"0644","slices":["pkg1_manifest"]}
{"kind":"slice","name":"pkg1_manifest"}
Expand All @@ -50,8 +52,9 @@ var readManifestTests = []struct {
`,
mfest: &manifestContents{
Paths: []*manifest.Path{
{Kind: "path", Path: "/dir/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", FinalSHA256: "8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6", Size: 0x15, Link: ""},
{Kind: "path", Path: "/dir/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", FinalSHA256: "8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6", Size: 0x15, Link: "", HardLinkId: 1},
{Kind: "path", Path: "/dir/foo/bar/", Mode: "01777", Slices: []string{"pkg2_myotherslice", "pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""},
{Kind: "path", Path: "/dir/hardlink/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", FinalSHA256: "8067926c032c090867013d14fb0eb21ae858344f62ad07086fd32375845c91a6", Size: 0x15, Link: "", HardLinkId: 1},
{Kind: "path", Path: "/dir/link/file", Mode: "0644", Slices: []string{"pkg1_myslice"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: "/dir/file"},
{Kind: "path", Path: "/manifest/manifest.wall", Mode: "0644", Slices: []string{"pkg1_manifest"}, SHA256: "", FinalSHA256: "", Size: 0x0, Link: ""},
},
Expand All @@ -68,6 +71,7 @@ var readManifestTests = []struct {
{Kind: "content", Slice: "pkg1_manifest", Path: "/manifest/manifest.wall"},
{Kind: "content", Slice: "pkg1_myslice", Path: "/dir/file"},
{Kind: "content", Slice: "pkg1_myslice", Path: "/dir/foo/bar/"},
{Kind: "content", Slice: "pkg1_myslice", Path: "/dir/hardlink/file"},
{Kind: "content", Slice: "pkg1_myslice", Path: "/dir/link/file"},
{Kind: "content", Slice: "pkg2_myotherslice", Path: "/dir/foo/bar/"},
},
Expand Down
106 changes: 72 additions & 34 deletions internal/manifest/report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ var reportTests = []struct {
mutate: []*fsutil.Entry{&sampleDir},
err: `cannot mutate path in report: /example-dir/ is a directory`,
}, {
summary: "Regular file hard link",
summary: "Hard link to regular file",
add: []sliceAndEntry{
{entry: sampleFile, slice: oneSlice},
{entry: sampleHardLinkReg, slice: oneSlice}},
Expand All @@ -293,7 +293,6 @@ var reportTests = []struct {
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
HardLinkId: 1,
},
"/example-hard-link-reg": {
Expand All @@ -302,11 +301,11 @@ var reportTests = []struct {
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
HardLinkId: 1,
}},
},
},
}, {
summary: "Symlink hard link",
summary: "Hard link to symlink",
add: []sliceAndEntry{
{entry: sampleLink, slice: oneSlice},
{entry: sampleHardLinkSym, slice: oneSlice}},
Expand All @@ -328,40 +327,40 @@ var reportTests = []struct {
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "/base/example-file",
HardLinkId: 1,
}},
},
},
}, {
summary: "Multiple hard links",
add: []sliceAndEntry{
{entry: sampleFile, slice: oneSlice},
{entry: sampleHardLinkReg, slice: oneSlice},
{
entry: fsutil.Entry{
Path: "/base/another-example-file",
Mode: 0777,
SHA256: "another-example-file_hash",
Size: 5678,
Link: "",
},
slice: oneSlice,
add: []sliceAndEntry{{
entry: sampleFile,
slice: oneSlice,
}, {
entry: sampleHardLinkReg,
slice: oneSlice,
}, {
entry: fsutil.Entry{
Path: "/base/another-example-file",
Mode: 0777,
SHA256: "another-example-file_hash",
Size: 5678,
},
{
entry: fsutil.Entry{
Path: "/base/another-example-hard-link-reg",
Mode: sampleFile.Mode,
Link: "/base/another-example-file",
LinkType: fsutil.TypeHardLink,
},
slice: oneSlice,
slice: otherSlice,
}, {
entry: fsutil.Entry{
Path: "/base/another-example-hard-link-reg",
Mode: sampleFile.Mode,
Link: "/base/another-example-file",
LinkType: fsutil.TypeHardLink,
},
},
slice: otherSlice,
}},
expected: map[string]manifest.ReportEntry{
"/example-file": {
Path: "/example-file",
Mode: 0777,
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
HardLinkId: 1,
},
"/example-hard-link-reg": {
Expand All @@ -370,27 +369,66 @@ var reportTests = []struct {
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
HardLinkId: 1,
},
"/another-example-file": {
Path: "/another-example-file",
Mode: 0777,
SHA256: "another-example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
Slices: map[*setup.Slice]bool{otherSlice: true},
HardLinkId: 2,
},
"/another-example-hard-link-reg": {
Path: "/another-example-hard-link-reg",
Mode: sampleFile.Mode,
SHA256: "another-example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
Link: "",
Slices: map[*setup.Slice]bool{otherSlice: true},
HardLinkId: 2,
}},
},
},
}, {
summary: "Hard links to same file in different slices",
add: []sliceAndEntry{{
entry: sampleFile, slice: oneSlice,
}, {
entry: sampleHardLinkReg, slice: oneSlice,
}, {
entry: fsutil.Entry{
Path: "/base/another-hard-link-reg",
Mode: sampleFile.Mode,
Link: "/base/example-file",
LinkType: fsutil.TypeHardLink,
},
slice: otherSlice,
}},
expected: map[string]manifest.ReportEntry{
"/example-file": {
Path: "/example-file",
Mode: 0777,
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
HardLinkId: 1,
},
"/example-hard-link-reg": {
Path: "/example-hard-link-reg",
Mode: sampleFile.Mode,
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{oneSlice: true},
HardLinkId: 1,
},
"/another-hard-link-reg": {
Path: "/another-hard-link-reg",
Mode: sampleFile.Mode,
SHA256: "example-file_hash",
Size: 5678,
Slices: map[*setup.Slice]bool{otherSlice: true},
HardLinkId: 1,
},
},
}}

func (s *S) TestReport(c *C) {
Expand Down
Loading

0 comments on commit 76e8281

Please sign in to comment.