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

F-Droid reproduceable build failed #1639

Closed
linsui opened this issue Apr 21, 2022 · 31 comments · Fixed by #1933 or #2558
Closed

F-Droid reproduceable build failed #1639

linsui opened this issue Apr 21, 2022 · 31 comments · Fixed by #1933 or #2558
Labels
bug Something isn't working

Comments

@linsui
Copy link

linsui commented Apr 21, 2022

Describe the bug

https://monitor.f-droid.org/builds/log/org.jellyfin.androidtv/130399#site-footer It built fine in the CI. Not sure why it failed on the build server. Could you please take a look? Thanks!

Logs

https://monitor.f-droid.org/builds/log/org.jellyfin.androidtv/130399#site-footer

Application version

130399

Where did you install the app from?

No response

Device information

None

Android version

None

Jellyfin server version

None

@linsui linsui added the bug Something isn't working label Apr 21, 2022
@sourpatched
Copy link

Seems like it could be due to APK Signature that's causing a mismatch?

F-Drod is more for mobile then ATV but in saying that, it's best to have an app be more accessible.

@nielsvanvelzen
Copy link
Member

Issue might be fixed in the v0.13.4 release.

@linsui
Copy link
Author

linsui commented May 8, 2022

Still not verified.

@linsui
Copy link
Author

linsui commented May 31, 2022

There are two different files.

Unexpected diff output:
Binary files /tmp/tmpn4i7qii7/unsigned_binaries_org.jellyfin.androidtv_130699.binary/content/assets/dexopt/baseline.prof and /tmp/tmpn4i7qii7/_tmp_tmpn4i7qii7_sigcp_org.jellyfin.androidtv_130699/content/assets/dexopt/baseline.prof differ
Binary files /tmp/tmpn4i7qii7/unsigned_binaries_org.jellyfin.androidtv_130699.binary/content/classes3.dex and /tmp/tmpn4i7qii7/_tmp_tmpn4i7qii7_sigcp_org.jellyfin.androidtv_130699/content/classes3.dex differ

@licaon-kter
Copy link

@nielsvanvelzen
Copy link
Member

Thanks for linking the job, I think I know why it fails now. The AboutLibraries plugin we use generates a JSON file with all licenses and adds a timestamp from when it generated the file. That timestamp obviously differs between our build and F-Droid's build. Will see what I can do to fix.

@nielsvanvelzen
Copy link
Member

Issue should be fixed in the next release (0.14.1)

@licaon-kter
Copy link

Still is failing locally: 2 files differ, can you dissect the APK? (remove .zip at the end of the filenames and use 7zip to unpack)

jtv.7z.001.zip
jtv.7z.002.zip
jtv.7z.003.zip
jtv.7z.004.zip

ref: https://gitlab.com/linsui/fdroiddata/-/commit/62ec23b72ffea96297c82641fbbfa85f162b7b1e#note_1100650641

@linsui
Copy link
Author

linsui commented Jan 9, 2023

With

tasks.whenTaskAdded {
    if(name.contains("ArtProfile")) {
        enabled = false
    }
}

we can disable the prefile. Not sure if it affects the performance.

@nielsvanvelzen
Copy link
Member

I'm quite sure that disabling ART will give us a performance penalty, so we shouldn't disable it. If that's the problem in creating a reproducible build I think our only choice is to not use reproducible builds for now.

@obfusk
Copy link

obfusk commented Jan 10, 2023

I'm quite sure that disabling ART will give us a performance penalty, so we shouldn't disable it.

It doesn't disable the Android Runtime (ART), just baseline profiles.

According to the docs:

Baseline Profiles improve code execution speed by around 30% from the first launch by avoiding interpretation and just-in-time (JIT) compilation steps for included code paths.

So it may very well indeed result in a performance penalty, but I have found no real-world data on whether 30% is accurate or not. And all the docs say that developers need to define these profiles, but I don't see e.g. a baseline-prof.txt in your repo, so I have no idea what the baseline.prof{,m} is currently based on and whether it's actually doing anything useful. Do you have any info on that?

@linsui
Copy link
Author

linsui commented Jan 10, 2023

Could you please test the apk build without the profile?

@linsui
Copy link
Author

linsui commented Jan 10, 2023

Every time it produces a different apk so the profile part is not reproducible at all.

@linsui
Copy link
Author

linsui commented Jan 11, 2023

@licaon-kter got the 0.14.3 version build reproduciblly by limiting it to use only 1 core.

@nielsvanvelzen Do you use more cores for newer versions? Could you please build the latest version with 1 core?

@nielsvanvelzen
Copy link
Member

nielsvanvelzen commented Jan 11, 2023

All releases are build with GitHub actions. According to the documentation these are the specs:

  • 2-core CPU (x86_64)
  • 7 GB of RAM
  • 14 GB of SSD space

I believe they used two cores from the start. Our 0.14 releases also used this release pipeline.

@linsui
Copy link
Author

linsui commented Jan 16, 2023

@obfusk's https://github.com/obfusk/reproducible-apk-tools#sort-baselinepy can fix the profm problem. You also need to run the script before sign the apk.

@obfusk
Copy link

obfusk commented Jan 20, 2023

You can also sort the baseline.profm file by adding something like this to build.gradle:

import com.android.tools.profgen.ArtProfileKt
import com.android.tools.profgen.ArtProfileSerializer
import com.android.tools.profgen.DexFile

project.afterEvaluate {
    tasks.compileReleaseArtProfile.doLast {
        outputs.files.each { file ->
            if (file.toString().endsWith(".profm")) {
                println("Sorting ${file} ...")
                def version = ArtProfileSerializer.valueOf("METADATA_0_0_2")
                def profile = ArtProfileKt.ArtProfile(file)
                def keys = new ArrayList(profile.profileData.keySet())
                def sortedData = new LinkedHashMap()
                Collections.sort keys, new DexFile.Companion()
                keys.each { key -> sortedData[key] = profile.profileData[key] }
                new FileOutputStream(file).with {
                    write(version.magicBytes$profgen)
                    write(version.versionBytes$profgen)
                    version.write$profgen(it, sortedData, "")
                }
            }
        }
    }
}

@nielsvanvelzen
Copy link
Member

Our Gradle build files use Kotlin and it looks like the compiler is slightly more strict. We can't use the properties/functions on ArtProfileSerializer because their visibility is internal.

@obfusk
Copy link

obfusk commented Jan 24, 2023

We can't use the properties/functions on ArtProfileSerializer because their visibility is internal.

I was afraid it would not work with build.gradle.kts because of the visibility, but thanks for confirming 😅

You might be able to put it in a separate .gradle file (e.g. fix-profm.grade) and use apply(from = "fix-profm.gradle") from build.gradle.kts, but I haven't tested this.

@obfusk
Copy link

obfusk commented Jan 24, 2023

A quick test suggest it will fail to find the required imports via apply(from = ...); but perhaps you can make it work?

FWIW, using the python script to sort instead of com.android.tools.profgen should work with Kotlin.

@obfusk
Copy link

obfusk commented Jan 24, 2023

You can also wait for a new AGP version with the fix, but that might take a while.

@obfusk
Copy link

obfusk commented Jan 24, 2023

You can put this in app/fix-profm.gradle:

buildscript {
    repositories {
        google()
        mavenCentral()
    }
    dependencies {
        // NB: use this with gradle/libs.versions.toml (and modify as needed):
        classpath "com.android.tools.build:gradle:${libs.versions.android.gradle.get()}"
    }
}

import com.android.tools.profgen.ArtProfileKt
import com.android.tools.profgen.ArtProfileSerializer
import com.android.tools.profgen.DexFile

project.afterEvaluate {
    tasks.compileReleaseArtProfile.doLast {
        outputs.files.each { file ->
            if (file.toString().endsWith(".profm")) {
                println("Sorting ${file} ...")
                def version = ArtProfileSerializer.valueOf("METADATA_0_0_2")
                def profile = ArtProfileKt.ArtProfile(file)
                def keys = new ArrayList(profile.profileData.keySet())
                def sortedData = new LinkedHashMap()
                Collections.sort keys, new DexFile.Companion()
                keys.each { key -> sortedData[key] = profile.profileData[key] }
                new FileOutputStream(file).with {
                    write(version.magicBytes$profgen)
                    write(version.versionBytes$profgen)
                    version.write$profgen(it, sortedData, "")
                }
            }
        }
    }
}

And add this to app/build.gradle.kts:

apply(from = "fix-profm.gradle")

And it should work.

@nielsvanvelzen nielsvanvelzen changed the title F-Droid reproduciable build failed F-Droid reproduceable build failed Feb 3, 2023
@nielsvanvelzen
Copy link
Member

nielsvanvelzen commented Mar 3, 2023

Thanks @obfusk, I've applied this workaround in #2558 and will backport it to our next patch release which will likely be on the 12th (edit: or earlier). @linsui do I need to make changes to f-droid data after the release or will you deal with that part?

@linsui
Copy link
Author

linsui commented Mar 4, 2023

You can open an MR or ping me when it's ready.

@nielsvanvelzen
Copy link
Member

@linsui https://github.com/jellyfin/jellyfin-androidtv/releases/tag/v0.15.5 includes the workaround for reproducible builds. Let me know if this issue persists or something else comes up. Thanks for the report!

@licaon-kter
Copy link

A quick test did fail to Repro :(

@nielsvanvelzen
Copy link
Member

Hmm weird, I tested it by creating multiple builds locally and using diff -r to see if there were binary differences and it worked after the pull request. I guess I'll need to do some more testing later. Was the issue still in the baseline files in your quick test?

@licaon-kter
Copy link

licaon-kter commented Mar 14, 2023

It's the "number of CPUs" issue, will test 1/2/4/etc cores and adapt the recipe.
see below

@licaon-kter
Copy link

Ok, builds fine, I've missed that it needs Java17 now 🥳

@nielsvanvelzen
Copy link
Member

Awesome, thanks for checking! Glad to hear it's working now.

@licaon-kter
Copy link

Done https://gitlab.com/fdroid/fdroiddata/-/commit/260ab2dd85fe5739476f756b6233ae556d36344e

Thanks for sticking with it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants