Package: bash
Version: 4.1-3
Severity: important
Tags: patch

Hi,

Currently both dash and bash include /bin/sh in their files
list.  Thus one of the two has to always be diverting /bin/sh
to prevent file conflicts.

This is a nuisance because:
 - the system administrator may want a shell other than dash
   and bash
 - hardcoding this pair is kind of annoying
 - upgrades are a pain; see Bug#546528, #538822, and #540512.

Luckily ajt taught us a way out: bash can stop providing
/bin/sh without causing it to be removed, using some diversion
trickery[1].

Here's a try.  The main complication is that when we play with
diversions, any existing diversions have to be forgotten (or
remembered explicitly).  These patches cope with that by copping out
and just preserving the target of the /bin/sh link instead of
specific diversions.

My main complaint is the duplication of dash preinst code that
restores diversions in the postinst.  I think that's okay as a
temporary hack but would be happy to see the logic centralized or
eliminated (perhaps by getting dash to drop its copy of /bin/sh,
too, so there would be nothing to divert).

Patches apply on top of the patch from Bug#602456 (bash: please drop
dpkg assertion in preinst).  Attached since debbugs seems to deal
best with that.

What do you think?

Jonathan Nieder (2):
  bash.preinst: document
  Remove /bin/sh from the bash package

 debian/bash.postinst  |   50 +++++++++-
 debian/bash.preinst.c |  254 ++++++++++++++++++++++++++++++-------------------
 debian/changelog      |    4 +
 debian/rules          |    3 -
 4 files changed, 209 insertions(+), 102 deletions(-)

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=85;bug=34717
Subject: [PATCH 1/2] bash.preinst: document

Paraphrase as a shell script.
---
 debian/bash.preinst.c |   39 ++++++++++++++++++++++++++++++++-------
 1 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/debian/bash.preinst.c b/debian/bash.preinst.c
index 8eec85b..76a8aeb 100644
--- a/debian/bash.preinst.c
+++ b/debian/bash.preinst.c
@@ -5,6 +5,38 @@
  * You may freely use, distribute, and modify this program.
  */
 
+static const char msg[] =
+  "As bash for Debian is destined to provide a working /bin/sh (pointing to\n"
+  "/bin/bash) your link will be overwritten by a default link.\n\n"
+  "If you don't want further upgrades to overwrite your customization, 
please\n"
+  "read /usr/share/doc/bash/README.Debian.gz for a more permanent 
solution.\n\n"
+  "[Press RETURN to continue]";
+
+/* if ! target=$(readlink /bin/sh)
+ * then
+ *     # error reading link. Will be overwritten.
+ *     echo 'The bash upgrade discovered that something is wrong with your 
/bin/sh link.'
+ *     echo "$msg"
+ *     read answer
+ *     exit 0
+ * fi
+ * if test "$target" != bash && test "$target" != /bin/bash
+ * then
+ *     read diversion < <(/usr/sbin/dpkg-divert --list /bin/sh) ||
+ *             exit 1
+ *     if test -n "$diversion"
+ *     then
+ *             # link is diverted
+ *             exit 0
+ *     fi
+ *     echo 'The bash upgrade discovered that your /bin/sh link points to 
$target'
+ *     echo "$msg"
+ *     read answer
+ *     exit 0
+ * fi
+ * exit 0
+ */
+
 // Don't rely on /bin/sh and popen!
 
 #include <stdio.h>
@@ -70,13 +102,6 @@ char *check_diversion(void)
     return NULL;
 }
 
-const char *msg =
-  "As bash for Debian is destined to provide a working /bin/sh (pointing to\n"
-  "/bin/bash) your link will be overwritten by a default link.\n\n"
-  "If you don't want further upgrades to overwrite your customization, 
please\n"
-  "read /usr/share/doc/bash/README.Debian.gz for a more permanent 
solution.\n\n"
-  "[Press RETURN to continue]";
-
 int main(void) {
     int targetlen;
     char target[PATH_MAX+1], answer[1024], *fn;
-- 
1.7.2.3.557.gab647.dirty

Subject: [PATCH 2/2] Remove /bin/sh from the bash package

---
 debian/bash.postinst  |   50 +++++++++-
 debian/bash.preinst.c |  261 ++++++++++++++++++++++++++++---------------------
 debian/changelog      |    4 +
 debian/rules          |    3 -
 4 files changed, 200 insertions(+), 118 deletions(-)

diff --git a/debian/bash.postinst b/debian/bash.postinst
index 02b2302..8baf4f5 100644
--- a/debian/bash.postinst
+++ b/debian/bash.postinst
@@ -1,12 +1,58 @@
 #! /bin/bash
-
 set -e
 
-# the symlink is in the package now. So this should never happen ...
+set_diversions () {
+       if test "$(dpkg-divert --listpackage /bin/sh)" != "$1"
+       then
+               dpkg-divert --quiet --remove /bin/sh
+               dpkg-divert --package "$1" --divert /bin/sh.distrib --add 
/bin/sh
+       fi
+       if test "$(dpkg-divert --listpackage /usr/share/man/man1/sh.1.gz)" != 
"$1"
+       then
+               dpkg-divert --remove /usr/share/man/man1/sh.1.gz
+               dpkg-divert --package "$1" \
+                       --divert /usr/share/man/man1/sh.distrib.1.gz \
+                       --add /usr/share/man/man1/sh.1.gz
+       fi
+}
+
+divert_sh () {
+       shell=$(readlink /bin/sh)
+       shell=${shell#/bin/}
+
+       case "$shell" in
+       dash|bash|mksh|ksh|posh|pdksh|zsh)
+               set_diversions "$shell"
+               ;;
+       ksh93)
+               set_diversions ksh
+               ;;
+       mksh-static)
+               set_diversions mksh
+               ;;
+       zsh4|/usr/bin/zsh)
+               set_diversions zsh
+               ;;
+       *)
+               # Unknown shell.  Trust the existing diversion.
+               case "$(dpkg-divert --listpackage /bin/sh)" in
+               dash)
+                       echo >&2 \
+ "dash.preinst: Overwriting /bin/sh -> $shell link as requested by diversion"
+                       ;;
+               "")
+                       echo >&2 "dash.preinst: Overwriting /bin/sh -> $shell 
link."
+               esac
+       esac
+}
+
+# This should never happen ...
 if [ ! -e /bin/sh ]; then
     ln -s bash /bin/sh
 fi
 
+divert_sh
+
 update-alternatives --install \
     /usr/share/man/man7/builtins.7.gz \
     builtins.7.gz \
diff --git a/debian/bash.preinst.c b/debian/bash.preinst.c
index 76a8aeb..d7f0751 100644
--- a/debian/bash.preinst.c
+++ b/debian/bash.preinst.c
@@ -1,134 +1,169 @@
-/* Copyright (c) 1999 Anthony Towns
- * Copyright (c) 2000, 2002 Matthias Klose
- * Copyright (c) 2008 Canonical Ltd
+/*
+ * This file is in the public domain.
+ * You may freely use, modify, distribute, and relicense it.
  *
- * You may freely use, distribute, and modify this program.
+ * Don't rely on /bin/sh or popen()!
  */
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+
 static const char msg[] =
-  "As bash for Debian is destined to provide a working /bin/sh (pointing to\n"
-  "/bin/bash) your link will be overwritten by a default link.\n\n"
+  "The bash upgrade discovered that something is wrong with your /bin/sh 
link.\n"
+  "As bash for Debian is dedicated to maintaining a working /bin/sh\n"
+  "(pointing to /bin/bash or your shell of choice), your link will be 
replaced\n"
+  "by a link to /bin/bash.\n\n"
   "If you don't want further upgrades to overwrite your customization, 
please\n"
   "read /usr/share/doc/bash/README.Debian.gz for a more permanent 
solution.\n\n"
   "[Press RETURN to continue]";
 
