[pacman-dev] [PATCH] Fix bug 9395 by allowing pacman to remove unresolvable packages from a transaction

[Nagy Gabor]

Hi!

Some comments/suggestions. (Note: First wait for Dan's response, whether
he can support your concept (behavior change) or not. I would be sad, if
you did useless work because of me.)

I must repeat some of my earlier suggestions.

We prefer shorter patches. I can fully support in your patch: dropping
PM_TRANS_CONV_INSTALL_IGNOREPKG, I don't know why we haven't done this
earlier. But: I think we should give a warning where we asked the user
earlier. For example, this part could be factorized into a seperate
patch.

> +static int is_needed(alpm_list_t *infolist, pkginfo_t *info, int marker)
> +{
> +	if (info->unresolvable) {
> +		/* Obviously if it's already been marked unresolvable, it is not
> +		   needed */
> +		return(0);
> +	} else if (!info->pulled) {
> +		/* If it's top-level (not pulled), then it's needed */
> +		return(1);
> +	} else {
> +		/* Now, if all of the top-level packages which depend on it are
> +		   unresolvable, then it is unneeded */
> +		alpm_list_t *i;
> +		for (i = info->dependents; i; i = i->next) {
> +			pmpkg_t *deppkg = (pmpkg_t *) i->data;
> +			if (info->marker == marker) {
> +				/* This means that a dependency loop has been detected; we've
> +				   already marked this package meaning that it's already been
> +				   seen this time through.  So ignore this dependency. */
> +				continue;
> +			}
> +
> +			info->marker = marker;
> +
> +			if (!is_needed(infolist, findinfo(infolist, deppkg), marker)) {
> +				return(0);
> +			}

???

> +		}
> +		
> +		return(1);

???
I don't understand this part. (I checked, your earlier patch also had
this part.) Probably want to do this: If any dependent is needed, then 
this package is also needed, otherwise not.

> +	}
> +}



> +			pm_errno = PM_ERR_UNSATISFIED_DEPS;
> +			_alpm_log(PM_LOG_ERROR, _("cannot resolve dependencies for:\n"));
> +			for (i = unresolvable; i; i = i->next) {
> +				_alpm_log(PM_LOG_ERROR, _("\t%s\n"), alpm_pkg_get_name((pmpkg_t *) i->data));

2. The result of this:
error: cannot resolve dependencies for:
error:	foo
error:	bar
...

This is ugly, we use data list for this purpose, the front-end should
build this message.

> +				if (data) {
> +					alpm_list_t *targ = alpm_list_add(NULL, i->data);
> +					deps = alpm_checkdeps(_alpm_db_get_pkgcache(local), 0, remove, targ);

This is not OK. (If this part existed in your previous patch, sorry for
missing it.) In the target list we may have a satisfier.

> +					alpm_list_free(targ);
> +				}
> +			}