Bug#989334: bash: remove obsolete upgrade code from pre-wheezy from preinst
Hi,
Quoting Johannes Schauer Marin Rodrigues (2021-10-04 18:16:01)
> On Tue, 1 Jun 2021 13:35:53 +0200 Helmut Grohne wrote:
> > While #958083 asks for removing bash.preinst, this bug asks for even less:
> > The bash.preinst contains code for upgrading a bash that did contain /bin/sh
> > to a current one. Since at least wheezy, bash did not contain /bin/sh, so
> > this code can quite surely go away without trouble.
> >
> > I'm attaching a patch to makes just this code go away. The deleted code
> > is practically dead for years. Removing it should be uncontroversial and
> > after it is removed, it becomes easier to reason about #958083 which
> > asks to remove the rest of the preinst.
> >
> > Essentially, this bug splits #958083 into a removing the majority of code
> > that should be uncontroversial to remove.
>
> since this is blocking our work on adding DPKG_ROOT support to the essential
> package set and since bash is among the 10 remaining source packages that we
> still have to patch, I would like to NMU bash with the patch provided by
> Helmut. If you think this is inappropriate, please speak up. Otherwise, my
> plan
> is to upload to DELAYED/10 in two weeks time. I'll send another mail to this
> bug with the debdiff once I have the NMU ready.
I have uploaded bash to DELAYED/10 with the attached debdiff.
Thanks!
cheers, joschdiff -Nru bash-5.1/debian/bash.preinst.c bash-5.1/debian/bash.preinst.c
--- bash-5.1/debian/bash.preinst.c 2013-10-23 14:41:22.0 +0200
+++ bash-5.1/debian/bash.preinst.c 2021-10-23 11:36:25.0 +0200
@@ -4,13 +4,8 @@
*/
#include "bash.preinst.h"
-#include
-#include
#include
-#include
-#include
#include
-#include
static void backup(const char *file, const char *dest)
{
@@ -34,103 +29,6 @@
die_errno("cannot create symlink %s -> %s", link, target);
}
-static void reset_diversion(const char *package, const char *file,
- const char *distrib)
-{
- const char * const remove_old_diversion[] =
- {"dpkg-divert", "--package", "bash", "--remove", file, NULL};
- const char * const new_diversion[] =
- {"dpkg-divert", "--package", package,
- "--divert", distrib, "--add", file, NULL};
- run(remove_old_diversion);
- run(new_diversion);
-}
-
-static int has_binsh_line(FILE *file)
-{
- char item[sizeof("/bin/sh\n")];
-
- while (fgets(item, sizeof(item), file)) {
- int ch;
-
- if (!memcmp(item, "/bin/sh\n", strlen("/bin/sh\n") + 1))
- return 1;
- if (strchr(item, '\n'))
- continue;
-
- /* Finish the line. */
- for (ch = 0; ch != '\n' && ch != EOF; ch = fgetc(file))
- ; /* just reading */
- if (ch == EOF)
- break;
- }
- if (ferror(file))
- die_errno("cannot read pipe");
- return 0;
-}
-
-static int binsh_in_filelist(const char *package)
-{
- const char * const cmd[] = {"dpkg-query", "-L", package, NULL};
- pid_t child;
- int sink;
- FILE *in;
- int found;
-
- /*
- * dpkg -L $package 2>/dev/null | ...
- *
- * Redirection of stderr is for quieter output
- * when $package is not installed. If opening /dev/null
- * fails, no problem; leave stderr alone in that case.
- */
- sink = open("/dev/null", O_WRONLY);
- if (sink >= 0)
- set_cloexec(sink);
- in = spawn_pipe(, cmd, sink);
-
- /* ... | grep "^/bin/sh\$" */
- found = has_binsh_line(in);
- if (fclose(in))
- die_errno("cannot close read end of pipe");
-
- /*
- * dpkg -L will error out if $package is not already installed.
- *
- * We stopped reading early if we found a match, so
- * tolerate SIGPIPE in that case.
- */
- wait_or_die(child, "dpkg-query -L", ERROR_OK |
- (found ? SIGPIPE_OK : 0));
- return found;
-}
-
-static int undiverted(const char *path)
-{
- const char * const cmd[] =
- {"dpkg-divert", "--listpackage", path, NULL};
- pid_t child;
- char packagename[sizeof("bash\n")];
- size_t len;
- FILE *in = spawn_pipe(, cmd, -1);
- int diverted = 1;
-
- /* Is $path diverted by someone other than bash? */
-
- len = fread(packagename, 1, sizeof(packagename), in);
- if (ferror(in))
- die_errno("cannot read from dpkg-divert");
- if (len == 0)
- diverted = 0; /* No diversion. */
- if (len == strlen("bash\n") && !memcmp(packagename, "bash\n", len))
- diverted = 0; /* Diverted by bash. */
-
- if (fclose(in))
- die_errno("cannot close read end of pipe");
- wait_or_die(child, "dpkg-divert", ERROR_OK | SIGPIPE_OK);
- return !diverted;
-}
-
int main(int argc, char *argv[])
{
/* /bin/sh needs to point to a valid target. */
@@ -144,27 +42,5 @@
force_symlink("bash.1.gz", "/usr/share/man/man1/sh.1.gz",
"/usr/share/man/man1/sh.1.gz.temp");
}
- if (!binsh_in_filelist("bash"))
- /* Ready. */
- return 0;
-
- /*
- * In bash (<= 4.1-3), the bash package included symlinks for
- * /bin/sh and the sh(1) manpage in its data.tar.
- *
- * Unless we are careful, unpacking the new version of bash
- * will remove them. So we tell dpkg that the files from bash
- * to be removed are elsewhere, using a diversion on behalf of
- * another package.
- *
- * Based on an
Bug#989334: bash: remove obsolete upgrade code from pre-wheezy from preinst
Hi, On Tue, 1 Jun 2021 13:35:53 +0200 Helmut Grohne wrote: > While #958083 asks for removing bash.preinst, this bug asks for even less: > The bash.preinst contains code for upgrading a bash that did contain /bin/sh > to a current one. Since at least wheezy, bash did not contain /bin/sh, so > this code can quite surely go away without trouble. > > I'm attaching a patch to makes just this code go away. The deleted code > is practically dead for years. Removing it should be uncontroversial and > after it is removed, it becomes easier to reason about #958083 which > asks to remove the rest of the preinst. > > Essentially, this bug splits #958083 into a removing the majority of code > that should be uncontroversial to remove. since this is blocking our work on adding DPKG_ROOT support to the essential package set and since bash is among the 10 remaining source packages that we still have to patch, I would like to NMU bash with the patch provided by Helmut. If you think this is inappropriate, please speak up. Otherwise, my plan is to upload to DELAYED/10 in two weeks time. I'll send another mail to this bug with the debdiff once I have the NMU ready. Thanks! cheers, josch signature.asc Description: signature
Bug#989334: bash: remove obsolete upgrade code from pre-wheezy from preinst
Package: bash
Version: 5.1-3
Severity: wishlist
Tags: patch
X-Debbugs-Cc: Niels Thykier , Guillem Jover
Control: block 958083 by -1
While #958083 asks for removing bash.preinst, this bug asks for even
less: The bash.preinst contains code for upgrading a bash that did
contain /bin/sh to a current one. Since at least wheezy, bash did not
contain /bin/sh, so this code can quite surely go away without trouble.
I'm attaching a patch to makes just this code go away. The deleted code
is practically dead for years. Removing it should be uncontroversial and
after it is removed, it becomes easier to reason about #958083 which
asks to remove the rest of the preinst.
Essentially, this bug splits #958083 into a removing the majority of
code that should be uncontroversial to remove.
Helmut
diff --minimal -Nru bash-5.1/debian/bash.preinst-lib.c
bash-5.1/debian/bash.preinst-lib.c
--- bash-5.1/debian/bash.preinst-lib.c 2013-10-23 14:41:22.0 +0200
+++ bash-5.1/debian/bash.preinst-lib.c 2021-05-31 14:29:20.0 +0200
@@ -13,7 +13,6 @@
#include
#include
#include
-#include
#include
extern char **environ;
@@ -58,30 +57,12 @@
die_errno("cannot get status of %s", file);
}
-void set_cloexec(int fd)
-{
- int flags = fcntl(fd, F_GETFD);
- if (flags < 0 || fcntl(fd, F_SETFD, flags | FD_CLOEXEC))
- die_errno("cannot set close-on-exec flag");
-}
-
-void xpipe(int pipefd[2])
-{
- if (pipe(pipefd))
- die_errno("cannot create pipe");
- set_cloexec(pipefd[0]);
- set_cloexec(pipefd[1]);
-}
-
-void wait_or_die(pid_t child, const char *name, int flags)
+static void wait_or_die(pid_t child, const char *name)
{
int status;
if (waitpid(child, , 0) != child)
die_errno("cannot wait for %s", name);
- if ((WIFEXITED(status) && WEXITSTATUS(status) == 0) ||
- ((flags & ERROR_OK) && WIFEXITED(status)) ||
- ((flags & SIGPIPE_OK) &&
-WIFSIGNALED(status) && WTERMSIG(status) == SIGPIPE))
+ if (WIFEXITED(status) && WEXITSTATUS(status) == 0)
return;
if (WIFEXITED(status))
@@ -93,7 +74,7 @@
die("waitpid is confused (status=%d)", status);
}
-pid_t spawn(const char * const cmd[], int out, int err)
+static pid_t spawn(const char * const cmd[], int out, int err)
{
pid_t child;
posix_spawn_file_actions_t redir;
@@ -111,21 +92,5 @@
void run(const char * const cmd[])
{
pid_t child = spawn(cmd, -1, -1);
- wait_or_die(child, cmd[0], 0);
-}
-
-FILE *spawn_pipe(pid_t *pid, const char * const cmd[], int errfd)
-{
- int pipefd[2];
- FILE *f;
-
- xpipe(pipefd);
- *pid = spawn(cmd, pipefd[1], errfd);
- if (close(pipefd[1]) || (errfd != -1 && close(errfd)))
- die_errno("cannot close unneeded fd");
-
- f = fdopen(pipefd[0], "r");
- if (!f)
- die_errno("cannot stream read end of pipe");
- return f;
+ wait_or_die(child, cmd[0]);
}
diff --minimal -Nru bash-5.1/debian/bash.preinst.c
bash-5.1/debian/bash.preinst.c
--- bash-5.1/debian/bash.preinst.c 2013-10-23 14:41:22.0 +0200
+++ bash-5.1/debian/bash.preinst.c 2021-05-31 14:29:20.0 +0200
@@ -4,13 +4,8 @@
*/
#include "bash.preinst.h"
-#include
-#include
#include
-#include
-#include
#include
-#include
static void backup(const char *file, const char *dest)
{
@@ -34,103 +29,6 @@
die_errno("cannot create symlink %s -> %s", link, target);
}
-static void reset_diversion(const char *package, const char *file,
- const char *distrib)
-{
- const char * const remove_old_diversion[] =
- {"dpkg-divert", "--package", "bash", "--remove", file, NULL};
- const char * const new_diversion[] =
- {"dpkg-divert", "--package", package,
- "--divert", distrib, "--add", file, NULL};
- run(remove_old_diversion);
- run(new_diversion);
-}
-
-static int has_binsh_line(FILE *file)
-{
- char item[sizeof("/bin/sh\n")];
-
- while (fgets(item, sizeof(item), file)) {
- int ch;
-
- if (!memcmp(item, "/bin/sh\n", strlen("/bin/sh\n") + 1))
- return 1;
- if (strchr(item, '\n'))
- continue;
-
- /* Finish the line. */
- for (ch = 0; ch != '\n' && ch != EOF; ch = fgetc(file))
- ; /* just reading */
- if (ch == EOF)
- break;
- }
- if (ferror(file))
- die_errno("cannot read pipe");
- return 0;
-}
-
-static int binsh_in_filelist(const char *package)
-{
- const char * const cmd[] = {"dpkg-query", "-L", package, NULL};
- pid_t child;
- int sink;
- FILE *in;
- int found;
-
- /*
-* dpkg -L $package 2>/dev/null | ...
-
