lxqt / lxqt-archiver

A simple & lightweight desktop-agnostic Qt file archiver
https://lxqt.github.io
GNU General Public License v2.0
39 stars 29 forks source link

Can't create zip archive under Debian-based systems #379

Closed K0-RR closed 9 months ago

K0-RR commented 9 months ago

I'm using fresh source build (although the bug happens also on the Ubuntu package)

[ 98%] Linking CXX executable lxqt-archiver
[100%] Built target lxqt-archiver
(master) [~/Documents/lxqt-archiver/build] k0rr@core $ cd src/
(master) [~/Documents/lxqt-archiver/build/src] k0rr@core $ ./lxqt-archiver
src_dir: /home/k0rr/Downloads/Takeout/Takeout/Poczta, dst: /
final: /home/k0rr/Downloads/.fr-nZGpdG/lxqt.zip -> /home/k0rr/Downloads/lxqt.zip, 0
Screenshot ![Screenshot_2024-02-09_10-53-28](https://github.com/lxqt/lxqt-archiver/assets/43277609/3836b11b-9619-47a0-bf0e-5c28c3766a3d)
Steps to Reproduce (for bugs)
  1. Build from the source
  2. Run GUI
  3. Try to create zip archive
Context

Other archivers like File roller, Engrampa, Ark do not have this issue, so no library should be missing.

System Information
K0-RR commented 9 months ago

This is the same as https://github.com/lxqt/lxqt-archiver/issues/349 except I managed to build from the source, which wasn't easy not knowing what dependencies are needed.

tsujan commented 9 months ago

Not reproducible. Zip archives are created without problem with the latest git source here.

This is the same as #349

Then please add any new info you may have to that report. A closed report doesn't disappear; it'll be reopened if it can be reproduced based on the available info.

K0-RR commented 9 months ago

Reported to Ubuntu - https://bugs.launchpad.net/ubuntu/+source/lxqt-archiver/+bug/2052880

May I ask what distribution are you using (or rather have used to test this issue)?

Then please add any new info you may have to that report. A closed report doesn't disappear

I stopped doing that because they always tell me to create my own issue… (when I reply to an older one, like in this case)

tsujan commented 9 months ago

they always tell me to create my own issue

Who are "they"? LXQt has no relation to Ubuntu.

sudipm-mukherjee commented 9 months ago

I happened to notice the Ubuntu bug today and tested on Ubuntu Noble. And for me, if I use the command lxqt-archiver --force -a /tmp/archive.tar.bz2 ./*.txt it is working properly and the archive.tar.bz2 is created with the files. But if I give the command lxqt-archiver --force -a /tmp/archive.zip ./*.txt to create a zip file then it fails. Attaching the screenshot. And I have confirmed I have zip installed in the system.

dpkg -l | grep zip
ii  7zip                                          23.01+dfsg-8                             amd64        7-Zip file archiver with a high compression ratio
ii  bzip2                                         1.0.8-5build1                            amd64        high-quality block-sorting file compressor - utilities
ii  gzip                                          1.12-1ubuntu1                            amd64        GNU compression utilities
ii  p7zip-full                                    16.02+transitional.1                     all          transitional package
ii  unzip                                         6.0-28ubuntu2                            amd64        De-archiver for .zip files
ii  zip                                           3.0-13                                   amd64        Archiver for .zip files

And the screenshot of the error:

Screenshot from 2024-02-11 15-56-21

tsujan commented 9 months ago

There should be a piece of info hidden somewhere; otherwise, with the current info, there's no problem here and no --force is needed:

zipped

sudipm-mukherjee commented 9 months ago

@tsujan I ran it with strace, and the problem seems to be coming from:

6488  execve("/usr/lib/7zip/7z", ["/usr/lib/7zip/7z", "a", "-w/home/sudip", "-tzip", "-mem=AES128", "-bd", "-y", "-l", "-mx=7", "--", "/tmp/.fr-9xQMAN/archive.zip", "9.txt", "8.txt", "7.txt", "6.txt", "5.txt", "4.txt", "3.txt", "2.txt", "1.txt", "10.txt"], 0x65423d0a8888 /* 19 vars */) = 0

When I ran that same command manually it fails:

$ /usr/lib/7zip/7z a -w/home/sudip -tzip -mem=AES128 -bd -y -l -mx=7 -- /tmp/1archive.zip 9.txt 8.txt 7.txt 6.txt 5.txt

Command Line Error:
Unknown switch:
-l

And then I removed -l from the command I manually executed and the zip file was created. The file /usr/lib/7zip/7z is from 7zip package and Ubuntu has 23.01+dfsg-8 version now. Also, /usr/lib/7zip/7z --help shows me that l is a command its not listed under switches.

tsujan commented 9 months ago

It's about zip, not 7z; isn't it?

As for 7zip, Arch has p7zip and 7-zip-full (and more). As far as I've tested with Arqiver, p7zip works better.

sudipm-mukherjee commented 9 months ago

Yes, its about zip. But I dont know why lxqt-archiver is choosing to use 7z in this case. But even if it uses 7z since -tzip has been given in the options so it will create a zip file. I also have p7zip and p7zip-full installed, but lxqt-archiver is still using 7z in my system. I was looking at your source and it seems to be coming from https://github.com/lxqt/lxqt-archiver/blob/master/src/core/fr-command-7z.c#L348.

