On Wed, Nov 23, 2011 at 9:51 AM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote:
Issue a warning if the user for whatever reason tries to remove an optdepend of an installed package.
Signed-off-by: Benedikt Morbach <benedikt.morbach@googlemail.com> --- lib/libalpm/alpm.h | 5 ++++- lib/libalpm/deps.c | 35 ++++++++++++++++++++++++++--------- lib/libalpm/error.c | 2 ++ lib/libalpm/remove.c | 16 +++++++++++++--- lib/libalpm/sync.c | 2 +- lib/libalpm/trans.c | 10 ++++++++-- src/pacman/remove.c | 23 +++++++++++++++++++---- src/util/testdb.c | 2 +- 8 files changed, 74 insertions(+), 21 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index d06ad0d..34dbaa1 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -162,6 +162,8 @@ typedef struct _alpm_depmissing_t { alpm_depend_t *depend; /* this is used in case of remove dependency error only */ char *causingpkg; + /* this is used for optdepends only */ + char *description; This is needed only because depmissing_t points at a depend_t and not an optdepend_t? Yet another reason to make them one and the same.
} alpm_depmissing_t;
/** Conflict */ @@ -1096,7 +1098,7 @@ int alpm_remove_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg); */
alpm_list_t *alpm_checkdeps(alpm_handle_t *handle, alpm_list_t *pkglist, - alpm_list_t *remove, alpm_list_t *upgrade, int reversedeps); + alpm_list_t *remove, alpm_list_t *upgrade, int reversedeps, int consider_optdeps); alpm_pkg_t *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstring); alpm_pkg_t *alpm_find_dbs_satisfier(alpm_handle_t *handle, alpm_list_t *dbs, const char *depstring); @@ -1184,6 +1186,7 @@ typedef enum _alpm_errno_t { ALPM_ERR_DLT_PATCHFAILED, /* Dependencies */ ALPM_ERR_UNSATISFIED_DEPS, + ALPM_ERR_UNSATISFIED_OPTDEPS, ALPM_ERR_CONFLICTING_DEPS, ALPM_ERR_FILE_CONFLICTS, /* Misc */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 83bc619..c9efce4 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -52,7 +52,7 @@ void _alpm_optdep_free(alpm_optdepend_t *optdep) }
static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, - const char *causingpkg) + const char *causingpkg, const char *description) { alpm_depmissing_t *miss;
@@ -61,6 +61,7 @@ static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, STRDUP(miss->target, target, return NULL); miss->depend = _alpm_dep_dup(dep); STRDUP(miss->causingpkg, causingpkg, return NULL); + STRDUP(miss->description, description, return NULL);
return miss; } @@ -70,6 +71,7 @@ void _alpm_depmiss_free(alpm_depmissing_t *miss) _alpm_dep_free(miss->depend); FREE(miss->target); FREE(miss->causingpkg); + FREE(miss->description); FREE(miss); }
@@ -283,11 +285,12 @@ alpm_pkg_t SYMEXPORT *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstri * @param remove an alpm_list_t* of packages to be removed * @param upgrade an alpm_list_t* of packages to be upgraded (remove-then-upgrade) * @param reversedeps handles the backward dependencies + * @param consider_optdeps handles optional dependencies instead of normal ones * @return an alpm_list_t* of alpm_depmissing_t pointers. */ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle, alpm_list_t *pkglist, alpm_list_t *remove, alpm_list_t *upgrade, - int reversedeps) + int reversedeps, int consider_optdeps) { alpm_list_t *i, *j; alpm_list_t *dblist = NULL, *modified = NULL; @@ -326,7 +329,7 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle, _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: missing dependency '%s' for package '%s'\n", missdepstring, tp->name); free(missdepstring); - miss = depmiss_new(tp->name, depend, NULL); + miss = depmiss_new(tp->name, depend, NULL, NULL); baddeps = alpm_list_add(baddeps, miss); } release_filtered_depend(depend, nodepversion); @@ -338,8 +341,17 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle, * the packages listed in the requiredby field. */ for(i = dblist; i; i = i->next) { alpm_pkg_t *lp = i->data; - for(j = alpm_pkg_get_depends(lp); j; j = j->next) { - alpm_depend_t *depend = j->data; + j = consider_optdeps ? alpm_pkg_get_optdepends(lp) : alpm_pkg_get_depends(lp); + for(; j; j = j->next) { + alpm_depend_t *depend; + const char *description = NULL; + if(consider_optdeps) { + alpm_optdepend_t *optdep = j->data; + depend = optdep->depend; + description = optdep->description; + } else { + depend = j->data; + } OMG please unify depend_t and optdepend_t. :)
depend = filtered_depend(depend, nodepversion); alpm_pkg_t *causingpkg = find_dep_satisfier(modified, depend); /* we won't break this depend, if it is already broken, we ignore it */ @@ -350,10 +362,15 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle, !find_dep_satisfier(dblist, depend)) { alpm_depmissing_t *miss; char *missdepstring = alpm_dep_compute_string(depend); - _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' dependency of '%s'\n", - missdepstring, lp->name); + if(consider_optdeps) { + _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' optional dependency of '%s'\n", We're way over the normal line wrap width here, so please move the start of the string to the next line. + missdepstring, lp->name); + } else { + _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' dependency of '%s'\n", + missdepstring, lp->name); + } free(missdepstring); - miss = depmiss_new(lp->name, depend, causingpkg->name); + miss = depmiss_new(lp->name, depend, causingpkg->name, description); baddeps = alpm_list_add(baddeps, miss); } release_filtered_depend(depend, nodepversion); @@ -801,7 +818,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, for(i = alpm_list_last(*packages); i; i = i->next) { alpm_pkg_t *tpkg = i->data; targ = alpm_list_add(NULL, tpkg); - deps = alpm_checkdeps(handle, localpkgs, remove, targ, 0); + deps = alpm_checkdeps(handle, localpkgs, remove, targ, 0, 0); alpm_list_free(targ);
for(j = deps; j; j = j->next) { diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 044dec7..5283392 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -136,6 +136,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err) /* Dependencies */ case ALPM_ERR_UNSATISFIED_DEPS: return _("could not satisfy dependencies"); + case ALPM_ERR_UNSATISFIED_OPTDEPS: + return _("could not satisfy optional dependencies"); case ALPM_ERR_CONFLICTING_DEPS: return _("conflicting dependencies"); case ALPM_ERR_FILE_CONFLICTS: diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index d7e06bc..012f9c8 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -102,7 +102,7 @@ static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), - trans->remove, NULL, 1); + trans->remove, NULL, 1, 0); } return 0; } @@ -133,7 +133,7 @@ static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp) alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), - trans->remove, NULL, 1); + trans->remove, NULL, 1, 0); } }
@@ -164,7 +164,7 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) EVENT(handle, ALPM_EVENT_CHECKDEPS_START, NULL, NULL);
_alpm_log(handle, ALPM_LOG_DEBUG, "looking for unsatisfied dependencies\n"); - lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1); + lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1, 0); if(lp != NULL) {
if(trans->flags & ALPM_TRANS_FLAG_CASCADE) { @@ -206,6 +206,16 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)
if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) { EVENT(handle, ALPM_EVENT_CHECKDEPS_DONE, NULL, NULL); + lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1, 1); + if(lp != NULL) { + if(data) { + *data = lp; + } else { + alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free(lp); + } + RET_ERR(handle, ALPM_ERR_UNSATISFIED_OPTDEPS, -1); + } We should not be checking deps, whether optional or required, after we have issued CHECKDEPS_DONE. This can't be done here, you need to move this to a more logical place earlier in teh method.
}
return 0; diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 3817ec8..b501807 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -605,7 +605,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) { _alpm_log(handle, ALPM_LOG_DEBUG, "checking dependencies\n"); deps = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), - trans->remove, trans->add, 1); + trans->remove, trans->add, 1, 0); if(deps) { handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; ret = -1; diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index cb97a4a..40e6786 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -101,6 +101,7 @@ static alpm_list_t *check_arch(alpm_handle_t *handle, alpm_list_t *pkgs) int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data) { alpm_trans_t *trans; + int retval = 0;
/* Sanity checks */ CHECK_HANDLE(handle, return -1); @@ -127,7 +128,12 @@ int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data) if(trans->add == NULL) { if(_alpm_remove_prepare(handle, data) == -1) { /* pm_errno is set by _alpm_remove_prepare() */ - return -1; + /* UNSATISFIED_OPTDEPS is nonfatal. */ + if(alpm_errno(handle) == ALPM_ERR_UNSATISFIED_OPTDEPS) { + retval = -1; + } else { + return -1; + } I don't like this special case at all, -1 from me implemented this way. Please use callbacks and resolve it during checkdeps resolution, rather than whatever we have going on here.
} } else { if(_alpm_sync_prepare(handle, data) == -1) { @@ -138,7 +144,7 @@ int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data)
trans->state = STATE_PREPARED;
- return 0; + return retval; }
/** Commit a transaction. */ diff --git a/src/pacman/remove.c b/src/pacman/remove.c index f56c1ec..739a6eb 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -111,8 +111,8 @@ int pacman_remove(alpm_list_t *targets) /* Step 2: prepare the transaction based on its type, targets and flags */ if(alpm_trans_prepare(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); - pm_printf(ALPM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), - alpm_strerror(err)); + retval = 1; + switch(err) { case ALPM_ERR_PKG_INVALID_ARCH: for(i = data; i; i = alpm_list_next(i)) { @@ -128,12 +128,27 @@ int pacman_remove(alpm_list_t *targets) free(depstring); } break; + case ALPM_ERR_UNSATISFIED_OPTDEPS: + for(i = data; i; i = alpm_list_next(i)) { + alpm_depmissing_t *miss = i->data; + char *depstring = alpm_dep_compute_string(miss->depend); + if(miss->description) { + printf(_(":: %s: optionally requires %s (%s)\n"), miss->target, depstring, miss->description); + } else { + printf(_(":: %s: optionally requires %s\n"), miss->target, depstring); + } + free(depstring); + } + retval = 0; + break; default: break; } FREELIST(data); - retval = 1; - goto cleanup; + if(retval) { + pm_printf(ALPM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), alpm_strerror(err)); + goto cleanup; + } }
/* Search for holdpkg in target list */ diff --git a/src/util/testdb.c b/src/util/testdb.c index b15bbe5..e1401cc 100644 --- a/src/util/testdb.c +++ b/src/util/testdb.c @@ -99,7 +99,7 @@ static int checkdeps(alpm_list_t *pkglist) alpm_list_t *data, *i; int ret = 0; /* check dependencies */ - data = alpm_checkdeps(handle, pkglist, NULL, pkglist, 0); + data = alpm_checkdeps(handle, pkglist, NULL, pkglist, 0, 0); for(i = data; i; i = alpm_list_next(i)) { alpm_depmissing_t *miss = i->data; char *depstring = alpm_dep_compute_string(miss->depend); -- 1.7.7.3