[apparmor] aa-notify still broken :-(

[John Johansen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/24/2011 05:51 AM, Jamie Strandboge wrote:
> On Sat, 2011-09-24 at 13:52 +0200, Christian Boltz wrote:
>> Hello,
>>
>> Am Samstag, 24. September 2011 schrieb John Johansen:
>>> On 09/23/2011 04:01 PM, Christian Boltz wrote:
>>>> After a long debugging session with John on IRC I found out that
>>>> sudo on openSUSE resets or deletes too many environment variables.
>>>> It turned out that $HOME and $DISPLAY need to be set to the
>>>> correct value - otherwise $notify_exe can't connect to DBUS to
>>>> display the message.
>>>>
>>>> Getting the correct $HOME is easy.
>>>>
>>>> $DISPLAY is a different beast - if sudo unsets it, the best thing I
>>>> can do is to hardcode it to ":0" which should fit most systems.
>>>> I'm open for better ideas, but please ACK my patch before - it
>>>> makes the situation much better compared to the current aa-notify
>>>> ;-)
>>  
>>> So I am not very happy with setting the display with a guess but the
>>> best I can come up with is either using a flag, but there is no
>>> point to doing that when you can do
>>>   sudo DISPLAY="$DISPLAY" aa-notify -p
>>
>> Maybe a flag (and/or an option in the config file) would still be better 
>> than "sudo DISPLAY=...". I'm not too familiar with sudo, but I'd guess 
>> that you can limit what a user can hand over as environment variables. 
>> Having an option for aa-notify might be more flexible regarding sudo.
>> (If I'm wrong about the restrictions in sudo, forget this note ;-)
>>
>>> I'm not sure setting DISPLAY = :0 is better than documenting the sudo
>>> case and that DISPLAY with need to be set.
>>
>> The point is that setting DISPLAY=:0 will fix the issue for (I'd guess) 
>> 99% of the users. That makes it a good default IMHO.
>>
>> Documentation is of course needed, and maybe even a warning at startup 
>> (if -p is given) saying
>>     Environment variable $DISPLAY not set - falling back to default :0
>>
>> That said: I also don't really like the solution with the hardcoded 
>> default of :0, but it's the least bad (!= best) solution I can imagine.
>>
>>> So Ack on the setting of HOME, and hold off on DISPLAY for the moment
>>> anyways.  I would like to hear more of what others have to say on
>>> that part
>>
>> OK, I commited the HOME part and a TODO note about $DISPLAY.
> 
> I didn't get a chance to comment on this. Setting DISPLAY to the default
> of ':0' feels wrong to me too. I don't have an alternative at this time.
> 
How about an optional flag?  It sounds like Christian would prefer that
to passing DISPLAY on the command line and that way you are opting in

> As for HOME, I think putting it in send_message is the wrong place.
> While it is guaranteed to always be up to date, I don't think that
> people are changing their HOME all that often and I think all the hits
> on an LDAP database for each message is too much. I think that something
> like the attached patch (against current trunk) would be better
> (untested).
> 
> 
Well if HOME is changing are current implementation is broken too, because
we just use the value passed in the environment at start up so this is
good.  And yeah it should be faster as we don't have to lookup HOME each
time.

I say once its tested we commit this revision.
Acked-by: John Johansen <john.johansen at canonical.com>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5+qGkACgkQxAVxIsEKI+aL7wCgsRVi8J4EQgVFFQGogTujdVVF
6aUAnRO6UomGaAtPS96/QbId0mpr/lDw
=43qQ
-----END PGP SIGNATURE-----