[pacman-dev] [PATCH] New -Ru option

[Aaron Griffin]

On 10/30/07, Nagy Gabor <ngaba at bibl.u-szeged.hu> wrote:
> > On 10/26/07, Nagy Gabor <ngaba at bibl.u-szeged.hu> wrote:
> > > >+                      printf(_("  -u, --unneeded       remove only
> > > >unneeded packages (that won't break packages)\n"));
> > > Hm. This description may not be correct ("remove unneeded packages only"
> > > sounds better for me). But I won't fix it now, because I want to
> > > motivate you to fix these "Hunglish" problems (I'm not sure on
> > > "target-list" neither...). You have probably realized already, that my
> > > English is far from perfect ;-), so I leave these works for you, sorry.
> >
> > Hah. "Hunglish"
> >
> > Just for the record... both sound a little "forced". I think it's
> > actually the word "only" that is making it feel funny (to me). "remove
> > unneeded packages" sounds better, but that's just my opinion.
> >
> > Regarding the patch... a couple of comments:
> >
> > > --- a/lib/libalpm/deps.h
> > > +++ b/lib/libalpm/deps.h
> > > @@ -40,6 +40,7 @@ struct __pmdepmissing_t {
> > >         char target[PKG_NAME_LEN];
> > >         pmdeptype_t type;
> > >         pmdepend_t depend;
> > > +       char inducer[PKG_NAME_LEN]; /* this is used in case of remove
> > dependency error only */
> >
> > Hrm. I don't know if I like the name "inducer" - could you explain
> > what you meant with this so we could maybe use a clearer term?
>
> OK. First of all, this is useful with "pacman -R" only.(*)
> If you do a "pacman -R foo", checkdeps will collect the broken dependencies
> (after the theoretical commit), for it will return with "bar" requires "prov"
> ("foo" provides "prov" here, and there is no other installed "prov"-provider). So
> 1. user get a not really informative error message; that's why I wrote in patch
> description: "miss->depend (EDIT: typo, I meant miss->inducer) will be useful in
> -R dependency error messages too."
> 2. -Ru needs this information too to figure out the "needed" packages.
> Of course we can live without inducer field (which indicates the
> "dependency-broker" package): we just checking the target list again; but that
> is simply needless, and using alpm_depcmp and such things in pacman/remove.c is
> not nice imho.
> Note: I'm not sure on (*), this can be useful with other commands, too:
> I hate these type of error messages:
> "pacman -S foo"
> ERROR: bar requires baz=1.0
> (This can happen in the following case: resolvedeps pulls baz, since new version
> is needed for foo; but updating baz 1.0 to 1.1 would break the "baz=1.0" depend
> of bar <- resolvedeps doesn't resolve this.)

I'm actually just commenting on the variable name itself. Not the
code. The variable name is unclear. I didn't really need all the
rationalization behind it.

I just think that seeing "dep->inducer" gives me no information
because the variable name is confusing. Could you change this to be a
little more clear and resubmit?

> > > +int _alpm_pkgname_pkg_cmp(const void *pkgname, const void
> I can repeat myself: we can live without this. But IMHO this is nicer than get

Ok, in this case, could you resubmit without this function, to make
things cleaner. We can add this later as another patch. (You might
want to take a look at "git commit --amend"