[pacman-dev] PATCH: Fix bug 9395 (allow sync to remove unresolvable packages if user wishes)

Xavier shiningxc at gmail.com
Wed Dec 31 04:24:48 EST 2008


On Wed, Dec 31, 2008 at 6:10 AM, Bryan Ischo
<bji-keyword-pacman.3644cb at www.ischo.com> wrote:
> Hi, attached is a patch to fix bug 9395, against the current git tree for
> pacman, as described in an earlier email to this list.  You may find a
> description of the changes I made in the Arch bug database for bug 9395.
>  Please let me know if there are any questions.
>
> Thank you, and best wishes,
> Bryan
>
>
> diff -rupN -x '*\.git*' pacman/doc/pacman.8.txt
> pacman-fixbug9395/doc/pacman.8.txt
> --- pacman/doc/pacman.8.txt     2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/doc/pacman.8.txt  2008-12-31 17:52:33.000000000 +1300
> @@ -152,6 +152,11 @@ Options
>        If an install scriptlet exists, do not execute it. Do not use this
>        unless you know what you are doing.
>
> +*\--noignoreprompt*::
> +       Do not prompt the user to ask for confirmation that packages listed
> in
> +       IgnorePkg and IgnoreGroup really should be ignored; instead, assume
> +       that they should be and silently ignore them.
> +
>


Maybe we could simply get totally rid of this prompt, and avoid this
option altogether.
I think you already suggested that somewhere, and it might be fine. We
are usually against interaction.


>  Query Options[[QO]]
>  -------------------
> diff -rupN -x '*\.git*' pacman/lib/libalpm/alpm.h
> pacman-fixbug9395/lib/libalpm/alpm.h
> --- pacman/lib/libalpm/alpm.h   2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/lib/libalpm/alpm.h        2008-12-31
> 17:59:38.000000000 +1300
> @@ -377,7 +377,7 @@ typedef enum _pmtransconv_t {
>        PM_TRANS_CONV_CONFLICT_PKG = 0x04,
>        PM_TRANS_CONV_CORRUPTED_PKG = 0x08,
>        PM_TRANS_CONV_LOCAL_NEWER = 0x10,
> -       /* 0x20 flag can go here */
> +    PM_TRANS_CONV_REMOVE_PKGS = 0x20,
>        PM_TRANS_CONV_REMOVE_HOLDPKG = 0x40
>  } pmtransconv_t;
>
> diff -rupN -x '*\.git*' pacman/lib/libalpm/deps.c
> pacman-fixbug9395/lib/libalpm/deps.c
> --- pacman/lib/libalpm/deps.c   2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/lib/libalpm/deps.c        2008-12-31
> 17:52:41.000000000 +1300
> @@ -546,17 +546,92 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *de
>        return(NULL);
>  }
>
> +typedef struct __pkginfo_t
> +{
> +       /* The package for which this info is being kept */
> +       pmpkg_t *pkg;
> +       /* 0 if this package has been determined to be unresolvable, meaning
> that
> +          it has dependencies that cannot be resolved, nonzero otherwise */
> +       int unresolvable;
> +       /* 0 if this package was not pulled, nonzero if this package was
> pulled */
> +       int pulled;
> +       /* Packages that are immediately dependent on this package. */
> +       alpm_list_t *dependents;
> +} pkginfo_t;
> +
> +


I see that we need to keep tracks of all the parent dependencies, so
that when we want to remove a package from the targets, we remove all
the parent dependencies as well. We might want to re-use the existing
pmgraph_t structure we have for that, instead of adding a new one.

As for the rest, we could adopt a more general approach.
We add a SkipUnresolvable option in pacman.conf. If this option is
enabled, we simply skip all unresolvable packages from the target
list, no matter if it was caused by an ignored package or not.


> +static pkginfo_t *_alpm_findinfo(alpm_list_t *list, pmpkg_t *pkg)
> +{
> +       alpm_list_t *i;
> +
> +       for (i = list; i; i = i->next) {
> +               pkginfo_t *info = (pkginfo_t *) i->data;
> +               if (info->pkg == pkg) {
> +                       return info;
> +               }
> +       }
> +
> +       return NULL;
> +}
> +
> +
<snip>
> diff -rupN -x '*\.git*' pacman/lib/libalpm/deps.h
> pacman-fixbug9395/lib/libalpm/deps.h
> --- pacman/lib/libalpm/deps.h   2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/lib/libalpm/deps.h        2008-12-31
> 17:52:41.000000000 +1300
> @@ -48,8 +48,8 @@ void _alpm_depmiss_free(pmdepmissing_t *
>  alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, int reverse);
>  void _alpm_recursedeps(pmdb_t *db, alpm_list_t *targs, int
> include_explicit);
>  pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, alpm_list_t
> *excluding, pmpkg_t *tpkg);
> -int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, alpm_list_t
> *list,
> -                     alpm_list_t *remove, alpm_list_t **data);
> +int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, alpm_list_t
> **list,
> +                      alpm_list_t **pulled, alpm_list_t *remove,
> alpm_list_t **data);


Looks like these changes are indeed needed, to be able to remove
packages from "list".
Maybe for consistency we could do the same change for the "remove"
list, even if it is not needed.
Well, I am undecided on this one.


>  int _alpm_dep_edge(pmpkg_t *pkg1, pmpkg_t *pkg2);
>  pmdepend_t *_alpm_splitdep(const char *depstring);
>  pmpkg_t *_alpm_find_dep_satisfier(alpm_list_t *pkgs, pmdepend_t *dep);
> diff -rupN -x '*\.git*' pacman/lib/libalpm/sync.c
> pacman-fixbug9395/lib/libalpm/sync.c
> --- pacman/lib/libalpm/sync.c   2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/lib/libalpm/sync.c        2008-12-31
> 18:01:17.000000000 +1300
> @@ -417,9 +417,10 @@ int _alpm_sync_prepare(pmtrans_t *trans,
>        }
>
>        if(!(trans->flags & PM_TRANS_FLAG_NODEPS)) {
> -               /* store a pointer to the last original target so we can
> tell what was
> -                * pulled by resolvedeps */
> -               alpm_list_t *pulled = alpm_list_last(list);
> +               /* resolvedeps returns a pointer to the first element of the
> +                * list which is a pulled element, and all elements after
> that
> +                * are pulled as well */
> +               alpm_list_t *pulled;
>                /* Resolve targets dependencies */
>                EVENT(trans, PM_TRANS_EVT_RESOLVEDEPS_START, NULL, NULL);
>                _alpm_log(PM_LOG_DEBUG, "resolving target's dependencies\n");
> @@ -432,13 +433,13 @@ int _alpm_sync_prepare(pmtrans_t *trans,
>                        }
>                }
>
> -               if(_alpm_resolvedeps(db_local, dbs_sync, list, remove, data)
> == -1) {
> +               if(_alpm_resolvedeps(db_local, dbs_sync, &list, &pulled,
> remove, data) == -1) {
>                        /* pm_errno is set by resolvedeps */
>                        ret = -1;
>                        goto cleanup;
>                }
>
> -               for(i = pulled->next; i; i = i->next) {
> +               for(i = pulled; i; i = i->next) {
>                        pmpkg_t *spkg = i->data;
>                        pmsyncpkg_t *sync =
> _alpm_sync_new(PM_PKG_REASON_DEPEND, spkg, NULL);
>                        if(sync == NULL) {
> diff -rupN -x '*\.git*' pacman/src/pacman/callback.c
> pacman-fixbug9395/src/pacman/callback.c
> --- pacman/src/pacman/callback.c        2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/src/pacman/callback.c     2008-12-31
> 18:04:16.000000000 +1300
> @@ -248,7 +248,10 @@ void cb_trans_conv(pmtransconv_t event,
>  {
>        switch(event) {
>                case PM_TRANS_CONV_INSTALL_IGNOREPKG:
> -                       if(data2) {
> +                       if(config->noignoreprompt) {
> +                               *response = 0;
> +                       }
> +                       else if(data2) {
>                                /* TODO we take this route based on data2
> being not null? WTF */
>                                *response = yesno(_(":: %s requires
> installing %s from IgnorePkg/IgnoreGroup. Install anyway?"),
>                                                alpm_pkg_get_name(data2),
> @@ -274,6 +277,31 @@ void cb_trans_conv(pmtransconv_t event,
>                                        (char *)data2,
>                                        (char *)data2);
>                        break;
> +                case PM_TRANS_CONV_REMOVE_PKGS:
> +                    {
> +                        /* Allocate a buffer big enough to hold all of the
> +                           package names */
> +                        char *packagenames;
> +                        alpm_list_t *unresolved = (alpm_list_t *) data1;
> +                        alpm_list_t *i;
> +                        int len = 1, /* for trailing \0 */ where = 0, count
> = 0;
> +                        for (i = unresolved; i; i = i->next) {
> +                            count += 1;
> +                            len += 3 /* for \t, comma, and \n */ +
> +                                strlen(alpm_pkg_get_name(i->data));
> +                        }
> +                        packagenames = (char *) malloc(len);
> +                        for (i = unresolved; i; i = i->next) {
> +                            where += snprintf(&(packagenames[where]), len -
> where, "\t%s%s\n",
> +                                              alpm_pkg_get_name(i->data),
> (i->next) ? "," : "");
> +                        }
> +                        *response = yesno(_(":: the following package%s
> cannot be upgraded due to unresolvable "
> +
>             "dependencies:\n%s\nDo you want to skip %s package%s for this
> upgrade?"),
> +                                          (count > 1) ? "s" : "",
> packagenames, (count > 1) ? "these" : "this",
> +                                          (count > 1) ? "s" : "");
> +                        free(packagenames);
> +                    }
> +                    break;


We must have already something to display a list of targets, somewhere
in the pacman frontend code. If the existing functions are not
directly usable for this use case, we will probably need to factor out
some code.


> diff -rupN -x '*\.git*' pacman/src/pacman/pacman.c
> pacman-fixbug9395/src/pacman/pacman.c
> --- pacman/src/pacman/pacman.c  2008-12-31 16:45:31.000000000 +1300
> +++ pacman-fixbug9395/src/pacman/pacman.c       2008-12-31
> 17:53:00.000000000 +1300
> @@ -137,15 +137,16 @@ static void usage(int op, const char * c
>                                 "                       ignore a group
> upgrade (can be used more than once)\n"));
>                        printf(_("  -q, --quiet          show less
> information for query and search\n"));
>                }
> -               printf(_("      --config <path>  set an alternate
> configuration file\n"));
> -               printf(_("      --logfile <path> set an alternate log
> file\n"));
> -               printf(_("      --noconfirm      do not ask for any
> confirmation\n"));
> -               printf(_("      --noprogressbar  do not show a progress bar
> when downloading files\n"));
> -               printf(_("      --noscriptlet    do not execute the install
> scriptlet if one exists\n"));
> -               printf(_("  -v, --verbose        be verbose\n"));
> -               printf(_("  -r, --root <path>    set an alternate
> installation root\n"));
> -               printf(_("  -b, --dbpath <path>  set an alternate database
> location\n"));
> -               printf(_("      --cachedir <dir> set an alternate package
> cache location\n"));
> +               printf(_("      --config <path>   set an alternate
> configuration file\n"));
> +               printf(_("      --logfile <path>  set an alternate log
> file\n"));
> +               printf(_("      --noconfirm       do not ask for any
> confirmation\n"));
> +               printf(_("      --noprogressbar   do not show a progress bar
> when downloading files\n"));
> +               printf(_("      --noscriptlet     do not execute the install
> scriptlet if one exists\n"));
> +               printf(_("      --noignoreprompt  don't prompt to confirm
> packages in IngorePkg and IgnoreGroup\n"));
> +               printf(_("  -v, --verbose         be verbose\n"));
> +               printf(_("  -r, --root <path>     set an alternate
> installation root\n"));
> +               printf(_("  -b, --dbpath <path>   set an alternate database
> location\n"));
> +               printf(_("      --cachedir <dir>  set an alternate package
> cache location\n"));
>        }
>  }
>


These changes are strange. Maybe a whitespace / tab issue?


More information about the pacman-dev mailing list