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.

Sign up for GitHub

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

Jump to bottom

Remove suffix for Wayland session #997

Merged
merged 3 commits into from
Mar 14, 2021
Merged

Remove suffix for Wayland session #997

merged 3 commits into from
Mar 14, 2021

Conversation

plfiorini
Copy link
Member

@plfiorini plfiorini commented Mar 20, 2018 •
edited
Loading

Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

[ChangeLog][Greeter] Remove suffix for Wayland sessions

@plfiorini plfiorini added this to the 0.18 milestone Mar 20, 2018
@plfiorini plfiorini changed the title Feature/session name Remove suffix for Wayland session Mar 20, 2018
@Vogtinator
Copy link
Contributor

I agree, but keeping backwards-compat in mind: How hard would it be to still apply the suffix if a conflicting session already exists?

@plfiorini plfiorini modified the milestones: 0.18, 0.19 Jul 17, 2018
@plfiorini
Copy link
Member Author

plfiorini commented Sep 30, 2019 •
edited
Loading

@Vogtinator What's a conflicting session?
EDIT: A session that exist both in xsessions and wayland-sessions?

@Vogtinator
Copy link
Contributor

@Vogtinator What's a conflicting session?

Current Plasma releases have plasma.desktop and plasmawayland.desktop which both have Name=Plasma. It relies on this suffix for distinction.

See also https://phabricator.kde.org/D22210

@Vogtinator Vogtinator mentioned this pull request Oct 1, 2019
Merged
@Pointedstick
Copy link
Collaborator

This seems like the right approach to me, and then the session itself becomes responsible for having an accurate name.

What's blocking this?

@davidedmundson
Copy link
Member

What's blocking this?

Me. We need to fix Plasma first.

We should have done it for 5.17, that's arguably my fault, sorry. I'll make sure we do for Plasma 5.18.

@a17r
Copy link
Contributor

a17r commented Oct 15, 2019

I think it is enough to make distro packagers aware through mail/changelog, no need for workarounds. We patch stuff now, we'll patch stuff in the meantime until it fits by default.

kdesysadmin pushed a commit to KDE/plasma-workspace that referenced this pull request Oct 16, 2019
@a17r
Fix Name conflict between plasma.desktop and plasmawayland.desktop
7f0ce07 
Summary:
SDDM until 0.18.1 appends " (Wayland)" to the name of any session file it
finds in wayland-sessions dir, and Plasma is relying on that behaviour to
distinguish between its X11 and Wayland sessions. This leads to duplicate
"Plasma" entries on any other DM not applying the same hack, e.g. lightdm,
and users are lost without downstream patching.

SDDM in 0.19 only appends " (Wayland)" in case the session name does
not already end with this, to avoid duplicating it.

CCBUG: https://bugs.kde.org/show_bug.cgi?id=368409
See also: sddm/sddm#997

Reviewers: #plasma, davidedmundson, fvogt, ngraham

Reviewed By: #plasma, davidedmundson, fvogt, ngraham

Subscribers: ngraham, pino, rdieter, fvogt, davidedmundson, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D22210
kdesysadmin pushed a commit to KDE/plasma-workspace that referenced this pull request Oct 16, 2019
@Pointedstick
Be explicit about X11 or Wayland for the dev session names
63dca88 
Summary:
I just switched to openSUSE Tumbleweed and noticed that the session names in SDDM's
chooser don't include "X11" or "Wayland" like they do in other distros I've used before. This
makes it impossible to tell which one you're going to boot into.

This patch adds those identifiers into the translated string so it's always clear.

Ideally sddm/sddm#997 would get done first, but even it it doesn't,
it's no big deal.

Reviewers: fvogt, davidedmundson, apol, #plasma

Reviewed By: davidedmundson, #plasma

Subscribers: fvogt, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D24666
@plfiorini
Remove suffix for Wayland session
7e49c43 
Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

Plasma is also headed in that direction.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

However if two sessions has the same display name we deduplicate
appending an indicator to the Wayland counterpart, this helps
Plasma during the transition to removing the session type from
their display name.

[ChangeLog][Greeter] Remove suffix for Wayland sessions and
deduplicate display names.
@plfiorini
Copy link
Member Author

Quick fix to deduplicate name, don't have time to even test this right now.
We'll do it later, but if you have time to test it please report what's wrong.
Basically it still appends " (Wayland)" if more than one session has the same name.

@plfiorini
Copy link
Member Author

That should help you with your transition,

@Vogtinator
Copy link
Contributor

Basically what I requested a few comments above, yay. Code looks good, but I didn't test it yet either.

