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

RDA support: Make MATE's screensaver aware of being run inside a remo… #159

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

RDA support: Make MATE's screensaver aware of being run inside a remo… #159

wants to merge 2 commits into from

Conversation

sunweaver
Copy link
Member

@sunweaver sunweaver commented Jun 28, 2018 •
edited
Loading

…te desktop technology.

This patch adds a "Disconnect <remote-tech>" button to MATE's screensaver dialog
(if the <remote-tech> supports disconnecting from a running session).

When this button gets clicked, the remote session will get suspended
via librda's API (RDA = Remote Desktop Awareness).

Note on the PR: This feature is not yet fully ready for production. At least not under X2Go.

  • When resuming the session, I can take a quick glimpse at the underlying desktop session, only after a fraction of a second the screensaver dialog kicks in.
  • In fact, when resuming a MATE session via X2Go (or whatever tech that supports session resuming), my favourite approach would in fact be to unlock the session right away.
  • It may even make sense, to unlock the session directly after the session has been suspended to reduce the CPU load impact of the actual graphical screensaver (which assumingly runs in background)

Request for feedback on the above. Thanks!

@sunweaver
Copy link
Member Author

  • Alternatively, a suspended session would simply blacken its screen (and hide all graphical activities from the underlying Xserver of the remote framework software)

@vkareh vkareh requested a review from a team July 2, 2018 15:47
@raveit65 raveit65 requested a review from monsta July 3, 2018 08:42
@sunweaver
Copy link
Member Author

@monsta: @raveit65: what's the status on this PR? From IRC I read that remote desktop awareness should rather be moved into mate-session.

Basically, the mate-session code should query some parameters at session start up then, I guess, and then serve those to other MATE components.

The basic idea about all this is: MATE is nearly highly usable on remote desktop connections, even after the GTK-3 switch. For remote usage, it would be cool to switch on/off some features here and there, to make things really optimal.

This is what RDA is all about...

@sunweaver
Copy link
Member Author

sunweaver commented Aug 13, 2018 via email

@monsta
Copy link
Contributor

monsta commented Aug 14, 2018

I think it's ok, but I'm not sure if multiple MATE components should all be using this rda library... I thought about handling at least some common things in the session manager.

As this currently implies changes in more than one repo, I was going to start a team discussion on this.
@sunweaver: do you have write access to our Core Team discussions?

@monsta
Copy link
Contributor

monsta commented Aug 14, 2018

Oh, and we'll need librda in distros too 🙂

@sunweaver
Copy link
Member Author

sunweaver commented Aug 14, 2018 via email

@sunweaver
Copy link
Member Author

sunweaver commented Aug 14, 2018 via email

@sunweaver
Copy link
Member Author

sunweaver commented Aug 14, 2018 via email

@sunweaver
Copy link
Member Author

This PR has been included in mate-screensaver 1.20.3-3 in Debian (and Ubuntu disco). Please test. Thanks.

@sunweaver
Copy link
Member Author

sunweaver commented Jan 22, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Jan 22, 2019 via email

@sunweaver
Copy link
Member Author

Hi,

This PR has been included in mate-screensaver 1.20.3-3 in Debian (and Ubuntu disco). Please test. Thanks.

So, this is independent from mate-panel PR?

Yes.

And what test result is expected in x2go session?
before/ after?

Before applying this PR: MATE screensavers in a local session and in an X2Go session look the same

After applying this PR: MATE screensaver in X2Go will offer a [ Disconnect X2Go ] button. Clicking onto it will execute an x2gosuspend-session command in the background.

lukefromdc
lukefromdc reviewed Jan 25, 2019
View reviewed changes
Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I cannot test a remote session with no virtual machines (all is bare metal here), this builds and runs fine on bare metal with or without librda0 installed on Debian Unstable. Note that on Debian, /etc/pam.d/mate-screensaver has to be replaced with Debian's version or just dropped for the unlock dialog to come up

@lukefromdc
Copy link
Member

Note that Debian already builds with this now:
https://www.phoronix.com/scan.php?page=news_item&px=MATE-Debian-Remote-Aware
so if they don't get reports of this not working we can probably proceed to merge it.

@sunweaver
Copy link
Member Author

sunweaver commented Jan 25, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Jan 25, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Jan 25, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Jan 25, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Jan 25, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Feb 3, 2019 •
edited
Loading

Hi @raveit65

I finally managed to build mate-screensaver against librda 0.0.5 on Fedora 29 using mock:

mate-screensaver rda

It took me quite a while to figure that out. In general. After some trial and error, I built mate-screensaver 1.20.3 from Fedora 29 and added the RDA patch to it via Patch0: in the .spec file.

I built it inside mock. The not publicly available librda I added to the build chroot and started the next mock run with --no-clean option.

But the issue I encountered then was that the mock build would apply the patch but not show any signs of the RDA patch being applied to the configure script.

After a loooong time... my issue was: I built the modified mate-screensaver.spec from a release tarball (not from Git snapshot). And when building from release tarball, the .spec file won't call autogen.sh. Thus, no autoreconf run, either. And thus, no RDA support during configure and during build.

So... my guess now is... With your tests, you never did an autoreconf of mate-screensaver (and neither of mate-panel). Is that possible?

Thanks+Greets,
Mike

PS: here is the .spec file I used for mate-screensaver:

mate-screensaver.spec.txt

the diff to recent mate-screensaver in Fedora is: (ignore the bit regarding %autosetup -> %setup, it was done test-wise, but not the cause)

--- mate-screensaver.spec	2018-12-22 18:20:09.000000000 +0100
+++ mate-screensaver.spec.rda	2019-02-03 22:18:18.468413012 +0100
@@ -15,7 +15,7 @@
 Name:           mate-screensaver
 Version:        %{branch}.3
 %if 0%{?rel_build}
-Release:        1%{?dist}
+Release:        1.1%{?dist}
 %else
 Release:        0.9%{?git_rel}%{?dist}
 %endif
@@ -28,6 +28,7 @@
 %{?rel_build:Source0:     http://pub.mate-desktop.org/releases/%{branch}/%{name}-%{version}.tar.xz}
 # Source for snapshot-builds.
 %{!?rel_build:Source0:    http://git.mate-desktop.org/%{name}/snapshot/%{name}-%{commit}.tar.xz#/%{git_tar}}
+Patch0:         mate-screensaver-rda.patch
 
 Requires:       redhat-menus
 Requires:       system-logos
@@ -53,6 +54,7 @@
 BuildRequires:  systemd-devel
 BuildRequires:  xorg-x11-proto-devel
 BuildRequires:  xmlto
+BuildRequires:  librda-devel
 
 %description
 mate-screensaver is a screen saver and locker that aims to have
@@ -68,14 +70,16 @@
 
 
 %prep
-%if 0%{?rel_build}
-%autosetup -p1
-%else
-%autosetup -n %{name}-%{commit} -p1
-%endif
+# if 0%{?rel_build}
+# autosetup -p1
+# else
+# autosetup -n %{name}-%{commit} -p1
+# endif
+%setup
+%patch0 -p1 -b .rda
 
 %if 0%{?rel_build}
-#NOCONFIGURE=1 ./autogen.sh
+NOCONFIGURE=1 ./autogen.sh
 %else # 0%{?rel_build}
 # for snapshots
 # needed for git snapshots

PPS: and here is the patch file that I used:

mate-screensaver-rda.patch.txt

PPPS: and here is the librda.spec file that I used to build my librda.rpm:

librda.spec.txt

@sunweaver
Copy link
Member Author

When testing RDA functionality with the rdacheck utility, you should see this...

mate rdacheck

@sunweaver
Copy link
Member Author

sunweaver commented Feb 4, 2019 via email

@sunweaver
Copy link
Member Author

sunweaver commented Feb 4, 2019 via email

@raveit65
Copy link
Member

raveit65 commented Feb 4, 2019

for #159

NOCONFIGURE=1 ./autogen.sh

All is done here ^^^^^^

%build
%configure
--with-x
--disable-schemas-compile
--enable-docbook-docs
--with-mit-ext
--with-xf86gamma-ext
--with-libgl
--with-shadow
--enable-locking
--with-systemd
--enable-pam
--without-console-kit

make %{?_smp_mflags} V=1

Yes and the most important part is missing in the above spec snippet: what do you set 0%{?rel_build} to (at the very top of the spec file)? If it is set to 1, the configure file won't be recreated (as autogen.sh is commented out) and thus, RDA support won't compiled in.

Sadly, you don't have the skills to proper read a spec file.
Better look for another reviewer.

@sunweaver
Copy link
Member Author

sunweaver commented Feb 4, 2019 via email

@sunweaver
RDA support: Make MATE's screensaver aware of being run inside a remo…
78dcf1e 
…te desktop technology.

     This patch adds "Disconnect <remote-tech>" button to MATE's screensaver
     (if the <remote-tech> supports disconnecting from a running session).

     When this button gets clicked, the remote session will get suspended
     via librda's API (RDA = Remote Desktop Awareness).
@sunweaver
RDA: Disable switch user button, if session is remote.
140a0b7
@sunweaver
Copy link
Member Author

branch has been rebased against latest master + 1 more patch got added that disabled the switch user button, if session is remote.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukefromdc lukefromdc lukefromdc left review comments

@monsta monsta Awaiting requested review from monsta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sunweaver @monsta @lukefromdc @raveit65