Bug#843531: diffoscope: walks through packages twice when comparing two .changes

[Chris Lamb]

tags 843531 + pending
thanks

Pushed to master:

  https://anonscm.debian.org/git/reproducible/diffoscope.git/commit/?id=f47459f


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-

Bug#843531: diffoscope: walks through packages twice when comparing two .changes

[Chris Lamb]

tags 843531 - patch
thanks

> diffoscope: walks through packages twice when comparing two .changes

Thu 22 13:17  lamby: btw, your commit does seem to do 
the work, but I'm still not confident  
as to whether there are other  
implications or whatnot, so I'd prefer  
to stare at it some more before merging 
on master 
Thu 22 13:18 <  lamby > mapreri: Could you easily construct a  
test? 
Thu 22 13:18 <  lamby > I'm not committing it without one. (In  
fact, I regret adding the patch tag.) 
Thu 22 13:20 < mapreri> ah, no, it's broken :( 
Thu 22 13:20 < mapreri> if you try to diff 2 .buildinfo it won't  
recurse over the .debs anymore 
Thu 22 13:20 < mapreri> and no, I haven't forged a nice test for the  
testsuite, just running it against an  
unreproducible package 
Thu 22 13:20 < mapreri> I should probably do that, but I'm not sure
about how to do it.

[..]

Thu 22 13:23  lamby: if it wasn't clear enough, this is what 
happens (first with your patch, after without 
https://paste.debian.net/plain/903968).  if comparing .changes runs as wanted.


(Using mock module might be helpful for a testcase)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-

Bug#843531: diffoscope: walks through packages twice when comparing two .changes

[Chris Lamb]

Hi again,

> diffoscope: walks through packages twice when comparing two .changes

(Cleaner version attached)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-
>From 2d2fc6377c635cdc5c9281bbd59d89f9563bef60 Mon Sep 17 00:00:00 2001
From: Chris Lamb 
Date: Wed, 21 Dec 2016 23:06:48 +
Subject: [PATCH] Avoid walking through packages twice when comparing two
 .changes (Closes: #843531)

Signed-off-by: Chris Lamb 
---
 diffoscope/comparators/debian.py | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/diffoscope/comparators/debian.py b/diffoscope/comparators/debian.py
index fa02064..67d74cb 100644
--- a/diffoscope/comparators/debian.py
+++ b/diffoscope/comparators/debian.py
@@ -181,8 +181,18 @@ class DotDscFile(DebControlFile):
 file._deb822 = dsc
 return True
 
+class DotBuildinfoContainer(DebControlContainer):
+def get_member_names(self):
+# If path exists in the parent container, don't recurse
+parent_container_names = self.source.container.get_member_names()
+
+return [
+x for x in super(DotBuildinfoContainer, self).get_member_names()
+if x not in parent_container_names
+]
 
 class DotBuildinfoFile(DebControlFile):
+CONTAINER_CLASS = DotBuildinfoContainer
 RE_FILE_EXTENSION = re.compile(r'\.buildinfo$')
 
 @staticmethod
-- 
2.11.0

Bug#843531: diffoscope: walks through packages twice when comparing two .changes

[Chris Lamb]

Hi Mattia,

> diffoscope: walks through packages twice when comparing two .changes

Could you try the attached patch?

  commit 55240ad42188c963dde092ba1b090f95bb8cf83c
  Author: Chris Lamb 
  Date:   Wed Dec 21 23:00:56 2016 +
  
  Avoid walking through packages twice when comparing two .changes (Closes: 
#843531)
  
  Signed-off-by: Chris Lamb 
  
   diffoscope/comparators/debian.py | 7 +++
   1 file changed, 7 insertions(+)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-
>From 55240ad42188c963dde092ba1b090f95bb8cf83c Mon Sep 17 00:00:00 2001
From: Chris Lamb 
Date: Wed, 21 Dec 2016 23:00:56 +
Subject: [PATCH] Avoid walking through packages twice when comparing two
 .changes (Closes: #843531)

Signed-off-by: Chris Lamb 
---
 diffoscope/comparators/debian.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/diffoscope/comparators/debian.py b/diffoscope/comparators/debian.py
index fa02064..337a3dd 100644
--- a/diffoscope/comparators/debian.py
+++ b/diffoscope/comparators/debian.py
@@ -181,8 +181,15 @@ class DotDscFile(DebControlFile):
 file._deb822 = dsc
 return True
 
+class DotBuildinfoContainer(DebControlContainer):
+def get_member_names(self):
+field = self.source.deb822.get('Files') or self.source.deb822.get('Checksums-Sha256')
+# If path exists in parent, don't recurse
+parent_fields = self.source.container.get_member_names()
+return [d['name'] for d in field if d['name'] not in parent_fields]
 
 class DotBuildinfoFile(DebControlFile):
+CONTAINER_CLASS = DotBuildinfoContainer
 RE_FILE_EXTENSION = re.compile(r'\.buildinfo$')
 
 @staticmethod
-- 
2.11.0