[RFC] /etc/shells management (fish, mksh, posh, tcsh, zsh)

[Yaakov Selkowitz <yselkowitz@cygwin.com>]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On 2016-05-11 14:06, Yaakov Selkowitz wrote:
> On 2016-05-11 12:09, Andrew Schulman wrote:
>>> Am 10.05.2016 um 20:19 schrieb Andrew Schulman:
>>>> Achim, can you please add /bin/fish and /usr/bin/fish to /etc/shells in
>>>> base-files?
>>>
>>> I seem to remember that this was discussed before.  If you could perhaps
>>> look up that discussion and fill me in what the conclusion was last time
>>> around?
>>
>> Hm, you're right, it was discussed before:
>>
>> https://www.cygwin.com/ml/cygwin/2014-02/msg00696.html
>>
>> I don't know if there was consensus, but the last word there from CGF was
>> that shell packages should run a postinstall step to add themselves to
>> /etc/shells.
>
> While I'm always ready to reconsider previous decisions, this is how it
> appears to be handled in Linux distros.  The implication thereof is that
> (once all packages have been adapted) the default /etc/shells should
> only contain those shells available by default (namely, sh, bash, and
> /sbin/nologin), e.g.:
>
> https://git.fedorahosted.org/cgit/setup.git/tree/shells
>
> (except that /sbin != /usr/sbin on Cygwin.)

AFAICS this should be a two-step process.

1) base-files' default /etc/shells should contain only the shells in a 
Base install, namely:

/bin/sh
/bin/ash
/bin/bash
/bin/dash
/usr/bin/sh
/usr/bin/ash
/usr/bin/bash
/usr/bin/dash
/sbin/nologin

2) Then all non-Base shells, namely:

fish                                         Andrew Schulman
mksh                                         Chris Sutcliffe
posh                                         Jari Aalto
tcsh                                         Corinna Vinschen
zsh                                          Peter A. Castro

will bump release adding an update_etc_shells call, per the attached 
patch, with the path of their shell(s).

>> That seems reasonable. There are questions about the right way to do it,
>> but I'll ask those in a separate thread.
>
> Probably best if we have a cygport function for creating the necessary
> postinstall and preremove commands.

Attached.  Any questions or comments before I make this official?

Or, is this just not worth the trouble?  What are the consequences of 
having shells listed in /etc/shells which aren't on the system?

-- 
Yaakov

[-- Attachment #2: 0002-Add-update_etc_shells-for-etc-shells-management.patch --]
[-- Type: text/plain, Size: 2608 bytes --]

From 68d32dfee3ed2b0b8c1f356ef794846f086d3583 Mon Sep 17 00:00:00 2001
From: Yaakov Selkowitz <yselkowi@redhat.com>
Date: Thu, 12 May 2016 16:14:33 -0500
Subject: [PATCH] Add update_etc_shells for /etc/shells management

See https://cygwin.com/ml/cygwin/2016-05/msg00135.html
---
 lib/src_install.cygpart | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/lib/src_install.cygpart b/lib/src_install.cygpart
index 1fe8176..41adcb3 100644
--- a/lib/src_install.cygpart
+++ b/lib/src_install.cygpart
@@ -877,6 +877,60 @@ make_etc_defaults() {
 	done
 }
 
+#****I* Installing/update_etc_shells
+#  SYNOPSIS
+#  update_etc_shells PATH_TO_SHELL [PATH_TO_SHELL] ...
+#  DESCRIPTION
+#  Indicates that the given fully-qualified path(s) are shells which should be
+#  listed in /etc/shells.  update_etc_shells creates a postinstall script to
+#  add this listing if it doesn't already exist, and a preremove script which
+#  removes it when uninstalling.
+#  NOTES
+#  * Only one of the /bin or /usr/bin paths should be specified for any given
+#    shell; the other will be added automatically.
+#  * Generic aliases should be listed as well, e.g. tcsh would use
+#    update_etc_shells /bin/tcsh /bin/csh
+#  * Shells which are part of the Base install (namely, bash/sh and dash/ash)
+#    should not use this function, as they are already included in the default
+#    /etc/shells.
+#****
+update_etc_shells() {
+	local alt sh
+
+	for sh in ${@}
+	do
+		case ${sh} in
+		/bin/*)		alt=/usr${sh} ;;
+		/usr/bin/*)	alt=${sh#/usr} ;;
+		*)		alt= ;;
+		esac
+
+		if [ ! -e ${D}${sh} ] && [ ! -e ${D}${alt:-${sh}} ]
+		then
+			error "shell ${sh} does not exist"
+		fi
+
+		dodir /etc/postinstall
+		cat >> ${D}/etc/postinstall/${PN}.sh <<-_EOF
+			if [ ! -f /etc/shells ] || ! grep -q "^${sh}$" /etc/shells
+			then
+			    echo -e "${sh}${alt:+\n}${alt}" >> /etc/shells
+			fi
+
+		_EOF
+
+		dodir /etc/preremove
+		cat >> ${D}/etc/preremove/${PN}.sh <<-_EOF
+			if [ -f /etc/shells ]
+			then
+			    sed -i -e '\|^${sh}$|d' /etc/shells
+			    ${alt:+sed -i -e '\|^${alt}$|d' /etc/shells}
+			fi
+
+		_EOF
+	done
+}
+
 __prepinstalldirs() {
 	rm -fr ${D}/*;
 }
@@ -950,4 +1004,4 @@ readonly -f __doinstall __fix_shebang \
             exeinto doexe newexe insinto doins newins doicon newicon \
             dolib doman newman domenu newmenu dosbin newsbin dosym \
             make_autostart_entry make_desktop_entry make_etc_defaults \
-            __prepinstalldirs cyginstall
+            update_etc_shells __prepinstalldirs cyginstall
-- 
2.8.0