Bug#985045: unblock: apt/2.2.2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: jak@debian.org
Please unblock package apt
[ Reason ]
Three things:
1. Improvements for rred with empty patches. APT would display errors
and fall back to fetching full files when it saw empty patches,
confusing people and causing them to ask questions about it
2. Fix the flakiness of autopkgtest on armhf
3. Make a MACRO be a single statement by wrapping it in do { ... } while (0)
as we used it in an if statement without { } in 2.2.1 and that caused
compiler warning because the second statement was run outside the if
statement. This caused us to split dpkg commandlines earlier than
necessary, so not a huge deal, but still.
Adding this change, rather than "fixing" the call such that if we
backport another change in that area we don't forget about putting {}
around it again :D
[ Impact ]
I think I explained that above already. Flaky tests on armhf, and
confusing behavior when apt stumbles upon empty rred patches.
[ Tests ]
Changes in 1 are accompanied by integration tests, change 2 is a test
itself, and change 3 is essentially tested too - it's the macro adding
arguments to dpkg, the test suite wouldn't work without it.
All these tests are run by autopkgtest, so hooray, automatic validation of
all our changes :)
[ Risks ]
I don't see any huge risk here. If there's a regression in the pdiff
changes, we'd fall back to fetching full files (again), if there's a
regression in the flaky test it could be more flaky; and the macro
wrapped in do ... while (0) is trivial.
[ Checklist ]
[x] all changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in testing
[ Other info ]
unblock apt/2.2.2
--
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer i speak de, en
diff -Nru apt-2.2.1/apt-pkg/acquire-item.cc apt-2.2.2/apt-pkg/acquire-item.cc
--- apt-2.2.1/apt-pkg/acquire-item.cc 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/apt-pkg/acquire-item.cc 2021-03-12 09:15:59.000000000 +0100
@@ -2564,26 +2564,16 @@
}
}
-
- bool foundStart = false;
- for (std::vector<DiffInfo>::iterator cur = available_patches.begin();
- cur != available_patches.end(); ++cur)
- {
- if (LocalHashes != cur->result_hashes)
- continue;
-
- available_patches.erase(available_patches.begin(), cur);
- foundStart = true;
- break;
- }
-
- if (foundStart == false || unlikely(available_patches.empty() == true))
{
- ErrorText = "Couldn't find the start of the patch series";
- return false;
- }
+ auto const foundStart = std::find_if(available_patches.rbegin(), available_patches.rend(),
+ [&](auto const &cur) { return LocalHashes == cur.result_hashes; });
+ if (foundStart == available_patches.rend() || unlikely(available_patches.empty()))
+ {
+ ErrorText = "Couldn't find the start of the patch series";
+ return false;
+ }
+ available_patches.erase(available_patches.begin(), std::prev(foundStart.base()));
- {
auto const patch = std::find_if(available_patches.cbegin(), available_patches.cend(), [](auto const &patch) {
return not patch.result_hashes.usable() ||
not patch.patch_hashes.usable() ||
@@ -3050,14 +3040,11 @@
State = StateErrorDiff;
return;
}
- std::string const PatchFile = GetMergeDiffsPatchFileName(UnpatchedFile, patch.file);
std::string const PatchedFile = GetKeepCompressedFileName(UncompressedUnpatchedFile, Target);
switch (State)
{
case StateFetchDiff:
- Rename(DestFile, PatchFile);
-
// check if this is the last completed diff
State = StateDoneDiff;
for (std::vector<pkgAcqIndexMergeDiffs *>::const_iterator I = allPatches->begin();
@@ -3068,6 +3055,8 @@
std::clog << "Not the last done diff in the batch: " << Desc.URI << std::endl;
return;
}
+ for (auto * diff : *allPatches)
+ Rename(diff->DestFile, GetMergeDiffsPatchFileName(UnpatchedFile, diff->patch.file));
// this is the last completed diff, so we are ready to apply now
DestFile = GetKeepCompressedFileName(UncompressedUnpatchedFile + "-patched", Target);
if(Debug)
@@ -3098,8 +3087,8 @@
if(Debug)
std::clog << "allDone: " << DestFile << "\n" << std::endl;
return;
- case StateDoneDiff: _error->Fatal("Done called for %s which is in an invalid Done state", PatchFile.c_str()); break;
- case StateErrorDiff: _error->Fatal("Done called for %s which is in an invalid Error state", PatchFile.c_str()); break;
+ case StateDoneDiff: _error->Fatal("Done called for %s which is in an invalid Done state", patch.file.c_str()); break;
+ case StateErrorDiff: _error->Fatal("Done called for %s which is in an invalid Error state", patch.file.c_str()); break;
}
}
/*}}}*/
@@ -3188,8 +3177,8 @@
/* The only header we use is the last-modified header. */
string pkgAcqIndex::Custom600Headers() const
{
-
- string msg = "\nIndex-File: true";
+ std::string msg = pkgAcqBaseIndex::Custom600Headers();
+ msg.append("\nIndex-File: true");
if (TransactionManager->LastMetaIndexParser == NULL)
{
@@ -3941,9 +3930,10 @@
/*}}}*/
string pkgAcqFile::Custom600Headers() const /*{{{*/
{
- if (IsIndexFile)
- return "\nIndex-File: true";
- return "";
+ string Header = pkgAcquire::Item::Custom600Headers();
+ if (not IsIndexFile)
+ return Header;
+ return Header + "\nIndex-File: true";
}
/*}}}*/
pkgAcqFile::~pkgAcqFile() {}
diff -Nru apt-2.2.1/apt-pkg/deb/dpkgpm.cc apt-2.2.2/apt-pkg/deb/dpkgpm.cc
--- apt-2.2.1/apt-pkg/deb/dpkgpm.cc 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/apt-pkg/deb/dpkgpm.cc 2021-03-12 09:15:59.000000000 +0100
@@ -1765,8 +1765,8 @@
if (pipe(fd) != 0)
return _error->Errno("pipe","Failed to create IPC pipe to dpkg");
-#define ADDARG(X) Args.push_back(X); Size += strlen(X)
-#define ADDARGC(X) Args.push_back(X); Size += sizeof(X) - 1
+#define ADDARG(X) do { const char *arg = (X); Args.push_back(arg); Size += strlen(arg); } while (0)
+#define ADDARGC(X) ADDARG(X)
ADDARGC("--status-fd");
char status_fd_buf[20];
diff -Nru apt-2.2.1/CMakeLists.txt apt-2.2.2/CMakeLists.txt
--- apt-2.2.1/CMakeLists.txt 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/CMakeLists.txt 2021-03-12 09:15:59.000000000 +0100
@@ -200,7 +200,7 @@
# Configure some variables like package, version and architecture.
set(PACKAGE ${PROJECT_NAME})
set(PACKAGE_MAIL "APT Development Team <deity@lists.debian.org>")
-set(PACKAGE_VERSION "2.2.1")
+set(PACKAGE_VERSION "2.2.2")
string(REGEX MATCH "^[0-9.]+" PROJECT_VERSION ${PACKAGE_VERSION})
if (NOT DEFINED DPKG_DATADIR)
diff -Nru apt-2.2.1/debian/changelog apt-2.2.2/debian/changelog
--- apt-2.2.1/debian/changelog 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/debian/changelog 2021-03-12 09:15:59.000000000 +0100
@@ -1,3 +1,18 @@
+apt (2.2.2) unstable; urgency=medium
+
+ [ David Kalnischkies ]
+ * Deal with rred shortcomings around empty patch files (LP: #1918112)
+ - Allow merging with empty pdiff patches
+ - Rename pdiff merge patches only after they are all downloaded
+ - Start pdiff patching from the last possible starting point
+ - Ensure all index files sent custom tags to the methods
+ * Harden test for no new acquires after transaction abort (Closes: #984966)
+
+ [ Julian Andres Klode ]
+ * Make ADDARG{,C}() macros expand to single statements
+
+ -- Julian Andres Klode <jak@debian.org> Fri, 12 Mar 2021 09:15:59 +0100
+
apt (2.2.1) unstable; urgency=medium
[ Julian Andres Klode ]
diff -Nru apt-2.2.1/doc/apt-verbatim.ent apt-2.2.2/doc/apt-verbatim.ent
--- apt-2.2.1/doc/apt-verbatim.ent 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/doc/apt-verbatim.ent 2021-03-12 09:15:59.000000000 +0100
@@ -274,7 +274,7 @@
">
<!-- this will be updated by 'prepare-release' -->
-<!ENTITY apt-product-version "2.2.1">
+<!ENTITY apt-product-version "2.2.2">
<!-- (Code)names for various things used all over the place -->
<!ENTITY debian-oldstable-codename "buster">
diff -Nru apt-2.2.1/doc/po/apt-doc.pot apt-2.2.2/doc/po/apt-doc.pot
--- apt-2.2.1/doc/po/apt-doc.pot 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/doc/po/apt-doc.pot 2021-03-12 09:15:59.000000000 +0100
@@ -5,9 +5,9 @@
#, fuzzy
msgid ""
msgstr ""
-"Project-Id-Version: apt-doc 2.2.1\n"
+"Project-Id-Version: apt-doc 2.2.2\n"
"Report-Msgid-Bugs-To: APT Development Team <deity@lists.debian.org>\n"
-"POT-Creation-Date: 2021-03-01 22:28+0100\n"
+"POT-Creation-Date: 2021-03-12 09:16+0100\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
diff -Nru apt-2.2.1/methods/rred.cc apt-2.2.2/methods/rred.cc
--- apt-2.2.1/methods/rred.cc 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/methods/rred.cc 2021-03-12 09:15:59.000000000 +0100
@@ -441,8 +441,12 @@
bool cmdwanted = true;
Change ch(std::numeric_limits<size_t>::max());
- if (f.ReadLine(buffer, sizeof(buffer)) == NULL)
+ if (f.ReadLine(buffer, sizeof(buffer)) == nullptr)
+ {
+ if (f.Eof())
+ return true;
return _error->Error("Reading first line of patchfile %s failed", f.Name().c_str());
+ }
do {
if (h != NULL)
h->Add(buffer);
diff -Nru apt-2.2.1/po/apt-all.pot apt-2.2.2/po/apt-all.pot
--- apt-2.2.1/po/apt-all.pot 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/po/apt-all.pot 2021-03-12 09:15:59.000000000 +0100
@@ -5,9 +5,9 @@
#, fuzzy
msgid ""
msgstr ""
-"Project-Id-Version: apt 2.2.1\n"
+"Project-Id-Version: apt 2.2.2\n"
"Report-Msgid-Bugs-To: APT Development Team <deity@lists.debian.org>\n"
-"POT-Creation-Date: 2021-03-01 22:28+0100\n"
+"POT-Creation-Date: 2021-03-12 09:16+0100\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
diff -Nru apt-2.2.1/test/integration/framework apt-2.2.2/test/integration/framework
--- apt-2.2.1/test/integration/framework 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/test/integration/framework 2021-03-12 09:15:59.000000000 +0100
@@ -2126,6 +2126,9 @@
fi
}
+cdfind() {
+ ( cd /; find "$@" )
+}
aptautotest_aptget_update() {
local TESTCALL="$1"
while [ -n "$2" ]; do
@@ -2135,24 +2138,21 @@
if ! test -d "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists"; then return; fi
testfilestats "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt" '%U:%G:%a' '=' "${TEST_DEFAULT_USER}:${TEST_DEFAULT_GROUP}:755"
testfilestats "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists" '%U:%G:%a' '=' "${TEST_DEFAULT_USER}:${TEST_DEFAULT_GROUP}:755"
- (
- cd /
# all copied files are properly chmodded
local backupIFS="$IFS"
IFS="$(printf "\n\b")"
- find "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists" -type f ! -name 'lock' | while read file; do
+ cdfind "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists" -type f ! -name 'lock' | while read file; do
testfilestats "$file" '%U:%G:%a' '=' "${TEST_DEFAULT_USER}:${TEST_DEFAULT_GROUP}:644"
done
IFS="$backupIFS"
if [ "$TESTCALL" = 'testsuccess' ]; then
# failure cases can retain partial files and such
- testempty find "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial" -mindepth 1 ! \( -name 'lock' -o -name '*.FAILED' \)
+ testempty cdfind "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial" -mindepth 1 ! \( -name 'lock' -o -name '*.FAILED' \)
fi
if [ -s "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" ]; then
testfileequal "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" \
- "$(find "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do stat --format '%U:%G:%a:%n' "$line"; done | sort)"
+ "$(cdfind "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do stat --format '%U:%G:%a:%n' "$line"; done | sort)"
fi
- )
}
aptautotest_apt_update() { aptautotest_aptget_update "$@"; }
aptautotest_aptcdrom_add() { aptautotest_aptget_update "$@"; }
diff -Nru apt-2.2.1/test/integration/test-method-mirror apt-2.2.2/test/integration/test-method-mirror
--- apt-2.2.1/test/integration/test-method-mirror 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/test/integration/test-method-mirror 2021-03-12 09:15:59.000000000 +0100
@@ -192,6 +192,16 @@
testfailure apt update
testrundownload 'foo=2'
+msgmsg 'Mirrors can be filtered by' 'by-hash type'
+echo "http://localhost:${APTHTTPPORT}/failure type:foobar priority:1
+http://localhost:${APTHTTPPORT}/redirectme type:index type:deb
+" > aptarchive/mirror.txt
+rm -rf rootdir/var/lib/apt/lists
+testsuccess apt update -o Acquire::By-Hash=force #-o Debug::pkgAcquire::Worker=1
+cp rootdir/tmp/testsuccess.output forcedbyhash.output
+testfailure grep "localhost:${APTHTTPPORT}/failure" forcedbyhash.output
+testrundownload 'foo=2'
+
msgmsg 'The prefix for the mirrorlist is' 'passed on'
echo 'Dir::Bin::Methods::foo+mirror+file "mirror";
Dir::Bin::Methods::foo+mirror+http "mirror";
diff -Nru apt-2.2.1/test/integration/test-method-rred apt-2.2.2/test/integration/test-method-rred
--- apt-2.2.1/test/integration/test-method-rred 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/test/integration/test-method-rred 2021-03-12 09:15:59.000000000 +0100
@@ -146,6 +146,7 @@
- even more good stuff
- bonus good stuff
$(tail -n 12 ./Packages)"
+testrred 'Patch file' 'empty' '' "$(cat ./Packages)"
failrred() {
msgtest 'Failure caused by' "$1"
@@ -161,7 +162,6 @@
</html>'
# not a problem per-se, but we want our parser to be really strict
-failrred 'Empty patch file' ''
failrred 'Empty line patch file' '
'
failrred 'Empty line before command' '
@@ -222,7 +222,8 @@
testsuccess apthelper cat-file --compress gzip Packages.ed-1
mv rootdir/tmp/testsuccess.output Packages.ed-1.gz
testsuccess rm Packages.ed-1
-createpatch 'Remove (old) dog paragraph' '10,19d' > Packages.ed-2
+touch Packages.ed-2 # an empty patch
+createpatch 'Remove (old) dog paragraph' '10,19d' > Packages.ed-3
mergepatches '11,19c
Package: extra-kittens
Version: unavailable
diff -Nru apt-2.2.1/test/integration/test-pdiff-usage apt-2.2.2/test/integration/test-pdiff-usage
--- apt-2.2.1/test/integration/test-pdiff-usage 2021-03-01 22:27:55.000000000 +0100
+++ apt-2.2.2/test/integration/test-pdiff-usage 2021-03-12 09:15:59.000000000 +0100
@@ -100,12 +100,15 @@
PATCHINDEX='aptarchive/Packages.diff/Index'
echo "SHA256-Current: $(sha256sum "${PKGFILE}-new" | cut -d' ' -f 1) $(stat -c%s "${PKGFILE}-new")
SHA256-History:
+ $(sha256sum "$PKGFILE" | cut -d' ' -f 1) $(stat -c%s "$PKGFILE") 2000-08-18-2013.28
01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b 33053002 2010-08-18-2013.28
$(sha256sum "$PKGFILE" | cut -d' ' -f 1) $(stat -c%s "$PKGFILE") $(basename "$PATCHFILE")
SHA256-Patches:
+ $(sha256sum "$PATCHFILE" | cut -d' ' -f 1) $(stat -c%s "$PATCHFILE") 2000-08-18-2013.28
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 19722 2010-08-18-2013.28
$(sha256sum "$PATCHFILE" | cut -d' ' -f 1) $(stat -c%s "$PATCHFILE") $(basename "$PATCHFILE")
SHA256-Download:
+ $(sha256sum "${PATCHFILE}.gz" | cut -d' ' -f 1) $(stat -c%s "${PATCHFILE}.gz") 2000-08-18-2013.28.gz
d2a1b33187ed2d248eeae3b1223ea71791ea35f2138a713ed371332a6421f467 197 2010-08-18-2013.28.gz
$(sha256sum "${PATCHFILE}.gz" | cut -d' ' -f 1) $(stat -c%s "${PATCHFILE}.gz") $(basename "${PATCHFILE}.gz")" > "$PATCHINDEX"
@@ -251,16 +254,10 @@
cp Packages-future aptarchive/Packages
rm -f rootdir/var/lib/apt/lists/*_Contents-*
webserverconfig 'aptwebserver::overwrite::.*Contents-.*::filename' '/hacked-i386.gz'
- # This should work in at least 4% of the cases...
- for i in $(seq 25); do
- testfailure apt update "$@"
- if ! grep 'rred:600' rootdir/tmp/testfailure.output; then
- break
- fi
- done
- webserverconfig 'aptwebserver::overwrite::.*Contents-.*::filename' '/Contents-i386.gz'
+ testfailure apt update "$@"
cp rootdir/tmp/testfailure.output patchdownload.output
- testfailure grep 'rred:600' patchdownload.output
+ webserverconfig 'aptwebserver::overwrite::.*Contents-.*::filename' '/Contents-i386.gz'
+ testfailure grep -q -z 'AbortTransaction.* [a-zA-Z0-9+-]\+:600%20' patchdownload.output
testnopackage newstuff futurestuff
testsuccessequal "$(cat "${PKGFILE}")
" aptcache show apt oldstuff
@@ -281,7 +278,8 @@
rm "${PATCHFILE}.gz"
testsuccess apt update "$@"
cp rootdir/tmp/testsuccess.output patchdownload.output
- testsuccess grep '^Falling back to normal index file acquire' patchdownload.output
+ # it should be anchored on line start, but multiple processes on the same output stream…
+ testsuccess grep 'Falling back to normal index file acquire' patchdownload.output
testnopackage oldstuff
testsuccessequal "$(cat Packages-future)
" aptcache show apt newstuff futurestuff
Reply to: