[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#335925: apt-cache rdepends shows duplicated reverse dependencies/dependants



I tried to fix the issue and this seems to work fine with --recurse as
well. I attached a patch and the modified testcase that David wrote
earlier.
I also modified the output format. This way it's much easier to
interpret the meaning of the output because earlier when the '|'
character is used in front of a package name, we must also consider
the next package name to be in the group. But once we remove
duplicates, it is possible that the last package name has been already
printed earlier and hence it does not get printed the next time
causing another different package name to be included in the group.

For instance, when I queried for apt-cache depends python-numpy, the
output was (which contained couple of duplicate items),
python-numpy
  Depends: python
  Depends: python
  Depends: python-central
 |Depends: libblas3gf
 |Depends: <libblas.so.3gf>
    libatlas3gf-base
    libblas3gf
  Depends: libatlas3gf-base
  Depends: libc6
  Depends: libgcc1
  Depends: libgfortran3
 |Depends: liblapack3gf
 |Depends: <liblapack.so.3gf>
    libatlas3gf-base
    liblapack3gf
  Depends: libatlas3gf-base
  Suggests: python-numpy-doc
  Suggests: python-numpy-dbg
  Suggests: python-nose
  Conflicts: <python-f2py>
  Conflicts: python-matplotlib
  Conflicts: <python-numpy-dev>
  Conflicts: python-scipy
  Conflicts: <python2.3-f2py>
  Conflicts: <python2.4-f2py>

But when I modified the code in such a way that it removed duplicates,
the output in the same output format was (please look at the section
highlighted with '---'),
python-numpy
  Depends: python
  Depends: python-central
 |Depends: libblas3gf
 |Depends: <libblas.so.3gf>
    libatlas3gf-base
    libblas3gf
  Depends: libatlas3gf-base
  Depends: libc6
  Depends: libgcc1
  Depends: libgfortran3
 |Depends: liblapack3gf              ---
 |Depends: <liblapack.so.3gf>    ---
    libatlas3gf-base                     ---
    liblapack3gf                           ---
  Suggests: python-numpy-doc   ---
  Suggests: python-numpy-dbg
  Suggests: python-nose
  Conflicts: <python-f2py>
  Conflicts: python-matplotlib
  Conflicts: <python-numpy-dev>
  Conflicts: python-scipy
  Conflicts: <python2.3-f2py>
  Conflicts: <python2.4-f2py>

And I guessed the intended correct output to be something like the following,
python-numpy
  Depends: python
  Depends: python-central
  Depends: libblas3gf
    | Depends: <libblas.so.3gf> libatlas3gf-base libblas3gf
    | Depends: libatlas3gf-base
  Depends: libc6
  Depends: libgcc1
  Depends: libgfortran3
  Depends: liblapack3gf
    | Depends: <liblapack.so.3gf> libatlas3gf-base liblapack3gf
  Suggests: python-numpy-doc
  Suggests: python-numpy-dbg
  Suggests: python-nose
  Conflicts: <python-f2py>
  Conflicts: python-matplotlib
  Conflicts: <python-numpy-dev>
  Conflicts: python-scipy
  Conflicts: <python2.3-f2py>
  Conflicts: <python2.4-f2py>

With my modifications, apt-cache will output the result in the above
format. Please correct me if this is not the required output or if I
have misunderstood. I would like to know your feedback.
On 2/9/11, Ishan Jayawardena <udeshike@gmail.com> wrote:
> I have attached a modified patch
>
> On 2/9/11, Ishan Jayawardena <udeshike@gmail.com> wrote:
>> Hi David,
>>
>> Thank you for your reply. I will look into your suggestions and modify
>> it appropriately.
>>
>> On 2/9/11, David Kalnischkies <kalnischkies+debian@gmail.com> wrote:
>>> Hi Ishan,
>>>
>>> On Tue, Feb 8, 2011 at 16:46, Ishan Jayawardena <udeshike@gmail.com>
>>> wrote:
>>>> I tried to come up with a fix.
>>>
>>> First of all, the patch really fixes the issue for rdepends, so you are
>>> on
>>> the right track, but…
>>>
>>>> Please find the attached patch and give
>>>> me your feedback.
>>>
>>> * the indent-style is wrong. I know that the indent-style is… lets say
>>>   strange, but thats how it is and changing it would just destroy the
>>> history
>>>   in the vcs. The style is easiest described by indent with 3 spaces on
>>> each
>>>   curly bracket - but replace 8 consecutive spaces with a tab. You have
>>> just
>>>   used spaces and/or not the tabstop at 8 so your code is various
>>> indent-
>>>   levels higher than the rest which make it harder to read.
>>> * The code is reused for the depends command which breaks with your
>>> patch.
>>>   Attached is a testcase you can copy to test/integration in the source
>>> tree
>>>   which will run the tools build in this tree, so you can test your
>>> patch
>>>   without installing anything, you just need to build the tree and run
>>> the
>>>   testcase(s). This will show you the next two things:
>>> * You are printing your s string to often (you don't need it, just print
>>> it
>>>   as the code did it before and move the duplication check before it
>>> instead)
>>> * If the DependencyType is shown you should disable the duplication
>>> check
>>> as
>>>   otherwise only one of possible many dependencies is shown.
>>>
>>> In general, it seems to be a burden at first, but writing a testcase for
>>> the
>>> issue at hand can save you from a lot of problems later on.
>>>
>>> Hope that helps.
>>>
>>>
>>> Best regards
>>>
>>> David Kalnischkies
>>>
>>
>>
>> --
>> Regards,
>> Ishan Jayawardena.
>>
>
>
> --
> Regards,
> Ishan Jayawardena.
>


-- 
Regards,
Ishan Jayawardena.
=== modified file 'cmdline/apt-cache.cc'
--- cmdline/apt-cache.cc	2011-02-04 21:56:51 +0000
+++ cmdline/apt-cache.cc	2011-02-18 06:08:25 +0000
@@ -605,6 +605,7 @@
    bool const ShowEnhances = _config->FindB("APT::Cache::ShowEnhances", Important == false);
    bool const ShowOnlyFirstOr = _config->FindB("APT::Cache::ShowOnlyFirstOr", false);
 
+   std::set<std::string> output;
    while (verset.empty() != true)
    {
       pkgCache::VerIterator Ver = *verset.begin();
@@ -612,75 +613,106 @@
       pkgCache::PkgIterator Pkg = Ver.ParentPkg();
       Shown[Pkg->ID] = true;
 
-	 cout << Pkg.FullName(true) << endl;
-
-	 if (RevDepends == true)
-	    cout << "Reverse Depends:" << endl;
-	 for (pkgCache::DepIterator D = RevDepends ? Pkg.RevDependsList() : Ver.DependsList();
-	      D.end() == false; D++)
-	 {
-	    switch (D->Type) {
-	    case pkgCache::Dep::PreDepends: if (!ShowPreDepends) continue; break;
-	    case pkgCache::Dep::Depends: if (!ShowDepends) continue; break;
-	    case pkgCache::Dep::Recommends: if (!ShowRecommends) continue; break;
-	    case pkgCache::Dep::Suggests: if (!ShowSuggests) continue; break;
-	    case pkgCache::Dep::Replaces: if (!ShowReplaces) continue; break;
-	    case pkgCache::Dep::Conflicts: if (!ShowConflicts) continue; break;
-	    case pkgCache::Dep::DpkgBreaks: if (!ShowBreaks) continue; break;
-	    case pkgCache::Dep::Enhances: if (!ShowEnhances) continue; break;
-	    }
-
-	    pkgCache::PkgIterator Trg = RevDepends ? D.ParentPkg() : D.TargetPkg();
-
-	    if((Installed && Trg->CurrentVer != 0) || !Installed)
-	      {
-
-		if ((D->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or && ShowOnlyFirstOr == false)
-		  cout << " |";
-		else
-		  cout << "  ";
-	    
-		// Show the package
-		if (ShowDepType == true)
-		  cout << D.DepType() << ": ";
-		if (Trg->VersionList == 0)
-		  cout << "<" << Trg.FullName(true) << ">" << endl;
-		else
-		  cout << Trg.FullName(true) << endl;
-	    
-		if (Recurse == true && Shown[Trg->ID] == false)
-		{
-		  Shown[Trg->ID] = true;
-		  verset.insert(APT::VersionSet::FromPackage(CacheFile, Trg, APT::VersionSet::CANDIDATE, helper));
-		}
-
-	      }
-	    
-	    // Display all solutions
-	    SPtrArray<pkgCache::Version *> List = D.AllTargets();
-	    pkgPrioSortList(*Cache,List);
-	    for (pkgCache::Version **I = List; *I != 0; I++)
-	    {
-	       pkgCache::VerIterator V(*Cache,*I);
-	       if (V != Cache->VerP + V.ParentPkg()->VersionList ||
-		   V->ParentPkg == D->Package)
-		  continue;
-	       cout << "    " << V.ParentPkg().FullName(true) << endl;
-
-		if (Recurse == true && Shown[V.ParentPkg()->ID] == false)
-		{
-		  Shown[V.ParentPkg()->ID] = true;
-		  verset.insert(APT::VersionSet::FromPackage(CacheFile, V.ParentPkg(), APT::VersionSet::CANDIDATE, helper));
-		}
-	    }
-
-	    if (ShowOnlyFirstOr == true)
-	       while ((D->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or) ++D;
-	 }
+     cout << Pkg.FullName(true) << endl;
+
+     if (RevDepends == true)
+        cout << "Reverse Depends:" << endl;
+     bool nextAlso = false;
+     output.clear();
+     for (pkgCache::DepIterator D = RevDepends ? Pkg.RevDependsList() : Ver.DependsList();
+          D.end() == false; D++)
+     {
+        string depType = " ";
+        string prefix = "";
+        string pkgName = "";
+        
+        switch (D->Type) {
+        case pkgCache::Dep::PreDepends: if (!ShowPreDepends) continue; break;
+        case pkgCache::Dep::Depends: if (!ShowDepends) continue; break;
+        case pkgCache::Dep::Recommends: if (!ShowRecommends) continue; break;
+        case pkgCache::Dep::Suggests: if (!ShowSuggests) continue; break;
+        case pkgCache::Dep::Replaces: if (!ShowReplaces) continue; break;
+        case pkgCache::Dep::Conflicts: if (!ShowConflicts) continue; break;
+        case pkgCache::Dep::DpkgBreaks: if (!ShowBreaks) continue; break;
+        case pkgCache::Dep::Enhances: if (!ShowEnhances) continue; break;
+        }
+
+        pkgCache::PkgIterator Trg = RevDepends ? D.ParentPkg() : D.TargetPkg();
+
+        if ((Installed && Trg->CurrentVer != 0) || !Installed)
+        {
+          if ((D->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or && ShowOnlyFirstOr == false)
+          {
+             if (nextAlso == true)
+             	 prefix = "|";
+             else
+                nextAlso = true;
+          }
+          else
+          {
+             if (nextAlso == true)
+             {
+                prefix = "|";
+                nextAlso = false;
+             }
+          }
+          
+          // Show the package
+          if (ShowDepType == true)
+             depType = depType + D.DepType() + ": ";
+          if (Trg->VersionList == 0)
+             pkgName = "<" + Trg.FullName(true) + ">";
+          else
+             pkgName = Trg.FullName(true);
+
+          if (Recurse == true && Shown[Trg->ID] == false)
+          {
+             Shown[Trg->ID] = true;
+             verset.insert(APT::VersionSet::FromPackage(CacheFile, Trg, APT::VersionSet::CANDIDATE, helper));
+          }
+        }
+
+        // Display all solutions
+        SPtrArray<pkgCache::Version *> List = D.AllTargets();
+        pkgPrioSortList(*Cache,List);
+        string provides = "";
+        for (pkgCache::Version **I = List; *I != 0; I++)
+        {
+           pkgCache::VerIterator V(*Cache,*I);
+           if (V != Cache->VerP + V.ParentPkg()->VersionList ||
+               V->ParentPkg == D->Package)
+              continue;
+           provides = provides + " " + V.ParentPkg().FullName(true);
+
+           if (Recurse == true && Shown[V.ParentPkg()->ID] == false)
+           {
+              Shown[V.ParentPkg()->ID] = true;
+              verset.insert(APT::VersionSet::FromPackage(CacheFile, V.ParentPkg(), APT::VersionSet::CANDIDATE, helper));
+           }
+        }
+
+        if (ShowOnlyFirstOr == true)
+           while ((D->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or) ++D;
+
+        if ((output.insert(depType + pkgName)).second == true)
+        {
+           std::string start = " ";
+           if (prefix == "")
+              start += depType;
+           else
+           {
+              start += "   ";
+              prefix += depType;        	     	  
+           }
+           cout << start << prefix << pkgName << provides << endl;
+        }
+        else
+           nextAlso = false;
+     }
    }
 
    for (APT::PackageSet::const_iterator Pkg = helper.virtualPkgs.begin();
-	Pkg != helper.virtualPkgs.end(); ++Pkg)
+    Pkg != helper.virtualPkgs.end(); ++Pkg)
       cout << '<' << Pkg.FullName(true) << '>' << endl;
 
    return true;

Attachment: test-bug-335925-rdepends-duplicates
Description: Binary data


Reply to: