feat: add support for hard links #179
Conversation
Hard links can now be extracted and they are correctly reflected in the manifest. --------- Co-authored-by: Rafid Bin Mostofa <rafid.mostofa@canonical.com> Co-authored-by: Alberto Carretero <angelalbertoc.r@gmail.com>
niemeyer
left a comment
niemeyer
left a comment
There was a problem hiding this comment.
Thanks, Alberto. This is looking great.
A few comments, and maybe the last pass:
internal/deb/extract.go
Outdated
|
|
||
| // sanitizeTarPath removes the leading "./" from the source path in the tarball, | ||
| // and verifies that the path is not empty. | ||
| func sanitizeTarPath(path string) string { |
There was a problem hiding this comment.
Do we have any logic that prevents the tar from leaving the target directory?
There was a problem hiding this comment.
No, we don't at the moment.
There was a problem hiding this comment.
Agreed offline to do it, just not in this PR.
niemeyer
left a comment
niemeyer
left a comment
There was a problem hiding this comment.
Thanks! A few more trivials, mainly just follow ups of the previous points.
LGTM.
internal/deb/extract.go
Outdated
| } | ||
| } | ||
| sort.Strings(errs) | ||
| return fmt.Errorf("%s", errs[0]) |
There was a problem hiding this comment.
Per comment in the prior review, we don't need to append all the errors to then return the first one. We can simply stop on the first thing we find and return the error.
There was a problem hiding this comment.
The problem with that is that the error won't be deterministic, right? Maybe we can sort the links instead of the error if that is more performant.
4 participants
Hard links can now be extracted and they are correctly reflected in the manifest.