[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#874326: [PATCH 2/3] private-update: return early if `ListUpdate()` fails



`ListUpdate()` returns a bool to indicate whether locking succeeded or
not, but we don't look at the return value. Teach the caller to return
early if `ListUpdate()` returns false.

To give a concrete example, this means that a non-root user with the
default configuration will see this on `apt-get update`:

  E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
  E: Unable to lock directory /var/lib/apt/lists/

as opposed to also seeing a couple of follow-on warnings:

  W: Problem unlinking the file /var/cache/apt/pkgcache.bin - RemoveCaches (13: Permission denied)
  W: Problem unlinking the file /var/cache/apt/srcpkgcache.bin - RemoveCaches (13: Permission denied)
---
 apt-private/private-update.cc            |  3 ++-
 test/integration/test-apt-update-locking | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100755 test/integration/test-apt-update-locking

diff --git a/apt-private/private-update.cc b/apt-private/private-update.cc
index c9113ddd3..d56593505 100644
--- a/apt-private/private-update.cc
+++ b/apt-private/private-update.cc
@@ -69,7 +69,8 @@ bool DoUpdate(CommandLine &CmdL)
    if (_config->FindB("APT::Get::Download",true) == true)
    {
       AcqTextStatus Stat(std::cout, ScreenWidth,_config->FindI("quiet",0));
-      ListUpdate(Stat, *List);
+      if (ListUpdate(Stat, *List) == false)
+	 return false;
    }
 
    if (_config->FindB("pkgCacheFile::Generate", true) == false)
diff --git a/test/integration/test-apt-update-locking b/test/integration/test-apt-update-locking
new file mode 100755
index 000000000..e2e1e3649
--- /dev/null
+++ b/test/integration/test-apt-update-locking
@@ -0,0 +1,18 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+setupaptarchive
+
+rm "$TMPWORKINGDIRECTORY/rootdir/var/lib/apt/lists/lock"
+# mkdir() in the framework is too clever for us, so avoid it
+command mkdir "$TMPWORKINGDIRECTORY/rootdir/var/lib/apt/lists/lock"
+testfailure aptget update
+cp "${TMPWORKINGDIRECTORY}/rootdir/tmp/testfailure.output" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testfailure"
+testsuccess grep "^E: Could not open lock file" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testfailure"
+testsuccess grep "^E: Unable to lock directory" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testfailure"
+testfailure grep "^W: " "${TMPWORKINGDIRECTORY}/rootdir/tmp/testfailure"
-- 
2.21.0.rc2.5.gc65a2884ea


Reply to: