Debian Bug report logs - #850501
diffoscope: apktool.yml file created by apktool is shown as file from APK & contain input filenames

Toggle useless messages

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

From: Emanuel Bronshtein <e3amn2l@gmx.com>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: diffoscope: APK support issues - traceback on existent directory & missing zipinfo & misleading apktool.yml file

Date: Fri, 06 Jan 2017 23:49:50 -0500

Package: diffoscope
Version: 60
Severity: normal

Dear Maintainer,

3 issues regarding APK files (apk.py comparator) below:

#1 - Diffoscope fail to run on APKs if supplied via absolute paths.

Running: (using diffoscope from GIT)

/data/repbdiffs/repos/diffoscope/bin/diffoscope /tmp/1.apk /tmp/2.apk

Result:

Destination directory (/tmp/1.apk) already exists. Use -f switch if you want to overwrite it.
Traceback (most recent call last):
  File "/data/repbdiffs/repos/diffoscope/diffoscope/main.py", line 260, in main
    sys.exit(run_diffoscope(parsed_args))
  File "/data/repbdiffs/repos/diffoscope/diffoscope/main.py", line 236, in run_diffoscope
    parsed_args.path1, parsed_args.path2)
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/utils/compare.py", line 61, in compare_root_paths
    return compare_files(file1, file2)
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/utils/compare.py", line 78, in compare_files
    return file1.compare(file2, source)
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/utils/file.py", line 199, in compare
    if hasattr(self, 'compare_details') or self.as_container:
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/utils/file.py", line 108, in as_container
    self._as_container = self.__class__.CONTAINER_CLASS(self)
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/utils/archive.py", line 44, in __init__
    self._archive = self.open_archive()
  File "/data/repbdiffs/repos/diffoscope/diffoscope/tools.py", line 50, in tool_check
    return original_function(*args, **kwargs)
  File "/data/repbdiffs/repos/diffoscope/diffoscope/comparators/apk.py", line 45, in open_archive
    shell=False, stderr=None, stdout=subprocess.PIPE)
  File "/usr/lib/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['apktool', 'd', '-k', '-m', '-o', '/tmp/1.apk', '/tmp/1.apk']' returned non-zero exit status 1

it does work when running as:
cd /tmp && /data/repbdiffs/repos/diffoscope/bin/diffoscope 1.apk 2.apk

fix:
	use temporary directory for apktool unpacking.

#2 - apktool.yml file created by apktool is shown as file from APK & contain input filenames (might be unrelated to files content)

apktool generate apktool.yml which contain metadata about the APK, more information:
https://ibotpeaches.github.io/Apktool/documentation/
but shown as file from APK which is incorrect, for example:
diffoscope 1.apk 2.apk
1.apk is: https://f-droid.org/repo/com.poinsart.votar_9.apk
2.apk is: https://verification.f-droid.org/com.poinsart.votar_9.apk

Result:

--- 1.apk
+++ 2.apk
├── apktool.yml
@@ -1,9 +1,9 @@
│  !!brut.androlib.meta.MetaInfo
│ -apkFileName: 1.apk
│ +apkFileName: 2.apk
│  compressionType: false
│  doNotCompress:
│  - arsc
│  isFrameworkApk: false
│  packageInfo: null
│  sdkInfo:
│    minSdkVersion: '9'


it's better to show it as "APK metadata" (similar to "file list","metadata", etc..) instead of apktool.yml

also the apktool.yml contain the filename recevied by apktool at apkFileName field, thus if apktool was run directly on files supplied via command-line (instead of files inside archive) it will show difference that not related to APK content, example above and in:
https://verification.f-droid.org/org.sufficientlysecure.ical_54.apk.diffoscope.html

thus apkFileName field need to be striped from apktool.yml file. (the archive case is supported via zipinfo information, see next issue)

fix:
	1. show apktool.yml difference as "APK metadata" instead of apktool.yml file
	2. remove apkFileName field from apktool.yml result.

