search

devhyper / open-video-editor

Open source Android video editor, built with Media3 and Jetpack Compose.
GNU General Public License v3.0
284 stars 12 forks source link

F-Droid can't build #63

Closed licaon-kter closed 1 month ago

licaon-kter commented 3 months ago

ref: https://gitlab.com/fdroid/fdroiddata/-/jobs/6417503136#L426

while the APK content files of CI (https://gitlab.com/fdroid/fdroiddata/-/jobs/6417503136/artifacts/file/tmp/io.github.devhyper.openvideoeditor_5.apk) and your release (https://github.com/devhyper/open-video-editor/releases/tag/v1.1.1) are identical, the actual structure of the archive is padded very different.

Did you change some tooling, the way you build your APKs?

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

devhyper commented 3 months ago

Only changes were updating Android Studio and the build tools to the latest version, and adding the ffmpeg-kit dependency. Could these have something to do with that?

mario0318 commented 3 months ago

Only changes were updating Android Studio and the build tools to the latest version, and adding the ffmpeg-kit dependency. Could these have something to do with that?

Is this also the reason the latest apk release is massive compared to earlier ones?

licaon-kter commented 3 months ago

I keep rebuilding...

reading https://github.com/obfusk/reproducible-apk-tools?tab=readme-ov-file#inplace-fixpy "NB: build-tools 31.0.0 and 32.0.0 are skipped because their zipalign is broken; --page-size requires build-tools >= 35.0.0-rc1." made me think that this might be the issue somehow, because the difference is massive padding in the APK

@mario0318 fyi ^^^ not libs differences

@devhyper wonder how many build-tools do you have installed and which ones?

devhyper commented 3 months ago

Only changes were updating Android Studio and the build tools to the latest version, and adding the ffmpeg-kit dependency. Could these have something to do with that?

Is this also the reason the latest apk release is massive compared to earlier ones?

Size difference is because of the addition of ffmpeg, I swapped to the min version so the next release will be 10 MB less.

devhyper commented 3 months ago

I keep rebuilding...

reading https://github.com/obfusk/reproducible-apk-tools?tab=readme-ov-file#inplace-fixpy "NB: build-tools 31.0.0 and 32.0.0 are skipped because their zipalign is broken; --page-size requires build-tools >= 35.0.0-rc1." made me think that this might be the issue somehow, because the difference is massive padding in the APK

@mario0318 fyi ^^^ not libs differences

@devhyper wonder how many build-tools do you have installed and which ones?

Looks like I have 34.0.0 installed, the build has always used that since that was the minimum version required for the android gradle plugin. Turns out the updated 35 rc was not used.

devhyper commented 3 months ago

Just realized that gradle was updated from 8.2.1 to 8.4, that is probably it.

devhyper commented 3 months ago

Building with gradle 8.2.1 changes the apk size slightly, but the hashes are still different, probably because fdroid built it with gradle 8.4 as well. Is there any way for me to verify if this fixed it? @licaon-kter

licaon-kter commented 3 months ago

It's easier to just attach an APK and point me to a commit so I can check.

devhyper commented 3 months ago

d0801dc app-release.zip

licaon-kter commented 3 months ago

Same error, but oddly the APKs are 20Mb smaller now, wonder why...

devhyper commented 3 months ago

Apks are smaller because I changed ffmpeg to min version. I think I need to add 35 rc to the build file.

devhyper commented 3 months ago

130152e app-release.zip Added build tools 35.0.0 rc2 to build.gradle, the warning should disappear now. @licaon-kter

devhyper commented 3 months ago

@licaon-kter

licaon-kter commented 3 months ago

Didn't got a chance to retry, will do asap

linsui commented 2 months ago

1.1.2 has the same problem.

linsui commented 1 month ago

https://gitlab.com/fdroid/fdroiddata/-/merge_requests/15112 Fixed.

obfusk commented 1 month ago

https://gitlab.com/fdroid/fdroiddata/-/merge_requests/15112 Fixed.

Wow. You still didn't realise you've simply been calling inplace-fix.py with the wrong parameters all this time?

You could have just replaced the incorrect use of --zipalign with --page-align (or --page-size 16 for v1.1.1 since it used AGP >= 8.3 before it was downgraded again in v1.1.2 and the alignment changed back to 4K pages) months ago instead of taking ages to come up with that hack to work around the fact you've been calling my RB tools with the wrong parameters and thus getting the ZIP alignment wrong on your end all this time. I guess F-Droid could use an RB expert.

linsui commented 1 month ago

You could have just replaced the incorrect use of --zipalign with --page-align (or --page-size 16 for v1.1.1 since it used AGP >= 8.3 before it was downgraded again in v1.1.2 and the alignment changed back to 4K pages) months ago instead of taking ages to come up with that hack to work around the fact you've been calling my RB tools with the wrong parameters and thus getting the ZIP alignment wrong on your end all this time.

Tried with 16 and 64 but it doesn't work.

I guess F-Droid could use an RB expert.

No need to guess since you were the only RB expert.

obfusk commented 1 month ago

Tried with 16 and 64 but it doesn't work.

Try 4. It works.

linsui commented 1 month ago

Trying now.

obfusk commented 1 month ago

And 4 has been the default for ages (AGP < 8.3). You just need to actually use page alignment by passing --page-align or --page-size, which you haven't been doing.

linsui commented 1 month ago

https://gitlab.com/fdroid/fdroiddata/-/merge_requests/15113 Fixed, thanks!

You just need to actually use page alignment by passing --page-align or --page-size, which you haven't been doing.

I don't know this arg before yesterday and I just realize that page align is not enabled by default. :shrug:

licaon-kter commented 1 month ago

I've tested align 4 for https://github.com/elastic-rock/KeepScreenOn/issues/26 and there it needed 16 heh

Will need to juggle all these values then when the next one fails, based on AGP version.

Thanks

obfusk commented 1 month ago

Will need to juggle all these values then when the next one fails, based on AGP version.

Yeah, you need to know what ZIP alignment parameters should be used to match the upstream APK. Google changing the defaults complicates that. But you always needed --page-align to properly align uncompressed .so files.

FYI I recommend using --internal to always use the built-in pure-Python zipalign.py instead of relying on the code to automatically detect a suitable version from build-tools (and the Debian package doesn't support different page sizes).

obfusk commented 1 month ago

No need to guess since you were the only RB expert.

Then maybe F-Droid shouldn't have forced its only RB expert to resign :woman_shrugging: Good luck keeping things running.