Debian Bug report logs - #866120
diffoscope: please add an xml comparator

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Mattia Rizzolo <mattia@debian.org>

To: submit@bugs.debian.org

Subject: diffoscope: please add an xml comparator

Date: Tue, 27 Jun 2017 15:51:13 +0200

[Message part 1 (text/plain, inline)]

Source: diffoscope
Version: 83
Severity: wishlist

Currently scribus treats XML files as regular text files.  I believe it
could be better by trying to understand them, like it's done for .json,
and provide a more smart output.

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

[signature.asc (application/pgp-signature, inline)]

Message #10 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 866120@bugs.debian.org

Cc: Juliana Rodrigues <juliana.orod@gmail.com>

Subject: Re: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 10:21:19 +0100

forcemerge 866120 869154
tags 866120 + patch
thanks

Woo, thanks! First, am going to merge this into the existing wishlist bug…


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb, Debian Project Leader
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #21 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 866120@bugs.debian.org

Cc: Juliana Rodrigues <juliana.orod@gmail.com>

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 10:48:07 +0100

tags 866120 + pending
thanks

Chris Lamb wrote:

> Woo, thanks! First, am going to merge this into the existing
> wishlist bug…

… And I've now also merged the code into our Git repo. Thanks!

I did make some changes after committing which might be useful
to know/understand for next time. First two (easy) style fixups
that should be fairly self-explanatory:

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

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

Next, I had to add MissingFile support to your comparator; did you run
the entire testsuite? *g*  In particular, the epub tests were failing
for me before I added:

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

Anyway, thank you again for your contribution. Let me know if you have
any questions on the above, but we all look forward to future patches
from yourself…


Best wishes,

