[pacman-dev] [PATCH 10/14] Warn on optdep removal
Dan McGee
dpmcgee at gmail.com
Wed Nov 30 22:54:18 EST 2011
On Wed, Nov 23, 2011 at 9:51 AM, Benedikt Morbach
<benedikt.morbach at 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 at 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
>
>
More information about the pacman-dev
mailing list