Skip to content

Commit

Permalink
chore: apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Rafid Bin Mostofa <[email protected]>
  • Loading branch information
zhijie-yang and rebornplusplus authored Nov 27, 2024
1 parent b4d8ff8 commit 67c88b9
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 27 deletions.
24 changes: 10 additions & 14 deletions internal/deb/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ type ExtractInfo struct {
type HardLinkInfo struct {
// The target path of the hard link logged in the tarball header.
TargetPath string
// The extract infos of the hard links to be extracted, which has
// the link target TargetPath.
// Info of the hard links to extract, which has the link target TargetPath.
ExtractInfos []ExtractInfo
}

Expand Down Expand Up @@ -281,17 +280,16 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
}
err := options.Create(extractInfos, createOptions)
if err != nil {
// Handle the hard link where its counterpart is not extracted
if tarHeader.Typeflag == tar.TypeLink && errors.Is(err, fsutil.ErrLinkTargetNotExist) {
if errors.Is(err, fsutil.ErrLinkTargetNotExist) && tarHeader.Typeflag == tar.TypeLink {
// This means that the hard link target file has not been
// extracted. Add the entry to the pending list.
// extracted. Add this hard link entry to the pending list
// to extract later.
basePath := sanitizeTarSourcePath(tarHeader.Linkname)
pendingHardLinks[basePath] = append(pendingHardLinks[basePath],
HardLinkInfo{
TargetPath: targetPath,
ExtractInfos: extractInfos,
})
pendingPaths[basePath] = true
info := HardLinkInfo{
TargetPath: targetPath,
ExtractInfos: extractInfos,
}
pendingHardLinks[basePath] = append(pendingHardLinks[basePath], info)
} else {
return err
}
Expand All @@ -317,7 +315,6 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
if err != nil {
return err
}

}

if len(pendingPaths) > 0 {
Expand Down Expand Up @@ -359,7 +356,7 @@ func handlePendingHardLinks(options *ExtractOptions, pendingHardLinks map[string
continue
}

// Since the hard link target file is not extracted in the first pass,
// 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 := hardLinks[0].TargetPath
Expand All @@ -375,7 +372,6 @@ func handlePendingHardLinks(options *ExtractOptions, pendingHardLinks map[string
if err != nil {
return err
}
delete(pendingPaths, sourcePath)
delete(pendingPaths, targetPath)

// Create the remaining hard links.
Expand Down
2 changes: 1 addition & 1 deletion internal/deb/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ var extractTests = []extractTest{{
},
error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`,
}, {
summary: "Hard link is filled with the target file",
summary: "Hard link is inflated with the target file",
pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
testutil.Dir(0755, "./"),
testutil.Reg(0644, "./file", "text for file"),
Expand Down
1 change: 0 additions & 1 deletion internal/fsutil/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func Create(options *CreateOptions) (*Entry, error) {
}
mode := s.Mode()
if o.OverrideMode && mode != o.Mode && o.Link == "" {
fmt.Printf("Changing mode of %s from %#o to %#o\n", o.Path, mode, o.Mode)
err := os.Chmod(o.Path, o.Mode)
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions internal/fsutil/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var createTests = []createTest{{
}, {
summary: "Create a hard link",
options: fsutil.CreateOptions{
Path: "dir/hardlink",
Path: "hardlink",
Link: "file",
Mode: 0644,
MakeParents: true,
Expand All @@ -122,7 +122,7 @@ var createTests = []createTest{{
}, {
summary: "Cannot create a hard link if the link target does not exist",
options: fsutil.CreateOptions{
Path: "dir/hardlink",
Path: "hardlink",
Link: "missing-file",
Mode: 0644,
MakeParents: true,
Expand All @@ -132,7 +132,7 @@ var createTests = []createTest{{
},
error: `link target does not exist`,
}, {
summary: "Re-creating a duplicated hard link keeps the original link",
summary: "Hard link is not created if it already exists",
options: fsutil.CreateOptions{
Path: "hardlink",
Link: "file",
Expand All @@ -149,7 +149,7 @@ var createTests = []createTest{{
"/hardlink": "file 0644 3a6eb079",
},
}, {
summary: "Cannot create a hard link if the link path exists and it is not a hard link to the target",
summary: "Cannot create a hard link if it exists and is not a hard link to target",
options: fsutil.CreateOptions{
Path: "hardlink",
Link: "file",
Expand Down
14 changes: 8 additions & 6 deletions internal/manifest/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ type Report struct {
Root string
// Entries holds all reported content, indexed by their path.
Entries map[string]ReportEntry
// curHardLinkId is used to allocate unique HardLinkId for hard links.

// curHardLinkId is used internally to allocate unique HardLinkId for hard
// links.
curHardLinkId int
}

Expand Down Expand Up @@ -149,11 +151,11 @@ func (r *Report) sanitizeAbsPath(path string, isDir bool) (relPath string, err e
return relPath, nil
}

// entryIsHardLink determines if a link path belongs to a hard link by
// checking if the link path has a prefix of the root path, since
// hard links are created with absolute paths prefixing the report's root
// path, while symlinks are created either with relative paths or absolute
// paths that do not have the report's root path as a prefix.
// entryIsHardLink determines if a link is a hard link by checking if the link
// has a prefix of the root dir, since hard links are created with absolute
// paths prefixing the report's root dir, while symlinks are created either with
// relative paths or absolute paths that do not have the report's root dir as a
// prefix.
func (r *Report) entryIsHardLink(link string) bool {
return link != "" && strings.HasPrefix(link, r.Root)
}
2 changes: 1 addition & 1 deletion internal/testutil/nopcloser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"io"
)

// NopSeekCloser is an io.Reader that does nothing on Close, and
// readSeekerNopCloser is an io.Reader that does nothing on Close, and
// seeks to the beginning of the stream on Seek.
// It is an extension of io.NopCloser that also implements io.Seeker.
type readSeekerNopCloser struct {
Expand Down

0 comments on commit 67c88b9

Please sign in to comment.