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
>From 68d32dfee3ed2b0b8c1f356ef794846f086d3583 Mon Sep 17 00:00:00 2001
From: Yaakov Selkowitz <yselk...@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

Reply via email to