improve "#2522 use startup command from /usr/share/xsession ..."

[Hiero32]

It is good way to set STARTUP in startwm.sh.
Many desktop environments do not start wthout it on debian/ubuntu.
Users need to write lines in .xsessionrc/.xsession., however it is difficult to set environment variables correctly.

This fixes an issue in #2522 and adds some improvements.

• Escape of double quote in following line does not work as expected.

    echo "env XDG_CURRENT_DESKTOP=\"$XDG_CURRENT_DESKTOP\" $STARTUP"

• replace ';' to ':', if ';' exists in XDG_CURRENT_DESKTOP.
  This is a workaround for some desktop environments. e.g. gnome-flashback-metacity.
• add to source the file "/etc/xrdp/export_desktop_session", if exists.
  use startup command from /usr/share/xsession if DISPLAY_SESSION is set #2522 expects DESKTOP_SESSION is set in sesman.ini in the SessionVariables section.
  I think it is a bit inconvenient for users to modify sesman.ini.
  I added to source the file /etc/xrdp/export_desktop_session, if exists.
  Simplest content in /etc/xrdp/export_desktop_session is to just one line to export DESKTOP_SESSION environment variable as follows.

export DESKTOP_SESSION=ubuntu

Because /etc/xrdp/export_desktop_session is shell script, more functionality can be added.
For example, if multiple users use the server and only userA wants to use gnome-flashback-metacity, and others want to use ubuntu, following /etc/xrdp/export_desktop_session works.

if [ "$USER" = "userA" ]; then
  export DESKTOP_SESSION=gnome-flashback-metacity
else
  export DESKTOP_SESSION=ubuntu
fi

[matt335672]

Personally I'm not keen on the /etc/xrdp/export_desktop_session. This can easily be added into /etc/profile.d by the system manager, or even .profile for the user (which works around your user test above).

The rest of it looks good to me though.

What do you think @akarl10 ?

[akarl10]

looks good.
agree about export_desktop_session.
What I recommended in my ppa instruction is putting something like

[ -n "$XRDP_SESSION" ] && export DESKTOP_SESSION=<your preferred desktop>

in ~/.profile, but of course /etc/profile.d is a good place too

[matt335672]

As discussed in the PR, please remove this bit.

By all means add a comment to point the user to ~/.profile or /etc/profile.d/

[akarl10]

I would suggest "ls -1 /usr/share/xsessions/" without .desktop suffix

[akarl10]

should probably be DESKTOP_SESSION=ubuntu

[akarl10]

I like the documentation. This way we give some kind of "best practice" and prevent too many custom solutions/hacks that might break if upstream changes something

[Hiero32]

I tried to put

[ -n "XRDP_SESSION" ] && export DESKTOP_SESSION=ubuntu

into ~/.profile.
It works. However, I found an small issue in the following steps.

1. local login (gnome-flashback-metacity) (from GDM) -> XRDP_SESSION is not set. DESKTOP_SESSION=gnome-flashback-metacity is set
2. logout
3. xrdp login -> XRDP_SESSION is set. DESKTOP_SESSION=ubuntu is set
4. logout
5. local login (gnome-flashback-metacity) if interval between 3. and 5. is less than around 15sec XRDP_SESSION, XRDP_PULSE_SINK_SOCKET, XRDP_PULSE_SOURCE_SOCKET, XRDP_SOCKET_PATH and PULSE_SCRIPT are still set.

[akarl10]

I tried to put

[ -n "XRDP_SESSION" ] && export DESKTOP_SESSION=ubuntu

into ~/.profile. It works. However, I found an small issue in the following steps.

1. local login (gnome-flashback-metacity) (from GDM) -> XRDP_SESSION is not set.  DESKTOP_SESSION=gnome-flashback-metacity is set

2. logout

3. xrdp  login -> XRDP_SESSION is set. DESKTOP_SESSION=ubuntu is set

4. logout

5. local login (gnome-flashback-metacity)  if interval between 3. and 5. is less than around 15sec XRDP_SESSION, XRDP_PULSE_SINK_SOCKET, XRDP_PULSE_SOURCE_SOCKET, XRDP_SOCKET_PATH and PULSE_SCRIPT are still set.

say Hooray to user@.service
nowadays some desktop environments don't start applications directly with "exec" but using a transient systemd user unit. the environment used by applications executed this way is systemctl --user show-environment

the 3-5 sec timeout is the time it takes to stop that unit after the LAST session of the user (if you would have a ssh session active this environment will not clear)

[Hiero32]

Understood. Thanks.

[matt335672]

@Hiero32 - there's probably not much you can do in the scope of this change to fix this problem. See #2491 for more background on this. @akarl10 has spent quite a bit of effort looking into this.

Personally, once you've fixed up @akarl10's documentation sections I'd be happy to merge this.

[akarl10]

I think in a2a8a0b the documentation is already fixed. At least I am happy with this inline doc

If we stick to the .profile recommendation we could remove the comment about SessionVariables (note that sesman.ini never contained DESKTOP_SESSION so this would be the only place that was mentioned)

[matt335672]

Agreed.

@Hiero32 - do you want to remove this comment at lines 60 and 61?

 # DESKTOP_SESSION should be set in sesman.ini in the SessionVariables section.
 # If set and valid then the STARTUP command will be taken from there

then we'll merge this.

[Hiero32]

There are a couple of reasons I prefer .profile and /etc/profile.d.

• .profile and /etc/profile.d support both per-user and system-wide.
• log out then log in is enough to reflect modification of .profile or /etc/profile.d.
• sesman.ini can be over written when apt upgrade is executed.
• reboot is required to reflect modification of sesman.ini.

I replaced

# DESKTOP_SESSION should be set in sesman.ini in the SessionVariables section.
# If set and valid then the STARTUP command will be taken from there

with

# If DESKTOP_SESSION is set and valid then the STARTUP command will be taken from there

Also I confirmed /etc/profile.
I found .sh suffix is necessary for filename in /etc/profile.d.
changed to

 # 2. create a file (any_filename.sh is OK) in /etc/profile.d

[matt335672]

Thanks @Hiero32 for sticking with this.