[pkg-fso-maint] [PATCH] install.sh: add $DESKTOP_PANEL user variable (was Re: install.sh - 4 proposals for patches)

[Luca Capello]

Hi Steffen!

Continuing with the third patch...

On Fri, 13 Feb 2009 02:47:31 +0100, Steffen Moeller wrote:
>>From 1d74bb83dac1fc1d852bc3165f3265d548d49842 Mon Sep 17 00:00:00 2001
> From: Steffen Moeller <steffen_moeller at gmx.de>
> Date: Fri, 13 Feb 2009 00:27:58 +0100
> Subject: [PATCH] Allowing another (more user-friendly) window manager than the default zhone-only user interface.

I strongly object to this new variable, read below for the reasons.

> +WINDOW_MANAGER=${WINDOW_MANAGER:-zhone}
[...]
> +  WINDOW_MANAGER   the presentation of the phone to the user. The default is
> +                   to just use the 'zhone' (phone) application. Alternatively,
> +                   'fbpanel' is a tiny but usable window manager.

There is a misunderstanding here: Zhone is not a window manager, but a
phone application.  The window manager now installed by default is
Matchbox [1], which IMHO is working great and is probably the best
solution for embedded devices.

[1] http://matchbox-project.org/

Thus, what you want to change is the desktop panel, however:

1) we should provide a *default* installation, then if people want to
   customize it they are free, but giving more options reduce
   consistency between default installations

2) we should install the panel which requires the least disk space and
   provides the best default options

Here the comparison between the various panel packages from `apt-cache
search panel` (the second field is the Installed-Size as showed by
`apt-cache show $PKG`)

- avant-window-navigator, 328kB, too many dependencies not automatically
  pulled in

- docker, 84kB, openmoko-panel-plugin is not showed, while it should be
  (I do not have the time right now to report this bug, if someone wants
  to do it, please go on!)

- fbpanel, 732kB plus librsvg2-common for a total of 909kB, it works as
  expect with minor bugs (requires gnome-terminal icon, its menu
  includes an 'emacs' entry but emacs is not installed)

- fspanel, 68kB, it does not support systray applications, thus
  openmoko-panel-plugin is not showed

- gnome-panel, 1068kB, too many dependencies not automatically pulled in

- kicker, 5864kB, too many dependencies for a total of 107MB

- lxpanel, 1172kB, it works but the default background color is black
  (it is difficult to see the openmoko-panel-plugin icons) and it has
  minor bugs very similar to the fbpanel ones

- matchbox-panel, 380kB plus libiw29 (maybe it should not depend on it,
  but simply recommends) and matchbox-common for a total of 676kB, it
  works and IMHO it is the best solution, with the openmoko-panel-plugin
  icons bigger than expected

- trayer, 100kB, already used :-)

- xfce4-panel, 1844kB, too many dependencies for a total of 7549kB

>  echo " * Installing /root/.xsession"
> -cat > $INST_DIR/root/.xsession <<__END__
> +case $WINDOW_MANAGER in
> +fbpanel) 
> +	# from http://wiki.openmoko.org/wiki/Debian#Matchbox_with_fbpanel

Please insert comments only when they are useful to understand what the
script is doing or you explain a technical choice.

> +	apt-get -yes install fbpanel

This is wrong, because the above command will be executed on the host
distribution (i.e. the one from which you have started install.sh) and
not on the target one (i.e. Debian).  The correct command is

  chroot $INST_DIR apt-get -yes install fbpanel

> +	cat > $INST_DIR/root/.xsession <<__END__
> +#!/bin/sh
> +export GTK_MODULES=libgtkstylus.so

This has nothing to do with the panel.

> +zhone &
> +xsetroot -solid black

Again, this has nothing to do with the panel.

> +matchbox-keyboard-toggle &
> +matchbox-window-manager -use_titlebar yes &
> +# -use_titlebar yes to minimize & toggle between apps

If we need a comment here, it should be put before the command.
However, IMHO a mobile phone is not like a desktop where you want to
minimize applications: they should all be fullscreen and then you switch
between them.

> +# fbpanel's taskbar does not work with matchbox-window-manager

If fbpanel has a problem with matchbox-window-manager, this should be
filed and resolved ASAP.  Please note that this is already a bad start
for fbpanel.

> +#~/bin/auxlaunch &

Again, this has nothing to do with the panel.

> +while true;
> +do
> +fbpanel;
> +sleep 1;
> +done;

This is not how a panel should be started.  The order in which you start
applications in ~/.xsession is important, since the first application
not started in the background (thus not listed with the final &) blocks
the execution of ~/.xsession until it quits.  If this is the last
application started, this also means that quitting this application
causes the X session to be closed as well.  That is why the last
application started by ~/.xsession is either a terminal or a window
manager.

> +__END__
> +	

No trailing spaces, please :-)

> +*)
> +	cat > $INST_DIR/root/.xsession <<__END__
>  #!/bin/sh
>  trayer --edge top &
>  openmoko-panel-plugin &
>  zhone &
>  exec matchbox-window-manager -use_titlebar no -use_cursor no
>  __END__

Instead of the above statement which duplicates information, I would go
for something like:

--8<---------------cut here---------------start------------->8---
	chroot $INST_DIR apt-get -yes install $DESKTOP_PANEL
	echo "#!/bin/sh" >$INST_DIR/root/.xsession
	case $DESKTOP_PANEL in
	fbpanel)
		echo "fbpanel &" >>$INST_DIR/root/.xsession ;;
	trayer)
		echo "trayer --edge top &" >>$INST_DIR/root/.xsession ;;
	*)
		exit 1 ;;
	esac
        cat > $INST_DIR/root/.xsession <<__END__
openmoko-panel-plugin &
zhone &
exec matchbox-window-manager -use_titlebar no -use_cursor no
__END__
--8<---------------cut here---------------end--------------->8---

NB, not that I want the above one implemented, but to give an idea ;-)

Thx, bye,
Gismo / Luca
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 314 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-fso-maint/attachments/20090218/e0bf5e46/attachment-0001.pgp