#3 missing zipinfo information

on ZIP files the zipinfo utility used to list files inside the archive (may contain difference in file-ordering/permissions/timestamps/etc..), but it is not used on APK files which are ZIP/JAR files.
for example, comparing the zipinfo on APKs:
https://f-droid.org/repo/com.nbossard.packlist_16.apk
https://verification.f-droid.org/com.nbossard.packlist_16.apk
show that there are new-files added & there is file-ordering issue, as happened before apk.py was added. (zip.py handled APK files)

fix:
	use also the zipinfo mechanism as used currently on ZIP files via zip.py comparator on APK files.

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

From: Chris Lamb <lamby@debian.org>

To: Emanuel Bronshtein <e3amn2l@gmx.com>, 850485@bugs.debian.org

Subject: Re: Bug#850485: diffoscope: APK support issues - traceback on existent directory & missing zipinfo & misleading apktool.yml file

Date: Sat, 07 Jan 2017 09:43:11 +0000

clone 850485 -1 -2
retitle 850485 diffoscope: fails to run on APKs if supplied via absolute paths
retitle -1 diffoscope: apktool.yml file created by apktool is shown as file from APK & contain input filenames
retitle -2 diffoscope: apk output is missing zipinfo information
thanks

Emanuel Bronshtein wrote:

> 3 issues regarding APK files (apk.py comparator) below:

Thanks for these! Have split the bug up so that they can be tackled
individually.

[Almost certainly not necessary this time, but please provide --debug
output in bug reports!]


Regards,

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

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

From: Маша Глухова <siamezzze@gmail.com>

To: 850501@bugs.debian.org

Subject: Re: diffoscope: APK support issues - traceback on existent directory & missing zipinfo & misleading apktool.yml file

Date: Mon, 09 Jan 2017 05:28:36 +0000

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

On Fri, 06 Jan 2017 23:49:50 -0500 Emanuel Bronshtein <e3amn2l@gmx.com>
wrote:

> #2 - apktool.yml file created by apktool is shown as file from APK &
contain input filenames (might be unrelated to files content)

With the attached patch, input filenames are removed from apktool.yml and
the file is renamed to "APK metadata".
It is still compared as a file, though. Let me know if this is a problem.


Thanks,
Maria Glukhova

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

[0001-Remove-archive-name-from-apktool.yml-and-rename-it.patch (text/x-patch, attachment)]

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

From: Chris Lamb <lamby@debian.org>

To: 850501@bugs.debian.org, Маша Глухова <siamezzze@gmail.com>

Cc: Emanuel Bronshtein <e3amn2l@gmx.com>

Subject: Re: diffoscope: apktool.yml file created by apktool is shown as file from APK & contain input filenames

Date: Mon, 09 Jan 2017 11:33:57 +0000

tags 850501 + pending
thanks

Applied in Git as:

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

Many thanks!


Regards,

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

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

From: Hans-Christoph Steiner <hans@eds.org>

To: Chris Lamb <lamby@debian.org>, Emanuel Bronshtein <e3amn2l@gmx.com>, Маша Глухова <siamezzze@gmail.com>, 850501@bugs.debian.org

Subject: rerunning using diffoscope 67-21-gfe7ae15

Date: Mon, 9 Jan 2017 15:50:13 +0100

Thanks for your work on the APK diffing!  I had to fix a typo to get it
running that was introduced in diffoscope commit
fe7ae15e1c177866acd478af4cc4a51bd5002017 at the bottom of it. It turned
'f_out' into a non-existent 'w'.

With that change, diffoscope is now working for me again.  I'm running
it on the fdroid verification server, you can see the output here:

https://verification.f-droid.org/


Also, about apktool.yml, I don't think we need it at all in diffoscope.
It is not a file from the APK and it is not produced by Android SDK
tools. As far as I know, it is only used by apktool if you want to use
apktool to reconstruct a previously "decoded" APK.

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

From: Chris Lamb <lamby@debian.org>

To: "Hans-Christoph Steiner" <hans@eds.org>, Emanuel Bronshtein <e3amn2l@gmx.com>, Маша Глухова <siamezzze@gmail.com>, 850501@bugs.debian.org

Subject: Re: rerunning using diffoscope 67-21-gfe7ae15

Date: Mon, 09 Jan 2017 17:11:39 +0000

Hans-Christoph Steiner wrote:

> Thanks for your work on the APK diffing!  I had to fix a typo to get it
> running that was introduced in diffoscope commit

Fixed - so sorry about that! Now I wonder if we should insist on tests
with patches... ;)

> With that change, diffoscope is now working for me again.  I'm running
> it on the fdroid verification server, you can see the output here:
> 
> https://verification.f-droid.org/

Awesome!


Regards,

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

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

From: Maria Glukhova <siamezzze@gmail.com>

To: 850501@bugs.debian.org

Subject: Re: rerunning using diffoscope 67-21-gfe7ae15

Date: Sat, 14 Jan 2017 00:49:26 +0200

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

On Mon, 09 Jan 2017 17:11:39 +0000 Chris Lamb <lamby@debian.org> wrote:
> Fixed - so sorry about that! Now I wonder if we should insist on tests
> with patches... ;)

Added some tests for APK comparator, see attached patch.
However, please keep in mind apktool takes a lot of time to run even on toy
examples, so adding these tests would extend the time taken by the whole
testing process significantly (almost doubles it at my system). Which makes
me wonder if that was the reason such tests were not added together with
APK comparator in the first place...


Maria Glukhova

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

[0001-Add-some-tests-for-APK-comarator.patch (text/x-diff, attachment)]

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

From: Chris Lamb <lamby@debian.org>

To: Maria Glukhova <siamezzze@gmail.com>, 850501@bugs.debian.org

Subject: Re: Bug#850501: rerunning using diffoscope 67-21-gfe7ae15

Date: Sun, 15 Jan 2017 00:43:36 +0000

Maria,

Thank you so much for these :)

> Added some tests for APK comparator, see attached patch.

Unfortunately I'm seeing:

differences = [<Difference zipinfo {} -- zipinfo {} []>, <Difference APK metadata -- APK metadata []>, <Difference AndroidManifest.xml -- AndroidManifest.xml []>, <Difference original/AndroidManifest.xml -- original/AndroidManifest.xml []>]

    @skip_unless_tools_exist('apktool', 'zipinfo')
    def test_android_manifest(differences):
>       assert differences[1].source1 == 'AndroidManifest.xml'
E       assert 'APK metadata' == 'AndroidManifest.xml'
E         - APK metadata
E         + AndroidManifest.xml

Any ideas?

> However, please keep in mind apktool takes a lot of time to run even on toy
> examples

Whilst I am not a fan of it slowing down this much, I think I'd
prefer to see some tests in there (that we can speed up later)
rather than none.


Regards,

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

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

From: Maria Glukhova <siamezzze@gmail.com>

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

Subject: Re: Bug#850501: rerunning using diffoscope 67-21-gfe7ae15

Date: Sun, 15 Jan 2017 02:39:44 +0000

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

Chris,

I see. Seems like file ordering differs for some reason. I should not have
relied on it, anyway.
Rewrote it so file ordering is not taken into account. Does it work now?

Thank you,
Maria

вс, 15 янв. 2017 г. в 2:43, Chris Lamb <lamby@debian.org>:

> Maria,
>
> Thank you so much for these :)
>
> > Added some tests for APK comparator, see attached patch.
>
> Unfortunately I'm seeing:
>
> differences = [<Difference zipinfo {} -- zipinfo {} []>, <Difference APK
> metadata -- APK metadata []>, <Difference AndroidManifest.xml --
> AndroidManifest.xml []>, <Difference original/AndroidManifest.xml --
> original/AndroidManifest.xml []>]
>
>     @skip_unless_tools_exist('apktool', 'zipinfo')
>     def test_android_manifest(differences):
> >       assert differences[1].source1 == 'AndroidManifest.xml'
> E       assert 'APK metadata' == 'AndroidManifest.xml'
> E         - APK metadata
> E         + AndroidManifest.xml
>
> Any ideas?
>
> > However, please keep in mind apktool takes a lot of time to run even on
> toy
> > examples
>
> Whilst I am not a fan of it slowing down this much, I think I'd
> prefer to see some tests in there (that we can speed up later)
> rather than none.
>
>
> Regards,
>
> --
>       ,''`.
>      : :'  :     Chris Lamb
>      `. `'`      lamby@debian.org / chris-lamb.co.uk
>        `-
>

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

[0001-Add-some-tests-for-APK-comarator.patch (text/x-diff, attachment)]

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

From: Chris Lamb <lamby@debian.org>

To: Maria Glukhova <siamezzze@gmail.com>, 850501@bugs.debian.org

Subject: Re: Bug#850501: rerunning using diffoscope 67-21-gfe7ae15

Date: Sun, 15 Jan 2017 02:57:37 +0000

Hi Maria,

> Rewrote it so file ordering is not taken into account. Does it work now?

Yes, although this would still mean the output of diffoscope itself
is not determinstic. As it happens I wrote my own fix before I saw your email
and I have pushed it — what do you think?


Regards,

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

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

From: Maria Glukhova <siamezzze@gmail.com>

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

Subject: Re: Bug#850501: rerunning using diffoscope 67-21-gfe7ae15

Date: Sun, 15 Jan 2017 03:07:32 +0000

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

Chris,

вс, 15 янв. 2017 г. в 4:57, Chris Lamb <lamby@debian.org>:

> As it happens I wrote my own fix before I saw your email
> and I have pushed it — what do you think?
>
>
Making APK metadata appear at fixed place surely looks better. IMO, that
approach follows the original intent of the request much more closely :)
Thank you very much for your work on this!  (And sorry, once again, for not
doing it properly from the start)

Maria

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

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

From: Chris Lamb <lamby@debian.org>

To: 850501-close@bugs.debian.org

Subject: Bug#850501: fixed in diffoscope 68

Date: Mon, 16 Jan 2017 00:48:26 +0000

Source: diffoscope
Source-Version: 68

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 850501@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Chris Lamb <lamby@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: SHA256

Format: 1.8
Date: Mon, 16 Jan 2017 11:24:22 +1100
Source: diffoscope
Binary: diffoscope
Architecture: source
Version: 68
Distribution: unstable
Urgency: medium
Maintainer: Reproducible builds folks <reproducible-builds@lists.alioth.debian.org>
Changed-By: Chris Lamb <lamby@debian.org>
Description:
 diffoscope - in-depth comparison of files, archives, and directories
Closes: 849395 850055 850485 850501 850502 850730 850807 850850
Changes:
 diffoscope (68) unstable; urgency=medium
 .
   [ Chris Lamb ]
 .
   * Don't blow up if directory containing ELF debug symbols already exists.
     (Closes: #850807)
   * Fix .APK extration when provided with absolute filenames.
     (Closes: #850485)
   * Support comparing .ico files using img2txt. (Closes: #850730)
   * comparators.utils.file: If we don't have an archive-extraction tool (eg.
     apktool), don't blow up when attempting to unpack it.
   * Include magic file type when we know the file format but can't find
     file-specific details. (Closes: #850850)
   * Ensure fake "APK metadata" file appears first, fixing non-deterministic
     tests/output.
   * Correctly escape value of href="" elements (re. #849411)
 .
   * Optimisations:
     - Disable profiling entirely (unless enabled) for a 2%+ optimisation
     - Compile APK filename regex instead of generating it each loop.
 .
   * Logging:
     - Log tempfile cleanup process
     - Log when we add a progress observer.
     - Drop milliseconds from log output
 .
   * Misc:
     - Many unused import removals, indentation changes, etc.
     - Fix duplicated word and long line errors in debian/changelog.
     - Suggest some promotion in post-release documentation.
 .
   [ Maria Glukhova ]
   * comparators/device: don't crash when comparing a non-device against a
     device (Closes: #850055)
   * Remove archive name from apktool.yml and rename it. (Closes: #850501)
   * Zipinfo included in APK files comparison. (Closes: #850502)
     - Add some tests for APK comparator.
   * Add image metadata comparison. (Closes: #849395)
   * Ensure imagemagick version is new enough for image metadata tests.
 .
   [ Mattia Rizzolo ]
   * Skip the openssh_pub_key test if the version of ssh is < 6.9.
   * comparators/icc: rename RE_FILE_EXTENSION to RE_FILE_TYPE, as that's what
     the regular expression is looking for.
   * Make use of a new mechanism to remove a bunch of recognizes() methods
     dealing with simple RE_FILE_TYPE matching.
Checksums-Sha1:
 30784a46bd47bd40ea061544dbd8efa80c2f70b9 2940 diffoscope_68.dsc
 c2bb3ade5270b5eb1a74f04601a48daa04f6d979 324376 diffoscope_68.tar.xz
Checksums-Sha256:
 825057d67e926532df3fd06ea63e2c57a0075f5e0709dccd6bd393f77ac556e1 2940 diffoscope_68.dsc
 42db84379df4a6932c09b5674b64828880ab9a64c5426628220c8c127d66f18b 324376 diffoscope_68.tar.xz
Files:
 4237325fb0e88b2d2c5571f5fc9f0cac 2940 devel optional diffoscope_68.dsc
 aeb6a542b7db5b8d478b55d5a06abfec 324376 devel optional diffoscope_68.tar.xz

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

iQIzBAEBCAAdFiEEwv5L0nHBObhsUz5GHpU+J9QxHlgFAlh8FEkACgkQHpU+J9Qx
Hlj3WBAAtsuSuNT0D0vgccnZq5y+S9byiuwotBQA5V/iDgpnuhmCRJnN4paFJfXn
wHCoFoTyqZZuX548DLC3UooidTJmSf1Bw5uCIBv++EyE4cSNwLaR2kV+7XE+WGnV
JA/WuwdH3QprhVzvGfCmLSDcsW7XHwS96jJlv/f4Qf42GSiRcZn30t9iaZlZAtPm
O9+oSZdWL8HKCgpdluB+YKe5nq7lH/qTtGecAzJvytlR9TqRlaId05jfc4f0dc8w
/znweuf0nWg4mbGRWhOll765XLp27puw4/bSee879990Q+8EBJKmmKpogWN+UiZR
NPMmDKV+caxOWeGT/uHWrr7A4ViMWlPw6jl1Z7jrNUZijYD1zQzYhSZMNOX5O7oT
wih2ynSYLpOSqRBjjx8U8lRhoKXsB9Yz9FHLYDLhtKAEymGld7hcsL7OdM/m9FNk
kQqHCUCpSkrO3RZQLmf+9dDKgwyuzR9z/AtmluYmJMJ7TntENn5+4Fk0sOCUxTah
QMnipPeBpku+lYFRAwwI9aa8CHRzsPb28PfSbRJKRrrvrCxXoyRxB0mZkiQpVm0H
HlIaddchgElL61zWcCXS9oZ9EMb8lQ4Pl7ArxMpyJxGJCap4MnmtrFTxzSQOe7v2
NYI9QHDDJj9eyBQMgvDqtTrEwCO0EhNwc8jmUC8EorvqhZPVhVE=
=1s7L
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.