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 can't build #6455

Closed
3 tasks done
licaon-kter opened this issue Sep 6, 2024 · 40 comments · Fixed by #6471
Closed
3 tasks done

F-Droid can't build #6455

licaon-kter opened this issue Sep 6, 2024 · 40 comments · Fixed by #6471
Labels
enhancement New feature or request

Comments

@licaon-kter
Copy link

licaon-kter commented Sep 6, 2024

Describe your suggested feature

ref: https://gitlab.com/fdroid/fdroiddata/-/jobs/7760790503#L379

Other details

No response

Acknowledgements

  • I have searched the existing issues and this is a new ticket, NOT a duplicate or related to another open issue.
  • I have written a short but informative title.
  • I will fill out all of the requested information in this form.

/LE: fyi https://gitlab.com/fdroid/fdroiddata/-/commit/f3e76bdabb388c9a7a197388af5222bb01d8a91c

@licaon-kter licaon-kter added the enhancement New feature or request label Sep 6, 2024
@Bnyro
Copy link
Member

Bnyro commented Sep 6, 2024

According to TeamNewPipe/NewPipeExtractor#1066, the dependency might need to be lowercased to ...teamnewpipe:NewPipeExtractor... instead of ...TeamNewpipe:NewPipeExtractor..., although it worked fine for me the other way yesterday, probably due to dependency caching.

In CI (gh actions) of this repository, it sometimes passes and sometimes fails with the error you mentioned, I love deterministic build failures...

I hope I can find time for looking at it tomorrow.

@Bnyro
Copy link
Member

Bnyro commented Sep 8, 2024

Can you please try building from the latest commit (1d4304c)?

This works for me now after clearing the IDEA caches.

@licaon-kter
Copy link
Author

@Bnyro 1d4304c builds fine locally

@licaon-kter
Copy link
Author

@Bnyro
Copy link
Member

Bnyro commented Nov 18, 2024

LibreTube has used that dependency for 2 years now, I don't see the issue?

@licaon-kter
Copy link
Author

licaon-kter commented Nov 18, 2024

New scanner code picked it up.

Just exclude does not make that dep FOSS, when it was built from non-FOSS code.

@IzzySoft
Copy link

@Bnyro for what it's worth, I try to build the versions since 1.25.1 and fail with the same error. With v0.26.0 it is

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:mergeReleaseNativeLibs'.
> Could not resolve all files for configuration ':app:releaseRuntimeClasspath'.
   > Could not find com.github.teamnewpipe:NewPipeExtractor:6e3a4a6d9de61eafb73e7eb1b714847b5077856d.
     Searched in the following locations:
       - https://dl.google.com/dl/android/maven2/com/github/teamnewpipe/NewPipeExtractor/6e3a4a6d9de61eafb73e7eb1b714847b5077856d/NewPipeExtractor-6e3a4a6d9de61eafb73e7eb1b714847b5077856d.pom
       - https://repo.maven.apache.org/maven2/com/github/teamnewpipe/NewPipeExtractor/6e3a4a6d9de61eafb73e7eb1b714847b5077856d/NewPipeExtractor-6e3a4a6d9de61eafb73e7eb1b714847b5077856d.pom
       - https://jitpack.io/com/github/teamnewpipe/NewPipeExtractor/6e3a4a6d9de61eafb73e7eb1b714847b5077856d/NewPipeExtractor-6e3a4a6d9de61eafb73e7eb1b714847b5077856d.pom
     Required by:
         project :app

Not sure if maybe switching to the latest release including that commit (v0.24.3 was release 3 days ago) might solve that issue?

@Bnyro
Copy link
Member

Bnyro commented Nov 27, 2024

Not sure if maybe switching to the latest release including that commit (v0.24.3 was release 3 days ago) might solve that issue?

Indeed it might be a good idea to switch to their latest stable release now, we previously required an unreleased version due to breaking changes from YouTube.

@Bnyro
Copy link
Member

Bnyro commented Nov 27, 2024

New scanner code picked it up.

Just exclude does not make that dep FOSS, when it was built from non-FOSS code.

Thus we either need to drop that dependency or get the Upstream Non-Free anti-feature, right?

@IzzySoft
Copy link

switch to their latest stable release

If you have a commit with that, and an APK built from that commit, feel free to give me a ping to check for RB again (that was why I tried to build, see Reproducible Builds, special client support and more at IzzyOnDroid – currently 354 apps (29.1%) are RB here already).

@licaon-kter
Copy link
Author

@Bnyro drop ye

UpstreamNonFree means that upstream has non free but F-Droid has all free because it cleans up the non-free before building

There's never a non-free to build from.

@Bnyro
Copy link
Member

Bnyro commented Nov 28, 2024

@IzzySoft 2d66a6a

@IzzySoft
Copy link

Thanks @Bnyro! If you have an APK built from that commit with your workflow, I can check if it's RB if you want (so we're prepared for the next release and can have the "green shield up").

@Bnyro
Copy link
Member

Bnyro commented Nov 28, 2024

Sure, I'll send you a build this evening 👍

@Bnyro
Copy link
Member

Bnyro commented Nov 28, 2024

The universal apk: app-universal-release.zip, built from 2d66a6a.

@IzzySoft
Copy link

That was not build from the commit you specified, @Bnyro 😉

  -rw-r--r--  0.0 unx       56 b-       51 defN 1981-01-01 01:01:02 a18242e2 META-INF/com/android/build/gradle/app-metadata.properties
- -rw-r--r--  0.0 unx      120 b-      118 defN 1981-01-01 01:01:02 fd28c097 META-INF/version-control-info.textproto
- -rw-r--r--  0.0 unx    11663 b-    11663 stor 1981-01-01 01:01:02 eaa2c9cd assets/dexopt/baseline.prof
+ -rw-r--r--  0.0 unx      120 b-      118 defN 1981-01-01 01:01:02 d47bfabf META-INF/version-control-info.textproto
+ -rw-r--r--  0.0 unx    11656 b-    11656 stor 1981-01-01 01:01:02 ee9c069c assets/dexopt/baseline.prof
  -rw-r--r--  0.0 unx      292 b-      292 stor 1981-01-01 01:01:02 227b7cba assets/dexopt/baseline.profm
  -rw-r--r--  0.0 unx  2422680 b-  1141281 defN 1981-01-01 01:01:02 57021890 classes.dex
  -rw-r--r--  0.0 unx   673128 b-   286620 defN 1981-01-01 01:01:02 58f32d66 classes2.dex
- -rw-r--r--  0.0 unx  5822264 b-  2621671 defN 1981-01-01 01:01:02 4f352f35 classes3.dex
+ -rw-r--r--  0.0 unx  5822272 b-  2621689 defN 1981-01-01 01:01:02 230e9716 classes3.dex
  -rw-r--r--  0.0 unx  5019616 b-  2382937 defN 1981-01-01 01:01:02 c9aacb69 lib/arm64-v8a/libcronet.119.0.6045.31.so

it was built from 9458b55 according to META-INF/version-control-info.textproto. But I'm flexible, so let me try with that commit again…

  -rw-r--r--  0.0 unx      120 b-      118 defN 1981-01-01 01:01:02 fd28c097 META-INF/version-control-info.textproto
- -rw-r--r--  0.0 unx    11663 b-    11663 stor 1981-01-01 01:01:02 eaa2c9cd assets/dexopt/baseline.prof
+ -rw-r--r--  0.0 unx    11656 b-    11656 stor 1981-01-01 01:01:02 ee9c069c assets/dexopt/baseline.prof
  -rw-r--r--  0.0 unx      292 b-      292 stor 1981-01-01 01:01:02 227b7cba assets/dexopt/baseline.profm
  -rw-r--r--  0.0 unx  2422680 b-  1141281 defN 1981-01-01 01:01:02 57021890 classes.dex
  -rw-r--r--  0.0 unx   673128 b-   286620 defN 1981-01-01 01:01:02 58f32d66 classes2.dex
- -rw-r--r--  0.0 unx  5822264 b-  2621671 defN 1981-01-01 01:01:02 4f352f35 classes3.dex
+ -rw-r--r--  0.0 unx  5822272 b-  2621689 defN 1981-01-01 01:01:02 230e9716 classes3.dex
  -rw-r--r--  0.0 unx  5019616 b-  2382937 defN 1981-01-01 01:01:02 c9aacb69 lib/arm64-v8a/libcronet.119.0.6045.31.so

Marginally better. Let's check the Dex… Hm, strange. OK, according to your Github workflows you build on Ubuntu 24 using JDK-17. I've been building on Debian bookworm with JDK-17. Should be close enough, but wouldn't be the first time this made a difference – so let me make another run on Ubuntu. We have 22.04 here (ubuntu:jammy), giving the same results as the Debian Bookworm run. Let me try ubuntu:noble (same results), then I'm out asking if you maybe use something other than your workflow says, e.g. JDK-21? OK, no dice either, gives the same hash.

Looking closer at the dex diff, it doesn't really look like something related to a specific JDK or OS mismatch. I'm attaching it here for you, maybe it gives you a better idea than it gives me: dex.diff.gz

@IzzySoft
Copy link

PS: a guess would be a "dirty tree" (local changes) or left-over artifacts from previous build. Can we exclude those here?

@Bnyro
Copy link
Member

Bnyro commented Nov 28, 2024

That was not build from the commit you specified, @Bnyro 😉

Oh, I didn't think a merge commit would matter 😌

I'm building releases on my machine locally, not via workflows, but still I'm using OpenJDK 17.

Possibly, as you said, there might be some leftovers from previous builds, I'll try my luck again when I get to have time (sorry for possible delays).

@IzzySoft
Copy link

No prob. When building locally, it's a good idea to insert clean for "release builds" (and those I shall test for RB), e.g. ./gradlew clean assembleRelease – or hitting "clean" before "build" in Studio if you're a cat chasing your mouse 😜 As it sometimes matters, maybe also mention which JDK (OpenJDK preferred) you're using and on which OS you're building. Plus of course any other "specific" things there might be (e.g. using a different NDK or whatever).

@Bnyro
Copy link
Member

Bnyro commented Dec 2, 2024

@IzzySoft same commit as before (9458b55), built on Void Linux using OpenJDK 17 and a "cleaned" build environment.
app-universal-release.zip

@IzzySoft
Copy link

IzzySoft commented Dec 2, 2024

    "upstream_signed_apk_sha256": "0312167fd80c653e4f5cb16f53cfbdd9fdc07b4223a2b6487055e1ef827858ac",
    "built_unsigned_apk_sha256": "69ff7344aacd6654d2d6728c5b62e95efeb786eda45b78fef27de36cd0e976df",
    "signature_copied_apk_sha256": "0312167fd80c653e4f5cb16f53cfbdd9fdc07b4223a2b6487055e1ef827858ac"

That means RB 🥳 Congrats!

PS: Please let me know when it's available as release, so I establish it with my builder here. Thanks!

@Bnyro
Copy link
Member

Bnyro commented Dec 2, 2024

That means RB 🥳 Congrats!

Awesome, thanks for your guidance!

@Bnyro Bnyro mentioned this issue Dec 8, 2024
3 tasks
@FireMasterK
Copy link
Member

New scanner code picked it up.

Just exclude does not make that dep FOSS, when it was built from non-FOSS code.

There are alternative cronet implementations that are ABI compatible and FOSS. This is the cronet-embedded dependency. Why does this matter when the resulting binary doesn't use any non-free code? The non-free code is used at compile-time and never at runtime.

https://github.com/androidx/media/blob/1a7da45dd9c6419a43fab95132ed0aa0999300aa/libraries/datasource_cronet/build.gradle#L27
I'm sure this line patched to use the cronet-embedded dependency and the resulting binary would still be the same, so why do we have this limitation?

@licaon-kter
Copy link
Author

Ask yourself this: can you build that lib without that non-FOSS code? If not... it's not FOSS

@FireMasterK
Copy link
Member

Ask yourself this: can you build that lib without that non-FOSS code? If not... it's not FOSS

Yes you can, I just did this in my fork to test this: https://github.com/FireMasterK/LibreTube/tree/media3-cronet

@licaon-kter
Copy link
Author

@FireMasterK that's not what I've asked 🤷

but thanks for providing a solution maybe 👍

@Bnyro
Copy link
Member

Bnyro commented Jan 24, 2025

@IzzySoft v0.27.0 was released just now: https://github.com/libre-tube/LibreTube/releases/tag/v0.27.0 🎉
I really hope everything is still reproducible, that'd be awesome!

