From 9fc0b435593839de47098212f0ae5f15b6263099 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 17 Nov 2014 15:06:35 +0100 Subject: close leaking slave fd after setting up pty magic The fd moves out of scope here anyway, so we should close it properly instead of leaking it which will tickle down to dpkg maintainer scripts. Closes: 767774 --- .../test-no-fds-leaked-to-maintainer-scripts | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100755 test/integration/test-no-fds-leaked-to-maintainer-scripts (limited to 'test/integration/test-no-fds-leaked-to-maintainer-scripts') diff --git a/test/integration/test-no-fds-leaked-to-maintainer-scripts b/test/integration/test-no-fds-leaked-to-maintainer-scripts new file mode 100755 index 000000000..6ed120090 --- /dev/null +++ b/test/integration/test-no-fds-leaked-to-maintainer-scripts @@ -0,0 +1,40 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'native' +configdpkgnoopchroot + +setupsimplenativepackage "fdleaks" 'native' '1.0' 'unstable' +BUILDDIR="incoming/fdleaks-1.0" +for script in 'preinst' 'postinst' 'prerm' 'postrm'; do + echo '#!/bin/sh +ls -l /proc/self/fd/' > ${BUILDDIR}/debian/$script +done +buildpackage "$BUILDDIR" 'unstable' 'main' 'native' +rm -rf "$BUILDDIR" + +setupaptarchive + +testsuccess aptget install -y fdleaks +msgtest 'Check if fds were not' 'leaked' +if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '8' ]; then + msgpass +else + echo + cat rootdir/tmp/testsuccess.output + msgfail +fi + +testsuccess aptget purge -y fdleaks +msgtest 'Check if fds were not' 'leaked' +if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '12' ]; then + msgpass +else + echo + cat rootdir/tmp/testsuccess.output + msgfail +fi -- cgit v1.2.3-70-g09d2 From 299aea924ccef428219ed6f1a026c122678429e6 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 18 Nov 2014 00:59:39 +0100 Subject: fix PTY interaction on linux and kfreebsd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We run dpkg on its own pty, so we can log its output and have our own output around it (like the progress bar), while also allowing debconf and configfile prompts to happen. In commit 223ae57d468fdcac451209a095047a07a5698212 we changed to constantly reopening the slave for kfreebsd. This has the sideeffect though that in some cases slave and master will lose their connection on linux, so that no output is passed along anymore. We fix this by having always an fd referencing the slave open (linux), but we don't use it (kfreebsd). Failing to get our PTY up and running has many (bad) consequences including (not limited to, nor all at ones or in any case) garbled ouput, no output, no logging, a (partial) mixture of the previous items, … This commit is therefore also reshuffling quiet a bit of the creation code to get especially the output part up and running on linux and the logging for kfreebsd. Note that the testcase tries to cover some cases, but this is an interactivity issue so only interactive usage can really be a good test. Closes: 765687 --- apt-pkg/deb/dpkgpm.cc | 99 ++++++++++++---------- .../test-no-fds-leaked-to-maintainer-scripts | 40 ++++++++- 2 files changed, 92 insertions(+), 47 deletions(-) (limited to 'test/integration/test-no-fds-leaked-to-maintainer-scripts') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index e1f1ec680..e36a52c3e 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -72,7 +72,8 @@ class pkgDPkgPMPrivate public: pkgDPkgPMPrivate() : stdin_is_dev_null(false), dpkgbuf_pos(0), term_out(NULL), history_out(NULL), - progress(NULL), master(-1), slave(NULL) + progress(NULL), tt_is_valid(false), master(-1), + slave(NULL), protect_slave_from_dying(-1) { dpkgbuf[0] = '\0'; } @@ -90,8 +91,10 @@ public: // pty stuff struct termios tt; + bool tt_is_valid; int master; char * slave; + int protect_slave_from_dying; // signals sigset_t sigmask; @@ -1071,47 +1074,40 @@ void pkgDPkgPM::StartPtyMagic() } _error->PushToStack(); - // if tcgetattr for both stdin/stdout returns 0 (no error) - // we do the pty magic - if (tcgetattr(STDOUT_FILENO, &d->tt) == 0 && - tcgetattr(STDIN_FILENO, &d->tt) == 0) + + d->master = posix_openpt(O_RDWR | O_NOCTTY); + if (d->master == -1) + _error->Errno("posix_openpt", _("Can not write log (%s)"), _("Is /dev/pts mounted?")); + else if (unlockpt(d->master) == -1) + _error->Errno("unlockpt", "Unlocking the slave of master fd %d failed!", d->master); + else { - d->master = posix_openpt(O_RDWR | O_NOCTTY); - if (d->master == -1) - _error->Errno("posix_openpt", _("Can not write log (%s)"), _("Is /dev/pts mounted?")); - else if (unlockpt(d->master) == -1) - { - _error->Errno("unlockpt", "Unlocking the slave of master fd %d failed!", d->master); - close(d->master); - d->master = -1; - } + char const * const slave_name = ptsname(d->master); + if (slave_name == NULL) + _error->Errno("ptsname", "Getting name for slave of master fd %d failed!", d->master); else { - char const * const slave_name = ptsname(d->master); - if (slave_name == NULL) + d->slave = strdup(slave_name); + if (d->slave == NULL) + _error->Errno("strdup", "Copying name %s for slave of master fd %d failed!", slave_name, d->master); + else if (grantpt(d->master) == -1) + _error->Errno("grantpt", "Granting access to slave %s based on master fd %d failed!", slave_name, d->master); + else if (tcgetattr(STDIN_FILENO, &d->tt) == 0) { - _error->Errno("unlockpt", "Getting name for slave of master fd %d failed!", d->master); - close(d->master); - d->master = -1; - } - else - { - d->slave = strdup(slave_name); - if (d->slave == NULL) + d->tt_is_valid = true; + struct termios raw_tt; + // copy window size of stdout if its a 'good' terminal + if (tcgetattr(STDOUT_FILENO, &raw_tt) == 0) { - _error->Errno("strdup", "Copying name %s for slave of master fd %d failed!", slave_name, d->master); - close(d->master); - d->master = -1; + struct winsize win; + if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &win) < 0) + _error->Errno("ioctl", "Getting TIOCGWINSZ from stdout failed!"); + if (ioctl(d->master, TIOCSWINSZ, &win) < 0) + _error->Errno("ioctl", "Setting TIOCSWINSZ for master fd %d failed!", d->master); } - struct winsize win; - if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &win) < 0) - _error->Errno("ioctl", "Getting TIOCGWINSZ from stdout failed!"); - if (ioctl(d->master, TIOCSWINSZ, &win) < 0) - _error->Errno("ioctl", "Setting TIOCSWINSZ for master fd %d failed!", d->master); if (tcsetattr(d->master, TCSANOW, &d->tt) == -1) _error->Errno("tcsetattr", "Setting in Start via TCSANOW for master fd %d failed!", d->master); - struct termios raw_tt; raw_tt = d->tt; cfmakeraw(&raw_tt); raw_tt.c_lflag &= ~ECHO; @@ -1123,18 +1119,22 @@ void pkgDPkgPM::StartPtyMagic() sigaddset(&d->sigmask, SIGTTOU); sigprocmask(SIG_BLOCK,&d->sigmask, &d->original_sigmask); if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &raw_tt) == -1) - _error->Errno("tcsetattr", "Setting in Start via TCSAFLUSH for stdout failed!"); + _error->Errno("tcsetattr", "Setting in Start via TCSAFLUSH for stdin failed!"); sigprocmask(SIG_SETMASK, &d->original_sigmask, NULL); + + } + if (d->slave != NULL) + { + /* on linux, closing (and later reopening) all references to the slave + makes the slave a death end, so we open it here to have one open all + the time. We could use this fd in SetupSlavePtyMagic() for linux, but + on kfreebsd we get an incorrect ("step like") output then while it has + no problem with closing all references… so to avoid platform specific + code here we combine both and be happy once more */ + d->protect_slave_from_dying = open(d->slave, O_RDWR | O_CLOEXEC); } } } - else - { - // complain only if stdout is either a terminal (but still failed) or is an invalid - // descriptor otherwise we would complain about redirection to e.g. /dev/null as well. - if (isatty(STDOUT_FILENO) == 1 || errno == EBADF) - _error->Errno("tcgetattr", _("Can not write log (%s)"), _("Is stdout a terminal?")); - } if (_error->PendingError() == true) { @@ -1143,17 +1143,23 @@ void pkgDPkgPM::StartPtyMagic() close(d->master); d->master = -1; } + if (d->slave != NULL) + { + free(d->slave); + d->slave = NULL; + } _error->DumpErrors(std::cerr); } _error->RevertToStack(); } void pkgDPkgPM::SetupSlavePtyMagic() { - if(d->master == -1) + if(d->master == -1 || d->slave == NULL) return; if (close(d->master) == -1) _error->FatalE("close", "Closing master %d in child failed!", d->master); + d->master = -1; if (setsid() == -1) _error->FatalE("setsid", "Starting a new session for child failed!"); @@ -1168,7 +1174,7 @@ void pkgDPkgPM::SetupSlavePtyMagic() if (dup2(slaveFd, i) == -1) _error->FatalE("dup2", "Dupping %d to %d in child failed!", slaveFd, i); - if (tcsetattr(0, TCSANOW, &d->tt) < 0) + if (d->tt_is_valid == true && tcsetattr(STDIN_FILENO, TCSANOW, &d->tt) < 0) _error->FatalE("tcsetattr", "Setting in Setup via TCSANOW for slave fd %d failed!", slaveFd); } @@ -1180,9 +1186,14 @@ void pkgDPkgPM::StopPtyMagic() if (d->slave != NULL) free(d->slave); d->slave = NULL; + if (d->protect_slave_from_dying != -1) + { + close(d->protect_slave_from_dying); + d->protect_slave_from_dying = -1; + } if(d->master >= 0) { - if (tcsetattr(0, TCSAFLUSH, &d->tt) == -1) + if (d->tt_is_valid == true && tcsetattr(STDIN_FILENO, TCSAFLUSH, &d->tt) == -1) _error->FatalE("tcsetattr", "Setting in Stop via TCSAFLUSH for stdin failed!"); close(d->master); d->master = -1; diff --git a/test/integration/test-no-fds-leaked-to-maintainer-scripts b/test/integration/test-no-fds-leaked-to-maintainer-scripts index 6ed120090..3c6457cab 100755 --- a/test/integration/test-no-fds-leaked-to-maintainer-scripts +++ b/test/integration/test-no-fds-leaked-to-maintainer-scripts @@ -8,7 +8,7 @@ setupenvironment configarchitecture 'native' configdpkgnoopchroot -setupsimplenativepackage "fdleaks" 'native' '1.0' 'unstable' +setupsimplenativepackage "fdleaks" 'all' '1.0' 'unstable' BUILDDIR="incoming/fdleaks-1.0" for script in 'preinst' 'postinst' 'prerm' 'postrm'; do echo '#!/bin/sh @@ -19,7 +19,8 @@ rm -rf "$BUILDDIR" setupaptarchive -testsuccess aptget install -y fdleaks +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +testsuccess aptget install -y fdleaks -qq < /dev/null msgtest 'Check if fds were not' 'leaked' if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '8' ]; then msgpass @@ -29,7 +30,23 @@ else msgfail fi -testsuccess aptget purge -y fdleaks +cp rootdir/tmp/testsuccess.output terminal.output +tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log +testfileequal 'terminal.log' "$(cat terminal.output)" + +testequal 'startup archives unpack +install fdleaks:all 1.0 +status half-installed fdleaks:all 1.0 +status unpacked fdleaks:all 1.0 +status unpacked fdleaks:all 1.0 +startup packages configure +configure fdleaks:all 1.0 +status unpacked fdleaks:all 1.0 +status half-configured fdleaks:all 1.0 +status installed fdleaks:all 1.0' cut -f 3- -d' ' rootdir/var/log/dpkg.log + +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +testsuccess aptget purge -y fdleaks -qq msgtest 'Check if fds were not' 'leaked' if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '12' ]; then msgpass @@ -38,3 +55,20 @@ else cat rootdir/tmp/testsuccess.output msgfail fi +cp rootdir/tmp/testsuccess.output terminal.output +tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log +testfileequal 'terminal.log' "$(cat terminal.output)" + +testequal 'startup packages purge +status installed fdleaks:all 1.0 +remove fdleaks:all 1.0 +status half-configured fdleaks:all 1.0 +status half-installed fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +purge fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +status config-files fdleaks:all 1.0 +status not-installed fdleaks:all ' cut -f 3- -d' ' rootdir/var/log/dpkg.log -- cgit v1.2.3-70-g09d2 From e18f6133b254db9e1dc7b202366b067b15a68123 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 10 Dec 2014 22:26:59 +0100 Subject: do not make PTY slave the controlling terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we have no controlling terminal opening a terminal will make this terminal our controller, which is a serious problem if this happens to be the pseudo terminal we created to run dpkg in as we will close this terminal at the end hanging ourself up in the process… The offending open is the one we do to have at least one slave fd open all the time, but for good measure, we apply the flag also to the slave fd opening in the child process as we set the controlling terminal explicitely here. This is a regression from 150bdc9ca5d656f9fba94d37c5f4f183b02bd746 with the slight twist that this usecase was silently broken before in that it wasn't logging the output in term.log (as a pseudo terminal wasn't created). Closes: 772641 --- apt-pkg/deb/dpkgpm.cc | 4 +- test/integration/framework | 2 +- .../test-no-fds-leaked-to-maintainer-scripts | 109 +++++++++++++-------- 3 files changed, 70 insertions(+), 45 deletions(-) (limited to 'test/integration/test-no-fds-leaked-to-maintainer-scripts') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index e36a52c3e..93a007d14 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -1131,7 +1131,7 @@ void pkgDPkgPM::StartPtyMagic() on kfreebsd we get an incorrect ("step like") output then while it has no problem with closing all references… so to avoid platform specific code here we combine both and be happy once more */ - d->protect_slave_from_dying = open(d->slave, O_RDWR | O_CLOEXEC); + d->protect_slave_from_dying = open(d->slave, O_RDWR | O_CLOEXEC | O_NOCTTY); } } } @@ -1163,7 +1163,7 @@ void pkgDPkgPM::SetupSlavePtyMagic() if (setsid() == -1) _error->FatalE("setsid", "Starting a new session for child failed!"); - int const slaveFd = open(d->slave, O_RDWR); + int const slaveFd = open(d->slave, O_RDWR | O_NOCTTY); if (slaveFd == -1) _error->FatalE("open", _("Can not write log (%s)"), _("Is /dev/pts mounted?")); else if (ioctl(slaveFd, TIOCSCTTY, 0) < 0) diff --git a/test/integration/framework b/test/integration/framework index ac482a7a0..9e183057f 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -102,7 +102,7 @@ runapt() { local CMD="$1" shift case $CMD in - sh|aptitude|*/*) ;; + sh|aptitude|*/*|command) ;; *) CMD="${BUILDDIRECTORY}/$CMD";; esac MALLOC_PERTURB_=21 MALLOC_CHECK_=2 APT_CONFIG="$(getaptconfig)" LD_LIBRARY_PATH=${BUILDDIRECTORY} $CMD "$@" diff --git a/test/integration/test-no-fds-leaked-to-maintainer-scripts b/test/integration/test-no-fds-leaked-to-maintainer-scripts index 3c6457cab..6eb033055 100755 --- a/test/integration/test-no-fds-leaked-to-maintainer-scripts +++ b/test/integration/test-no-fds-leaked-to-maintainer-scripts @@ -5,7 +5,7 @@ TESTDIR=$(readlink -f $(dirname $0)) . $TESTDIR/framework setupenvironment -configarchitecture 'native' +configarchitecture 'amd64' 'i386' configdpkgnoopchroot setupsimplenativepackage "fdleaks" 'all' '1.0' 'unstable' @@ -17,58 +17,83 @@ done buildpackage "$BUILDDIR" 'unstable' 'main' 'native' rm -rf "$BUILDDIR" +PKGNAME='fdleaks:all' +if ! dpkg-checkbuilddeps -d 'dpkg (>= 1.16.2)' /dev/null >/dev/null 2>&1; then + PKGNAME='fdleaks' +fi + setupaptarchive rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log testsuccess aptget install -y fdleaks -qq < /dev/null -msgtest 'Check if fds were not' 'leaked' -if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '8' ]; then - msgpass -else - echo - cat rootdir/tmp/testsuccess.output - msgfail -fi -cp rootdir/tmp/testsuccess.output terminal.output -tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log -testfileequal 'terminal.log' "$(cat terminal.output)" +checkfdleak() { + msgtest 'Check if fds were not' 'leaked' + if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = "$1" ]; then + msgpass + else + echo + cat rootdir/tmp/testsuccess.output + msgfail + fi +} +checkinstall() { + checkfdleak 8 + + cp rootdir/tmp/testsuccess.output terminal.output + tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log + testfileequal 'terminal.log' "$(cat terminal.output)" -testequal 'startup archives unpack -install fdleaks:all 1.0 -status half-installed fdleaks:all 1.0 -status unpacked fdleaks:all 1.0 -status unpacked fdleaks:all 1.0 + testequal "startup archives unpack +install $PKGNAME 1.0 +status half-installed $PKGNAME 1.0 +status unpacked $PKGNAME 1.0 +status unpacked $PKGNAME 1.0 startup packages configure -configure fdleaks:all 1.0 -status unpacked fdleaks:all 1.0 -status half-configured fdleaks:all 1.0 -status installed fdleaks:all 1.0' cut -f 3- -d' ' rootdir/var/log/dpkg.log +configure $PKGNAME 1.0 +status unpacked $PKGNAME 1.0 +status half-configured $PKGNAME 1.0 +status installed $PKGNAME 1.0" cut -f 3- -d' ' rootdir/var/log/dpkg.log +} +checkinstall rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log testsuccess aptget purge -y fdleaks -qq -msgtest 'Check if fds were not' 'leaked' -if [ "$(grep 'root root' rootdir/tmp/testsuccess.output | wc -l)" = '12' ]; then +checkpurge() { + checkfdleak 12 + + cp rootdir/tmp/testsuccess.output terminal.output + tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log + testfileequal 'terminal.log' "$(cat terminal.output)" + + testequal "startup packages purge +status installed $PKGNAME 1.0 +remove $PKGNAME 1.0 +status half-configured $PKGNAME 1.0 +status half-installed $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +purge $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +status config-files $PKGNAME 1.0 +status not-installed $PKGNAME " cut -f 3- -d' ' rootdir/var/log/dpkg.log +} +checkpurge + +msgtest 'setsid provided is new enough to support' '-w' +if dpkg-checkbuilddeps -d 'util-linux (>= 2.24.2-1)' /dev/null >/dev/null 2>&1; then msgpass else - echo - cat rootdir/tmp/testsuccess.output - msgfail + msgskip "$(command dpkg -l util-linux)" + exit fi -cp rootdir/tmp/testsuccess.output terminal.output -tail -n +3 rootdir/var/log/apt/term.log | head -n -1 > terminal.log -testfileequal 'terminal.log' "$(cat terminal.output)" -testequal 'startup packages purge -status installed fdleaks:all 1.0 -remove fdleaks:all 1.0 -status half-configured fdleaks:all 1.0 -status half-installed fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -purge fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -status config-files fdleaks:all 1.0 -status not-installed fdleaks:all ' cut -f 3- -d' ' rootdir/var/log/dpkg.log +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +testsuccess runapt command setsid -w "${BUILDDIRECTORY}/apt-get" install -y fdleaks -qq < /dev/null +checkinstall + +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +testsuccess runapt command setsid -w "${BUILDDIRECTORY}/apt-get" purge -y fdleaks -qq +checkpurge -- cgit v1.2.3-70-g09d2 From a2a75ff4516f7609f4c55b42270abb8d08943c60 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 18 Nov 2014 19:53:56 +0100 Subject: always run 'dpkg --configure -a' at the end of our dpkg callings dpkg checks now for dependencies before running triggers, so that packages can now end up in trigger states (especially those we are not touching at all with our calls) after apt is done running. The solution to this is trivial: Just tell dpkg to configure everything after we have (supposely) configured everything already. In the worst case this means dpkg will have to run a bunch of triggers, usually it will just do nothing though. The code to make this happen was already available, so we just flip a config option here to cause it to be run. This way we can keep pretending that triggers are an implementation detail of dpkg. --triggers-only would supposely work as well, but --configure is more robust in regards to future changes to dpkg and something we will hopefully make use of in future versions anyway (as it was planed at the time this and related options were implemented). Note that dpkg currently has a workaround implemented to allow upgrades to jessie to be clean, so that the test works before and after. Also note that test (compared to the one in the bug) drops the await test as its is considered a loop by dpkg now. Closes: 769609 --- apt-pkg/deb/dpkgpm.cc | 9 ++- test/integration/framework | 25 ++++---- test/integration/test-apt-progress-fd | 67 ++++++++++--------- test/integration/test-apt-progress-fd-deb822 | 18 ++++-- test/integration/test-apt-progress-fd-error | 2 +- ...est-bug-769609-triggers-still-pending-after-run | 75 ++++++++++++++++++++++ .../test-no-fds-leaked-to-maintainer-scripts | 6 +- 7 files changed, 146 insertions(+), 56 deletions(-) create mode 100755 test/integration/test-bug-769609-triggers-still-pending-after-run (limited to 'test/integration/test-no-fds-leaked-to-maintainer-scripts') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 93a007d14..d54b7b50f 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -1047,6 +1047,12 @@ void pkgDPkgPM::BuildPackagesProgressMap() PackagesTotal++; } } + /* one extra: We don't want the progress bar to reach 100%, especially not + if we call dpkg --configure --pending and process a bunch of triggers + while showing 100%. Also, spindown takes a while, so never reaching 100% + is way more correct than reaching 100% while still doing stuff even if + doing it this way is slightly bending the rules */ + ++PackagesTotal; } /*}}}*/ #if (APT_PKG_MAJOR >= 4 && APT_PKG_MINOR < 13) @@ -1280,9 +1286,8 @@ bool pkgDPkgPM::GoNoABIBreak(APT::Progress::PackageManager *progress) // support subpressing of triggers processing for special // cases like d-i that runs the triggers handling manually - bool const SmartConf = (_config->Find("PackageManager::Configure", "all") != "all"); bool const TriggersPending = _config->FindB("DPkg::TriggersPending", false); - if (_config->FindB("DPkg::ConfigurePending", SmartConf) == true) + if (_config->FindB("DPkg::ConfigurePending", true) == true) List.push_back(Item(Item::ConfigurePending, PkgIterator())); // for the progress diff --git a/test/integration/framework b/test/integration/framework index 9e183057f..c9445065b 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1178,10 +1178,13 @@ testnopackage() { fi } -testdpkginstalled() { - msgtest "Test for correctly installed package(s) with" "dpkg -l $*" - local PKGS="$(dpkg -l "$@" 2>/dev/null | grep '^i' | wc -l)" - if [ "$PKGS" != $# ]; then +testdpkgstatus() { + local STATE="$1" + local NR="$2" + shift 2 + msgtest "Test that $NR package(s) are in state $STATE with" "dpkg -l $*" + local PKGS="$(dpkg -l "$@" 2>/dev/null | grep "^${STATE}" | wc -l)" + if [ "$PKGS" != $NR ]; then echo >&2 $PKGS dpkg -l "$@" | grep '^[a-z]' >&2 msgfail @@ -1190,16 +1193,12 @@ testdpkginstalled() { fi } +testdpkginstalled() { + testdpkgstatus 'ii' "$#" "$@" +} + testdpkgnotinstalled() { - msgtest "Test for correctly not-installed package(s) with" "dpkg -l $*" - local PKGS="$(dpkg -l "$@" 2> /dev/null | grep '^i' | wc -l)" - if [ "$PKGS" != 0 ]; then - echo - dpkg -l "$@" | grep '^[a-z]' >&2 - msgfail - else - msgpass - fi + testdpkgstatus 'ii' '0' "$@" } testmarkedauto() { diff --git a/test/integration/test-apt-progress-fd b/test/integration/test-apt-progress-fd index d72e7e72d..68cc0439c 100755 --- a/test/integration/test-apt-progress-fd +++ b/test/integration/test-apt-progress-fd @@ -19,13 +19,14 @@ testequal "dlstatus:1:0:Retrieving file 1 of 1 dlstatus:1:0:Retrieving file 1 of 1 pmstatus:dpkg-exec:0:Running dpkg pmstatus:testing:0:Installing testing (amd64) -pmstatus:testing:20:Preparing testing (amd64) -pmstatus:testing:40:Unpacking testing (amd64) -pmstatus:testing:60:Preparing to configure testing (amd64) -pmstatus:dpkg-exec:60:Running dpkg -pmstatus:testing:60:Configuring testing (amd64) -pmstatus:testing:80:Configuring testing (amd64) -pmstatus:testing:100:Installed testing (amd64)" cat apt-progress.log +pmstatus:testing:16.6667:Preparing testing (amd64) +pmstatus:testing:33.3333:Unpacking testing (amd64) +pmstatus:testing:50:Preparing to configure testing (amd64) +pmstatus:dpkg-exec:50:Running dpkg +pmstatus:testing:50:Configuring testing (amd64) +pmstatus:testing:66.6667:Configuring testing (amd64) +pmstatus:testing:83.3333:Installed testing (amd64) +pmstatus:dpkg-exec:83.3333:Running dpkg" cat apt-progress.log # upgrade exec 3> apt-progress.log @@ -34,13 +35,14 @@ testequal "dlstatus:1:0:Retrieving file 1 of 1 dlstatus:1:0:Retrieving file 1 of 1 pmstatus:dpkg-exec:0:Running dpkg pmstatus:testing:0:Installing testing (amd64) -pmstatus:testing:20:Preparing testing (amd64) -pmstatus:testing:40:Unpacking testing (amd64) -pmstatus:testing:60:Preparing to configure testing (amd64) -pmstatus:dpkg-exec:60:Running dpkg -pmstatus:testing:60:Configuring testing (amd64) -pmstatus:testing:80:Configuring testing (amd64) -pmstatus:testing:100:Installed testing (amd64)" cat apt-progress.log +pmstatus:testing:16.6667:Preparing testing (amd64) +pmstatus:testing:33.3333:Unpacking testing (amd64) +pmstatus:testing:50:Preparing to configure testing (amd64) +pmstatus:dpkg-exec:50:Running dpkg +pmstatus:testing:50:Configuring testing (amd64) +pmstatus:testing:66.6667:Configuring testing (amd64) +pmstatus:testing:83.3333:Installed testing (amd64) +pmstatus:dpkg-exec:83.3333:Running dpkg" cat apt-progress.log # reinstall exec 3> apt-progress.log @@ -49,22 +51,24 @@ testequal "dlstatus:1:0:Retrieving file 1 of 1 dlstatus:1:0:Retrieving file 1 of 1 pmstatus:dpkg-exec:0:Running dpkg pmstatus:testing:0:Installing testing (amd64) -pmstatus:testing:20:Preparing testing (amd64) -pmstatus:testing:40:Unpacking testing (amd64) -pmstatus:testing:60:Preparing to configure testing (amd64) -pmstatus:dpkg-exec:60:Running dpkg -pmstatus:testing:60:Configuring testing (amd64) -pmstatus:testing:80:Configuring testing (amd64) -pmstatus:testing:100:Installed testing (amd64)" cat apt-progress.log +pmstatus:testing:16.6667:Preparing testing (amd64) +pmstatus:testing:33.3333:Unpacking testing (amd64) +pmstatus:testing:50:Preparing to configure testing (amd64) +pmstatus:dpkg-exec:50:Running dpkg +pmstatus:testing:50:Configuring testing (amd64) +pmstatus:testing:66.6667:Configuring testing (amd64) +pmstatus:testing:83.3333:Installed testing (amd64) +pmstatus:dpkg-exec:83.3333:Running dpkg" cat apt-progress.log # and remove exec 3> apt-progress.log testsuccess aptget remove testing -y -o APT::Status-Fd=3 testequal "pmstatus:dpkg-exec:0:Running dpkg pmstatus:testing:0:Removing testing (amd64) -pmstatus:testing:33.3333:Preparing for removal of testing (amd64) -pmstatus:testing:66.6667:Removing testing (amd64) -pmstatus:testing:100:Removed testing (amd64)" cat apt-progress.log +pmstatus:testing:25:Preparing for removal of testing (amd64) +pmstatus:testing:50:Removing testing (amd64) +pmstatus:testing:75:Removed testing (amd64) +pmstatus:dpkg-exec:75:Running dpkg" cat apt-progress.log # install non-native and ensure we get proper progress info exec 3> apt-progress.log @@ -75,12 +79,13 @@ testequal "dlstatus:1:0:Retrieving file 1 of 1 dlstatus:1:0:Retrieving file 1 of 1 pmstatus:dpkg-exec:0:Running dpkg pmstatus:testing2:0:Installing testing2 (i386) -pmstatus:testing2:20:Preparing testing2 (i386) -pmstatus:testing2:40:Unpacking testing2 (i386) -pmstatus:testing2:60:Preparing to configure testing2 (i386) -pmstatus:dpkg-exec:60:Running dpkg -pmstatus:testing2:60:Configuring testing2 (i386) -pmstatus:testing2:80:Configuring testing2 (i386) -pmstatus:testing2:100:Installed testing2 (i386)" cat apt-progress.log +pmstatus:testing2:16.6667:Preparing testing2 (i386) +pmstatus:testing2:33.3333:Unpacking testing2 (i386) +pmstatus:testing2:50:Preparing to configure testing2 (i386) +pmstatus:dpkg-exec:50:Running dpkg +pmstatus:testing2:50:Configuring testing2 (i386) +pmstatus:testing2:66.6667:Configuring testing2 (i386) +pmstatus:testing2:83.3333:Installed testing2 (i386) +pmstatus:dpkg-exec:83.3333:Running dpkg" cat apt-progress.log rm -f apt-progress*.log diff --git a/test/integration/test-apt-progress-fd-deb822 b/test/integration/test-apt-progress-fd-deb822 index 9d227942d..badc985e4 100755 --- a/test/integration/test-apt-progress-fd-deb822 +++ b/test/integration/test-apt-progress-fd-deb822 @@ -27,37 +27,41 @@ Message: Installing testing (amd64) Status: progress Package: testing:amd64 -Percent: 20 +Percent: 16.6667 Message: Preparing testing (amd64) Status: progress Package: testing:amd64 -Percent: 40 +Percent: 33.3333 Message: Unpacking testing (amd64) Status: progress Package: testing:amd64 -Percent: 60 +Percent: 50 Message: Preparing to configure testing (amd64) Status: progress -Percent: 60 +Percent: 50 Message: Running dpkg Status: progress Package: testing:amd64 -Percent: 60 +Percent: 50 Message: Configuring testing (amd64) Status: progress Package: testing:amd64 -Percent: 80 +Percent: 66.6667 Message: Configuring testing (amd64) Status: progress Package: testing:amd64 -Percent: 100 +Percent: 83.3333 Message: Installed testing (amd64) + +Status: progress +Percent: 83.3333 +Message: Running dpkg " cat apt-progress.log diff --git a/test/integration/test-apt-progress-fd-error b/test/integration/test-apt-progress-fd-error index a47095b9b..632300765 100755 --- a/test/integration/test-apt-progress-fd-error +++ b/test/integration/test-apt-progress-fd-error @@ -18,7 +18,7 @@ setupaptarchive exec 3> apt-progress.log testfailure aptget install foo1 foo2 -y -o APT::Status-Fd=3 msgtest "Ensure correct error message" -if grep -q "aptarchive/pool/foo2_0.8.15_amd64.deb:40:trying to overwrite '/usr/bin/file-conflict', which is also in package foo1 0.8.15" apt-progress.log; then +if grep -q "aptarchive/pool/foo2_0.8.15_amd64.deb:36.3636:trying to overwrite '/usr/bin/file-conflict', which is also in package foo1 0.8.15" apt-progress.log; then msgpass else cat apt-progress.log diff --git a/test/integration/test-bug-769609-triggers-still-pending-after-run b/test/integration/test-bug-769609-triggers-still-pending-after-run new file mode 100755 index 000000000..146fa766b --- /dev/null +++ b/test/integration/test-bug-769609-triggers-still-pending-after-run @@ -0,0 +1,75 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'amd64' + +msgtest 'Check if installed dpkg supports' 'noawait trigger' +if dpkg-checkbuilddeps -d 'dpkg (>= 1.16.1)' /dev/null; then + msgpass +else + msgskip 'dpkg version too old' + exit 0 +fi +configdpkgnoopchroot + +buildtriggerpackages() { + local TYPE="$1" + setupsimplenativepackage "triggerable-$TYPE" 'all' '1.0' 'unstable' "Depends: trigdepends-$TYPE" + BUILDDIR="incoming/triggerable-${TYPE}-1.0" + cat >${BUILDDIR}/debian/postinst < ${BUILDDIR}/debian/triggers + buildpackage "$BUILDDIR" 'unstable' 'main' 'native' + rm -rf "$BUILDDIR" + buildsimplenativepackage "trigdepends-$TYPE" 'all' '1.0' 'unstable' +} + +#buildtriggerpackages 'interest' +buildtriggerpackages 'interest-noawait' +buildsimplenativepackage "trigstuff" 'all' '1.0' 'unstable' + +setupaptarchive + +runtests() { + local TYPE="$1" + msgmsg 'Working with trigger type' "$TYPE" + testsuccess aptget install triggerable-$TYPE -y + cp rootdir/tmp/testsuccess.output terminal.output + testsuccess grep '^REWRITE ' terminal.output + testdpkginstalled triggerable-$TYPE trigdepends-$TYPE + + testsuccess aptget install trigdepends-$TYPE -y --reinstall + cp rootdir/tmp/testsuccess.output terminal.output + testsuccess grep '^REWRITE ' terminal.output + testsuccess grep ' root root ' terminal.output + testdpkginstalled triggerable-$TYPE trigdepends-$TYPE + + testsuccess aptget install trigstuff -y + cp rootdir/tmp/testsuccess.output terminal.output + testsuccess grep '^REWRITE ' terminal.output + testsuccess grep ' root root ' terminal.output + testdpkginstalled triggerable-$TYPE trigdepends-$TYPE trigstuff + + testsuccess aptget purge trigstuff -y + cp rootdir/tmp/testsuccess.output terminal.output + testsuccess grep '^REWRITE ' terminal.output + testsuccess grep ' root root ' terminal.output + testdpkginstalled triggerable-$TYPE trigdepends-$TYPE + testdpkgnotinstalled trigstuff + + testsuccess aptget purge trigdepends-$TYPE -y + cp rootdir/tmp/testsuccess.output terminal.output + testfailure grep '^REWRITE ' terminal.output + testfailure grep ' root root ' terminal.output + testdpkgnotinstalled triggerable-$TYPE trigdepends-$TYPE +} +#runtests 'interest' +runtests 'interest-noawait' diff --git a/test/integration/test-no-fds-leaked-to-maintainer-scripts b/test/integration/test-no-fds-leaked-to-maintainer-scripts index 6eb033055..7d0c1c6c1 100755 --- a/test/integration/test-no-fds-leaked-to-maintainer-scripts +++ b/test/integration/test-no-fds-leaked-to-maintainer-scripts @@ -53,7 +53,8 @@ startup packages configure configure $PKGNAME 1.0 status unpacked $PKGNAME 1.0 status half-configured $PKGNAME 1.0 -status installed $PKGNAME 1.0" cut -f 3- -d' ' rootdir/var/log/dpkg.log +status installed $PKGNAME 1.0 +startup packages configure" cut -f 3- -d' ' rootdir/var/log/dpkg.log } checkinstall @@ -78,7 +79,8 @@ status config-files $PKGNAME 1.0 status config-files $PKGNAME 1.0 status config-files $PKGNAME 1.0 status config-files $PKGNAME 1.0 -status not-installed $PKGNAME " cut -f 3- -d' ' rootdir/var/log/dpkg.log +status not-installed $PKGNAME +startup packages configure" cut -f 3- -d' ' rootdir/var/log/dpkg.log } checkpurge -- cgit v1.2.3-70-g09d2 From 748a2177dcf8ff72bca90f5c7d516559ddd67352 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 22 Dec 2014 23:14:08 +0100 Subject: pass-through stdin fd instead of content if not a terminal Commit 299aea924ccef428219ed6f1a026c122678429e6 fixes the problem of not logging terminal in case stdin & stdout are not a terminal. The problem is that we are then trying to pass-through stdin content by reading from the apt-process stdin and writing it to the stdin of the child (dpkg), which works great for users who can control themselves, but pipes and co are a bit less forgiving causing us to pass everything to the first child process, which if the sending part of the pipe is e.g. 'yes' we will never see the end of it (as the pipe is full at some point and further writing blocks). There is a simple solution for that of course: If stdin isn't a terminal, we us the apt-process stdin as stdin for the child directly (We don't do this if it is a terminal to be able to save the typed input in the log). Closes: 773061 --- apt-pkg/deb/dpkgpm.cc | 16 ++++++++++++---- .../test-no-fds-leaked-to-maintainer-scripts | 22 ++++++++++++++++++++-- 2 files changed, 32 insertions(+), 6 deletions(-) (limited to 'test/integration/test-no-fds-leaked-to-maintainer-scripts') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index d54b7b50f..e23ca466d 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -73,7 +73,8 @@ public: pkgDPkgPMPrivate() : stdin_is_dev_null(false), dpkgbuf_pos(0), term_out(NULL), history_out(NULL), progress(NULL), tt_is_valid(false), master(-1), - slave(NULL), protect_slave_from_dying(-1) + slave(NULL), protect_slave_from_dying(-1), + direct_stdin(false) { dpkgbuf[0] = '\0'; } @@ -100,6 +101,7 @@ public: sigset_t sigmask; sigset_t original_sigmask; + bool direct_stdin; }; namespace @@ -1079,6 +1081,9 @@ void pkgDPkgPM::StartPtyMagic() return; } + if (isatty(STDIN_FILENO) == 0) + d->direct_stdin = true; + _error->PushToStack(); d->master = posix_openpt(O_RDWR | O_NOCTTY); @@ -1176,7 +1181,10 @@ void pkgDPkgPM::SetupSlavePtyMagic() _error->FatalE("ioctl", "Setting TIOCSCTTY for slave fd %d failed!", slaveFd); else { - for (unsigned short i = 0; i < 3; ++i) + unsigned short i = 0; + if (d->direct_stdin == true) + ++i; + for (; i < 3; ++i) if (dup2(slaveFd, i) == -1) _error->FatalE("dup2", "Dupping %d to %d in child failed!", slaveFd, i); @@ -1596,8 +1604,8 @@ bool pkgDPkgPM::GoNoABIBreak(APT::Progress::PackageManager *progress) // wait for input or output here FD_ZERO(&rfds); - if (d->master >= 0 && !d->stdin_is_dev_null) - FD_SET(0, &rfds); + if (d->master >= 0 && d->direct_stdin == false && d->stdin_is_dev_null == false) + FD_SET(STDIN_FILENO, &rfds); FD_SET(_dpkgin, &rfds); if(d->master >= 0) FD_SET(d->master, &rfds); diff --git a/test/integration/test-no-fds-leaked-to-maintainer-scripts b/test/integration/test-no-fds-leaked-to-maintainer-scripts index 7d0c1c6c1..41c057042 100755 --- a/test/integration/test-no-fds-leaked-to-maintainer-scripts +++ b/test/integration/test-no-fds-leaked-to-maintainer-scripts @@ -11,8 +11,14 @@ configdpkgnoopchroot setupsimplenativepackage "fdleaks" 'all' '1.0' 'unstable' BUILDDIR="incoming/fdleaks-1.0" for script in 'preinst' 'postinst' 'prerm' 'postrm'; do - echo '#!/bin/sh -ls -l /proc/self/fd/' > ${BUILDDIR}/debian/$script + cat > ${BUILDDIR}/debian/$script << EOF +#!/bin/sh +if [ -e "$(pwd)/rootdir/tmp/read_stdin" ]; then + read line; + echo "STDIN: -\$line-" +fi +ls -l /proc/self/fd/ +EOF done buildpackage "$BUILDDIR" 'unstable' 'main' 'native' rm -rf "$BUILDDIR" @@ -99,3 +105,15 @@ checkinstall rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log testsuccess runapt command setsid -w "${BUILDDIRECTORY}/apt-get" purge -y fdleaks -qq checkpurge + +touch rootdir/tmp/read_stdin + +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +for i in $(seq 1 10); do echo "$i"; done | testsuccess aptget install -y fdleaks -qq +checkinstall +testequal '2' grep -c '^STDIN: ' rootdir/var/log/apt/term.log + +rm -f rootdir/var/log/dpkg.log rootdir/var/log/apt/term.log +yes '' | testsuccess runapt command setsid -w "${BUILDDIRECTORY}/apt-get" purge -y fdleaks -qq +checkpurge +testequal '3' grep -c '^STDIN: ' rootdir/var/log/apt/term.log -- cgit v1.2.3-70-g09d2