Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix APK builds to enhance build reproduceability #675

Closed
LeoColman opened this issue Aug 15, 2024 · 15 comments
Closed

Fix APK builds to enhance build reproduceability #675

LeoColman opened this issue Aug 15, 2024 · 15 comments
Labels
Maintenance Fixing a dependency, code-cleaning

Comments

@LeoColman
Copy link
Owner

Ref: #672
Ref: https://gitlab.com/IzzyOnDroid/repo/-/issues/593

As discussed, the current way builds the APK file from the bundle. That hurts RB and we should change it, as there is no reason our pipeline is this way and we can probably save some valuable bits.

@LeoColman LeoColman added Maintenance Fixing a dependency, code-cleaning Good First Issue Defines a good issue for a new contributor to try labels Aug 15, 2024
@LeoColman
Copy link
Owner Author

@IzzySoft

Please check the latest release for reproducibility

@IzzySoft
Copy link

APKs differ:

--- /dev/fd/63  2024-08-20 09:19:17.497061195 +0200
+++ /dev/fd/62  2024-08-20 09:19:17.501061169 +0200
@@ -1,11 +1,11 @@
   META-INF/com/android/build/gradle/app-metadata.properties
   32-bit CRC value (hex):                         37fbe4f4
   assets/dexopt/baseline.prof
-  32-bit CRC value (hex):                         1610ff52
+  32-bit CRC value (hex):                         d718940e
   assets/dexopt/baseline.profm
   32-bit CRC value (hex):                         8e940fcb
   classes.dex
-  32-bit CRC value (hex):                         dd04bfa7
+  32-bit CRC value (hex):                         340bb292
   classes2.dex
   32-bit CRC value (hex):                         94d9bdfb

Dex diff:

--- /tmp/tmp.hdn1Wgyj7b 2024-08-20 09:20:29.244595223 +0200
+++ /tmp/tmp.gSEzKuyHo5 2024-08-20 09:20:30.000590311 +0200
@@ -1,9 +1,9 @@
-Processing '/tmp/tmp.O1dC637WPI'...
-Opened '/tmp/tmp.O1dC637WPI', DEX version '035'
+Processing '/tmp/tmp.paR2Swn4qp'...
+Opened '/tmp/tmp.paR2Swn4qp', DEX version '035'
 DEX file header:
 magic               : 'dex\n035\0'
-checksum            : 7c013042
-signature           : e2b4...6e08
+checksum            : bf3e2ec6
+signature           : 9f68...e55c
 file_size           : 3756724
 header_size         : 112
 link_size           : 0
@@ -465176,10 +465176,10 @@
 |: iget-object v4, v3, LJ2/d;.b:LK2/d;
 |: check-cast v4, LZ1/a;
 |: invoke-virtual {v4}, Ljava/lang/Object;.getClass:()Ljava/lang/Class;
-|: const-string v4, "CREATE TABLE Use(\n    date TEXT NOT NULL,\n    amount_grams TEXT NOT NULL,\n    cost_per_gram TEXT NOT NULL,\n    id TEXT PRIMARY KEY\n)"
-|: invoke-virtual {v0, v2, v4, v2}, LJ2/i;.b:(Ljava/lang/Integer;Ljava/lang/String;Lc3/c;)V
 |: const-string v4, "CREATE TABLE Pause(\n  start_time TEXT NOT NULL,\n  end_time TEXT NOT NULL,\n  id TEXT PRIMARY KEY,\n  is_enabled INTEGER DEFAULT 1\n)"
 |: invoke-virtual {v0, v2, v4, v2}, LJ2/i;.b:(Ljava/lang/Integer;Ljava/lang/String;Lc3/c;)V
+|: const-string v4, "CREATE TABLE Use(\n    date TEXT NOT NULL,\n    amount_grams TEXT NOT NULL,\n    cost_per_gram TEXT NOT NULL,\n    id TEXT PRIMARY KEY\n)"
+|: invoke-virtual {v0, v2, v4, v2}, LJ2/i;.b:(Ljava/lang/Integer;Ljava/lang/String;Lc3/c;)V
 |: return-void

Looks like different ordering, but I don't know how that happens or how to cure it. As the ex differs, so does the baseline.prof as it contains the hashes of the Dex files.

Just to make sure: Your APK was built from a clean tree at the commit the tag points to, right? And in case it matters, I've used ./gradlew assembleGithubRelease to build the APK here – which seems to match your workflow here.

@LeoColman LeoColman reopened this Aug 20, 2024
@LeoColman
Copy link
Owner Author

Your assumption is correct.

Could GH actions cache interfere with this? I'm not even sure I have them on 😅

Could it be due to SQL delight? Maybe it's not deterministic by default. I see that one of the out of order lines is related to it

@LeoColman
Copy link
Owner Author

Is there a way for me to easily check it locally?

"Generate it twice and run the checks"

@LeoColman LeoColman removed the Good First Issue Defines a good issue for a new contributor to try label Aug 20, 2024
@LeoColman
Copy link
Owner Author

@IzzySoft
Copy link

Apologies, I was too slow 🙈 Yeah, after you first mentioned it, I thought it might: the diff is about SQL statements ordered differently. Let me cc @obfusk as this could be interesting for her as well (Fay is our RB expert).

Nice finds of the other issues, too, thanks for the pointers! So do you have a plan there already as well?

@IzzySoft
Copy link

Is there a way for me to easily check it locally?

Sure. The tool we use is FOSS, and Fay is its author: https://github.com/obfusk/rbtlog (especially look at obfusk/rbtlog#471 (comment) 😉)

@LeoColman
Copy link
Owner Author

According to someone in the main issue I sent, this problem is gone in library version 2.x

I'll try to upgrade to that version given it's stable and see if the problem is gone.

Otherwise I have no plan

@obfusk
Copy link

obfusk commented Aug 20, 2024

I might be able to think of a workaround. But upgrading to a fixed version would of course be the best fix :)

@LeoColman
Copy link
Owner Author

@obfusk and @IzzySoft
https://gitlab.com/walletscrutiny/walletScrutinyCom/-/issues/197#note_543234399

It appears to be a very weird workaround to get SQLDelight to generate the same APK twice.

I think it's not worthy as a solution for my project, but for research reasons you might want to take a look at this

@LeoColman
Copy link
Owner Author

LeoColman commented Aug 21, 2024

sqldelight/sqldelight#1548 (comment)
This comment shows that the issue is fixed on the latest releases.

This makes it unlikely to be fixed in 1.x version. This fork seems to do it, but I didn't look into it https://github.com/muun/sqldelight/tree/reproducible-builds

@obfusk
Copy link

obfusk commented Aug 21, 2024

Yeah, disorderfs is one of the tools of the Reproducible Builds project, where I'm a contributor. I've used it before :)

I think there might be an easier (but ugly) workaround: putting each .sq(m) file in a separate directory and specifying all of those -- in a deterministic order -- using sourceFolders (can do that with a fileTree().visit {} or similar, as long as you sort the results, if you don't want to hardcode it). But I haven't tested that.

@LeoColman
Copy link
Owner Author

@IzzySoft
Let's see if the version upgrade really takes the issue away:

https://github.com/LeoColman/Petals/releases/tag/3.28.1

@IzzySoft
Copy link

It pretty much looks like, congrats!

    "upstream_signed_apk_sha256": "2364cac58c0ab3d9624d8439526214858e89da6f8d911c0bee2ba391948e96a1",
    "built_unsigned_apk_sha256": "9d71a2eb551d7cb66ace6ad87c7e8a014782471145d441e71ba49d70c75e9919",
    "signature_copied_apk_sha256": "2364cac58c0ab3d9624d8439526214858e89da6f8d911c0bee2ba391948e96a1"

Reproducible: True

Now creating the logs – and with the next sync, your app will carry the green shield at its latest release 🥳

@LeoColman
Copy link
Owner Author

Wohoo!
Thanks for all the partnership on this one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Fixing a dependency, code-cleaning
Projects
None yet
Development

No branches or pull requests

3 participants