Merge lp:~evfool/synaptic/lp945997 into lp:synaptic

[Robert Roth]

This branch contains the following fixes:
* installed version and latest available version field values made selectable in the embedded package properties pane (related to bug #173464)
* installed files listing did not work for multiarch packages as the name() already contained the arch, so i have added a check to make sure we try both with and without the arch suffix (if name contains : we already tried with the arch suffix, try without it, and if the name doesn't contain :, add the arch suffix and try with that). This fixes bug #945997.

[Daniel Hartwig (wigs)]

The comment “try multiarch name next” no longer applies, replaced by the new comment you insert.

The logic here seems ok.

Portability issues:

+ int index = namestr.find_first_of(":");
+ if (index > -1) {

string::find_first_of returns string::size_type, not int. It should not be coerced prior to comparing it to the “char not found“ value, or using it with any string functions. Generally, it is never necessary to explicitly coerce string::size_type.

The “char not found” value is string::npos, not -1. Although equivalent for some implementations of string in some libraries, you can not rely on that:

string::size_type index = namestr.find_first_of(":");
if (index != string::npos) {

review: Needs Fixing

[lp:~evfool/synaptic/lp945997]

2116. By Robert Roth on 2012-11-23

Fixed string searching issues based on review feedback

[Robert Roth (evfool)]

Thanks for the review Daniel, I have made the changes suggested.
I did not make my research,, just simply checked that it works for me for both multiarch and simple packages, I'm not (yet) used to think about portability, but hey, I'm learning :).

[Daniel Hartwig (wigs)]

> I did not make my research,, just simply checked that it
> works for me for both multiarch and simple packages

Indeed my initial look at the logic was not deep enough either! Consider:

$ apt-cache show bison:i386 bison:amd64 | grep …
Package: bison
Status: install ok installed
Architecture: i386
Multi-Arch: foreign

Package: bison
Architecture: amd64
Multi-Arch: foreign

$ ls /var/lib/dpkg/info/bison*.list
/var/lib/dpkg/info/bison.list

The properties screen for bison:amd64 (which is *not* installed) will display the installed files belonging to bison:i386.

Quick fix: determine whether the package is installed before looking for the files:

const char *RPackage::installedFiles()
{
static string filelist;
static string notInstalled = _("The list of installed files is only available for installed packages");
vector<string> sV;
string s;

if (!(getFlags() & FInstalled))
  return notInstalled.c_str();
…

[*] The sensible thing to do is not guess where dpkg stores the file list and instead call “dpkg -L PKGNAME”. There are various issues with that (in fact, even the released code has issues such as being ignorant of DPkg::Run-Directory), none of which will be addressed until after Wheezy.

[lp:~evfool/synaptic/lp945997]

2117. By on 2012-11-23

Added check to see if package is installed

[Robert Roth (evfool)]

Updated proposal with the check for installed package.

[Daniel Hartwig (wigs)]

Looks ok to me, and suitable for now until improvements are made to the libapt–dpkg interface.

review: Approve

Unmerged revisions

2117. By on 2012-11-23

Added check to see if package is installed

2116. By Robert Roth on 2012-11-23

Fixed string searching issues based on review feedback

2115. By Robert Roth on 2012-11-22

Fixed installed files listing for multiarch packages (LP: #945997)

2114. By Robert Roth on 2012-11-22

Made installed version and available version in the embedded package properties pane selectable (LP: #173464)

=== modified file 'common/rpackage.cc'
--- common/rpackage.cc	2012-07-12 20:03:08 +0000
+++ common/rpackage.cc	2012-11-23 09:19:20 +0000
@@ -213,16 +213,29 @@
 const char *RPackage::installedFiles()
 {
    static string filelist;
+   static string notInstalled = _("The list of installed files is only available for installed packages");
    vector<string> sV;
    string s;
 
    filelist.erase(filelist.begin(), filelist.end());
 
+   if (!(getFlags() || FInstalled)) {
+      return notInstalled.c_str();
+   }
    // try normal file first
-   string f = "/var/lib/dpkg/info/" + string(name()) + ".list";
-   // try multiarch name next
-   if (!FileExists(f))
-      f = "/var/lib/dpkg/info/" + string(name()) + ":" + arch() + ".list";
+   string namestr = string(name());
+   string f = "/var/lib/dpkg/info/" + namestr + ".list";
+   if (!FileExists(f)) {
+      // truncate the multiarch suffix if exists, add it if it does not exist
+      // to make sure we have tried both multiarch suffixed and simple package name
+      string::size_type index = namestr.find_first_of(":"); 
+      if (index != string::npos) {
+         namestr = namestr.substr(0, index);
+      } else {
+         namestr = namestr + ":" + arch();
+      }
+      f = "/var/lib/dpkg/info/" + namestr + ".list";
+   }
    if (FileExists(f)) {
       ifstream in(f.c_str());
       if (!in != 0)
@@ -237,9 +250,7 @@
 
       return filelist.c_str();
    }
-   filelist = _("The list of installed files is only available for installed packages");
-
-   return filelist.c_str();
+   return notInstalled.c_str();
 }
 #else
 const char *RPackage::installedFiles()

=== modified file 'gtk/gtkbuilder/window_main.ui'
--- gtk/gtkbuilder/window_main.ui	2012-11-21 19:37:43 +0000
+++ gtk/gtkbuilder/window_main.ui	2012-11-23 09:19:20 +0000
@@ -1511,6 +1511,7 @@
                                                 <property name="visible">True</property>
                                                 <property name="can_focus">False</property>
                                                 <property name="xalign">0</property>
+                                                <property name="selectable">True</property>
                                               </object>
                                               <packing>
                                                 <property name="left_attach">1</property>
@@ -1644,6 +1645,7 @@
                                                 <property name="visible">True</property>
                                                 <property name="can_focus">False</property>
                                                 <property name="xalign">0</property>
+                                                <property name="selectable">True</property>
                                               </object>
                                               <packing>
                                                 <property name="left_attach">1</property>