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

plasma-workspace: add systementry sessionmanagement patch #99582

Closed

Conversation

@NinjaTrappeur
Copy link
Member

@NinjaTrappeur NinjaTrappeur commented Oct 4, 2020

Motivation for this change

Systemd v246 hides the interface used by KDE to fetch the user session
details. We're applying this commit from 5.19.90 upgrading the
systementry applet to use the new sessionmanagement API.

Because of this API breakage, the "Switch User" item was absent from
the KDE application laucher.

See the #98141 issue for the
full story.

Things done

Pick https://invent.kde.org/plasma/plasma-workspace/-/commit/05414ed58d43d87d907326636faac53ae2e7bd60 from upstream.

The missing items are appearing again and are fully functional.

Screenshot showing the "switch user" item re-appearing in the launch menu

How to Test?

I used this trick #98141 (comment).

Don't forget to point to your local nixpkgs checkout:

mid=$(nixops create nixops-kde-test.nix)
nixops modify -d $mid -I nixpkgs=/path/to/your/checkout nixops-kde-test.nix
nixops deploy -d $mid

Note for @ttuegel: I'll let you handle the backport (if you're ok with this PR ofc :)).

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Member

@ttuegel ttuegel left a comment

We should use fetchpatch instead of fetchurl.

pkgs/desktops/plasma-5/plasma-workspace/default.nix Outdated Show resolved Hide resolved
pkgs/desktops/plasma-5/plasma-workspace/default.nix Outdated Show resolved Hide resolved
@ttuegel
Copy link
Member

@ttuegel ttuegel commented Oct 4, 2020

@NinjaTrappeur Does this also fix the lock screen?

@NinjaTrappeur NinjaTrappeur force-pushed the NinjaTrappeur:nin-fix-plasma-workspace branch from 1b82778 to 7cabb77 Oct 4, 2020
Systemd v246 hides the interface used by KDE to fetch the user session
details. We're applying this commit from 5.19.90 upgrading the
systementry applet to use the new sessionmanagement API.

Because of this API breakage, the "Switch User" item was absent from
the KDE application laucher.

See the #98141 issue for the
full story.
@NinjaTrappeur NinjaTrappeur force-pushed the NinjaTrappeur:nin-fix-plasma-workspace branch from 7cabb77 to a054b43 Oct 4, 2020
@NinjaTrappeur
Copy link
Member Author

@NinjaTrappeur NinjaTrappeur commented Oct 4, 2020

Does this also fix the lock screen?

I did not check the lock screen, I only verified the item was appearing in the login screen and application launcher.

I destroyed the VM and garbage collected the store. I think I'll call it a day for now.

I'll check this tomorrow night CEST (if you cannot do it by then).

PTAL.

@ttuegel
Copy link
Member

@ttuegel ttuegel commented Oct 4, 2020

The launcher is fixed, but the lock screen is not fixed. I guess that we need a patch to kscreenlocker, too.

@worldofpeace worldofpeace added this to To Do in 20.09 Blockers via automation Oct 4, 2020
@worldofpeace worldofpeace moved this from To Do to In progress in 20.09 Blockers Oct 4, 2020
@worldofpeace worldofpeace moved this from In progress to Needs Review in 20.09 Blockers Oct 4, 2020
( # Systemd v246 hides the interface used by KDE to fetch the
# user session details.
# We're applying this commit from 5.19.90 upgrading the
# systementry applet to use the new sessionmanagement API.
# See https://github.com/NixOS/nixpkgs/issues/98141 for more
# details.
# /!\ Remove this patch for version >= v5.19.90
fetchpatch {
name = "0003-port-systementry-to-sessionmanagement-api.patch";
url = "https://invent.kde.org/plasma/plasma-workspace/-/commit/05414ed58d43d87d907326636faac53ae2e7bd60.patch";
sha256 = "16izfcwxjkdn99j777ywjkzl01iyl542h4hpsbpkckccj7hz8sin";
})
Comment on lines +49 to +60

This comment has been minimized.

@jonringer

jonringer Oct 4, 2020
Contributor

This may be hacky since there's not a good "Qt source of truth", but would we be able to conditionally disable this, something like:

] ++ lib.optional (lib.versionOlder qtquickcontrols.version "5.19") [
  <patch>
 ];

This comment has been minimized.

@jonringer

jonringer Oct 4, 2020
Contributor

actually, the Qt PRs will probably visit this, and might not be necessary.

Too much time spent doing python PRs :)

@jonringer
Copy link
Contributor

@jonringer jonringer commented Oct 4, 2020

seems like no regressions from a program standpoint. Haven't tested

https://github.com/NixOS/nixpkgs/pull/99582
3 packages marked as broken and skipped:
libsForQt5.khotkeys libsForQt512.khotkeys libsForQt515.khotkeys

16 packages built:
kde-cli-tools kdeplasma-addons kdev-php kdev-python kdevelop kdevelop-unwrapped kmenuedit krohnkite kwin-dynamic-workspaces kwin-tiling plasma5.khotkeys plasma-desktop plasma-workspace powerdevil systemsettings wacomtablet
@NinjaTrappeur
Copy link
Member Author

@NinjaTrappeur NinjaTrappeur commented Oct 5, 2020

Indeed, I can also reproduce the problem on the lockscreen. I don't see any fix upstream be it in code or as a related bugtracker ticket.

I need help here, there's a hole in my understanding of kde-workspace.

As far I understand, the item visibility is defined at https://invent.kde.org/plasma/plasma-workspace/-/blob/master/lookandfeel/contents/lockscreen/LockScreenUi.qml#L277

The sessionsModel.canSwitchUser is the right way to get the capability, I don't see anything wrong :/

However, this action item is clearly not visible. Meaning either sessionsModel.canStartNewSession or sessionsModel.canSwitchUser is not verified in our context.

Any idea what could happen here @ttuegel? Are you in contact with some upstream dev. They might be able to help here.

I'd be interested to see if your systemd patch fixes this issue. If it is, I guess we should investigate the sessionsModel.canSwitchUser implementation.

@worldofpeace worldofpeace removed this from Needs Review in 20.09 Blockers Oct 5, 2020
@NinjaTrappeur
Copy link
Member Author

@NinjaTrappeur NinjaTrappeur commented Oct 5, 2020

Fixed by #99629

@NinjaTrappeur NinjaTrappeur deleted the NinjaTrappeur:nin-fix-plasma-workspace branch Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.