[pacman-dev] [PATCH] Attempt to stop installation when we encounter problems
Dan McGee
dan at archlinux.org
Fri Oct 31 23:49:47 EDT 2008
This should hopefully address some of the concerns raised in FS#11639 with
regards to continuing after filling the disk up. I'm not sure if we are
being too quick on the trigger here by failing after the package on every
warning libarchive can emit, but it remains to be seen.
Add some more checks and passing of error conditions between our functions.
We now ensure errors are returned when warnings or errors occur extracting a
single file, and then note the presence of errors after extraction of an
entire package is complete. If so, we abort the transaction to be on the
safe side and keep damage to a minimum.
Signed-off-by: Dan McGee <dan at archlinux.org>
---
Hey guys,
I would *love* it if someone could take this patch and run with it. I really
just wanted to hash out a possible fix, but unfortunately it isn't easy for
two reasons: our code structure is a bit rough, and we really don't have a
hard-and-fast set of rules for continuing or failing immediately. If anyone
wants to attack the problem further, you are more than welcome. Although
this patch doesn't cause any pactests to fail, I'm a bit wary of applying it
as-is, especially to maint.
-Dan
lib/libalpm/add.c | 89 +++++++++++++++++++++++++++++++++-------------------
1 files changed, 56 insertions(+), 33 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index f4caa5c..ee0898c 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -268,6 +268,32 @@ static int upgrade_remove(pmpkg_t *oldpkg, pmpkg_t *newpkg, pmtrans_t *trans, pm
return(0);
}
+static int perform_extraction(struct archive *archive,
+ struct archive_entry *entry, const char *filename, const char *origname)
+{
+ int ret;
+ const int archive_flags = ARCHIVE_EXTRACT_OWNER |
+ ARCHIVE_EXTRACT_PERM |
+ ARCHIVE_EXTRACT_TIME;
+
+ archive_entry_set_pathname(entry, filename);
+
+ ret = archive_read_extract(archive, entry, archive_flags);
+ if(ret == ARCHIVE_WARN) {
+ /* operation succeeded but a non-critical error was encountered */
+ _alpm_log(PM_LOG_WARNING, "warning extracting %s (%s)\n",
+ origname, archive_error_string(archive));
+ return(2);
+ } else if(ret != ARCHIVE_OK) {
+ _alpm_log(PM_LOG_ERROR, _("could not extract %s (%s)\n"),
+ origname, archive_error_string(archive));
+ alpm_logaction("error: could not extract %s (%s)\n",
+ origname, archive_error_string(archive));
+ return(1);
+ }
+ return(0);
+}
+
static int extract_single_file(struct archive *archive,
struct archive_entry *entry, pmpkg_t *newpkg, pmpkg_t *oldpkg,
pmtrans_t *trans, pmdb_t *db)
@@ -278,9 +304,6 @@ static int extract_single_file(struct archive *archive,
int needbackup = 0, notouch = 0;
char *hash_orig = NULL;
char *entryname_orig = NULL;
- const int archive_flags = ARCHIVE_EXTRACT_OWNER |
- ARCHIVE_EXTRACT_PERM |
- ARCHIVE_EXTRACT_TIME;
int errors = 0;
entryname = archive_entry_pathname(entry);
@@ -439,18 +462,13 @@ static int extract_single_file(struct archive *archive,
int ret;
snprintf(checkfile, PATH_MAX, "%s.paccheck", filename);
- archive_entry_set_pathname(entry, checkfile);
-
- ret = archive_read_extract(archive, entry, archive_flags);
- if(ret == ARCHIVE_WARN) {
- /* operation succeeded but a non-critical error was encountered */
- _alpm_log(PM_LOG_WARNING, "warning extracting %s (%s)\n",
- entryname_orig, archive_error_string(archive));
- } else if(ret != ARCHIVE_OK) {
- _alpm_log(PM_LOG_ERROR, _("could not extract %s (%s)\n"),
- entryname_orig, archive_error_string(archive));
- alpm_logaction("error: could not extract %s (%s)\n",
- entryname_orig, archive_error_string(archive));
+
+ ret = perform_extraction(archive, entry, checkfile, entryname_orig);
+ if(ret == 2) {
+ /* warning */
+ errors++;
+ } else if(ret == 1) {
+ /* error */
FREE(hash_orig);
FREE(entryname_orig);
return(1);
@@ -589,18 +607,12 @@ static int extract_single_file(struct archive *archive,
unlink(filename);
}
- archive_entry_set_pathname(entry, filename);
-
- ret = archive_read_extract(archive, entry, archive_flags);
- if(ret == ARCHIVE_WARN) {
- /* operation succeeded but a non-critical error was encountered */
- _alpm_log(PM_LOG_WARNING, "warning extracting %s (%s)\n",
- entryname_orig, archive_error_string(archive));
- } else if(ret != ARCHIVE_OK) {
- _alpm_log(PM_LOG_ERROR, _("could not extract %s (%s)\n"),
- entryname_orig, archive_error_string(archive));
- alpm_logaction("error: could not extract %s (%s)\n",
- entryname_orig, archive_error_string(archive));
+ ret = perform_extraction(archive, entry, filename, entryname_orig);
+ if(ret == 2) {
+ /* warning */
+ errors++;
+ } else if(ret == 1) {
+ /* error */
FREE(entryname_orig);
return(1);
}
@@ -851,6 +863,7 @@ cleanup:
int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
{
int pkg_count, pkg_current;
+ int skip_ldconfig = 0, ret = 0;
alpm_list_t *targ;
ALPM_LOG_FUNC;
@@ -868,19 +881,29 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
/* loop through our package list adding/upgrading one at a time */
for(targ = trans->packages; targ; targ = targ->next) {
if(handle->trans->state == STATE_INTERRUPTED) {
- return(0);
+ return(ret);
}
pmpkg_t *newpkg = (pmpkg_t *)targ->data;
- commit_single_pkg(newpkg, pkg_current, pkg_count, trans, db);
+ if(commit_single_pkg(newpkg, pkg_current, pkg_count, trans, db)) {
+ /* something screwed up on the commit, abort the trans */
+ trans->state = STATE_INTERRUPTED;
+ pm_errno = PM_ERR_TRANS_ABORT;
+ /* running ldconfig at this point could possibly screw system */
+ skip_ldconfig = 1;
+ ret = -1;
+ }
+
pkg_current++;
}
- /* run ldconfig if it exists */
- _alpm_log(PM_LOG_DEBUG, "running \"ldconfig -r %s\"\n", handle->root);
- _alpm_ldconfig(handle->root);
+ if(!skip_ldconfig) {
+ /* run ldconfig if it exists */
+ _alpm_log(PM_LOG_DEBUG, "running \"ldconfig -r %s\"\n", handle->root);
+ _alpm_ldconfig(handle->root);
+ }
- return(0);
+ return(ret);
}
/* vim: set ts=2 sw=2 noet: */
--
1.6.0.3
More information about the pacman-dev
mailing list