Error message for a faulty ~/.profile script

[Egon Elbre]

trusty and utopic SRU requests
==============================

[Impact]

In case of a syntax error in either of ~/.profile or a few other similar files, the Xorg login is interrupted, and the user is taken back to the login screen without an explanation. Debugging this problem may be a time consuming exercise, especially for non-experienced users.

With the proposed change, lightdm/gdm does not try to load such a file, but shows a warning dialog instead. (A warning dialog is also shown in case of some other type of error, which would not have caused the login to fail.)

[Test Case]

To reproduce:

* Edit ~/.profile and add something bad, e.g. a non-closed parenthesis.
* Log out and find that you can't log in to a graphical session.

After installing the proposed lightdm/gdm version, you'll instead see the dialog and can log in.

[Regression Potential]

Since this is only about improved exception handling, it does not at all affect users with correct configuration files. The regression risk ought to be minimal.

[Original description]

Binary package hint: gdm

After adding "function AddPath { PATH="$1:$PATH" }" to $HOME/.profile made the Xorg startup fail. (At that moment I had already forgotten the changes made to the .profile). As I had autologin that meant it kept trying to login and finally showed me the graphics reconfiguration screen. That sent me to a huge hunt for xorg conf and setup problems.

Anyway finally tracked this down. I had used bash syntax in .profile file whereas it was run with sh.

This is not a bug per se but I think a user mistake in ".profile" shouldn't bring the whole xorg startup to a halt as it does with autologin. I propose running user ".profile" and ".xprofile" scripts so that Xsession script continues running even if they have errors. I'm not sure whether this change would have some negative effects as well.

1) Ubuntu Lucid, Linux egon-laptop 2.6.32-25-generic #45-Ubuntu SMP Sat Oct 16 19:48:22 UTC 2010 i686 GNU/Linux
2) gdm 2.30.2.is.2.30.0-0ubuntu4

[Gunnar Hjalmarsson (gunnarhj)]

I had to set the -e option (errexit) in Xsession to make a ~/.profile error interrupt the startup. AFAICT that option is not set in the original Xsession script in GDM for Ubuntu.

Anyway, maybe there are still cases where a ~/.profile error may lead to the behavior you describe. The linked branch contains an idea for a fix that would handle ~/.profile and ~/.xprofile in a safer way.

If the solution is considered sensible, I suppose it should be forwarded upstream. Will await feedback before doing so.

[Gunnar Hjalmarsson (gunnarhj)]

I want to correct myself: When discussing this with more experienced developers, my attention was called to the fact that I had missed the distinction between syntax errors and other errors. While a syntax error kills the process, other errors, e.g. "not found", don't. Hence you can disregard my talk about the -e option.

A possible solution did come up. Replacing this /etc/gdm/Xsession line:

    test -f "$HOME/.profile" && . "$HOME/.profile"

with

    test -f "$HOME/.profile" && ( . "$HOME/.profile" ) && . "$HOME/.profile"

would protect against a ~/.profile syntax error causing the process to be killed. For two reasons it's not considered appropriate, at least not as soon before the 11.04 release as we are now:

1. Regression risk due to the ~/.profile commands being run twice (even if they would be run in a subprocess the first time).

2. Efficiency loss.

For the case the conclusion will change after the 11.04 release, a merge proposal with the above solution is available.

[Gunnar Hjalmarsson (gunnarhj)]

As regards efficiency, ~/.profile typically contains something like this:

if [ -n "$BASH_VERSION" ]; then
    # [ code that won't be run in bourne shell ]
fi

if [ -d "$HOME/bin" ] ; then
    PATH="$HOME/bin:$PATH"
fi
export LANGUAGE="en"
export LC_MESSAGES="en_US.UTF-8"
export LANG="sv_SE.UTF-8"

[Martin Pitt (pitti)]

I don't actually think that silently ignoring ~/.profile if it has errors is actually a good and expected behaviour. After all, the user just changed his ~/.profile and now restarted the session, perhaps to test the effect.

The best thing to me would be a dialog box that actually showed the error that the shell command encountered, and then exit the session. That might actually be possible with zenity or similar. We should at least ensure that the error appears in ~/.xsession-errors, then we at least have some point of feedback.

So from my POV the current bug title is "wontfix". I'm fine with renaming it to "better error message for broken ~/.profile", if you prefer that?

[Egon Elbre (egon-elbre)]

Sure, silently just ignoring is just as bad. I just think that, it's a mistake that shouldn't prevent you to login and start up everything.

The ~/.profile script can also easily be changed by non-experienced users (ones who may not know terminal that well). If that user doesn't get the session running then he'll be forced to use terminal.

The ideal case I think would be: everything starts up and an relevant error message is displayed.

I'm ok with scripts in /etc/.. preventing xsession to start as that requires sudo permissions (as sudo implies that caution must be taken). Essentially my POV is that if modifying a script doesn't require sudo permissions it should be relatively safe, meaning you should be still able to fix it from GUI interface. Of course practically it's impossible to do so, but it's a target to move towards.

[Gunnar Hjalmarsson (gunnarhj)]

I agree on the goal you describe, Egon, which is why I made an attempt to fix it. However, while the "run ~/.profile twice" proposal was rejected due to its disadvantages, it's the only way I was able to figure out that would prevent syntax errors from stopping the startup.

I leave it to you to decide if you want to go for Martin's idea and convert this bug to rather be about error messages. In the case of a syntax error, the error message currently is logged in .xsession-errors (as the last entry). Have no idea, though, how to make it trigger e.g. a zenity dialog.

Unassigning myself from this bug in the hope that someone with more experience from exception handling in dash will give it a shot.

[Pedro Villavicencio (pedro)]

We're closing this bug since it is has been some time with no response from the original reporter. However, if the issue still exists please feel free to reopen with the requested information. Also, if you could, please test against the latest development version of Ubuntu, since this confirms the bug is one we may be able to pass upstream for help.

[Egon Elbre (egon-elbre)]

Sorry, I forgot about this due to other duties (work and school).
This is not an active issue for me, but would be a nice touch to Ubuntu.

This still happens in Oneiric (tested in daily). There are some differences,
it just drops you into the default login screen without any explanation.
Which is quite bad because then you will be puzzled what just happened/went wrong.

Also I did some research how to keep the main script running when
sub-script has some syntax errors:

#####

set -e
ERRCODE=0
(. "$HOME/.profile") || ERRCODE="$?"
echo $ERRCODE

#####

ERRCODE will be
0 if everything was ok
127 if there was a syntax error
>0: there was some other problem with the script

That way we can get the correct error code without terminating the main script
and do graceful handling of the error.

[Gunnar Hjalmarsson (gunnarhj)]

Nice try, Egon, but I think your solution has the same drawbacks as the idea I mentioned in comment #2. The parentheses would make the ~/.profile commands be run in a subprocess, so if no error, you would need to source ~/.profile in the main process as well, or else settings of e.g. environment variables via ~/.profile would not work. In other words, normally ~/.profile would be sourced twice.

[Egon Elbre (egon-elbre)]

Right forgot about the variables.

After poking around in dash source I found that "$HOME/.profile" has been hardcoded. So the change needs be made in the shell to make ".profile" safe.

Also I found a way to keep the variables from subshell

### test.sh ###
ERRCODE=0
TEST=""
VARS=$(set -e; . $HOME/script.sh 1>/dev/null; set) || ERRCODE="$?"
if [ $ERRCODE==0 ]
then
    eval $VARS
fi
echo "ERRCODE =" $ERRCODE
echo " TEST =" $TEST
### script.sh ###
TEST="something"
function AddPath { echo "err" }
######

This also has one nice thing that when the script fails the environment stays unchanged. Also there may be some variables that shouldn't be taken from the subshell, but I have no idea which.

[Gunnar Hjalmarsson (gunnarhj)]

Hmm... Now this is above my head (again). ;-) One thing that it does not seem to handle is setting of aliases.

[Egon Elbre (egon-elbre)]

The idea is simple:

###

VARS=$( # this allows to capture subshell stdout to a variable
 . $HOME/script.sh 1>/dev/null # this prevents script printing something to stdout
set # this prints all variables to stdout
alias | sed 's/.*/alias \0/' # this prints aliases to stdout and prefixes them with "alias "
) || ERRCODE="$?"

if [ $ERRCODE==0 ]
then
    eval $VARS # if everything is ok, we reevaluate the variables and aliases in this shell
fi

###
Now the "VARS=$(command) || ERRCODE="$?"", || prevents the command from exiting the script if the next command exits with code 0. We could also use "VARS=$(command) || true", but then "$?" would contain the result of command "true", since ERRCODE="$?" also exits with code 0 we can prevent the shell from exiting and remember the correct error code.

[Gunnar Hjalmarsson (gunnarhj)]

I'll ask Martin P. to take a look.

[Martin Pitt (pitti)]

That indeed would run .profile only once, but now introduces the problem that it dumps the entire environment, and all bash functions in set, and has to read and evaluate it back:

$ set|wc -l
14698

Reading and evaluating 15.000 lines of shell is not a triviality.

Also, can we make sure that more complicated variable or alias definitions actually correctly survive this subshell/set/sed/eval round? I have some stuff like

  alias alert='notify-send --urgency=low -i "$([ $? = 0 ] && echo terminal || echo error)" "$(history|tail -n1|sed -e '\''s/^\s*[0-9]\+\s*//;s/[;&|]\s*alert$//'\'')"'

(now a standard value for new user accounts in Ubuntu) and also stuff like

if [ "$TERM" = xterm -a -z "$SSH_TTY" ]; then
    export PROMPT_COMMAND='echo -ne "\033]0;${USER}@${HOSTNAME}: ${PWD}\007"'
    export PS1="${debian_chroot:+\[\e[31m[$debian_chroot]\] }\[\e[31m\]\$? \[\e[30m\]\$(parse_git_branch)\[\e[32m\]\u@\h:\[\e[34m\]\w\n$\[\e[22m\]\[\e[30m\] "
fi

Also, functions that rely on variables like $$, or having a terminal as stdout/stderr would break.

As I already said, I don't think that there is any easy and efficient solution for this -- this is very much like t,he halting problem, shell is a Turing complete programming language after all. Also, it would again paper over the actual bug that the user introduced into his .profile -- we should tell him about it, instead of trying to interpret something half-working into broken scripts, which then also would only apply to particular environments like gdm. The next time the user would log into a VT, log into the box with ssh, use screen, or run a cron/at job it would break all over again.

I think the only really sensible solution here is to produce an error message.

[Egon Elbre (egon-elbre)]

For the sed/eval round we can always escape the special characters beforehand, there should be some function for that.

But I agree with your conclusion. Warning or message is a good enough solution, even though I would like to see some hand-holding. I agree it just isn't worth the effort and the only "nice" way (I can think) of doing this is to extend dash with such functionality.

[Gunnar Hjalmarsson (gunnarhj)]

Marked bug #1097903 a duplicate of this one, and added a lightdm (ubuntu) task.

[Matthieu Baerts (matttbe)]

Hello,

I also confirm that this bug can still be annoying with the latest versions of GDM and LightDM :-)
And it can be an easy way to make some jokes just by modifying the .profile! Then it will be hard for the user to understand why he can no longer start a new session.

Is it maybe possible to just add an error message if there are problems with the .profile file?
We can easily detect if there are errors in this .profile file as already written before:

    . ~/.profile || echo ERROR

But also without executing any code:

    sh -n ~/.profile || echo ERROR

Thank you for maintaining GDM and LightDM!

[Gunnar Hjalmarsson (gunnarhj)]

Hi Matthieu,

That "sh -n" option sounds interesting indeed, and I just prepared a merge proposal for lightdm. It would make the file not being loaded in case of a syntax error, and an entry printed to ~/.xsession-errors.

[Matthieu Baerts (matttbe)]

Hi Gunnar,

> It would make the file not being loaded in case of a syntax error, and an entry printed to ~/.xsession-errors.

I don't know if it's better to ignore this file in case of syntax errors or to refuse to start a new session but for me, the most interesting thing is to notify the user what's wrong. That can be made by adding a line in the ~/.xsession-errors file but I think it's better to display a new dialogue which explains the error.

Because if you just ignore this file and print an error in ~/.xsession-errors, I'm not sure that the user will see that there is a problem with his/her ~/.profile file.
Personally I don't often read this file :-)

[Alec Warner (antarus)]

I think we would also prefer a dialog, but I will take errors in ~/.xsession-errors over nothing. We can instruct our support staff to look there for these errors; right now there are no errors anywhere and so it is difficult to debug the root cause of the problem.

-A

[Gunnar Hjalmarsson (gunnarhj)]

And I who thought it was so elegant to fix this bug with a few characters of code. ;)

I suppose we could do a solution with zenity/kdialog, but before spending time on such a proposal (involving gettext etc.), I would like to know for sure that "sh -n" is an acceptable testing method. (Please note that other ideas were rejected previously in this bug report.)

@Alec: I think Matthieu meant that if we ignore an erroneous file and let the session start, the user may not be aware that there is an error, and hence has no reason to go reading .xsession-errors. I think he has a point.

[Gunnar Hjalmarsson (gunnarhj)]

I made a try.

As regards a syntax error, it currently results in an interrupted login, while other types of errors are silently ignored.

With my new merge proposal, a zenity/kdialog dialog is shown for all types of errors in ~/.profile and friends, and the login is completed (with a deficient configuration). They are also printed to ~/.xsession-errors.

Please feel free to test it by installing lightdm from my PPA at
https://launchpad.net/~gunnarhj/+archive/ubuntu/misc

[Matthieu Baerts (matttbe)]

It seems it's a good way to fix this bug!

Thank you for your help :-)

[Martin Pitt (pitti)]

Gunnar,

Running it through "sh -n" is definitively a good way how to test a shell script. It of course will miss a lot of errors (variable typos, failing programs in a set -e script, etc.), but it at least will catch obvious syntax errors. So ignoring faulty scripts and showing some error message (in ~/.xsession-errors or even a dialog) seems appropriate to me.

Thanks for working on this! I'll review the MP now.

[Robert Ancell (robert-ancell)]

I've removed the tasks on LightDM (the project) as the fixes for this were all in debian/.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.13.0-0ubuntu1

