[25468] in Source-Commits
Re: /svn/athena r25002 -
daemon@ATHENA.MIT.EDU (Benjamin Kaduk)
Sat Mar 5 17:32:21 2011
Date: Sat, 5 Mar 2011 17:32:13 -0500 (EST)
From: Benjamin Kaduk <kaduk@MIT.EDU>
To: Jonathan Reed <jdreed@MIT.EDU>
cc: source-commits@MIT.EDU
In-Reply-To: <C3E70ABF-B662-4853-8D20-20DC818C4AFF@MIT.EDU>
Message-ID: <alpine.GSO.1.10.1103051728170.19944@multics.mit.edu>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
On Sat, 5 Mar 2011, Jonathan Reed wrote:
>
> On Mar 5, 2011, at 4:34 PM, Benjamin Kaduk wrote:
>
>> On Fri, 4 Mar 2011, Jonathan D Reed wrote:
>>
>>> +PATH=/usr/bin:/bin:$PATH
>>> +
>>> +if [ "$(machtype -L)" != "debathena-cluster" ]; then
>>> + gnome-session-save --force-logout
>>> + exit 0
>>> +fi
>>> +
>>> +cell=athena.mit.edu
>>> +afspath=$HOME
>>> +if echo $HOME | grep -q ^/mit; then
>>> + # Shouldn't happen with std dotfiles
>>> + afspath=$(readlink $HOME)
>>> +fi
>>> +if echo $afspath | grep -q ^/afs; then
>>> + cell=$(echo $afspath | cut -d/ -f 3)
>>
>> This feels a little bit awkward. I think that logically, we should only be setting cell if $HOME is in /afs, and only doing the tokens check if cell is set. If some clever user decides to set $HOME to something else, well, then they hit the pkill branch.
>
> If HOME is not in AFS, you don't get to log into the cluster machine.
> Setting HOME to something else is unsupported.
Sure. We should be robust in handling such cases when possible, though
(which was the whole motivation for thinking enough to send the first
mail).
>
> This check is here for the following case: your homedir is in a foreign
> cell, and you have tokens for that cell, but you let your Athena tokens
> expire because oyu don't need them. In that case, we still want to do a
> graceful logout.
Right, that was clear. But somehow the "$afspath does not start with
/afs" case feels like an error case which may or may not merit treatment
other than defaulting to assuming that athena cell tokens are necessary.
>
>>
>>> +fi
>>> +if tokens | fgrep -q $cell; then
>>> + gnome-session-save --force-logout
>>> +else
>>> + pkill schroot
>>> +fi
>>> +
>>
>> No return value for this branch? We 'exit 0' after the gnome-session save in the non-cluster case.
>
> That case should never happen, because this code only gets installed on
> cluster machines. The metapackage check was there purely to avoid
Good point. I only mentioned it because it was inconsistent and I was too
lazy to look up whether it should be in both places or none.
-Ben
> wankery about the hypothetical case in which someone removes -cluster,
> but somehow the package ends up broken. I'm happy to remove it, because
> again, cluster-login-config should never end up on not-cluster.
