Bug 442268 - Review Request: lxsession-lite - Lightweight X11 session manager
Review Request: lxsession-lite - Lightweight X11 session manager
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Extras Quality Assurance
:
Depends On:
Blocks: LXDE 442270
  Show dependency treegraph
 
Reported: 2008-04-13 13:04 EDT by Christoph Wickert
Modified: 2009-06-13 15:23 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-15 22:06:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pertusus: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Christoph Wickert 2008-04-13 13:04:23 EDT
Spec URL: http://cwickert.fedorapeople.org/review/lxsession.spec
SRPM URL: http://cwickert.fedorapeople.org/review/lxsession-0.3.2-1.fc9.src.rpm
Description: LXSession is a standard-compliant X11 session manager with shutdown/reboot/suspend support via HAL and gdm. It is derived from XSM and is developed as default X11 session manager of LXDE, the Lightweight X11 Desktop Environment. Though being part of LXDE, it's totally desktop-independent and only has few dependencies.
Comment 1 Sebastian Vahl 2008-04-16 04:57:31 EDT
It seems that lxsession is not really working without smproxy from 
xorg-x11-xsm. Without it, it shows a logout screen with "logout" as the only 
option and seems to not save any running applications. With xorg-x11-xsm and 
started smproxy it really saves the session and also enhances the logout 
screen with other options (shutdown/reboot/suspend/hibernate).

So I suggest: Requires: xorg-x11-xsm
Comment 2 Christoph Wickert 2008-04-18 22:02:06 EDT
Good catch, will fix that. More comments anywone?
Comment 3 Patrice Dumas 2008-04-24 03:58:53 EDT
If xsm is not useful, maybe lxsession-lite should be packaged instead?

I tried lxsession-lite, but I couldn't make it work with fluxbox and gdm,
though I followed the README.
Comment 4 Christoph Wickert 2008-04-24 04:18:20 EDT
I updated to lxsession-lite 0.3.5 and everything is working for me. I will
update this review later today, but I'm still struggling with the new
lxde-common package, which now contains arch-specific and noarch content.
Comment 5 Patrice Dumas 2008-04-24 04:31:04 EDT
In case it can be useful, here is what I did:

$ cat /usr/share/xsessions/lxsessionfluxbox.desktop 
[Desktop Entry]
Encoding=UTF-8
Name=lxsession fluxbox
Comment=Fluxbox with lxsession
Exec=/usr/bin/startlxfluxbox
Type=Application

$ cat /usr/bin/startlxfluxbox
#! /bin/sh

exec /usr/bin/lxsession -s fluxbox

$ $ ls -1 /etc/xdg/lxsession/
fluxbox
Fluxbox
lxsession fluxbox