---------------
lightdm (1.13.0-0ubuntu1) vivid; urgency=medium

  * New upstream release:
    - Fix crash when having configuration keys defined in multiple places
      (LP: #1377373)
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Use correct syntax for DesktopNames key in session files (LP: #1383321)
    - Match seat configuration with globbing (LP: #1364911)
    - Allow user switching in multi-seat until bug stopping greeter showing on
      logout is fixed
    - Disable log message when AccountsService users change (LP: #1376357)
    - Update AppArmor scripts, requires AppArmor 2.9
    - Update tests to run better on servers
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Thu, 13 Nov 2014 11:08:17 +1300

[Brian Murray (brian-murray)]

Hello Egon, or anyone else affected,

Accepted lightdm into utopic-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/lightdm/1.12.2-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Gunnar Hjalmarsson (gunnarhj)]

I have successfully installed and run lightdm 1.12.2-0ubuntu1 from utopic-proposed. The behavior in case of a syntax error in ~/.profile has changed in accordance with the test case in the bug description.

[Barry Warsaw (barry)]

Hmm, as 1.13.0-0ubuntu1 just showed up in vivid, it broke my login in the follow way. Note that I always try devel channel updates in a VM with a snapshotted disk so I'm able to revert and do some further investigating. Looking at the dist-upgrade packages, I suspect the lightdm changes are affecting me, but I'm going to continue to investigate.

So, when I log in, I do get several warnings in a dialog. Most are because I use `source` instead of `.` to source other shell files. What shell is lightdm using these days? Still, even with the warnings, I can log in just fine.

Anyway, when I change them all to `.` I still get one warning about my use of shopt, but I'll just ignore that for now. However, logins are now completely broken. I see the screen blank and then I'm thrown back to the login screen. ~/.xsession-errors is empty and I can find no other log file either in $HOME or /var/log that contains any information about what's going wrong.

I'm going to pin lightdm and dist-upgrade everything else just to verify that the lightdm update is causing my problems. How can I debug the login crash after I fix my login scripts?

[Barry Warsaw (barry)]

apt-mark lightdm hold && apt-get dist-upgrade && reboot

Indeed, logins are fine.

[Barry Warsaw (barry)]

pay-service crashed, leaving a core file and the following in .xsesssion.old. Not sure if this is related.

upstart: pay-service main process (6149) killed by ABRT signal

[Barry Warsaw (barry)]

For now I'll keep the hold on lightdm, but if Robert or someone else wants to debug in real-time, ping me on irc.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Barry!

On 2014-11-14 17:56, Barry Warsaw wrote:
> So, when I log in, I do get several warnings in a dialog. Most are
> because I use `source` instead of `.` to source other shell files.
> What shell is lightdm using these days?

It has always been using Bourne shell, so "source" is not understood. You have obliviously been running your system without the configuration in those other shell files.

> Anyway, when I change them all to `.` I still get one warning about
> my use of shopt,

Another bash command, not understood by dash.

> However, logins are now completely broken. I see the screen blank and
> then I'm thrown back to the login screen. ~/.xsession-errors is
> empty and I can find no other log file either in $HOME or /var/log
> that contains any information about what's going wrong.

This is the code in /usr/sbin/lightdm-session which triggers those warning dialogs:

<code>
# temporary storage of error messages
ERR=$(mktemp --tmpdir config-err-XXXXXX)

source_with_error_check () {
    CONFIG_FILE="$1"
    if sh -n "$CONFIG_FILE" 2>"$ERR"; then
        echo "Loading $CONFIG_FILE";
        . "$CONFIG_FILE" 2>"$ERR"
        if [ -s "$ERR" ]; then
            SYNTAX=false
            . /usr/lib/lightdm/config-error-dialog.sh
        fi
    else
        SYNTAX=true
        . /usr/lib/lightdm/config-error-dialog.sh
    fi
    cat "$ERR" >>/dev/stderr
    truncate -s 0 "$ERR"
}

# Load profile
for file in "/etc/profile" "$HOME/.profile" \
            "/etc/xprofile" "$HOME/.xprofile"; do
    if [ -f "$file" ]; then
        source_with_error_check "$file"
    fi
done
</code>

I find it hard to believe that that code itself is causing the problem you describe. Considering that you could log in just fine before you changed the occurrences of "source" to ".", I would look for the cause in the code which unlike before is now sourced by lightdm.

[Barry Warsaw (barry)]

Hi Gunnar.

On Nov 14, 2014, at 07:07 PM, Gunnar Hjalmarsson wrote:

>It has always been using Bourne shell, so "source" is not understood.
>You have obliviously been running your system without the configuration
>in those other shell files.

Sure, it's entirely possibly that my login has been silently ignoring the
non-sh syntax errors, so in that respect, getting notification of them now is
a good thing.

I still wonder what in my shell code could be causing lightdm to exit back
to the login screen and where those errors would be logged so that I can view
them and debug them.

Cheers.

[Gunnar Hjalmarsson (gunnarhj)]

Being thrown back indicates a syntax error. One thing you could do is testing each one of your sourced files like this:

sh -n <file>

The files must pass that test, or else you won't be able to log in.

This is what the new lightdm code does, but it does not work recursively...

[Gunnar Hjalmarsson (gunnarhj)]

On 2014-11-14 20:43, Barry Warsaw wrote:
> Sure, it's entirely possibly that my login has been silently ignoring
> the non-sh syntax errors, so in that respect, getting notification of
> them now is a good thing.

The use of commands unknown to sh is one kind of error that was silently ignored. However, that's not a syntax error. Syntax errors was not ignored.

[Barry Warsaw (barry)]

On Nov 14, 2014, at 08:15 PM, Gunnar Hjalmarsson wrote:

>Being thrown back indicates a syntax error.

Are those errors logged anywhere permanently?

[Gunnar Hjalmarsson (gunnarhj)]

On 2014-11-14 21:45, Barry Warsaw wrote:
> On Nov 14, 2014, at 08:15 PM, Gunnar Hjalmarsson wrote:
>> Being thrown back indicates a syntax error.
>
> Are those errors logged anywhere permanently?

I don't know. Syntax errors directly in e.g. ~/.profile or /etc/profile are logged in ~/.xsession-errors both with the latest lightdm version and with previous versions, but I don't know whether syntax errors in files which are sourced from those files are logged anywhere.

[Barry Warsaw (barry)]

On Nov 14, 2014, at 09:59 PM, Gunnar Hjalmarsson wrote:

>I don't know. Syntax errors directly in e.g. ~/.profile or /etc/profile
>are logged in ~/.xsession-errors both with the latest lightdm version
>and with previous versions, but I don't know whether syntax errors in
>files which are sourced from those files are logged anywhere.

I guess they aren't because ~/.xsession-errors is empty when lightdm exits
back to the login screen. :(

[Gunnar Hjalmarsson (gunnarhj)]

I created the file /etc/profile.d/wrong.sh with a sh syntax error. With lightdm 1.12.1-0ubuntu1 the reason for the login failure is logged in ~/.xsession-errors, but with the latest lightdm version it's not. So there is a regression with the fix of this bug.

The reason is that stderr is temporarily redirected to /tmp/config-err-XXXXXX. Actually, if you are thrown back to the login screen and log in via a console to investigate the reason, you can find the error message in that /tmp file. OTOH, that's not documented anywhere...

Right now I don't know how to best deal with this issue. Any advice appreciated.

[Barry Warsaw (barry)]

On Nov 14, 2014, at 11:38 PM, Gunnar Hjalmarsson wrote:

>Right now I don't know how to best deal with this issue. Any advice
>appreciated.

Probably use tee(1) to get the output both into the temp file and
~/.xsession-errors.

[Gunnar Hjalmarsson (gunnarhj)]

On 2014-11-15 00:49, Barry Warsaw wrote:
> On Nov 14, 2014, at 11:38 PM, Gunnar Hjalmarsson wrote:
>> Right now I don't know how to best deal with this issue. Any
>> advice appreciated.
>
> Probably use tee(1) to get the output both into the temp file and
> ~/.xsession-errors.

Yeah.. But I fear that would make the code much more complicated. Or do you have an idea, given the current code as shown in comment #33?

[Gunnar Hjalmarsson (gunnarhj)]

I have been googling and experimenting a bit. Basically I'd like to change:

  . "$CONFIG_FILE" 2>"$ERR"

to something along these lines:

  . "$CONFIG_FILE" 2> >(tee "$ERR" >&2)

There is just one tiny problem: It's bash syntax and doesn't work in sh. If I change the shebang line of /usr/sbin/lightdm-session to "#!/bin/bash" it works like a charm. Actually, it works much better, since it recursively prevents syntax errors in sourced files from causing fatal errors. OTOH, I suppose that switching to bash wouldn't be approved just like that. ;)

I got the tip from the first answer at this page:

http://stackoverflow.com/questions/692000/how-do-i-write-stderr-to-a-file-while-using-tee-with-a-pipe

The answer also includes the equivalent code for sh. I have successfully applied that approach in stand-alone test scripts, but when moved to /usr/sbin/lightdm-session, all my attempts so far have caused buggy behavior.

You might think that something like this should work:

  . "$CONFIG_FILE" 2>&1 >/dev/null | tee "$ERR" >&2

It doesn't. As soon as you add a pipe like that, setting environment variables via the sourced files fails.

So I'm stuck. Any help from a shell scripting wizard would be much appreciated.

[Martin Pitt (pitti)]

> . "$CONFIG_FILE" 2>"$ERR"
> OTOH, I suppose that switching to bash wouldn't be approved just like that. ;)

Bash's input redirection simply is way more powerful. Granted, bash takes a tad longer to start up, but any tee or other external program which you call in addition will probably hurt performance just as much.

TBH I don't see a better solution either.

> You might think that something like this should work:
> . "$CONFIG_FILE" 2>&1 >/dev/null | tee "$ERR" >&2

No, as that would fork off the "." into a subprocess, and thus the exports only survive there.

[Barry Warsaw (barry)]

On Nov 18, 2014, at 02:41 PM, Martin Pitt wrote:

>Bash's input redirection simply is way more powerful. Granted, bash
>takes a tad longer to start up, but any tee or other external program
>which you call in addition will probably hurt performance just as much.

Purely selfishly, switching back to bash would probably solve my original
problem since all my (ancient) startup/login scripts are bash syntax. ;)

[Gunnar Hjalmarsson (gunnarhj)]

Thanks for looking at it, Martin.

Actually I may have found a relatively trivial solution using trap(); please see attached diff.

[Martin Pitt (pitti)]

Hah, nice! Works for me, too.

[Gunnar Hjalmarsson (gunnarhj)]

I filed a merge proposal, where lightdm-session is run under bash and the code for exception handling improved. A vivid build of lightdm, including the proposed changes, is available in my PPA at

https://launchpad.net/~gunnarhj/+archive/ubuntu/misc

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.13.0-0ubuntu2

---------------
lightdm (1.13.0-0ubuntu2) vivid; urgency=medium

  * debian/config-error-dialog.sh:
  * debian/lightdm-session:
    - Use bash for the session to improve error handling (LP: #678421)
  * debian/control:
    - Depend on bash
 -- Robert Ancell <email address hidden> Tue, 25 Nov 2014 11:28:11 +1300

[Brian Murray (brian-murray)]

Hello Egon, or anyone else affected,

Accepted lightdm into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.10.4-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Gunnar Hjalmarsson (gunnarhj)]

I have successfully installed and run lightdm 1.10.4-0ubuntu2 from trusty-proposed. The behavior in case of a syntax error in ~/.profile has changed in accordance with the test case in the bug description.

[Launchpad Janitor (janitor)]

This bug was fixed in the package gdm - 3.14.0-0ubuntu2

---------------
gdm (3.14.0-0ubuntu2) vivid; urgency=medium

  * debian/patches/ubuntu_config_error_dialog.patch:
    - Show warning dialog in case of error in ~/.profile etc. and
      don't let a syntax error make the login fail (LP: #678421).
  * debian/control.in:
    - Add bash to Depends.
 -- Gunnar Hjalmarsson <email address hidden> Tue, 25 Nov 2014 15:55:00 +0100

[Chris J Arges (arges)]

Hello Egon, or anyone else affected,

Accepted gdm into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/gdm/3.10.0.1-0ubuntu3.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Egon, or anyone else affected,

Accepted gdm into utopic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gdm/3.10.0.1-0ubuntu7.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Gunnar Hjalmarsson (gunnarhj)]

gdm 3.10.0.1-0ubuntu3.1 was successfully installed and run by Magnus Ewert. Magnus confirms that the behavior in case of a syntax error in ~/.profile has changed in accordance with the test case in the bug description.

[Brian Murray (brian-murray)]

@Gunnar - the SRU team tools understand the tags v-done and v-done-$release, but not v-done-$release-$package.

[Gunnar Hjalmarsson (gunnarhj)]

Thanks for the correction, Brian.

[Martin Pitt (pitti)]

Sorry, misunderstanding. I restored the previous tags, which seem to be correct.

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for gdm has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package gdm - 3.10.0.1-0ubuntu3.1

---------------
gdm (3.10.0.1-0ubuntu3.1) trusty; urgency=medium

  * debian/patches/ubuntu_config_error_dialog.patch:
    - Show warning dialog in case of error in ~/.profile etc. and
      don't let a syntax error make the login fail (LP: #678421).
  * debian/control.in:
    - Add bash to Depends.
 -- Gunnar Hjalmarsson <email address hidden> Sun, 14 Dec 2014 17:44:00 +0100

[Tim Lunn (darkxst)]

I verified gdm on utopic with both a valid and broken .profile, all working as expected

[Gunnar Hjalmarsson (gunnarhj)]

Thanks Tim, with that all the SRUs have been verified at last. :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package gdm - 3.10.0.1-0ubuntu7.1

---------------
gdm (3.10.0.1-0ubuntu7.1) utopic; urgency=medium

  * debian/patches/ubuntu_config_error_dialog.patch:
    - Show warning dialog in case of error in ~/.profile etc. and
      don't let a syntax error make the login fail (LP: #678421).
  * debian/control.in:
    - Add bash to Depends.
 -- Gunnar Hjalmarsson <email address hidden> Sun, 14 Dec 2014 17:43:00 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.10.4-0ubuntu2

---------------
lightdm (1.10.4-0ubuntu2) trusty; urgency=medium

  * debian/config-error-dialog.sh:
  * debian/lightdm-session:
    - Use bash for the session to improve error handling (LP: #678421)
  * debian/control:
    - Depend on bash

lightdm (1.10.4-0ubuntu1) trusty; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Support DesktopNames key in session files (LP: #1383321)
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Tue, 25 Nov 2014 11:25:23 +1300

[Daniel Hahler (blueyed)]

Thanks for working on this!

For what it's worth, I've been using the following, and had to move the "set -x" now below the "exec 2>>$logfile", otherwise this was detected as being an error.

1. I've been using the following in ~/.profile, mainly for debugging purposes:

    logfile=/tmp/debug-profile-calls.log
    echo -n "Calling .profile (via $0) " >> $logfile
    date >> $logfile
    echo "SHELL: $SHELL" >> $logfile

    set -x
    exec 1>>$logfile
    exec 2>>$logfile

2. I was confused that kdialog was being used / preferred: I am using some KDE apps, but use awesome WM and the software stack based on Unity/Gnome. Maybe there's a better method to detect a KDE system than just checking if "kdialog" is installed.

[Gunnar Hjalmarsson (gunnarhj)]

Hi Daniel,

Thanks for your comments!

On 2015-02-25 19:56, Daniel Hahler wrote:
> set -x
> exec 1>>$logfile
> exec 2>>$logfile

Really using the "exec" command in ~/.profile would stop the execution of the main process (/usr/sbin/lightdm-session), so I suppose your ~/.profile did not literally contain the above lines. To understand if it was a false positive, you need to tell us more.

As regards kdialog: Right, just checking if kdialog is installed is certainly not a safe way to detect a KDE system. OTOH, that's not really the intention here. Given that you seem to have kdialog installed, does it matter much if it's used to show the error dialog (in case of an error) instead of xenity? ;)

If you want to suggest that the code is changed somehow, I ask you to please file a new bug report, since it would be a little messy to handle it on this already pretty long report.

[Daniel Hahler (blueyed)]

On 25.02.2015 23:42, Gunnar Hjalmarsson wrote:

> Thanks for your comments!
>
> On 2015-02-25 19:56, Daniel Hahler wrote:
>> set -x
>> exec 1>>$logfile
>> exec 2>>$logfile
>
> Really using the "exec" command in ~/.profile would stop the execution
> of the main process (/usr/sbin/lightdm-session), so I suppose your
> ~/.profile did not literally contain the above lines. To understand if
> it was a false positive, you need to tell us more.

It does not stop the execution / replace the current process, but only
redirects the file descriptors when used like this (1 (stdout) and 2 (stderr)).

It's fine like it is and expected - just wanted to provide it as feedback
nonetheless.

Cheers,
Daniel.

--
http://daniel.hahler.de/

[Gunnar Hjalmarsson (gunnarhj)]

Ok, thanks, Daniel. This way I learned something new. :)

I think the reason for the confusion is that stderr is already redirected when the code in ~/.profile is run. It's done through this line in /usr/sbin/lightdm-session:

  . "$CONFIG_FILE" 2>"$ERR"

So if you want it to work as before, you need to modify your redirects (I'm not able to tell exactly how, though). Otherwise things probably don't end up as you expect, even if you were able to avoid the error dialog by moving down the 'set -x' statement.

[Timo Aaltonen (tjaalton)]

Hello Egon, or anyone else affected,

Accepted lightdm into utopic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.12.3-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Gunnar Hjalmarsson (gunnarhj)]

I have successfully installed and run lightdm 1.12.3-0ubuntu1 from utopic-proposed. The behavior in case of a syntax error in ~/.profile has changed in accordance with the test case in the bug description.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.12.3-0ubuntu1

---------------
lightdm (1.12.3-0ubuntu1) utopic; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each greeter session. (LP: #1190344)
    - Don't attempt generate D-Bus seat/session removal signals on shutdown.
  * debian/guest-account.sh:
    - Rename variables to make script compatible with Bash (LP: #1411100)
  * debian/lightdm-session:
    - Use bash for the session to improve error handling (LP: #678421)
  * debian/control:
    - Depend on bash

lightdm (1.12.2-0ubuntu1) utopic; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Use correct syntax for DesktopNames key in session files (LP: #1383321)
    - Mock /run in tests
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Tue, 10 Mar 2015 16:02:44 +1300