diff options
| author | Julian Andres Klode <jak@debian.org> | 2024-12-16 17:05:40 +0100 |
|---|---|---|
| committer | Julian Andres Klode <jak@debian.org> | 2024-12-18 10:25:24 +0100 |
| commit | 6e260e26e4cd671f781151e24d89a17b29a55530 (patch) | |
| tree | 0ae3b4f9122e5168df4d4d09102a88196d6baaf2 | |
| parent | 19000fab15ab0b30c95a1bf6e034aa3c6bdc36cb (diff) | |
Introduce automatic pager for read commands
Automatically show the output of `show`, `policy`, `list`,
`search`, `showsrc` in a pager.
The pager setup is inspired by git's pager setup. Notably,
the pager is found using APT_PAGER and PAGER variables.
We wait for the pager to be setup somewhat correctly by
using a notify pipe to figure out whether execvp() was
succesful - then the pipe will read EOF as the other end
got closed by CLOEXEC during exec - or not, then the pipe
will contain an errno.
We set up the correct handlers for signals and exit to close
the fds and wait for the pager. Notably inside the signal
handler we cannot flush our streams, only close them, so
there is some duplication.
We call the InitOutputPager() function from inside the
various Do...() functions rather than setting it up
generally in InitOutput(). Doing so allows us to first
render the progress without a pager, and then setup
the pager for the content only which improves user
experience.
When we setup a pager we also take care to disable
standard input, as we should not be prompting users
while a pager is running (the pager will be reading
from the tty directly). We do this by dup2-ing() a
/dev/null over it; if we just close()d the fd, another
open() might reuse the fd number and problems could
occur.
| -rw-r--r-- | CMake/config.h.in | 3 | ||||
| -rw-r--r-- | CMakeLists.txt | 3 | ||||
| -rw-r--r-- | apt-private/private-cmndline.cc | 1 | ||||
| -rw-r--r-- | apt-private/private-list.cc | 4 | ||||
| -rw-r--r-- | apt-private/private-main.cc | 5 | ||||
| -rw-r--r-- | apt-private/private-output.cc | 168 | ||||
| -rw-r--r-- | apt-private/private-output.h | 2 | ||||
| -rw-r--r-- | apt-private/private-search.cc | 3 | ||||
| -rw-r--r-- | apt-private/private-show.cc | 10 | ||||
| -rw-r--r-- | debian/tests/control | 2 | ||||
| -rw-r--r-- | doc/examples/configure-index | 1 | ||||
| -rw-r--r-- | test/integration/framework | 11 | ||||
| -rwxr-xr-x | test/integration/test-apt-cli-pager | 116 |
13 files changed, 322 insertions, 7 deletions
diff --git a/CMake/config.h.in b/CMake/config.h.in index 607f9d5ae..8346a5e9e 100644 --- a/CMake/config.h.in +++ b/CMake/config.h.in @@ -85,3 +85,6 @@ /* Whether to check for and require merged usr if a usrmerge package is available. */ #cmakedefine REQUIRE_MERGED_USR + +#cmakedefine DEFAULT_PAGER "${DEFAULT_PAGER}" +#cmakedefine PAGER_ENV "${PAGER_ENV}" diff --git a/CMakeLists.txt b/CMakeLists.txt index b304cc674..00f4a9bcf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -238,6 +238,9 @@ set(CONF_DIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/apt" CACHE PATH "Your /etc/apt") set(LIBEXEC_DIR "${CMAKE_INSTALL_FULL_LIBEXECDIR}/apt" CACHE PATH "Your /usr/libexec/apt") set(BIN_DIR "${CMAKE_INSTALL_FULL_BINDIR}") +# Setup the default pager and the environment +set(DEFAULT_PAGER "pager" CACHE STRING "The default pager to use for commands that support it.") +set(PAGER_ENV "LESS=FRX\\nMORE=FRX\\nLV=C" CACHE STRING "Environment to pass to the pager. One variable per line, with escaped newlines") # Configure our configuration headers (config.h and apti18n.h) configure_file(CMake/config.h.in ${PROJECT_BINARY_DIR}/include/config.h) diff --git a/apt-private/private-cmndline.cc b/apt-private/private-cmndline.cc index 5d4eeb5cb..cbf787aa9 100644 --- a/apt-private/private-cmndline.cc +++ b/apt-private/private-cmndline.cc @@ -509,6 +509,7 @@ static void BinarySpecificConfiguration(char const * const Binary) /*{{{*/ _config->CndSet("Binary::apt::APT::Keep-Downloaded-Packages", false); _config->CndSet("Binary::apt::APT::Get::Update::InteractiveReleaseInfoChanges", true); _config->CndSet("Binary::apt::APT::Cmd::Pattern-Only", true); + _config->CndSet("Binary::apt::Pager", true); if (isatty(STDIN_FILENO)) _config->CndSet("Binary::apt::Dpkg::Lock::Timeout", -1); diff --git a/apt-private/private-list.cc b/apt-private/private-list.cc index eee657c46..6536c8df7 100644 --- a/apt-private/private-list.cc +++ b/apt-private/private-list.cc @@ -131,6 +131,10 @@ bool DoList(CommandLine &Cmd) GetLocalitySortedVersionSet(CacheFile, &bag, matcher, &progress); bool const ShowAllVersions = _config->FindB("APT::Cmd::All-Versions", false); std::map<std::string, std::string> output_map; + + if (not InitOutputPager()) + return false; + for (LocalitySortedVersionSet::iterator V = bag.begin(); V != bag.end(); ++V) { std::stringstream outs; diff --git a/apt-private/private-main.cc b/apt-private/private-main.cc index 39ad07b36..7b4f05a50 100644 --- a/apt-private/private-main.cc +++ b/apt-private/private-main.cc @@ -6,6 +6,7 @@ #include <apt-pkg/strutl.h> #include <apt-private/private-main.h> +#include <apt-private/private-output.h> #include <iostream> #include <locale> @@ -78,8 +79,8 @@ void CheckIfCalledByScript(int argc, const char *argv[]) /*{{{*/ { if (unlikely(argc < 1)) return; - if(!isatty(STDOUT_FILENO) && - _config->FindB("Apt::Cmd::Disable-Script-Warning", false) == false) + if (not IsStdoutAtty() && + _config->FindB("Apt::Cmd::Disable-Script-Warning", false) == false) { // NOTE: CLI interface is redundant on the I/interface, this is // intentional to make it easier to read. diff --git a/apt-private/private-output.cc b/apt-private/private-output.cc index 79a97128c..e3f795cf3 100644 --- a/apt-private/private-output.cc +++ b/apt-private/private-output.cc @@ -6,6 +6,7 @@ #include <apt-pkg/configuration.h> #include <apt-pkg/depcache.h> #include <apt-pkg/error.h> +#include <apt-pkg/fileutl.h> #include <apt-pkg/pkgcache.h> #include <apt-pkg/pkgrecords.h> #include <apt-pkg/policy.h> @@ -18,11 +19,13 @@ #include <cstdio> #include <cstdlib> #include <cstring> +#include <fcntl.h> #include <iomanip> #include <iostream> #include <langinfo.h> #include <regex.h> #include <sys/ioctl.h> +#include <sys/wait.h> #include <termios.h> #include <unistd.h> @@ -41,6 +44,14 @@ std::ofstream devnull("/dev/null"); unsigned int ScreenWidth = 80 - 1; /* - 1 for the cursor */ +static pid_t Pager = -1; +static constexpr std::array<int, 5> PagerSignalsToHandle = { + SIGINT, + SIGHUP, + SIGTERM, + SIGQUIT, + SIGPIPE, +}; // SigWinch - Window size change signal handler /*{{{*/ // --------------------------------------------------------------------- @@ -56,9 +67,162 @@ static void SigWinch(int) #endif } /*}}}*/ + +bool IsStdoutAtty() +{ + static bool is = isatty(STDOUT_FILENO); + return is; +} + +static void WaitPager() +{ + if (Pager != -1) + { + c0out.flush(); + c1out.flush(); + c2out.flush(); + std::cerr.flush(); + std::cout.flush(); + fflush(stdout); + fflush(stderr); + close(STDOUT_FILENO); + close(STDERR_FILENO); + waitpid(Pager, nullptr, 0); + Pager = -1; + } +} + +static void SignalPager(int signo) +{ + if (Pager != -1) + { + // We can't flush from inside the signal handler. + close(STDOUT_FILENO); + close(STDERR_FILENO); + waitpid(Pager, nullptr, 0); + Pager = -1; + for (auto signalToHandle : PagerSignalsToHandle) + signal(signalToHandle, SIG_DFL); + raise(signo); + } +} + +static std::string DeterminePager() +{ + if (not IsStdoutAtty() || not _config->FindB("Pager", false)) + return ""; + if (auto pager = getenv("APT_PAGER"); pager) + return pager; + if (auto pager = getenv("PAGER"); pager) + return pager; + return DEFAULT_PAGER; +} + +bool InitOutputPager() +{ + // InitOutputPager() behaves like a singleton, do not run twice- + static bool alreadyRan = false; + if (alreadyRan) + return true; + alreadyRan = true; + + auto pager = DeterminePager(); + if (pager.empty() || pager == "cat") + return true; + + auto pagerEnv = VectorizeString(PAGER_ENV, '\n'); + + int pipefds[2] = {-1, -1}; + int notifyPipe[2] = {-1, -1}; + + DEFER([&] () { + close(pipefds[0]); + close(pipefds[1]); + close(notifyPipe[0]); + close(notifyPipe[1]); + }); + + if (pipe(pipefds) || pipe(notifyPipe)) + return _error->Errno("pipe", "Failed to setup pipe"); + + Pager = ExecFork(); + if (Pager < 0) + return _error->Errno("fork", "Failed to fork"); + if (Pager == 0) + { + // Child process + if (dup2(pipefds[0], STDIN_FILENO) == -1) + goto err; + if (dup2(STDOUT_FILENO, STDERR_FILENO) == -1) + goto err; + + for (int sig = 1; sig < NSIG; sig++) + signal(sig, SIG_DFL); + + for (auto &v: pagerEnv) + putenv(v.data()); + + { + // If our pager name contains a space we need to invoke it in a shell. Boooo! + char *cmd[] = {(char*)"/bin/sh", (char*)"-c", pager.data(), nullptr}; + if (std::none_of(pager.begin(), pager.end(), isspace_ascii)) + { + cmd[0] = pager.data(); + cmd[1] = nullptr; + } + execvp(cmd[0], cmd); + } + err: + int err = errno; + FileFd::Write(notifyPipe[1], &err, sizeof(err)); + _exit(128); + } + + // Parent process. + + // Figure out if we were able to execvp() the child successfully. To do so, + // read from the notifyPipe. If execvp() was successful, it was closed due + // to CLOEXEC. On failure, our child code writes an errno to it. + _error->PushToStack(); + close(notifyPipe[1]); // Need to close our write end so our read doesn't block + notifyPipe[1] = -1; + if (int err = 0; FileFd::Read(notifyPipe[0], &err, sizeof(err))) { + errno = err; + _error->WarningE("PagerSetup", "Could not execute pager"); + _error->MergeWithStack(); + return true; + } + _error->RevertToStack(); + + // When running a pager, we must not be reading from stdin/tty, so + // let's be safe and open /dev/null in its place. + if (int const nullfd = open("/dev/null", O_RDONLY); nullfd != -1) + { + dup2(nullfd, STDIN_FILENO); + close(nullfd); + } + + // Redirect the output(s) to the pager + if (dup2(pipefds[1], STDOUT_FILENO) == -1) + abort(); + if (isatty(STDERR_FILENO) && dup2(pipefds[1], STDERR_FILENO) == -1) + abort(); + + // From now on, we can't show any progress messages as we are outputting + // to the pager. + _config->CndSet("quiet::NoUpdate", "1"); + + // Setup signal handlers and exit handlers for the pager, such that + // we wait for it. + for (auto signalToHandle : PagerSignalsToHandle) + signal(signalToHandle, SignalPager); + atexit(WaitPager); + + return true; +} bool InitOutput(std::basic_streambuf<char> * const out) /*{{{*/ { - if (!isatty(STDOUT_FILENO) && _config->FindI("quiet", -1) == -1) + if (not IsStdoutAtty() && _config->FindI("quiet", -1) == -1) _config->Set("quiet","1"); c0out.rdbuf(out); @@ -89,7 +253,7 @@ bool InitOutput(std::basic_streambuf<char> * const out) /*{{{*/ SigWinch(0); } - if (isatty(STDOUT_FILENO) == 0 || not _config->FindB("APT::Color", true) || getenv("NO_COLOR") != nullptr || getenv("APT_NO_COLOR") != nullptr) + if (not IsStdoutAtty() || not _config->FindB("APT::Color", true) || getenv("NO_COLOR") != nullptr || getenv("APT_NO_COLOR") != nullptr) { _config->Set("APT::Color", false); _config->Set("APT::Color::Highlight", ""); diff --git a/apt-private/private-output.h b/apt-private/private-output.h index 8e1d47225..aaa9a8ed9 100644 --- a/apt-private/private-output.h +++ b/apt-private/private-output.h @@ -26,6 +26,8 @@ APT_PUBLIC extern std::ofstream devnull; APT_PUBLIC extern unsigned int ScreenWidth; APT_PUBLIC bool InitOutput(std::basic_streambuf<char> * const out = std::cout.rdbuf()); +APT_PUBLIC bool InitOutputPager() APT_MUSTCHECK; +APT_PUBLIC bool IsStdoutAtty(); void ListSingleVersion(pkgCacheFile &CacheFile, pkgRecords &records, pkgCache::VerIterator const &V, std::ostream &out, diff --git a/apt-private/private-search.cc b/apt-private/private-search.cc index 19a3bf0fa..d6654eb5e 100644 --- a/apt-private/private-search.cc +++ b/apt-private/private-search.cc @@ -169,6 +169,9 @@ static bool FullTextSearch(CommandLine &CmdL) /*{{{*/ APT_FREE_PATTERNS(); progress.Done(); + if (not InitOutputPager()) + return false; + // FIXME: SORT! and make sorting flexible (alphabetic, by pkg status) // output the sorted map std::map<std::string, std::string>::const_iterator K; diff --git a/apt-private/private-show.cc b/apt-private/private-show.cc index ceef6707e..58c5d92c9 100644 --- a/apt-private/private-show.cc +++ b/apt-private/private-show.cc @@ -351,6 +351,10 @@ bool ShowPackage(CommandLine &CmdL) /*{{{*/ int const ShowVersion = _config->FindI("APT::Cache::Show::Version", 1); pkgRecords Recs(CacheFile); + + if (not InitOutputPager()) + return false; + for (APT::VersionList::const_iterator Ver = verset.begin(); Ver != verset.end(); ++Ver) { pkgCache::VerFileIterator Vf; @@ -441,6 +445,9 @@ bool ShowSrcPackage(CommandLine &CmdL) /*{{{*/ if (_error->PendingError() == true) return false; + if (not InitOutputPager()) + return false; + bool found = false; // avoid showing identical records std::set<std::string> seen; @@ -496,6 +503,9 @@ bool Policy(CommandLine &CmdL) if (unlikely(Plcy == nullptr)) return false; + if (not InitOutputPager()) + return false; + // Print out all of the package files if (CmdL.FileList[1] == 0) { diff --git a/debian/tests/control b/debian/tests/control index c75f21bcf..e4bcad998 100644 --- a/debian/tests/control +++ b/debian/tests/control @@ -4,7 +4,7 @@ Depends: libapt-pkg-dev, pkg-config, g++ Tests: run-tests Restrictions: allow-stderr -Depends: @, @builddeps@, dpkg (>= 1.20.8), fakeroot, wget, stunnel4, lsof, db-util, +Depends: @, @builddeps@, dpkg (>= 1.20.8), expect, fakeroot, wget, stunnel4, lsof, db-util, gnupg (>= 2) | gnupg2, gnupg1 | gnupg (<< 2), gpgv (>= 2) | gpgv2, gpgv1 | gpgv (<< 2), gpgv-sq, diff --git a/doc/examples/configure-index b/doc/examples/configure-index index 9623514b8..15338c17d 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -696,6 +696,7 @@ acquire::*::by-hash "<STRING>"; // Unsorted options: Some of those are used only internally +pager "<BOOL>"; // true if a pager is to be used help "<BOOL>"; // true if the help message was requested via e.g. --help version "<BOOL>"; // true if the version number was requested via e.g. --version Binary "<STRING>"; // name of the program run like apt-get, apt-cache, … diff --git a/test/integration/framework b/test/integration/framework index 6d4d0b07f..a93168278 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -174,6 +174,13 @@ getaptconfig() { } runapt() { msgdebug "Executing: ${CCMD}$*${CDEBUG} " + local unbuffer= + unset NO_COLOR + if [ "$1" = "--unbuffer" ]; then + unbuffer="unbuffer" + export NO_COLOR=1 + shift + fi local CMD="$1" shift case "$CMD" in @@ -181,9 +188,9 @@ runapt() { *) CMD="${APTCMDLINEBINDIR}/$CMD";; esac if [ "$CMD" = 'aptitude' ]; then - MALLOC_PERTURB_=21 MALLOC_CHECK_=2 APT_CONFIG="$(getaptconfig)" LD_LIBRARY_PATH="${LIBRARYPATH}:${LD_LIBRARY_PATH}" command "$CMD" "$@" + MALLOC_PERTURB_=21 MALLOC_CHECK_=2 APT_CONFIG="$(getaptconfig)" LD_LIBRARY_PATH="${LIBRARYPATH}:${LD_LIBRARY_PATH}" command $unbuffer "$CMD" "$@" else - MALLOC_PERTURB_=21 MALLOC_CHECK_=2 APT_CONFIG="$(getaptconfig)" LD_LIBRARY_PATH="${LIBRARYPATH}:${LD_LIBRARY_PATH}" "$CMD" "$@" + MALLOC_PERTURB_=21 MALLOC_CHECK_=2 APT_CONFIG="$(getaptconfig)" LD_LIBRARY_PATH="${LIBRARYPATH}:${LD_LIBRARY_PATH}" $unbuffer "$CMD" "$@" fi } runpython3() { runapt command python3 "$@"; } diff --git a/test/integration/test-apt-cli-pager b/test/integration/test-apt-cli-pager new file mode 100755 index 000000000..6d0f88f54 --- /dev/null +++ b/test/integration/test-apt-cli-pager @@ -0,0 +1,116 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment +configarchitecture 'amd64' +DESCR='Some description + That has multiple lines' +insertsource 'unstable' 'foo' 'all' '1.0' +insertpackage 'unstable' 'foo' 'all' '1.0' '' '' "$DESCR" +insertpackage 'unstable' 'multi' 'all' '1.0' '' '' "$DESCR" +insertpackage 'unstable' 'multi' 'all' '2.0' '' '' "$DESCR" + +setupaptarchive + +APTARCHIVE=$(readlink -f ./aptarchive) + +for show in info show; do +msgmsg "$show supports pager" +PAGER=cat testsuccessequal "Package: multi +Version: 2.0 +Priority: optional +Section: other +Maintainer: Joe Sixpack <joe@example.org> +Installed-Size: 43.0 kB +Download-Size: 42 B +APT-Sources: file:$APTARCHIVE unstable/main all Packages +Description: Some description + That has multiple lines + +N: There is 1 additional record. Please use the '-a' switch to see it" runapt --unbuffer apt $show multi -o TestPager=cat + +PAGER="head -3" testsuccessequal "Package: multi +Version: 2.0 +Priority: optional" runapt --unbuffer apt $show multi -o TestPager="head -3" + +# Test that we are not blocking +PAGER=more testsuccessequal "Package: multi +Version: 2.0 +Priority: optional +Section: other +Maintainer: Joe Sixpack <joe@example.org> +Installed-Size: 43.0 kB +Download-Size: 42 B +APT-Sources: file:$APTARCHIVE unstable/main all Packages +Description: Some description + That has multiple lines + +N: There is 1 additional record. Please use the '-a' switch to see it" runapt --unbuffer apt $show multi -o TestThatWeAreNotBlocking=1 + +PAGER=not-a-valid-pager testsuccessequal "Package: multi +Version: 2.0 +Priority: optional +Section: other +Maintainer: Joe Sixpack <joe@example.org> +Installed-Size: 43.0 kB +Download-Size: 42 B +APT-Sources: file:$APTARCHIVE unstable/main all Packages +Description: Some description + That has multiple lines + +W: Could not execute pager - PagerSetup (2: No such file or directory) +N: There is 1 additional record. Please use the '-a' switch to see it" runapt --unbuffer apt $show multi -o TestPager="not-a-valid-pager" +PAGER="dd status=none of=/dev/null" testsuccessequal "" runapt --unbuffer apt $show multi -o Test="everything is paged" +done + +msgmsg "list supports pager" +PAGER="head -1" testsuccessequal "foo/unstable 1.0 all +multi/unstable 2.0 all" apt list -qq +PAGER="head -1" testsuccessequal "foo/unstable 1.0 all" runapt --unbuffer apt list -qq -o TestPager="head -1" + +PAGER="dd status=none of=/dev/null" testsuccessequal "Listing..." runapt --unbuffer apt list -o Test="progress is not paged" + +msgmsg "search supports pager" +PAGER="head -1" testsuccessequal "foo/unstable 1.0 all + Some description + +multi/unstable 2.0 all + Some description +" apt search -qq . + +PAGER="head -1" testsuccessequal "foo/unstable 1.0 all" runapt --unbuffer apt search -qq . -o TestPager="head -1" + +PAGER="dd status=none of=/dev/null" testsuccessequal "Sorting... +Full Text Search..." runapt --unbuffer apt search . -o Test="progress is not paged" + +msgmsg "policy supports pager" +PAGER="head -1" testsuccessequal "foo: + Installed: (none) + Candidate: 1.0 + Version table: + 1.0 500 + 500 file:${APTARCHIVE} unstable/main all Packages" apt policy foo + +PAGER="head -1" testsuccessequal "foo:" runapt --unbuffer apt policy foo -o TestPager="head -1" + +PAGER="dd status=none of=/dev/null" testsuccessequal "" runapt --unbuffer apt policy foo -o Test="everything is paged" + +msgmsg "showsrc supports pager" +PAGER="head -2" testsuccessequal "Package: foo +Binary: foo +Version: 1.0 +Maintainer: Joe Sixpack <joe@example.org> +Architecture: all +Files: + b998e085e36cf162e6a33c2801318fef 11 foo_1.0.dsc + d46b9a02af8487cbeb49165540c88184 14 foo_1.0.tar.gz +Checksums-Sha256: + ed7c25c832596339bee13e4e7c45cf49f869b60d2bf57252f18191d75866c2a7 11 foo_1.0.dsc + f3da8c6ebc62c8ef2dae439a498dddcdacc1a07f45ff67ad12f44b6e2353c239 14 foo_1.0.tar.gz +" runapt apt showsrc foo + +PAGER="head -2" testsuccessequal "Package: foo +Binary: foo" runapt --unbuffer apt showsrc foo -o TestPager="head- 2" |