-/* if ! target=$(readlink /bin/sh)
- * then
- *     # error reading link. Will be overwritten.
- *     echo 'The bash upgrade discovered that something is wrong with your 
/bin/sh link.'
- *     echo "$msg"
- *     read answer
- *     exit 0
- * fi
- * if test "$target" != bash && test "$target" != /bin/bash
- * then
- *     read diversion < <(/usr/sbin/dpkg-divert --list /bin/sh) ||
- *             exit 1
- *     if test -n "$diversion"
- *     then
- *             # link is diverted
- *             exit 0
- *     fi
- *     echo 'The bash upgrade discovered that your /bin/sh link points to 
$target'
- *     echo "$msg"
- *     read answer
- *     exit 0
- * fi
- * exit 0
- */
-
-// Don't rely on /bin/sh and popen!
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <limits.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-
-/* XXX: evil kludge, deal with arbitrary name lengths */
-#ifndef PATH_MAX
-#define PATH_MAX 4096
-#endif
+static void ensure_sh_is_symlink(void)
+{
+       struct stat buf;
+       char answer[256];
+       if (lstat("/bin/sh", &buf)) {
+               perror("bash.preinst: cannot stat /bin/sh");
+               exit(EXIT_FAILURE);
+       }
+       if (S_ISLNK(buf.st_mode))
+               return;
+       puts(msg);
+       fflush(stdout);
+       fgets(answer, sizeof(answer), stdin);
+       if (ferror(stdin)) {
+               perror("bash.preinst: cannot read reply");
+               exit(EXIT_FAILURE);
+       }
+       if (unlink("/bin/sh")) {
+               perror("bash.preinst: cannot unlink /bin/sh");
+               exit(EXIT_FAILURE);
+       }
+       if (symlink("bash", "/bin/sh")) {
+               perror("bash.preinst: cannot write new /bin/sh link");
+               exit(EXIT_FAILURE);
+       }
+}
 
-char *check_diversion(void)
+static pid_t fork_dpkg_divert(void)
 {
-    pid_t child;
-    int pipedes[2];
+       pid_t child = fork();
+       if (child == -1) {
+               perror("bash.preinst: cannot fork dpkg-divert");
+               exit(EXIT_FAILURE);
+       }
+       return child;
+}
 
-    if (pipe(pipedes))
-       return NULL;
+static void wait_on_dpkg_divert(pid_t pid)
+{
+       int status;
+       if (waitpid(pid, &status, 0) != pid) {
+               perror("bash.preinst: cannot wait on dpkg-divert");
+               exit(EXIT_FAILURE);
+       }
+       if (!WIFEXITED(status) || WEXITSTATUS(status)) {
+               fprintf(stderr, "bash.preinst: dpkg-divert failed\n");
+               exit(EXIT_FAILURE);
+       }
+}
 
+static void run_dpkg_divert(char *const *argv)
+{
+       pid_t child = fork_dpkg_divert();
+       if (!child)
+               execvp("dpkg-divert", argv);
+       wait_on_dpkg_divert(child);
+}
 
-    switch(child = fork()) {
-      case -1:
-        /* fork failed */
-       close(pipedes[0]);
-       close(pipedes[1]);
-        return NULL;
+static void divert_sh(void)
+{
+       char *const remove_argv[] = {
+               "dpkg-divert", "--quiet", "--remove", "/bin/sh", NULL
+       };
+       char *const add_argv[] = {
+               "dpkg-divert", "--package", "dash",
+               "--divert", "/bin/sh.removed", "--add", "/bin/sh", NULL
+       };
+       run_dpkg_divert(remove_argv);
+       run_dpkg_divert(add_argv);
+}
 
-      case 0:
-        /* i'm the child */
-        {
-           if (dup2(pipedes[STDOUT_FILENO], STDOUT_FILENO) < 0)
-               _exit(127);
-           close(pipedes[STDIN_FILENO]);
-           close(pipedes[STDOUT_FILENO]);
-            execl( "/usr/sbin/dpkg-divert", "/usr/sbin/dpkg-divert",
-                   "--list", "/bin/sh", NULL );
-           _exit(127);
-        }
+static void listpackage_sh(int out)
+{
+       if (dup2(out, STDOUT_FILENO) < 0) {
+               perror("bash.preinst: cannot redirect");
+               _exit(2);
+       }
+       close(out);
+       execlp("dpkg-divert", "dpkg-divert", "--listpackage", "/bin/sh", NULL);
+       perror("bash.preinst: cannot run dpkg-divert");
+       _exit(2);
+}
 
-      default:
-        /* i'm the parent */
-        {
-           static char line[1024];
-           FILE *fd;
+static int sh_is_diverted(void)
+{
+       int fds[2];
+       FILE *output;
+       char package[256];
+       pid_t child;
 
-           close (pipedes[STDOUT_FILENO]);
-           fcntl (pipedes[STDIN_FILENO], F_SETFD, FD_CLOEXEC);
-           fd = fdopen (pipedes[STDIN_FILENO], "r");
+       if (pipe(fds)) {
+               perror("bash.preinst: cannot create pipe");
+               exit(EXIT_FAILURE);
+       }
+       child = fork_dpkg_divert();
+       if (!child) {
+               close(fds[0]);
+               listpackage_sh(fds[1]);
+       }
+       close(fds[1]);
+       output = fdopen(fds[0], "r");
+       if (!output) {
+               perror("bash.preinst: cannot open read end of pipe");
+               exit(EXIT_FAILURE);
+       }
+       package[fread(package, sizeof(package) - 1, 1, output)] = '\0';
+       if (fclose(output)) {
+               perror("bash.preinst: error reading from pipe");
+               exit(EXIT_FAILURE);
+       }
+       wait_on_dpkg_divert(child);
 
-           while (fgets(line, 1024, fd) != NULL) {
-               line[strlen(line)-1] = '\0';
-               break;
-           }
-           fclose(fd);
-           return line;
-        }
-    }
-    return NULL;
+       if (!package[0] || !strcmp(package, "bash\n"))
+               return 0;
+       return 1;
 }
 
-int main(void) {
-    int targetlen;
-    char target[PATH_MAX+1], answer[1024], *fn;
-
-    targetlen = readlink("/bin/sh", target, PATH_MAX);
-    if (targetlen == -1) {
-       // error reading link. Will be overwritten.
-       puts("The bash upgrade discovered that something is wrong with your 
/bin/sh link.");
-       puts(msg);
-       fgets(answer, 1024, stdin);
-       return EXIT_SUCCESS;
-    }
-    target[targetlen] = '\0';
-    if (strcmp(target, "bash") != 0 && strcmp(target, "/bin/bash") != 0) {
-       char *diversion = check_diversion();
-
-       if (diversion == NULL)
-           return EXIT_FAILURE;
-       // printf("diversion: /%s/\n", diversion);
-       if (strcmp(diversion, "") != 0)
-           // link is diverted
-           return EXIT_SUCCESS;
-       printf("The bash upgrade discovered that your /bin/sh link points to 
%s.\n", target);
-       puts(msg);
-       fgets(answer, 1024, stdin);
-       return EXIT_SUCCESS;
-    }
+int main(void)
+{
+       /*
+        * # Warn if /bin/sh is broken.
+        * if ! test -h /bin/sh
+        * then
+        *      echo "$msg"
+        *      read answer
+        *      ln -sf bash /bin/sh
+        * fi
+        */
+       ensure_sh_is_symlink();
 
-    return EXIT_SUCCESS;
+       /*
+        * # Ensure /bin/sh from bash is diverted so bash can remove it.
+        * # bash.postinst or dash.preinst will put the old diversion back
+        * # in place.
+        *
+        * diverter=$(dpkg-divert --listpackage /bin/sh)
+        * case "$diverter" in
+        * bash | "")
+        *      dpkg-divert --quiet --remove /bin/sh
+        *      dpkg-divert --package dash --divert /bin/sh.removed --add 
/bin/sh
+        * esac
+        */
+       if (!sh_is_diverted())
+               divert_sh();
+       return 0;
 }
diff --git a/debian/changelog b/debian/changelog
index 1fd7738..2770a1e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,10 @@ bash (4.1-3.1) local; urgency=low
       the upgrade to lenny.
     - Remove dead code for handling /etc/bash_completion.
     - Remove dpkg --assert-support-predepends check.
+  * Stop shipping /bin/sh: divert it in preinst so the actual
+    /bin/sh is not removed, re-add diversions based on the target of
+    the /bin/sh link in postinst, and stop creating a new /bin/sh
+    link in debian/rules.
 
  -- Jonathan Nieder <jrnie...@gmail.com>  Mon, 01 Nov 2010 17:41:40 -0500
 
diff --git a/debian/rules b/debian/rules
index d5cf8ff..6badfb3 100755
--- a/debian/rules
+++ b/debian/rules
@@ -221,9 +221,6 @@ endif
 
        : # extra links
        ln -sf bash $(d)/bin/rbash
-#      : # now shipped by dash
-       ln -sf bash $(d)/bin/sh
-       ln -sf bash.1 $(d)/usr/share/man/man1/sh.1
 
        : # skeleton files
        $(ID) debian/etc.bash.bashrc $(d)/etc/bash.bashrc
-- 
1.7.2.3.557.gab647.dirty

Reply via email to