@plfiorini
Copy link
Member Author

Exactly that

@Conan-Kudo
Copy link
Contributor

@plfiorini @davidedmundson ping? Could this be rebased and merged? With Plasma 5.18 and 5.19, we now have this ugly Plasma (Wayland) (Wayland) descriptor now for the Wayland session.

@Vogtinator
Copy link
Contributor

@Conan-Kudo

That was already taken care of with #1202 though.

@Conan-Kudo
Copy link
Contributor

Welp... Thanks @Vogtinator!

@plfiorini
Copy link
Member Author

This patch is different: it adds the Wayland suffix only when there are more than one sessions with the same name.
If I recall the problem was that Plasma had the same name under both Xorg and Wayland.
Adding the Wayland suffix in SDDM is a hack, desktops that are available in multiple windowing systems should be named accordingly upstream.

@Vogtinator
Copy link
Contributor

This patch is different: it adds the Wayland suffix only when there are more than one sessions with the same name.
If I recall the problem was that Plasma had the same name under both Xorg and Wayland.

With this patch, it won't be GNOME (Wayland) anymore, but for the Plasma (Wayland) (Wayland) case it won't actually make a difference at all.

@plfiorini
Copy link
Member Author

I was under the impression that Plasma had changed their desktop entry name removing Wayland.
I still believe that SDDM should stop resorting to this hack and desktops should get their name right.

@Vogtinator
Copy link
Contributor

I was under the impression that Plasma had changed their desktop entry name removing Wayland.
I still believe that SDDM should stop resorting to this hack and desktops should get their name right.

https://phabricator.kde.org/D22210 got merged, so plasmawayland.desktop has Name=Plasma (Wayland) since 5.18. The code here for deduplicating makes sure that even older plasmawayland.desktop appears separate. The only issue is with new Plasma and sddm without this PR or #1202, but that's nothing we can fix.

@plfiorini plfiorini modified the milestones: 0.19, 0.20 Nov 3, 2020
@plfiorini plfiorini merged commit ec92b49 into sddm:develop Mar 14, 2021
@plfiorini plfiorini deleted the feature/session_name branch March 14, 2021 09:29
aleixpol pushed a commit to aleixpol/sddm that referenced this pull request Mar 19, 2021
@plfiorini @aleixpol
Remove suffix for Wayland session (sddm#997)
7526aca 
Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

Plasma is also headed in that direction.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

However if two sessions has the same display name we deduplicate
appending an indicator to the Wayland counterpart, this helps
Plasma during the transition to removing the session type from
their display name.

[ChangeLog][Greeter] Remove suffix for Wayland sessions and
deduplicate display names.
davidedmundson pushed a commit to davidedmundson/sddm that referenced this pull request Mar 21, 2021
@plfiorini @davidedmundson
Remove suffix for Wayland session (sddm#997)
461e440 
Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

Plasma is also headed in that direction.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

However if two sessions has the same display name we deduplicate
appending an indicator to the Wayland counterpart, this helps
Plasma during the transition to removing the session type from
their display name.

[ChangeLog][Greeter] Remove suffix for Wayland sessions and
deduplicate display names.
peremen pushed a commit to peremen/sddm that referenced this pull request Jun 30, 2022
@plfiorini @peremen
Remove suffix for Wayland session (sddm#997)
d91250d 
Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

Plasma is also headed in that direction.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

However if two sessions has the same display name we deduplicate
appending an indicator to the Wayland counterpart, this helps
Plasma during the transition to removing the session type from
their display name.

[ChangeLog][Greeter] Remove suffix for Wayland sessions and
deduplicate display names.
githubkusi pushed a commit to githubkusi/sddm that referenced this pull request Jul 6, 2023
@plfiorini
Remove suffix for Wayland session (sddm#997)
43a4f96 
Some desktops like GNOME specify which windowing system is in use
with the Name entry of their desktop file.

Plasma is also headed in that direction.

For Wayland-only desktops such as Liri this information is
redundant and so is for X11-only window managers.

Do not append the Wayland suffix and let desktops handle it
themeselves.

However if two sessions has the same display name we deduplicate
appending an indicator to the Wayland counterpart, this helps
Plasma during the transition to removing the session type from
their display name.

[ChangeLog][Greeter] Remove suffix for Wayland sessions and
deduplicate display names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vogtinator Vogtinator Vogtinator approved these changes

@jleclanche jleclanche Awaiting requested review from jleclanche

@davidedmundson davidedmundson Awaiting requested review from davidedmundson

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
0.20
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@plfiorini @Vogtinator @Pointedstick @davidedmundson @a17r @Conan-Kudo