all those directory have the same config file in 
$ ls  /etc/xdg/lxsession/*
/etc/xdg/lxsession/fluxbox:
config

/etc/xdg/lxsession/Fluxbox:
config

/etc/xdg/lxsession/lxsession fluxbox:
config

$ cat /etc/xdg/lxsession/fluxbox/config
[Session]
window_manager=startfluxbox



It goes as far as starting /usr/bin/lxsession -s fluxbox
but fluxbox itself is never started.
Comment 6 Christoph Wickert 2008-05-04 15:06:08 EDT
(In reply to comment #5)

> $ cat /etc/xdg/lxsession/fluxbox/config
> [Session]
> window_manager=startfluxbox

If you set this to 'startfloxbox' need to have a 'startfluxbox' script in $PATH.

  #!/bin/sh
  exec fluxbox
Comment 9 Patrice Dumas 2008-05-18 04:47:58 EDT
(In reply to comment #6)
> (In reply to comment #5)
> 
> > $ cat /etc/xdg/lxsession/fluxbox/config
> > [Session]
> > window_manager=startfluxbox
> 
> If you set this to 'startfloxbox' need to have a 'startfluxbox' script in $PATH.

It is the case. Using startfluxbox is the standard way to start 
fluxbox, it is what is in the .desktop file, for instance.
Comment 10 Patrice Dumas 2008-05-18 05:07:40 EDT
I suggest adding INSTALL='install -p' to the make install line to
keep timestamps:
make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'

And also I suggest using globs for man pages compression extensions
%{_mandir}/man1/lxsession*.1*

Thats all I see on the packaging side. I'll retest the functionality.
Comment 11 Christoph Wickert 2008-05-18 06:10:57 EDT
(In reply to comment #9)
> (In reply to comment #6)
> > (In reply to comment #5)
> > 
> > > $ cat /etc/xdg/lxsession/fluxbox/config
> > > [Session]
> > > window_manager=startfluxbox
> > 
> > If you set this to 'startfloxbox' need to have a 'startfluxbox' script in $PATH.
> 
> It is the case. Using startfluxbox is the standard way to start 
> fluxbox, it is what is in the .desktop file, for instance.

Ok, so startfluxbox is the normal startup script, but in comment #5 you also
said that startfluxbox executes "/usr/bin/lxsession -s fluxbox". Only one can be
true I think.

Your setup:
1. GDM starts lxsessionfluxbox.desktop -> /usr/bin/startlxfluxbox
2. startfluxbox executes "/usr/bin/lxsession -s fluxbox"
3. lxsession starts the command defined in /etc/xdg/lxsession/fluxbox/config
which is set to startfluxbox again. So you have created an infinite loop.
Comment 12 Patrice Dumas 2008-05-18 06:31:59 EDT
(In reply to comment #11)
> (In reply to comment #9)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > 
> > > > $ cat /etc/xdg/lxsession/fluxbox/config
> > > > [Session]
> > > > window_manager=startfluxbox
> > > 
> > > If you set this to 'startfloxbox' need to have a 'startfluxbox' script in
$PATH.
> > 
> > It is the case. Using startfluxbox is the standard way to start 
> > fluxbox, it is what is in the .desktop file, for instance.
> 
> Ok, so startfluxbox is the normal startup script, but in comment #5 you also
> said that startfluxbox executes "/usr/bin/lxsession -s fluxbox". Only one can be
> true I think.
> 
> Your setup:
> 1. GDM starts lxsessionfluxbox.desktop -> /usr/bin/startlxfluxbox

Right

> 2. startfluxbox executes "/usr/bin/lxsession -s fluxbox"

No, it is not startfluxbox, here, but startlxfluxbox (like above in 1.).


> 3. lxsession starts the command defined in /etc/xdg/lxsession/fluxbox/config
> which is set to startfluxbox again. So you have created an infinite loop.

Here it is really startfluxbox, but it never starts for real.
Comment 13 Christoph Wickert 2008-10-08 13:40:10 EDT
Patrice, I'm sure the package works correctly and there must be a fault in your configuration. Can you please try with the files from lxde-common (bug #442270) or base your configuration upon them?
Comment 14 Patrice Dumas 2008-10-08 18:50:09 EDT
I tried, and it also doesn't work. That being said, I don't think
that it should be a blocker, especially for devel.

Do you want to work more on this, or have the package formally 
reviewed?
Comment 15 Christoph Wickert 2008-10-08 19:28:43 EDT
I have no idea what's going wrong for you. It works reliable for me and others, on the other hand I see no obviuos error in your setup.

Anyway: If you want to do a review please try this one. Includes your suggestions from comment #10.
http://cwickert.fedorapeople.org/review/lxsession-lite-0.3.6-1.fc10.src.rpm
http://cwickert.fedorapeople.org/review/lxsession-lite.spec

Do you see the icons in the logout dialog? There should be icons since the new version now includes a copy of them, but some people reported missing icons. Looks like a problem with gtk-update-icon-cache.
Comment 16 Patrice Dumas 2008-10-09 04:37:26 EDT
During the build, there is:

./config.status: line 1119: ./intltool-extract.in: No such file or directory
mv: cannot stat `intltool-extract.out': No such file or directory
chmod: cannot access `intltool-extract': No such file or directory
chmod: cannot access `intltool-extract': No such file or directory
./config.status: line 1119: ./intltool-merge.in: No such file or directory
mv: cannot stat `intltool-merge.out': No such file or directory
chmod: cannot access `intltool-merge': No such file or directory
chmod: cannot access `intltool-merge': No such file or directory
./config.status: line 1119: ./intltool-update.in: No such file or directory
mv: cannot stat `intltool-update.out': No such file or directory
chmod: cannot access `intltool-update': No such file or directory
chmod: cannot access `intltool-update': No such file or directory

However .mo files are installed.

* rpmlint is silent
* free software with license file
* match upstream
909c3b0f4c6e4855f64dfbb47467c0b3  lxsession-lite-0.3.6.tar.gz
* follow guidelines
* %files section right


I still suggest
make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'
to keep the man pages timestamps, but if they are generated this
is pointless, I haven't verified. In any case it is not a blocker.


More problematic, is the fact that source timestamps is not kept:
-rw-rw-r-- 1 dumas dumas 229644 Jun 11 20:49 ../SOURCES/lxsession-lite-0.3.6.tar.gz
-rw-rw-r-- 1 dumas dumas 229644 Jun  8 19:13 lxsession-lite-0.3.6.tar.gz

please use wget -N, spectool -g or the appropriate curl option.
Since the stamps are similar it may also be an issue with sourceforge or
upstream, though.


If you fix this timestamp issue:
APPROVED

It seems to me that this package should need more testing before 
going into a stable branch, however, it could also be argued that
the whole lxde stack is not that mature and that it is better to have
more testing, and therefore add it to stable releases for maximal 
coverage.
Comment 17 Patrice Dumas 2008-10-09 04:42:24 EDT
(In reply to comment #15)
> I have no idea what's going wrong for you. It works reliable for me and others,
> on the other hand I see no obviuos error in your setup.

I have had many problems with X lately, but I don't think this is 
at that level. I don't have anything special I can think of. I use
xdm with custom session chooser in general, but here I tested with kdm.
In general I use fluxbox. I launched openbox after installing lxde-common
to see if it was an issue with openbox, and apparently openbox worked 
fine. I follow rawhide quite closely and I have a bunch of packages
installed, but I don't have an idea about what could be special.

> Do you see the icons in the logout dialog? There should be icons since the new
> version now includes a copy of them, but some people reported missing icons.
> Looks like a problem with gtk-update-icon-cache.

I have never seen a logout dialog since I have never login... the 
window manager is never started for me (still able to zap X).
Comment 18 Christoph Wickert 2008-10-09 19:23:27 EDT
(In reply to comment #16)
> 
> If you fix this timestamp issue:
> APPROVED

Fixed. If you want to take a (final) look at the package:
http://cwickert.fedorapeople.org/review/lxsession-lite.spec
http://cwickert.fedorapeople.org/review/lxsession-lite-0.3.6-2.fc10.src.rpm
Comment 19 Patrice Dumas 2008-10-10 04:12:23 EDT
The source tarball timestamp is still different from what I have
downloaded:

-rw-rw-r-- 1 dumas dumas 229644 Jun 11 20:49 ../SOURCES/lxsession-lite-0.3.6.tar.gz
-rw-rw-r-- 1 dumas dumas 229644 Jun  8 19:13 lxsession-lite-0.3.6.tar.gz
Comment 20 Christoph Wickert 2008-10-10 06:14:46 EDT
Ok, my bad, did not fix that because I thought you were talking about keeping the timestamps during install. 

But is the timestamp of the source really important? If so, why is it not in the Packaging or the Review Guidelines?
Comment 21 Patrice Dumas 2008-10-10 06:19:03 EDT
It is:
http://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
Comment 22 Christoph Wickert 2008-10-10 06:34:57 EDT
Ok, my fault. I really would like to know when tis was introduced but I can't find that change in the history nor in the old wiki. :(

Anyway, here's a new mockbuild:
http://cwickert.fedorapeople.org/review/lxsession-lite-0.3.6-2.fc10.src.rpm
Note: I did not increase the version nor did a changelog entry, just rebuilt the package with another source. Hope this is ok for you, for me the timestamp is matching now.
Comment 23 Patrice Dumas 2008-10-10 06:48:35 EDT
Perfect.

The guideline was added some months ago. I vaguely remember that it 
was rapidly discussed on the packaging list. But this is something
I insist on since quite a long time -- but although I don't think it should
be a must in general I also don't see why it shouldn't do be done.
Comment 24 Christoph Wickert 2008-10-10 06:57:58 EDT
I completely agree, it's something we should care about. I just didn't know it. :(

I guess my problem was kind of sourceforge-specific because I downloaded the file with my webbrowser and got redirected to a mirror.

New Package CVS Request
=======================
Package Name: lxsession-lite
Short Description: Lightweight X11 session manager
Owners: cwickert
Branches: F-8 F-9
InitialCC:
Comment 25 Patrice Dumas 2008-10-10 07:04:50 EDT
(In reply to comment #24)

> I guess my problem was kind of sourceforge-specific because I downloaded the
> file with my webbrowser and got redirected to a mirror.

I don't think that it is sourceforge specific since it doesn't seems to
me that webbrowser keep timestamps. Using wget there is also a redirection
but still the timestamps are kept.
Comment 26 Patrice Dumas 2008-10-10 07:45:24 EDT
(In reply to comment #15)
> I have no idea what's going wrong for you. It works reliable for me and others,
> on the other hand I see no obviuos error in your setup.

It works now. At least the lxde desktop works. However, my attemp to 
use fluwbox instead of the lxde desktop didn't worked, when I 
choose the session corresponding with the setup explained above, I also
get the lxde desktop...

> Do you see the icons in the logout dialog? There should be icons since the new
> version now includes a copy of them, but some people reported missing icons.
> Looks like a problem with gtk-update-icon-cache.

No problem here.
Comment 27 Kevin Fenzi 2008-10-10 19:12:27 EDT
cvs done.
Comment 28 Fedora Update System 2008-10-10 20:22:50 EDT
lxsession-lite-0.3.6-2.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/lxsession-lite-0.3.6-2.fc8
Comment 29 Christoph Wickert 2008-10-10 21:00:17 EDT
Has also been submitted for F9, but bodhi swallowed the notification.

(In reply to comment #26)
> At least the lxde desktop works. However, my attemp to 
> use fluwbox instead of the lxde desktop didn't worked, when I 
> choose the session corresponding with the setup explained above, I also
> get the lxde desktop...

I will look into that tomorrow.
Comment 30 Fedora Update System 2008-10-15 22:06:43 EDT
lxsession-lite-0.3.6-2.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 31 Fedora Update System 2008-10-15 22:11:24 EDT
lxsession-lite-0.3.6-2.fc9 has been pushed to the Fedora 9 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.