Fix of a (minor) issue: pkg add shows wrong version number in "package already installed" message #349

Merged
merged 4 commits into from Sep 6, 2012

Conversation

Projects
None yet
3 participants
Contributor

namore commented Sep 6, 2012

Problem description

Wrong error message when trying to add a local package which is
already installed. (wrong version message).

Example: I installed chromium-20.0.1132.57 vie pkgbeta.freebsd.org,
but due to security problems i compiled a newer package on another
machine: chromium-21.0.1180.89.txz

# pkg info chromium
chromium-20.0.1132.57          A mostly BSD-licensed web browser based on WebKit and Gtk+
# ls /tmp/chr*
/tmp/chromium-21.0.1180.89.txz
# pkg add /tmp/chromium-21.0.1180.89.txz
Installing chromium-21.0.1180.89...chromium-21.0.1180.89 already installed

Failed to install the following 1 package(s): /tmp/chromium-21.0.1180.89.txz

^ there's the error: it says "...chromium-21.0.1180.89 already
installed", but it should say: "...chromium-20.0.1132.57 already
installed", as "pkg info chromium" shows.

Affected Versions

I tested version 1.0rc5 (the pkgbeta.freebsd.org version), 1.0 (the
current ports version) and master (as of this pull request).
The problem is the same.

How serve is the problem?

It may cause minor confusion for users (as me), but it's not a serious
issue.

Source Problem and Fix

libpkg/pkg_add.c passes the to-be-installed pkg* to
emmit_pkg_already_installed(...).

To fix this, replaced the call to "pkg_is_installed" with it's content
(to get hold of the currently-installed package in from db),
restructured it slightly and kept the found package for the emmit
event. (after which it's pkg_freed again)

Tested

The patch was tested on 9.1-RC1 amd64 and works fine for me.

Contributor

dnaeon commented Sep 6, 2012

Now, that's how a good pull request is written :)

Thanks!

dnaeon added a commit that referenced this pull request Sep 6, 2012

Merge pull request #349 from namore/master
Fix of a (minor) issue: pkg add shows wrong version number in "package already installed" message

@dnaeon dnaeon merged commit a5cbd5a into freebsd:master Sep 6, 2012

Member

jlaffaye commented on libpkg/pkg_add.c in b4d7a5b Sep 6, 2012

Fixing this function seems more appropriate.

Contributor

namore commented Sep 6, 2012

The function pkg_is_installed() does exactly what it's named: it checks whether a package is installed (returns bool); but this information isn't enough here: we need to get the package itself.

I thought about extending
bool pkg_is_installed(pkgdb, origin)
to smth like:
bool pkg_is_installed(pkgdb, origin, &pkg)

But investigating a bit turned out this would break binary compatibility. (i don't know if it's important currently for libpkg)

Thus adding another function would be the way to go, like "pkg* get_pkg_if_installed(pkgdb,origin)". Introducing such a function makes sense if you need it more often, but it felt odd to add it for one single use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment