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

simple-extract: alternative (un)compressor - 7zip #177

Open
crpb opened this issue Jan 6, 2025 · 7 comments · May be fixed by #178
Open

simple-extract: alternative (un)compressor - 7zip #177

crpb opened this issue Jan 6, 2025 · 7 comments · May be fixed by #178

Comments

@crpb
Copy link
Contributor

crpb commented Jan 6, 2025

It might happen that you only have /usr/bin/7zz installed but not /usr/bin/7z which will make simple-extract foo.7z fail.

Given with those choices/coming changes:

% apt-file --filter-suites bookworm search bin/7z
7zip: /usr/bin/7zz
p7zip: /usr/bin/7zr
p7zip-full: /usr/bin/7z
p7zip-full: /usr/bin/7za
% apt-file --filter-suites trixie search bin/7z
7zip: /usr/bin/7z
7zip: /usr/bin/7za
7zip: /usr/bin/7zr
7zip-standalone: /usr/bin/7zz

We would need either something like this or similiar

grml-etc-core (git)-[master] % git diff master simple-extract-7z-ifelse
diff --git a/etc/zsh/zshrc b/etc/zsh/zshrc
index f26af6c..9179ec3 100644
--- a/etc/zsh/zshrc
+++ b/etc/zsh/zshrc
@@ -3367,6 +3367,11 @@ function simple-extract () {
                 ;;
             *7z)
                 DECOMP_CMD="7z x"
+                if ! check_com 7z; then
+                    if check_com "7zz"; then DECOMP_CMD="7zz x"
+                    elif check_com "7zr"; then DECOMP_CMD="7zr x"
+                    fi
+                fi
                 USES_STDIN=false
                 USES_STDOUT=false
                 ;;

Or maybe use an array (in general?) w/ fallback of DECOMP_CMD

grml-etc-core (git)-[master] % git diff master simple-extract-7z
diff --git a/etc/zsh/zshrc b/etc/zsh/zshrc
index f26af6c..1510725 100644
--- a/etc/zsh/zshrc
+++ b/etc/zsh/zshrc
@@ -3320,7 +3320,7 @@ function trans () {
 function simple-extract () {
     emulate -L zsh
     setopt extended_glob noclobber
-    local ARCHIVE DELETE_ORIGINAL DECOMP_CMD USES_STDIN USES_STDOUT GZTARGET WGET_CMD
+    local ARCHIVE DELETE_ORIGINAL DECOMP_CMD DECOMP_CMDS CMD USES_STDIN USES_STDOUT GZTARGET WGET_CMD
     local RC=0
     zparseopts -D -E "d=DELETE_ORIGINAL"
     for ARCHIVE in "${@}"; do
@@ -3366,6 +3366,7 @@ function simple-extract () {
                 USES_STDOUT=false
                 ;;
             *7z)
+                DECOMP_CMDS=( "7z x" "7zz x" "7zr x" )
                 DECOMP_CMD="7z x"
                 USES_STDIN=false
                 USES_STDOUT=false
@@ -3412,6 +3413,15 @@ function simple-extract () {
                 ;;
         esac

+        if (( ${+DECOMP_CMDS} )); then
+            for CMD in ${DECOMP_CMDS[@]}; do
+                if check_com "${CMD[(w)1]}"; then
+                    DECOMP_CMD="$CMD"
+                    break
+                fi
+            done
+        fi
+
         if ! check_com ${DECOMP_CMD[(w)1]}; then
             echo "ERROR: ${DECOMP_CMD[(w)1]} not installed." >&2
             RC=$((RC+2))

not really happy with it but just to illustrate how it could be achieved 🙈

Another idea i got was to check if we got an string(scalar) or array in $DECOMP_CMDS and do the magic accordingly ?_?

obelix% MOO=( "MOO moo" "OOM moo" "OOM oom" "MOO oom" ); [[ ${(t)MOO} == array ]] && print -l "\$MOO is a ${(t)MOO}\n${MOO[@]}" || print -l "\$MOO is a ${(t)MOO}\n$MOO"
$MOO is a array
MOO moo
OOM moo
OOM oom
MOO oom
obelix% MOO="MOO moo"; [[ ${(t)MOO} == array ]] && print -l "\$MOO is a ${(t)MOO}\n${MOO[@]}" || print -l "\$MOO is a ${(t)MOO}\n$MOO"
$MOO is a scalar
MOO moo

EDIT: maybe other commands also have alternatives that might need fix/adding?

@ft
Copy link
Member

ft commented Jan 6, 2025

Hm. Interestingly, simple-extract is one of the things that I thought we might remove at some point. That being said, there are certainly ways to improve the situation. Not sure how much work is worth it here, though. I find the function a little curious to start with. For instance, personally I always check the contents of an archive before extracting it, just to make sure the archive creates a directory, and won't litter my current working directory… I suppose we could add a -l option to support listing for all archive types as well… Then it also implicitly calls wget, which I find more surprising than useful. But maybe that's also just me.

The "one-ring-to-rule-them-all" solution, proposed might break, if different programs need different values for USE_STDIN and the like. Another thing to consider is that users might want to control which extracting program to use when multiple solutions are available, which could be done via zstyle. But I think that's all too much work for such a simple application.

Another thing is portability, since this implementation definitely requires GNU tar for many of the tar file types. That's not fantastic either. The fact that DECOMP_CMD is a string and not an array is dubious at least. I'd probably get rid of most of the magic, and have a function per file-type to dispatch to and make that function do the right thing, and put similar code structures into utility functions. But then and again I won't be the one doing it, so I guess I should stop rambling.

Getting back to the problem at hand, I think your initial suggestion would be a straight-forward, least intrusive solution. We could include something like that, without thinking too much about redesigning this thing.

@crpb crpb linked a pull request Jan 6, 2025 that will close this issue
@crpb
Copy link
Contributor Author

crpb commented Jan 6, 2025

just to make sure the archive creates a directory, and won't litter my current working directory

Thats why i have this function (which needs a bit of work! ^_^)

% whence -f sde
sde () {
        local f d
        test -f "${1}" || {
                echo "file missing"
                return 1
        }
        f=${1:a:q}
        d=${f:r}
        test ! -d "${d}" || {
                echo "folder ${d} already exists"
                return 1
        }
        mkdir -p "${d}"
        cd "${d}" && simple-extract "${f}"
}

especially files that have various brackets fail here so i usually use detox on those 🙈

requires GNU tar for many of the tar file types

well, i'm not sure since which version it's working but tar -xvf foo.tar{,.gz,.bz2,.xz} works here pretty much everywhere or at least on where i could test it quickly: debian+fbsd13(bsdtar)].

Anyhow, i guess that all should go into a new issue. Pushed the simple-fix for now.

@zeha
Copy link
Member

zeha commented Jan 6, 2025

i think bsdtar is kinda "better", in the sense that it uses libarchive and can unpack almost anything :)
dont really need simple-extrsct if you have bsdtar 😂

@crpb
Copy link
Contributor Author

crpb commented Jan 7, 2025

better

there might be odd behaviour anyway :----)

% find .
.
% cat << EOF > snapshots
p0/media/isos@auto-2024-11-01_07-30-1D-4W
p0/media/isos@auto-2024-11-02_07-30-1D-4W
p0/media/isos@auto-2024-11-03_07-30-1D-4W
p0/media/movies@auto-2024-11-01_07-30-1D-4W
p0/media/movies@auto-2024-11-02_07-30-1D-4W
p0/media/movies@auto-2024-11-03_07-30-1D-4W
p0/media/music@auto-2024-11-01_07-30-1D-4W
p0/media/music@auto-2024-11-02_07-30-1D-4W
p0/media/music@auto-2024-11-03_07-30-1D-4W
p0/media/tv@auto-2024-11-01_07-30-1D-4W
p0/media/tv@auto-2024-11-02_07-30-1D-4W
p0/media/tv@auto-2024-11-03_07-30-1D-4W
p0/media/tv@auto-2024-11-04_00-15-4H-1W
EOF

% find .
.
./snapshots
% zstd snapshots
snapshots            : 21.76%   (   547 =>    119 bytes, snapshots.zst)
% find .
.
./snapshots
./snapshots.zst
% tar xf snapshots
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Missing type keyword in mtree specification
tar: Error exit delayed from previous errors.
1 % find .
.
./p0
./p0/media
./p0/media/movies@auto-2024-11-02_07-30-1D-4W
./p0/media/isos@auto-2024-11-02_07-30-1D-4W
./p0/media/tv@auto-2024-11-01_07-30-1D-4W
./p0/media/music@auto-2024-11-02_07-30-1D-4W
./p0/media/tv@auto-2024-11-03_07-30-1D-4W
./p0/media/music@auto-2024-11-03_07-30-1D-4W
./p0/media/music@auto-2024-11-01_07-30-1D-4W
./p0/media/tv@auto-2024-11-02_07-30-1D-4W
./p0/media/tv@auto-2024-11-04_00-15-4H-1W
./p0/media/movies@auto-2024-11-03_07-30-1D-4W
./p0/media/isos@auto-2024-11-03_07-30-1D-4W
./p0/media/movies@auto-2024-11-01_07-30-1D-4W
./p0/media/isos@auto-2024-11-01_07-30-1D-4W
./snapshots
./snapshots.zst
% bsdtar --version
bsdtar 3.6.2 - libarchive 3.6.2 zlib/1.2.13 liblzma/5.4.1 bz2lib/1.0.8 libzstd/1.4.8

yes, those are folders and files in $PWD/p0/...

Maybe thats a feature, i called it very irritating when i stumbled over it..

@anarcat
Copy link

anarcat commented Jan 7, 2025

simple-extract is one of the things that I thought we might remove at some point

if you want to consider that more seriously, i think a few options already packaged in debian should be evaluated:

@ft
Copy link
Member

ft commented Jan 7, 2025

simple-extract is one of the things that I thought we might remove at some point

if you want to consider that more seriously, i think a few options already packaged in debian should be evaluated:
[…]

I would remove it for sure. But I also don't use it myself. I think it's a little too weird (shell — even zsh — has limitations with regards to expressivity) for what it does. I know none of the alternatives you suggested. From a cursory look, patool looks promising.

@mika
Copy link
Member

mika commented Jan 7, 2025

simple-extract is one of the things that I thought we might remove at some point

if you want to consider that more seriously, i think a few options already packaged in debian should be evaluated:
[…]

I would remove it for sure. But I also don't use it myself. I think it's a little too weird (shell — even zsh — has limitations with regards to expressivity) for what it does. I know none of the alternatives you suggested. From a cursory look, patool looks promising.

I must admit that I don't use simple-extract myself either (mainly because it's not in my memory muscles :)).
unp usually worked fine for me, though if we have a good option that works for most/everything I'm all for it. :)
But I also don't want to stop anyone from further iterating over improving simple-extract, as long it's not getting way too complex and would warrant its own shell script or so 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants