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

Copy/move progress between qubes in the UI copy dialog does not work properly #8723

Closed
jamke opened this issue Nov 22, 2023 · 28 comments · Fixed by QubesOS/qubes-core-agent-linux#476
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. r4.2-vm-bookworm-stable r4.2-vm-bullseye-stable r4.2-vm-centos-stream8-stable r4.2-vm-fc37-stable r4.2-vm-fc38-stable r4.2-vm-fc39-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Milestone

Comments

@jamke
Copy link

jamke commented Nov 22, 2023

Qubes OS release

R4.1 (R4.2, too?)

Brief summary

When I copy files from Dolphin with the qubes os service menu, it shows dialog in the process or copying, but the progress bar never properly reflects the progress. Probably it is based on files, not file data transfered (unlike terminal qvm-copy tool).

Steps to reproduce

Copy a large file from one qube to another.

Expected behavior

The progress slowly grows from 0 to 100% (full) with the almost constant speed.
Maybe, in case of move, the removing progress should be also reflected.

Actual behavior

Progress is wrong, e.g. always zero for the whole time.

@jamke jamke added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. labels Nov 22, 2023
@rustybird
Copy link

On the sender side: What's the TemplateVM and qubes-core-agent version? Also, are both kdialog and qdbus installed? Otherwise qvm-copy-to-vm.kde as used by Dolphin would (attempt to) fall back on zenity for the progress window.

With both kdialog and qdbus installed, it looks completely broken here on R4.2 Debian 12 bookworm:

$ truncate -s 2G file
$ bash -x /usr/lib/qubes/qvm-copy-to-vm.kde file; echo exited $?
[...]
+ ref=($(kdialog --progressbar "$DESCRIPTION"))
++ kdialog --progressbar 'Copying files...'
+ trap 'qdbus "${ref[@]}" close' EXIT
+ qdbus org.kde.kdialog-4632 /ProgressDialog Set '' maximum 2147483648
qdbus: could not find a Qt installation of ''
++ qdbus org.kde.kdialog-4632 /ProgressDialog close
qdbus: could not find a Qt installation of ''
exited 1

The Filecopy fails right away, but the progress bar window (showing an uninitialized progress bar) remains open.

@jamke
Copy link
Author

jamke commented Nov 22, 2023

I have kdialog and qdbus installed. I updated template (fedora-38-minimal based), the qubes-core-agent version is 4.1.46-1.fc38.

The problem still persists, when I copy 3GiB file, the progress in the KDialog dialog is zero for all the time and then the dialog closes after file successfully copied.

@rustybird
Copy link

rustybird commented Nov 22, 2023

If you can uninstall kdialog and/or qdbus and install zenity instead, that would work around the bug(s).

@andrewdavidwong andrewdavidwong added C: core needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. affects-4.1 This issue affects Qubes OS 4.1. labels Nov 22, 2023
@jamke
Copy link
Author

jamke commented Nov 23, 2023

If you can uninstall kdialog and/or qdbus and install zenity instead, that would work around the bug(s).

Not possible. Removing any of kdialog or qdbus (package qt) will also remove 42-43 dependent packages that I need.

So, I will have to wait when the bug(s) is fixed in Qubes OS. In my case the moving/copying files works, just progress is not properly shown. Maybe the code should be modified to always use zenity and do not fallback to kdialog if it is currently broken?

@GWeck
Copy link

GWeck commented Nov 23, 2023

This seems to be related to the usage of Dolphin: When copying with Nautilus (under XFCE as well as under KDE), the progress bar advances as it should.

@jamke
Copy link
Author

jamke commented Nov 23, 2023

@rustybird @GWeck on my fedora-38-minimal based system /usr/lib/qubes/qvm-copy-to-vm.kde is just a symlink to qvm-copy-to-vm.gnome. The difference is inside this script, which checks what is the script name ($0) and if it ends in "kde" and kdialog and qdbus exists, then it uses kdialog.

@jamke
Copy link
Author

jamke commented Nov 23, 2023

I commented out

elif [[ $0 == *.kde ]] && type kdialog qdbus >/dev/null; then
    PROGRESS_TYPE=gui  copy "$@" | progress_kdialog

To force using zenity and its progress shows properly. Then I uncommented the change back and after that the progress also works in kdialog dialog. Strange.

@jamke
Copy link
Author

jamke commented Nov 23, 2023

@rustybird I see that you are the person who was last modifying upstream bash scripts qvm-copy and qvm-copy-to-vm.gnome. Can you please help me to understand one of the latest commits (I have different source code in my system for now):

find_paths=( )
for path; do
    case "$path" in
        (-*) find_paths+=( ./"$path" ) ;;
        (*)  find_paths+=(   "$path" ) ;;
    esac
done

You break arguments adding './' to avoid passing it as a flag and make an array out of it. I do not understand, why the usual approach with passing -- to the command to avoid misinterpretation of it as flags does not work?

If possible I would like to ask you to avoid using for this way in bash, because it is not obvious, nor explicit. Wouldn't
for path in "$@" be better and cleaner?

@rustybird
Copy link

rustybird commented Nov 23, 2023

why the usual approach with passing -- to the command to avoid misinterpretation of it as flags does not work?

https://www.gnu.org/software/findutils/manual/html_mono/find.html#Starting-points

Wouldn't for path in "$@" be better and cleaner?

Eh, leaving out in "$@" is pretty common and behaves identically even in classic Bourne/POSIX sh, so this idiom has been around for a while. ;)

https://www.gnu.org/software/bash/manual/bash.html#index-for

@jamke
Copy link
Author

jamke commented Nov 23, 2023

https://www.gnu.org/software/findutils/manual/html_mono/find.html#Starting-points

I see, so you had to fight some kind of broken call convention of GNU find . Got it.

Eh, leaving out in "$@" is pretty common and behaves identically even in classic Bourne/POSIX sh, so this idiom has been around for a while. ;)

Yeah, I know. Just every time I read bash with such for omissions, I struggle: why would somebody drop the valuable part of the code! As every python developer would say: "Explicit is better than implicit". :)

@jamke
Copy link
Author

jamke commented Nov 23, 2023

I checked zenity and kdialog for possible improvements of these copy/move dialogs and unfortunately both these apps lack advanced features.
E.g. I would love to have a check box "Do not close dialog on finish", as it is presented in KDE when I copy files. It is very useful, because if copy process is important, I can check it and then switch to this dialog at any moment and check if copying was finished properly, to be sure that dialog auto-closed because some process or application died and collapsed the dialog.

On the other hand, I managed to add percentage and/or bytes count to the dialog, and time remaining estimation (flag --time-remaining) quite easily. What do you think, should something like that be implemented? At least until some better dialog implemented in the future?

@rustybird
Copy link

rustybird commented Nov 23, 2023

--time-remaining looks nice for sure, because it wouldn't add any complexity to the script.

% and byte count might be worth it too? Maybe break that out into a separate commit in the pull request. It's up to @marmarek whether it will be merged.

@jamke
Copy link
Author

jamke commented Nov 24, 2023

--time-remaining looks nice for sure, because it wouldn't add any complexity to the script.

% and byte count might be worth it too? Maybe break that out into a separate commit in the pull request. It's up to @marmarek whether it will be merged.

I can do that, and maybe will. The question is, though, should it be added if it is implemented only for zenity? I think kdialog is a bit more advanced, but it has more dependencies and no useful major features. Maybe the whole script should keep zenity-only part, and in case of kdialog it can just message something like "Please install zenity package to the template to see the progress" using kdialog. Maybe not, I do not know.

What I do know, implementing python script instead of this bash script with some small but more advanced UI (including checkbox I mentioned, including % progress in the title, maybe speed and etc.) would be interesting for me. It can be done with glade + python to keep the source consistent with the Qubes OS base.

@jamke
Copy link
Author

jamke commented Nov 24, 2023

What I do know, implementing python script instead of this bash script with some small but more advanced UI (including checkbox I mentioned, including % progress in the title, maybe speed and etc.) would be interesting for me. It can be done with glade + python to keep the source consistent with the Qubes OS base.

@marmarek @DemiMarie what do you guys think of it?

@DemiMarie
Copy link

@jamke Using a Python script would be a good idea, unless there is a compelling reason to not use Python (I don’t think there is).

@jamke
Copy link
Author

jamke commented Dec 1, 2023

To be clear, the issue is completely not fixed, the progress is not shown properly when kdialog is installed in the vast majority of cases.

rustybird added a commit to rustybird/qubes-core-agent-linux that referenced this issue Dec 2, 2023
The estimated total size in bytes can be too large to use as kdialog's
maximum. Use the default 100 instead and calculate the a percentage for
the current position, like in progress_zenity().

Fixes QubesOS/qubes-issues#8723
@andrewdavidwong andrewdavidwong added this to the Release 4.2 milestone Dec 4, 2023
@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). pr submitted A pull request has been submitted for this issue. and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Dec 4, 2023
@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing bullseye-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing bookworm-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 testing repository for the CentOS centos-stream8 template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.2-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 testing repository for the Fedora template.
To test this update, please install it with the following command:

sudo dnf update --enablerepo=qubes-vm-r4.2-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 testing repository for the Fedora template.
To test this update, please install it with the following command:

sudo dnf update --enablerepo=qubes-vm-r4.2-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 testing repository for the Fedora template.
To test this update, please install it with the following command:

sudo dnf update --enablerepo=qubes-vm-r4.2-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the CentOS centos-stream8 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. r4.2-vm-bookworm-stable r4.2-vm-bullseye-stable r4.2-vm-centos-stream8-stable r4.2-vm-fc37-stable r4.2-vm-fc38-stable r4.2-vm-fc39-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants