Add apkdiff tool #4221

radekdoulik · 2020-02-04T21:04:25Z

Context: https://github.com/xamarin/xamarin-android/projects/14
Context: #4191

And use it to produce .apkdesc files. These files will be later used
instead of the size reference files we have now. They contain more
detailed size information and also improve the readability compared to
the .csv files.

The .apkdesc files look like:

{
  "Comment": "HEAD/master: cdc04224edcb876ae6607693ea4011fee8c76893",
  "PackageSize": 75817581,
  "Entries": {
    "AndroidManifest.xml": {
      "Size": 5428
    },
    "res/drawable/android_button.xml": {
      "Size": 588
    },
...
}

Context: https://github.com/xamarin/xamarin-android/projects/14 Context: dotnet#4191 And use it to produce `.apkdesc` files. These files will be later used instead of the size reference files we have now. They contain more detailed size information and also improve the readability compared to the `.csv` files. The `.apkdesc` files look like: { "Comment": "HEAD/master: cdc04224edcb876ae6607693ea4011fee8c76893", "PackageSize": 75817581, "Entries": { "AndroidManifest.xml": { "Size": 5428 }, "res/drawable/android_button.xml": { "Size": 588 }, ... }

Instead of packages.config

dellis1972 · 2020-02-05T12:21:16Z

tools/apkdiff/apkdiff.csproj

@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


Now I think about it, should all new projects be SDK style?

Makes sense. Do we have a migration tool available on Mac or do I need to do it by hand?

@radekdoulik I found they are so short you can type them now:

<Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <TargetFramework>net472</TargetFramework> </PropertyGroup> </Project>

It has a bunch of conventions based on the csproj file name. You can also delete Properties\AssemblyInfo.cs.

Thanks! I did similar thing here radekdoulik/apkdiff@8608ae3

I guess I will need to modify the OutputPath in addition to that in XA.

radekdoulik · 2020-02-06T12:22:22Z

/azp run

azure-pipelines · 2020-02-06T12:22:47Z

Azure Pipelines successfully started running 1 pipeline(s).

jonpryor · 2020-02-06T13:27:10Z

"Silly" question: do we really want .apkdesc files to be JSON, and not XML? Yes, there's lots of XML hatred going around, but vast swaths of our toolchain are XML based, and I can't think of anything offhand in our toolchain which uses JSON.

radekdoulik · 2020-02-06T14:22:17Z

"Silly" question: do we really want .apkdesc files to be JSON, and not XML? Yes, there's lots of XML hatred going around, but vast swaths of our toolchain are XML based, and I can't think of anything offhand in our toolchain which uses JSON.

The choice is motivated mostly by better readability.

jonpryor · 2020-02-06T21:49:42Z

build-tools/scripts/TestApks.targets

+    </PropertyGroup>
+    <Exec
+        Condition=" Exists('$(ApkDiffPath)') "
+        Command="&quot;$(ApkDiffPath)&quot; -v -c &quot;HEAD/`git branch --show-current`: `git rev-parse HEAD`&quot; -s &quot;%(_AllArchives.Identity)&quot;;mv &quot;%(_AllArchives.RelativeDir)\%(_AllArchives.Filename).apkdesc&quot; &quot;%(_AllArchives.RelativeDir)\%(_AllArchives.Filename)-$(Configuration)-$(TestsFlavor).apkdesc&quot;"


I think that -$(TestsFlavor) can instead be $(TestsFlavor).

Right now the xa-test-results-10.2.99.215_739a18a-Darwin-Release.zip file contains filenames such as:

% find . -iname \*.apkd\* ./Xamarin.Android.EmbeddedDSO_Test-Signed-Release-.apkdesc ./Mono.Android_Tests-Signed-Release--Profiled-Aot.apkdesc ./Xamarin.Forms_Performance_Integration-Signed-Release-.apkdesc ./Xamarin.Forms_Performance_Integration-Signed-Release--Bundle.apkdesc ./Mono.Android_Tests-Signed-Release-.apkdesc ./Mono.Android_Tests-Signed-Release--Aot.apkdesc ./Xamarin.Forms_Performance_Integration-Signed-Release--Aot.apkdesc ./Xamarin.Android.Locale_Tests-Signed-Release-.apkdesc ./Mono.Android_Tests-Signed-Release--Bundle.apkdesc ./Xamarin.Android.JcwGen_Tests-Signed-Release-.apkdesc ./Xamarin.Android.Locale_Tests-Signed-Release--Aot.apkdesc ./Xamarin.Forms_Performance_Integration-Signed-Release--Profiled-Aot.apkdesc ./Mono.Android_TestsMultiDex-Signed-Release-.apkdesc ./Xamarin.Android.Locale_Tests-Signed-Release--Profiled-Aot.apkdesc

Note that many filenames have a trailing -, e.g. Xamarin.Android.JcwGen_Tests-Signed-Release-.apkdesc, or have --, e.g. Xamarin.Android.Locale_Tests-Signed-Release--Aot.apkdesc. I believe using just $(TestsFlavor) would fix this.

Visual Studio for Mac has a tendency to "re-format" `.csproj` files, e.g. if you add a new project to the `.sln`, it'll reformat *every* `.csproj` referenced by the `.sln`, and this "reformatting" invariably removes all "useful" whitespace, resulting in this "wonderful" fragment: ```xml <Target Name="CopyNewtonsoftJsonLicense" AfterTargets="Build"> <Copy SourceFiles="..\..\packages\newtonsoft.json\12.0.3\LICENSE.md" DestinationFiles="$(OutputPath)\Newtonsoft.Json-LICENSE.md" /> </Target> ``` To which I say NO. Custom targets do *not* go into the .csproj, precisely because VSMac will (eventually) screw it up. Move CopyNewtonsoftJsonLicense into `apkdiff.targets`, and then `<Import/>` apkdiff.targets.

Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3452921&view=logs&j=8556562a-ae5f-5bd1-7c4d-bf1af4b6f1e1&t=1a8bc9f1-aacc-59ec-6634-323d2bcb90ae Shortly `apkdiff` was added (2e28f2e), some PR builds started showing errors around its execution: RecordApkSizes: "/Library/Frameworks/Xamarin.Android.framework/Libraries/xbuild/Xamarin/Android/Darwin/apkdiff" -v -c "HEAD/`git branch --show-current`: `git rev-parse HEAD`" -s "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.aab";mv "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.apkdesc" "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed-Release.apkdesc" EXEC : error : apkdiff: Unknown file extension '.aab' mv: rename ../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.apkdesc to ../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed-Release.apkdesc: No such file or directory …/build-tools/scripts/TestApks.targets(354,5): error MSB3073: The command " "/Library/Frameworks/Xamarin.Android.framework/Libraries/xbuild/Xamarin/Android/Darwin/apkdiff" -v -c "HEAD/`git branch --show-current`: `git rev-parse HEAD`" -s "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.aab";mv "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.apkdesc" "../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed-Release.apkdesc" " exited with code 1. This error was likely overlooked in PR #4221 because PR builds are currently "noisy", with lots of "ignorable" errors occurring. Update `apkdiff` so that it will also process `.aab` files, thus avoiding the above error.

radekdoulik requested review from dellis1972, jonathanpeppers and jonpryor as code owners February 4, 2020 21:04

dellis1972 approved these changes Feb 5, 2020

View reviewed changes

radekdoulik added 2 commits February 5, 2020 12:54

Switch the project to use PackageReference

e4557e4

Instead of packages.config

Make xaprepare restore apkdiff project packages

a6d9acb

radekdoulik requested a review from grendello as a code owner February 5, 2020 11:59

grendello approved these changes Feb 5, 2020

View reviewed changes

dellis1972 reviewed Feb 5, 2020

View reviewed changes

radekdoulik added 3 commits February 5, 2020 13:24

Added forgotten file

28a686d

Install apkdiff

cb4015f

Try to get apkdiff from the installation

c218d9c

Install Newtonsoft.Json assembly

0afe791

Add apkdesc files to test results archive

37fc257

jonpryor reviewed Feb 6, 2020

View reviewed changes

jonpryor and others added 3 commits February 6, 2020 16:53

"Prettify" .apkdesc filename generation

06f493b

Add apkdiff.targets

4e294a4

jonpryor merged commit 2e28f2e into dotnet:master Feb 7, 2020

radekdoulik mentioned this pull request Feb 11, 2020

apkdiff tool #4192

Closed

brendanzagaeski added this to the In Preview: 10.4 (d16-7) milestone May 19, 2020

github-actions bot locked and limited conversation to collaborators Jan 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add apkdiff tool #4221

Add apkdiff tool #4221

Uh oh!

radekdoulik commented Feb 4, 2020

Uh oh!

dellis1972 Feb 5, 2020

Uh oh!

radekdoulik Feb 5, 2020

Uh oh!

jonathanpeppers Feb 5, 2020

Uh oh!

radekdoulik Feb 5, 2020

Uh oh!

radekdoulik commented Feb 6, 2020

Uh oh!

azure-pipelines bot commented Feb 6, 2020

Uh oh!

jonpryor commented Feb 6, 2020

Uh oh!

radekdoulik commented Feb 6, 2020

Uh oh!

jonpryor Feb 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		@@ -0,0 +1,60 @@
		<?xml version="1.0" encoding="utf-8"?>
		<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

Add apkdiff tool #4221

Add apkdiff tool #4221

Uh oh!

Conversation

radekdoulik commented Feb 4, 2020

Uh oh!

dellis1972 Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

radekdoulik Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

radekdoulik Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

radekdoulik commented Feb 6, 2020

Uh oh!

azure-pipelines bot commented Feb 6, 2020

Uh oh!

jonpryor commented Feb 6, 2020

Uh oh!

radekdoulik commented Feb 6, 2020

Uh oh!

jonpryor Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants