From a11b0048057f80b7a8a936b8f56be9f2fd121e7e Mon Sep 17 00:00:00 2001 From: just_fun Date: Sun, 18 Jun 2017 14:30:15 +0300 Subject: pkgadd: avoid fake installations if unpacking fails This patch aborts the package installation and remove the package from the database on any extraction error. This fixes FS#620. I don't know why the extraction errors were ignored (even documented). The initial commit already had this behaviour. Another odd thing is that the install status of the package was commited to the database before it was installed while there are exceptions used in pkg_install(): - archive open error - empty archive - archive read error Any of these errors will falsely mark the package as installed (maybe not a big problem with upgrades). To avoid breaking something (else), this fix kicks in only with fresh installs (not upgrades). Thanks Erich Eckner, for pointing out that the issue is reproductible with a read-only destination. Otherwise, I don't think I would have looked at this issue. A test case is presented with the FS issue, just in case someone who loves C++ enough to dig deeper or have more knowledge about the history of this program would take a closer look. --- pkgadd.cc | 10 +++++++++- pkgutil.cc | 7 +++++-- pkgutil.h | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkgadd.cc b/pkgadd.cc index 03cf0ec9..deeb50bc 100644 --- a/pkgadd.cc +++ b/pkgadd.cc @@ -103,7 +103,15 @@ void pkgadd::run(int argc, char** argv) db_add_pkg(package.first, package.second); db_commit(); - pkg_install(o_package, keep_list, non_install_files); + try { + pkg_install(o_package, keep_list, non_install_files, installed); + } catch (runtime_error&) { + if (!installed) { + db_rm_pkg(package.first); + db_commit(); + throw runtime_error("failed"); + } + } ldconfig(); } } diff --git a/pkgutil.cc b/pkgutil.cc index 87a4ffcc..926418cc 100644 --- a/pkgutil.cc +++ b/pkgutil.cc @@ -383,7 +383,7 @@ pair pkgutil::pkg_open(const string& filename) const return result; } -void pkgutil::pkg_install(const string& filename, const set& keep_list, const set& non_install_list) const +void pkgutil::pkg_install(const string& filename, const set& keep_list, const set& non_install_list, bool upgrade) const { struct archive* archive; struct archive_entry* entry; @@ -435,9 +435,12 @@ void pkgutil::pkg_install(const string& filename, const set& keep_list, if (archive_read_extract(archive, entry, flags) != ARCHIVE_OK) { // If a file fails to install we just print an error message and - // continue trying to install the rest of the package. + // continue trying to install the rest of the package, + // unless this is not an upgrade. const char* msg = archive_error_string(archive); cerr << utilname << ": could not install " + archive_filename << ": " << msg << endl; + if (!upgrade) + throw runtime_error("extract error: " + archive_filename + ": " + msg); continue; } diff --git a/pkgutil.h b/pkgutil.h index 4a4cabff..ce58f807 100644 --- a/pkgutil.h +++ b/pkgutil.h @@ -71,7 +71,7 @@ protected: // Tar.gz pair pkg_open(const string& filename) const; - void pkg_install(const string& filename, const set& keep_list, const set& non_install_files) const; + void pkg_install(const string& filename, const set& keep_list, const set& non_install_files, bool upgrade) const; void pkg_footprint(string& filename) const; void ldconfig() const; -- cgit v1.2.3