pkg: rewrite archive extraction with typed tar capabilities #12917
Conversation
We rewrite the extraction capabilities for package management, enforcing the invariants we wish to preserve at the type level. The aim is to separate the concerns of finding an appropriate archive binary, checking the capabilities of this binary, creating arguments for this binary and invoking this for extraction. The public API of extraction is still simple, so the typed invariants are more of an internal impelementation detail. A sumamry of the changes are as follows: - Use GADTs to encode which formats each tar implementation supports, preventing invalid combinations at compile time - Detect tar implementation (BSD/GNU/Other) via --version output - Prefer BSD tar over unzip for zip extraction - Check bsdtar before tar to get more capable implementation - Add gtar to binary list for GNU tar on macOS/BSD - Pass explicit -z/-j flags for OpenBSD/busybox tar Signed-off-by: Ali Caglayan <[email protected]>
Alizter
force-pushed
the
push-tkrvytnyslnk
branch
from
848c4f5 to
a6eb859
Compare
|
Does this actually solve #12818 ? The scope of that issue has been sharpened to be about ensuring that dev tools are removed if installation fails for any reason, but skimming the diff here, I don't see a fix addressing that. Am I just missing that logic, or should we remove that issues from the list of issues fixed by this PR? |
| | | | | | | ||
| | Gnu | GNU tar (most Linux) | yes | no | | ||
| | | | | | | ||
| | Other | OpenBSD tar, busybox tar | no | no | |
There was a problem hiding this comment.
@shonfeder this is the tar binary on Open BSD
There was a problem hiding this comment.
I am not sure why your are pointing these out to me, but thanks :)
There was a problem hiding this comment.
You mentioned if this solves #12818. The issue there is about openbsd not invoking tar correctly. Here we are doing the suggested fix.
There was a problem hiding this comment.
The title of that issue is "If dune tools install fails to install a tool it can break a user's environment". It happened that the failure in that case was caused by the tar issue, but that is covered in #10123, which is a separate issue.
See #12818 (comment)
I think your reply here helps clarify that the answer to my question is 'no', it does not close #12818.
| | K_bsd, (`Tar | `Tar_gz | `Tar_bz2 | `Zip) -> [] | ||
| | K_gnu, (`Tar | `Tar_gz | `Tar_bz2) -> [] | ||
| | K_other, `Tar -> [] | ||
| | K_other, `Tar_gz -> [ "-z" ] |
There was a problem hiding this comment.
@shonfeder Here is the -z flag for OpenBSD
|
Can you split off some work from this PR? It's a little hard to understand what's a fix and what's just a refactor. It would be good to start with the tests, then the fixes, and then the refactorings. |
I'll do that when I've done some more manual testing. |
4 participants
We rewrite the extraction capabilities for package management, enforcing the invariants we wish to preserve at the type level. The aim is to separate the concerns of finding an appropriate archive binary, checking the capabilities of this binary, creating arguments for this binary and invoking this for extraction.
The public API of extraction is still simple, so the typed invariants are more of an internal impelementation detail.
A sumamry of the changes are as follows:
taron OpenBSD #10123