tsujan commented 9 months ago

Yes, its about zip. But I dont know why lxqt-archiver is choosing to use 7z in this case.

See fr-command-zip.c inside the source. It uses the "zip" command, which is owned by zip 3.0-11 here.

I think were're getting somewhere now -- I mean that piece of info I mentioned... ;)

tsujan commented 9 months ago

Have you installed zip?

sudipm-mukherjee commented 9 months ago

I think there are two issues here.

  1. Its trying to use -l with 7z which is an invalid switch.
  2. In my system, its not trying to use zip even though its installed. Which we need to find out why. And. yes I have zip installed. Please see my comment at https://github.com/lxqt/lxqt-archiver/issues/379#issuecomment-1937794229 for the list.

I relooked at the strace log and I can see that your code has checked for zip and unzip and has found it. But even then it used 7z which is interesting.

tsujan commented 9 months ago

I can see that your code....

Not my code ;)

But even then it used 7z which is interesting.

I have no idea why it does that under Ubuntu; it doesn't here.

tsujan commented 9 months ago

Its trying to use -l with 7z which is an invalid switch.

With p7zip:

       -l     don't store symlinks; store the files/directories they point  to
              (CAUTION : the scanning stage can never end because of recursive
              symlinks like 'ln -s .. ldir')
sudipm-mukherjee commented 9 months ago

I can see that your code....

Not my code ;)

I meant upstream code. :)

But even then it used 7z which is interesting.

I have no idea why it does that under Ubuntu; it doesn't here.

I am trying to debug and see why its not using zip. But meanwhile can you please reopen this issue as the usage of -l with 7z is a valid bug.

sudipm-mukherjee commented 9 months ago

Its trying to use -l with 7z which is an invalid switch.

With p7zip:

       -l     don't store symlinks; store the files/directories they point  to
              (CAUTION : the scanning stage can never end because of recursive
              symlinks like 'ln -s .. ldir')

This is now getting confusing. :( In Ubuntu I have:

$ p7zip  --help
Usage: /usr/bin/p7zip [options] [--] [ name ... ]

Options:
    -c --stdout --to-stdout      output data to stdout
    -d --decompress --uncompress decompress file
    -f --force                   do not ask questions
    -k --keep                    keep original file
    -h --help                    print this help
    --                           treat subsequent arguments as file
                                 names, even if they start with a dash

There is no reference of -l.

sudipm-mukherjee commented 9 months ago

ok, so the man page of 7z from arch lists -l, but the man page from Ubuntu does not list it.

https://man.archlinux.org/man/7z.1 https://manpages.ubuntu.com/manpages/noble/en/man1/7z.1.html

tsujan commented 9 months ago

A shot in the dark: remove any package related to 7zip except for p7zip and retry.

tsujan commented 9 months ago

It seems to me that Debian-based systems don't have p7zip at all: p7zip is just a transitional package in them: https://packages.debian.org/sid/p7zip

sudipm-mukherjee commented 9 months ago

It seems to me that Debian-based systems don't have p7zip at all: p7zip is just a transitional package in them: https://packages.debian.org/sid/p7zip

Yes, I noticed that while trying to remove it. And, also p7zip-full is marked as a dependency of lxqt-archiver, so 7zip is now the default.

tsujan commented 9 months ago

In the meanwhile, I checked engrampa's code too: it also used -l.

tsujan commented 9 months ago

Also, except for differences that aren't related to this, fr-command-7z.c and fr-command-zip.c are almost identical in lxqt-archiver and engrampa (because the former was derived from the latter). So, I wonder if engrampa works without this problem there.

tsujan commented 9 months ago

Found this: https://github.com/mate-desktop/engrampa/issues/502

sudipm-mukherjee commented 9 months ago

Found this: mate-desktop/engrampa#502

So, that has been fixed in Debian by https://salsa.debian.org/debian-mate-team/engrampa/-/commit/b7961f856ce613f937f9a1259d1d3b5792a2bdf4

And the bug was https://bugs.debian.org/1061510

sudipm-mukherjee commented 9 months ago

I just tested a quick fix:

--- lxqt-archiver-0.9.0.orig/src/core/fr-command-7z.c
+++ lxqt-archiver-0.9.0/src/core/fr-command-7z.c
@@ -345,7 +345,9 @@ fr_command_7z_add (FrCommand     *comm,
        if (spd_support) fr_process_add_arg (comm->process, "-spd");
        fr_process_add_arg (comm->process, "-bd");
        fr_process_add_arg (comm->process, "-y");
+#ifndef USE_7Z
        fr_process_add_arg (comm->process, "-l");
+#endif
        add_password_arg (comm, comm->password, FALSE);
        if ((comm->password != NULL)
            && (*comm->password != 0)

Whoever wants to use 7z will have to define USE_7Z in their build flags. So, your ( upstream's :) ) normal builds will work as before and other distro will not see any affect.

tsujan commented 9 months ago

Please make a PR!

tsujan commented 9 months ago

Let's reopen this under a new title; to be closed when https://github.com/lxqt/lxqt-archiver/pull/380 is merged.