-- 
      ,''`.
     : :'  :     Chris Lamb, Debian Project Leader
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #28 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Juliana Rodrigues <juliana.orod@gmail.com>

To: Chris Lamb <lamby@debian.org>, 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 10:54:28 +0000

[Message part 1 (text/plain, inline)]

Hi Chris!

I see. I'll be more careful with python coding style
next time. (:

About the testsuite: Firstly I ran all tests, but some were naturally
failing (like test_debian, I guess). Then I focused on getting to work at
least the one test I was writting, since I don't know much about the
project yet.

Thank you very much for your support and for your quick fixes. I hope to be
able to contribute more in the future. :D

Juliana




Em sex, 21 de jul de 2017 06:48, Chris Lamb <lamby@debian.org> escreveu:

> tags 866120 + pending
> thanks
>
> Chris Lamb wrote:
>
> > Woo, thanks! First, am going to merge this into the existing
> > wishlist bug…
>
> … And I've now also merged the code into our Git repo. Thanks!
>
> I did make some changes after committing which might be useful
> to know/understand for next time. First two (easy) style fixups
> that should be fairly self-explanatory:
>
>
> https://anonscm.debian.org/git/reproducible/diffoscope.git/commit/?id=9d89fdda0101915eedf931a4c9127918cdfea79c
>
>
> https://anonscm.debian.org/git/reproducible/diffoscope.git/commit/?id=9123eb091c49db4d7685e36d33f539bcbecefae1
>
> Next, I had to add MissingFile support to your comparator; did you run
> the entire testsuite? *g*  In particular, the epub tests were failing
> for me before I added:
>
>
> https://anonscm.debian.org/git/reproducible/diffoscope.git/commit/?id=2422e0f0f8a8ae66da0438f9fc64376faa154b53
>
> Anyway, thank you again for your contribution. Let me know if you have
> any questions on the above, but we all look forward to future patches
> from yourself…
>
>
> Best wishes,
>
> --
>       ,''`.
>      : :'  :     Chris Lamb, Debian Project Leader
>      `. `'`      lamby@debian.org / chris-lamb.co.uk
>        `-
>

[Message part 2 (text/html, inline)]

Message #31 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Mattia Rizzolo <mattia@debian.org>

To: Juliana Rodrigues <juliana.orod@gmail.com>, 866120@bugs.debian.org

Cc: Chris Lamb <lamby@debian.org>

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 15:59:30 +0200

[Message part 1 (text/plain, inline)]

First:

\o/
Thank you!

On Fri, Jul 21, 2017 at 10:54:28AM +0000, Juliana Rodrigues wrote:
> About the testsuite: Firstly I ran all tests, but some were naturally
> failing (like test_debian, I guess).

As I said in other places, we expect the testsuite to pass in all
environments; the debian-related tests should either succeed for those
distributions shipping python-debian and stuff, or be skipped if those
modules are missing.

> Then I focused on getting to work at
> least the one test I was writting, since I don't know much about the
> project yet.

Please consider, either investigating those failures yourself and
propose fixing, or file bugs for them (with logs!).

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

[signature.asc (application/pgp-signature, inline)]

Message #36 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Juliana Rodrigues <juliana.orod@gmail.com>, 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 15:00:29 +0100

Hi Juliana,

> About the testsuite: Firstly I ran all tests, but some were naturally
> failing (like test_debian, I guess)

In general we try never to have any failing tests in diffoscope, so none
should ever be "naturally" failing. :)

> Then I focused on getting to work at least the one test I was writing

Can completely understand that.  As it happens, your change also causes
the APK tests to fail — would you like to work on fixing that? You can
see an output here:

  https://jenkins.debian.net/job/reproducible_diffoscope_from_git_master/301/

JFTR you can run parts of the testsuite with:

  $ PYTHONPATH=. pytest-3 tests/comparators/test_apk.py


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb, Debian Project Leader
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #41 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Juliana Rodrigues <juliana.orod@gmail.com>

To: Chris Lamb <lamby@debian.org>, 866120@bugs.debian.org, Mattia Rizzolo <mattia@debian.org>

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 14:20:15 +0000

[Message part 1 (text/plain, inline)]

Hi Chris and Mattia,

Yes! I would like to work on fixing that. Also, I'll take a look at the
other failed tests and get back at you. (:

Thank you very much for the feedback!

Juliana

Em sex, 21 de jul de 2017 11:00, Chris Lamb <lamby@debian.org> escreveu:

> Hi Juliana,
>
> > About the testsuite: Firstly I ran all tests, but some were naturally
> > failing (like test_debian, I guess)
>
> In general we try never to have any failing tests in diffoscope, so none
> should ever be "naturally" failing. :)
>
> > Then I focused on getting to work at least the one test I was writing
>
> Can completely understand that.  As it happens, your change also causes
> the APK tests to fail — would you like to work on fixing that? You can
> see an output here:
>
>
> https://jenkins.debian.net/job/reproducible_diffoscope_from_git_master/301/
>
> JFTR you can run parts of the testsuite with:
>
>   $ PYTHONPATH=. pytest-3 tests/comparators/test_apk.py
>
>
> Regards,
>
> --
>       ,''`.
>      : :'  :     Chris Lamb, Debian Project Leader
>      `. `'`      lamby@debian.org / chris-lamb.co.uk
>        `-
>

[Message part 2 (text/html, inline)]

Message #46 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Juliana Rodrigues <juliana.orod@gmail.com>, 866120@bugs.debian.org, Mattia Rizzolo <mattia@debian.org>

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 15:22:53 +0100

Hi Juliana,

> Hi Chris and Mattia,

Yes, it seems like we sent our mails within 59 seconds of each other.

Oh; feel free to join our IRC channels (on OFTC, #debian-reproducible &
#reproducible-builds) as that is where our automated testsuite prints
it's current status.


Best wishes,

-- 
      ,''`.
     : :'  :     Chris Lamb, Debian Project Leader
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #49 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Mattia Rizzolo <mattia@debian.org>

To: Chris Lamb <lamby@debian.org>, Juliana Rodrigues <juliana.orod@gmail.com>

Cc: 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Fri, 21 Jul 2017 19:22:30 +0200

[Message part 1 (text/plain, inline)]

On Fri, Jul 21, 2017 at 10:48:07AM +0100, Chris Lamb wrote:
> … And I've now also merged the code into our Git repo. Thanks!

Did you both go through
https://docs.python.org/3/library/xml.html#xml-vulnerabilities and
decided that the standard minidom was safe for our usages?

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

[signature.asc (application/pgp-signature, inline)]

Message #54 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Juliana Rodrigues <juliana.orod@gmail.com>, 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Mon, 24 Jul 2017 11:32:12 +0100

Hey Juliana,

> Yes! I would like to work on fixing that. Also, I'll take a look at the
> other failed tests and get back at you. (:

Any news on this? We'd love to get the tests all passing again :)


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb, Debian Project Leader
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #59 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Juliana Rodrigues <juliana.orod@gmail.com>

To: Chris Lamb <lamby@debian.org>

Cc: 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Mon, 24 Jul 2017 15:36:42 -0300

[Message part 1 (text/plain, inline)]

Hey Chris,

Just found out what was going wrong.

XMLFile returns an array with the Difference object, while the
previous TextFile class doesn't. So test_apk was looking for a
unified_diff in the wrong place.

This is easily fixed, but I've got a new question. Since XMLFile
now returns an array, the presenter displays the difference for
AndroidManifest file with two column pipes instead of one.
(https://paste.debian.net/977967/)

Should this be fixed or its the expected behavior?

Thanks!

2017-07-24 7:32 GMT-03:00 Chris Lamb <lamby@debian.org>:

> Hey Juliana,
>
> > Yes! I would like to work on fixing that. Also, I'll take a look at the
> > other failed tests and get back at you. (:
>
> Any news on this? We'd love to get the tests all passing again :)
>
>
> Regards,
>
> --
>       ,''`.
>      : :'  :     Chris Lamb, Debian Project Leader
>      `. `'`      lamby@debian.org / chris-lamb.co.uk
>        `-
>

[Message part 2 (text/html, inline)]

Message #64 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Juliana Rodrigues <juliana.orod@gmail.com>

To: Mattia Rizzolo <mattia@debian.org>

Cc: Chris Lamb <lamby@debian.org>, 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Mon, 24 Jul 2017 15:54:42 -0300

[Message part 1 (text/plain, inline)]

Hi Mattia,

Actually I havent. Looks like minidom is vulnerable to both
[billion laughs] and [quadratic blowup].

Should we migrate to defusexml? What you think? (:



2017-07-21 14:22 GMT-03:00 Mattia Rizzolo <mattia@debian.org>:

> On Fri, Jul 21, 2017 at 10:48:07AM +0100, Chris Lamb wrote:
> > … And I've now also merged the code into our Git repo. Thanks!
>
> Did you both go through
> https://docs.python.org/3/library/xml.html#xml-vulnerabilities and
> decided that the standard minidom was safe for our usages?
>
> --
> regards,
>                         Mattia Rizzolo
>
> GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
> more about me:  https://mapreri.org                             : :'  :
> Launchpad user: https://launchpad.net/~mapreri                  `. `'`
> Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-
>

[Message part 2 (text/html, inline)]

Message #69 received at 866120@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: Juliana Rodrigues <juliana.orod@gmail.com>, 866120@bugs.debian.org

Subject: Re: Bug#866120: diffoscope: please add an xml comparator

Date: Mon, 24 Jul 2017 21:54:24 +0100

Hi Juliana,

> Just found out what was going wrong.
> 
> XMLFile returns an array with the Difference object, while the
> previous TextFile class doesn't. So test_apk was looking for a
> unified_diff in the wrong place.

Neat — looking forward to your patch! In fact, can you commit it
directly? :)

> This is easily fixed, but I've got a new question. Since XMLFile
> now returns an array, the presenter displays the difference for
> AndroidManifest file with two column pipes instead of one.
> (https://paste.debian.net/977967/)

I guess my first question would be whether other comparators that do
similar pretty-printing / decoding do the same thing, eg. JSON, etc.
etc.?


Best wishes,

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

Message #74 received at 866120-close@bugs.debian.org (full text, mbox, reply):

From: Mattia Rizzolo <mattia@debian.org>

To: 866120-close@bugs.debian.org

Subject: Bug#866120: fixed in diffoscope 85

Date: Mon, 31 Jul 2017 21:05:11 +0000

Source: diffoscope
Source-Version: 85

We believe that the bug you reported is fixed in the latest version of
diffoscope, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 866120@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Mattia Rizzolo <mattia@debian.org> (supplier of updated diffoscope package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Mon, 31 Jul 2017 22:27:45 +0200
Source: diffoscope
Binary: diffoscope
Architecture: source
Version: 85
Distribution: unstable
Urgency: medium
Maintainer: Reproducible builds folks <reproducible-builds@lists.alioth.debian.org>
Changed-By: Mattia Rizzolo <mattia@debian.org>
Description:
 diffoscope - in-depth comparison of files, archives, and directories
Closes: 866120 868486 868534 870049
Changes:
 diffoscope (85) unstable; urgency=medium
 .
   [ Mattia Rizzolo ]
   * tools:
     + move from the deprecated platform.linux_distribution() to the external
       python package "distro".  Add it as an optioanl dependency, as without it
       distribution detection (i.e. `diffoscope --list-tools`) won't work.
       Also add it as a Debian (hard) dependency, as it's a very lightway
       package, and the usefulness it brings are great.
     + add a get_package_provider() function, returning the package name
       that best matches the system.
   * exc:
     + in RequiredToolNotFound.get_package(), just call the new
       get_package_provider()
   * debian/rules:
     + recommends the defusedxml python package, to avoid using the python3's
       standard xml library and its security holes.
 .
   [ Chris Lamb ]
   * comparators:
     + sqlite:
       - Simplify file detection by rewriting manual `recognizes` call
         with a `Sqlite3Database.RE_FILE_TYPE` definition.
     + xml:
       - Fix EPUB "missing file" tests; they ship a META-INF/container.xml file.
 .
   [ Ximin Luo ]
   * comparators:
     + factor common logic from various comparators into File.recognizes.
     + more tidying up and making names consistent.
     + directory:
       - make stat(1) warning textually like the other warnings.
       - bump stat(1) warning into an error.
       - use getfacl(1) before lsattr(1) as it's more general.
     + apk:
       - less kludgy way of detecting APKs.  Closes: #868486
   * main, logging:
     + restore old logger settings to avoid pytest fail in certain situations.
   * debian/rules:
     + add a check to prevent additions of "DOS/MBR" file type.
   * feeder:
     + force a flush when writing output to diff.  Closes: #870049
   * tests/comparators:
     + directory:
       - be less strict about the expected test output, to cope with a missing
         `getfacl`.  Closes: #868534
 .
   [ Juliana Oliveira Rodrigues ]
   * comparators:
     + Add new XML comparator.  Closes: #866120
       The comparator will use defusedxml if this is installed, to avoid
       falling in known security holes coming from the XML specification.
   * tests/comparators:
     + apk:
       - fix the tests after the addition of the XML comparator.
     + image:
       - fix test_ico_image for identify >= 6.9.8.
 .
   [ Guangyuan Yang ]
   * tests/comparators:
     + device:
       - fix 2 cases for FreeBSD.
Checksums-Sha1:
 fbf4fc1689b2b1241a3567aa972e5a4891c31c01 3228 diffoscope_85.dsc
 a781bde07e4705a0a3e1b2eb675905bd9a74f911 653004 diffoscope_85.tar.xz
 98181b38cc107175c30e083a31d6ab6441bbde06 18385 diffoscope_85_amd64.buildinfo
Checksums-Sha256:
 3af990a704ba3a33ce5e3cc8ec477c1e940fa6068e8523f9c337c87206c32588 3228 diffoscope_85.dsc
 2a71703db5199a6de56e21f8d1c50eea668c147e05ee06fdeda1bfd1bba27016 653004 diffoscope_85.tar.xz
 7cf18e23618d2fe4efbd4995ff4803f0af51a4e45ad0051fe5381c3f1b221b28 18385 diffoscope_85_amd64.buildinfo
Files:
 8c6156508c5386d5f6d03621212d23dd 3228 devel optional diffoscope_85.dsc
 0a6ae3525f191bff4b30b32cfc5e9b7f 653004 devel optional diffoscope_85.tar.xz
 e31ff3dcfd2b84502b318a43c55d263f 18385 devel optional diffoscope_85_amd64.buildinfo

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEi3hoeGwz5cZMTQpICBa54Yx2K60FAll/lV8ACgkQCBa54Yx2
K63H/w/+JwR4jI33AcYD1jxsgRho34BQ20t/F/0Eh2Y05q8sK0nh/8YxyvsmxhS4
/80ZJkNEm0bFqx378bLe18vC+wYoOTZDb4rPTQOWoxJ8NU6l4fASoXplgwYVCYo8
KDHFqWjbUH5eGYZtiT6KvIp7jHkvi5gjc29kyywZv7EC75kxqddCkQ8lzeHg5zls
XrwyOpKTRMRPmxP9uZS8Z6HiYVCmYjCuCN5uvnePldtxx+Ogde4E3z8SYZWgvnRY
SGyj2LnKsxvi5KIpCA6jIDwiDTiv99cdYg6G/F8nGcCBo2iHK96iyoGFmbg2t3ps
9Ru3EfFo7uou+N1G+nHUoDXoPxeB/1Y0ifrLCzrou3JFKPBFSxjiaXyhv0FAqxUF
QCsvzmo4pl9RRYJ7VSfb++s4ttMwBn8Hp+LI8mhAEOkeIxLnEGUrEsF02eF6ewC5
c2v0v5OoXW2nsCfSxEVGsYZ7IKMFwH5kia3zLUvY+A/n42h+R7MiBQ3aMucs5QoM
z9zxIY2LPRGP5D0VoEJ/tY45uA3Nz0hJDaKO1OTC/LyZ0AP5Qs+twVHMtqgcNmwS
ijnV+DlNUI1/TLP0sfYPK1U8J4JLbxgsSv2yjiK86H7QWaXWezsDAlY/krNyU4p3
ObyZx9AAxZOiQyALNbjzcc/x7LzczVYCRkDybTtMBclSulyVubA=
=pkPO
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.