@licaon-kter Cronet was removed with v0.27.0 (#7015), I assume the F-Droid builders will automatically continue updating the app now?

@IzzySoft
Copy link

@Bnyro thanks! RB confirmed for all 3 APKs, update pulled in manually already (well, triggered manually, framework took care for the rest). But… could you please take care for that blob?

Image

Easily achieved with a minor addition to your build.gradle:

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains. More details can be found e.g. here: Ramping up security: additional APK checks are in place with the IzzyOnDroid repo.

Thanks in advance!

@IzzySoft
Copy link

for all 3 APKs

What's the difference between them, now that the native libs have been dropped? Hint: None it seems. There I was wondering why I only found 2 APK files when expecting 6 (2 for each ABI, your build and ours). Cannot tell for the x86 as we've skipped them here, but both ARM + universal are identical.

@Bnyro
Copy link
Member

Bnyro commented Jan 25, 2025

What's the difference between them, now that the native libs have been dropped? Hint: None it seems. There I was wondering why I only found 2 APK files when expecting 6 (2 for each ABI, your build and ours). Cannot tell for the x86 as we've skipped them here, but both ARM + universal are identical.

Yes, chances are high that they're the same now that Cronet has been dropped.

However I know some people that have their Obtainium configured to download APKs from GitHub releases and probably some of them picked specific architectures, thus I'd prefer to continue publishing all the individual APKs for user convenience.

@Bnyro
Copy link
Member

Bnyro commented Jan 25, 2025

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains. More details can be found e.g. here: Ramping up security: additional APK checks are in place with the IzzyOnDroid repo.

It's crazy that Google includes such blobs by default in your APKs, I couldn't have guessed that (but yeah, it's Google...).
Thanks for the hint, I've done as you supposed 👍
I assume it's enough to just include this change in the next release, or do you want me to build new APKs?

@IzzySoft
Copy link

Yes, chances are high that they're the same now that Cronet has been dropped.

They are byte-identical. Run sha256sum, all 5 give the same. So I'll add a note to the recipe in my builder (FIXME, which prevents it from being automatically updated but instead "yells for adjustments") to drop the "additional ABIs", just keeping the universal one. As matching is done on the sha256sum, that would still cover all builds – while avoiding unnecessary additional builds on our end, thus saving resources. I guess that should be OK with you (as nobody would notice any difference anyhow except for the "missing" blocks when looking at our recipes/logs)?

However I know some people that have their Obtainium configured to download APKs from GitHub releases and probably some of them picked specific architectures, thus I'd prefer to continue publishing all the individual APKs for user convenience.

Fine with me. The IoD repo is pinned to the "universal" – but even if it would pick one of the other APKs, that would be fine as only their names differ 😉

It's crazy that Google includes such blobs by default in your APKs

Yeah, in a way. But remember they consider themselves to be the only trusted source ("thou shalt not have other stores beside me") – and their "PlayProtect" throws a big alert on (some) installs from other sources just because of them not being in their toy shop…

Thanks for the hint, I've done as you supposed 👍

Thanks! 🤗

I assume it's enough to just include this change in the next release

Yes, of course!

or do you want me to build new APKs?

Let me answer that with one of "my standard rules": "Never replace what has already been distributed." The current APKs are already "out in the wild": in the IoD repo, and also in multiple of our builders. None of those would fetch APKs from the same release again, as that's already marked as "we have this". So whenever an urgent replacement is needed, rather make a new (maintenance) release. Only if there are security implications (or similar strong reasons), remove the APKs from the affected release and make a new one.

Thanks a lot!

@Bnyro
Copy link
Member

Bnyro commented Jan 25, 2025

They are byte-identical. Run sha256sum, all 5 give the same. So I'll add a note to the recipe in my builder (FIXME, which prevents it from being automatically updated but instead "yells for adjustments") to drop the "additional ABIs", just keeping the universal one. As matching is done on the sha256sum, that would still cover all builds – while avoiding unnecessary additional builds on our end, thus saving resources. I guess that should be OK with you (as nobody would notice any difference anyhow except for the "missing" blocks when looking at our recipes/logs)?

Yes, of course!

Let me answer that with one of "my standard rules": "Never replace what has already been distributed." The current APKs are already "out in the wild": in the IoD repo, and also in multiple of our builders. None of those would fetch APKs from the same release again, as that's already marked as "we have this". So whenever an urgent replacement is needed, rather make a new (maintenance) release. Only if there are security implications (or similar strong reasons), remove the APKs from the affected release and make a new one.

Sure, I didn't mean to replace the current APKs :)
I was just wondering if you needed some new builds for proofing reproducibility of the builds now that these blobs are removed, or whether your apk diff tool can live with it until the next release :)
That doesn't seem to be the case though 😉

Thanks for all your help, especially with reproducible builds, very much appreciated ❤

@IzzySoft
Copy link

I'd say we just wait for the next release, @Bnyro – feel free to give me a ping then to confirm all is fine. And thanks for your help, too! 🤗

@licaon-kter
Copy link
Author

licaon-kter commented Jan 27, 2025

@licaon-kter
Copy link
Author

can we reopen?

@Bnyro
Copy link
Member

Bnyro commented Jan 27, 2025

I'm a bit confused at the "drop cronet" text

Indeed I removed it everywhere in the app's code and 2 cronet dependencies, only missed the data source. Removed with the latest commit.

@licaon-kter
Copy link
Author

licaon-kter commented Jan 27, 2025

So you say I can sed these lines as not needed so we can proceed with this version too?

@Bnyro
Copy link
Member

Bnyro commented Jan 27, 2025

So you say I can sed these lines as not needed so we can proceed with this version too?

Yeah, true

@licaon-kter
Copy link
Author

Done, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants