Fix menu file leftover on vm remove#73
Conversation
|
@marmarek @ben-grande |
|
There are extraneous commits. Pylint is complaining: https://gitlab.com/QubesOS/qubes-desktop-linux-common/-/jobs/13371730924. Wait for CI to finish to request a review. |
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
2 times, most recently
from
9c86361 to
d36815a
Compare
|
Hi @ben-grande repro jobs: fail with ModuleNotFoundError: No module named 'pkg_resources' — infrastructure issue in the runner, not related to my changes |
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
2 times, most recently
from
a600c8a to
1f84f0f
Compare
|
Tests are failing: https://gitlab.com/QubesOS/qubes-desktop-linux-common/-/jobs/13395859676 Other than that, looks good. |
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
from
1f84f0f to
14f1ec1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #73 +/- ##
==========================================
+ Coverage 65.73% 67.67% +1.93%
==========================================
Files 2 2
Lines 683 696 +13
==========================================
+ Hits 449 471 +22
+ Misses 234 225 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Please squash the commits. |
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
from
14f1ec1 to
10a699e
Compare
| with unittest.mock.patch('xdg.BaseDirectory.xdg_config_home', | ||
| config_dir): | ||
| # Missing applications-merged directory should be ignored. | ||
| qubesappmenus.AppmenusExtension._remove_menu_files('test-vm') |
There was a problem hiding this comment.
It is missing some assertion that it was ignored.
|
I pushed a CI fix for |
| - git clone https://github.com/QubesOS/qubes-linux-utils ~/linux-utils | ||
| - git clone https://github.com/QubesOS/qubes-core-admin-client ~/core-admin-client | ||
| - (cd ~/core-admin-client;python3 setup.py egg_info) | ||
| - (cd ~/core-admin-client; if [ -f setup.py ]; then python3 setup.py egg_info; else python3 -m pip install --quiet --no-deps -e .; fi) |
There was a problem hiding this comment.
I don't think the old code is needed here anymore. Remove the quiet flag, it is not used in run-tests for example: https://github.com/QubesOS/qubes-core-admin-client/pull/424/changes
There was a problem hiding this comment.
Keeping the old code for now is fine, it will be helpful if working with older branch (for example adding -b release4.3 to the earlier clone command)
There was a problem hiding this comment.
FYI I cherry-picked this one commit to main already.
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
4 times, most recently
from
05b102f to
7cf4860
Compare
|
@marmarek @ben-grande |
|
PipelineRetryFailed |
|
@marmarek @ben-grande |
ben-grande
left a comment
ben-grande
left a comment
There was a problem hiding this comment.
Please don't change the changelog and version. Marek bumps the version a part from a PR, so he can bundle multiple ones together.
There was a problem hiding this comment.
I'm okay with leaving --quiet here. And this one commit is in main already.
This might be rebase issue? That part of the commit is already in main. Anyway, yes, please clean this up - just rebasing on top of updated main should be enough. Otherwise this PR looks good. |
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
from
7cf4860 to
8dc8be7
Compare
Jayant-kernel
force-pushed
the
fix-menu-file-leftover-on-vm-remove
branch
from
8dc8be7 to
380dedf
Compare
|
@marmarek |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026032922-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026032404-devel&flavor=update
Failed tests26 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/170766#dependencies 28 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:13 performance degradations
Remaining performance tests:91 tests
|
|
Thanks for the merge |
4 participants
After removing a qube, the file
~/.config/menus/applications-merged/user-qubes-vm-directory.menu
is left behind, still referencing .desktop files that no longer exist.
The root cause is that xdg-desktop-menu uninstall does not reliably
clean up the .menu file — CalledProcessError is suppressed, and
accumulated install calls across multiple app syncs can leave residual
entries that prevent the file from being removed.
Fix by adding _remove_menu_files() which explicitly unlinks the known
.menu file paths for the VM after appmenus_remove() runs. Handles both
the current escaped-name format and the older dash-separated format.
Fixes QubesOS/qubes-issues#9668