Bug 1298715 - Review Request: xlogin - Automatic X login service for systemd
Summary: Review Request: xlogin - Automatic X login service for systemd
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Unspecified
unspecified
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-01-14 19:18 UTC by Raphael Groner
Modified: 2016-02-20 22:57 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2016-02-12 11:51:43 UTC
Type: Bug
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1305236 0 high CLOSED RFE: SELinux wrongly blocks xlogin session 2021-02-22 00:41:40 UTC

Internal Links: 1305236

Description Raphael Groner 2016-01-14 19:18:07 UTC
Spec URL: https://raphgro.fedorapeople.org/review/locker/xlogin.spec
SRPM URL: https://raphgro.fedorapeople.org/review/locker/xlogin-0-0.1.20160114git97667d7.fc23.src.rpm
Description: Automatic X login service for systemd 
Fedora Account System Username: raphgro


Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=12549897

Comment 1 Zbigniew Jędrzejewski-Szmek 2016-01-14 21:05:55 UTC
Nothing systemd related is ever trivial ;)

systemd-units is no more, it was merged into systemd around F18 or so.

Please, follow systemd guidelines at https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd (you most likely want %systemd_postun not %systemd_postun_with_restart). The comment "we do not want to automatically enable user services" shows a misunderstanding, those macros do not automatically enable anything, unless configured to with presets.

Comment 2 Raphael Groner 2016-01-15 10:53:01 UTC
Hi Zbigniew,
thanks for the explanation.

Upstream writes as an answer to my request about documentation enhancement (issue 8):

Currently, we don't install the README and there is no man page. I am unsure whether writing one would be worthwhile as xlogin was really designed as a stop-gap solution. Using .xinitrc-defined sessions is a bit weird in a systemd setting and the way we managing environment variables (e.g. DISPLAY) and bus addresses shows just how imperfect the current landscape is. In my opinion this has more to do with lack of consensus in the systemd camp than with xlogin, which is also indicated by the fact that xlogin has been the best `workaround' for about four years now!

The part of xlogin that is not a hack is the x-daemon script which actually does something useful in an elegant way. This script, however, is under 100 bytes(!) so maybe it can serve as its own documentation. Admitted, every line uses something clever and non-trivial. In the end, it just spawns X, passing the command line arguments, and terminates when X is started. Does anyone desire documentation for that?

Comment 3 Zbigniew Jędrzejewski-Szmek 2016-01-15 14:28:53 UTC
I'm confused by your comment ;) There's a README, and it explains everything. I certainly wouldn't expect a man pages in every package.

Comment 4 Raphael Groner 2016-01-15 14:38:56 UTC
Restarting any xlogin user sessions while a possible upgrade would log off all currently connected users from X. Do we want that really to happen?

Comment 5 Zbigniew Jędrzejewski-Szmek 2016-01-15 14:48:33 UTC
"... you most likely want %systemd_postun not %systemd_postun_with_restart ..."

Comment 6 Raphael Groner 2016-01-17 09:57:00 UTC
I understand that policy for services that run with root access. But how to do those %systemd_ macros for the dynamical user services?

Comment 7 Zbigniew Jędrzejewski-Szmek 2016-01-17 17:36:09 UTC
So... it seems that systemd presets are non-functional for templates. I'm pretty sure they worked at one point, but seem broken now:

$ systemctl preset getty@.service
Failed to execute operation: Unit name getty@.service is not valid.
This is supposed to enable DefaultInstance in the template. This might not be very useful, but it should work. https://github.com/systemd/systemd/issues/2345 duly files.

Ideally presets could be used to enable specific instances, like discussed in http://lists.freedesktop.org/archives/systemd-devel/2015-August/033832.html. But even without that, the scriptlets should be used. Presets should get fixed at some point, and you still want to keep the %preun, %postun parts. I think you need to use xlogin@*.service for the %preun, %postun scriptlets.

To summarize:
%post
%systemd_post xlogin@.service %{name}@.service

%preun
%systemd_preun xlogin@*.service %{name}@*.service

%postun
%systemd_postun xlogin@*.service %{name}@*.service

You're in uncharted territory here ;)

Comment 8 Upstream Release Monitoring 2016-01-24 19:24:53 UTC
raphgro's scratch build of xlogin-0-0.1.20160114git97667d7.fc23.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12669706

Comment 10 Zbigniew Jędrzejewski-Szmek 2016-01-24 23:31:02 UTC
The package name is OK, the license is OK, %license is used, package has the latest version, builds and installs OK. Packaging matches all guidelines.

If there are problems, it's because there are no guidelines and this package is doing something new. So there might be bugs, let's see how this goes. Package is APPROVED.

I see one small issue (and it's in the code I suggested ;)):
globs expansion should be prevented. Please change %preun, %postun to put the argument in quotes (%{name}@*.service → '%{name}@*.service').

Comment 11 Gwyn Ciesla 2016-02-03 13:16:02 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/xlogin

Comment 12 Fedora Update System 2016-02-03 16:08:06 UTC
xlogin-0-0.1.20160114git97667d7.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-39ef6d00d1

Comment 13 Fedora Update System 2016-02-03 16:08:06 UTC
xlogin-0-0.1.20160114git97667d7.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-ff96a1e0b9

Comment 14 Fedora Update System 2016-02-03 16:08:08 UTC
xlogin-0-0.1.20160114git97667d7.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-39ef6d00d1

Comment 15 Fedora Update System 2016-02-03 16:08:09 UTC
xlogin-0-0.1.20160114git97667d7.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-39ef6d00d1

Comment 16 Fedora Update System 2016-02-03 16:08:12 UTC
xlogin-0-0.1.20160114git97667d7.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-48ec072ee2

Comment 17 Fedora Update System 2016-02-03 22:23:10 UTC
xlogin-0-0.1.20160114git97667d7.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-39ef6d00d1

Comment 18 Fedora Update System 2016-02-03 22:59:13 UTC
xlogin-0-0.1.20160114git97667d7.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-ff96a1e0b9

Comment 19 Fedora Update System 2016-02-03 23:25:03 UTC
xlogin-0-0.1.20160114git97667d7.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-48ec072ee2

Comment 20 Fedora Update System 2016-02-12 11:51:42 UTC
xlogin-0-0.1.20160114git97667d7.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2016-02-12 12:19:39 UTC
xlogin-0-0.1.20160114git97667d7.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2016-02-20 22:57:56 UTC
xlogin-0-0.1.20160114git97667d7.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.