open-video-editor icon indicating copy to clipboard operation
open-video-editor copied to clipboard

F-Droid can't build

Open licaon-kter opened this issue 1 year ago • 15 comments

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

licaon-kter avatar Mar 18 '24 15:03 licaon-kter

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?

devhyper avatar Mar 18 '24 16:03 devhyper

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?

mario0318 avatar Mar 20 '24 15:03 mario0318

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?

licaon-kter avatar Mar 20 '24 16:03 licaon-kter

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 avatar Mar 21 '24 00:03 devhyper

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 avatar Mar 21 '24 00:03 devhyper

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

devhyper avatar Mar 21 '24 00:03 devhyper

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

devhyper avatar Mar 21 '24 02:03 devhyper

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

licaon-kter avatar Mar 21 '24 04:03 licaon-kter

d0801dc app-release.zip

devhyper avatar Mar 21 '24 07:03 devhyper

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

licaon-kter avatar Mar 21 '24 15:03 licaon-kter

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

devhyper avatar Mar 21 '24 20:03 devhyper

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

devhyper avatar Mar 22 '24 02:03 devhyper

@licaon-kter

devhyper avatar Apr 05 '24 19:04 devhyper

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

licaon-kter avatar Apr 05 '24 20:04 licaon-kter

1.1.2 has the same problem.

linsui avatar Apr 30 '24 07:04 linsui

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

linsui avatar May 30 '24 11:05 linsui

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.

obfusk avatar May 30 '24 17:05 obfusk

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.

linsui avatar May 30 '24 17:05 linsui

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

Try 4. It works.

obfusk avatar May 30 '24 17:05 obfusk

Trying now.

linsui avatar May 30 '24 17:05 linsui

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.

obfusk avatar May 30 '24 17:05 obfusk

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:

linsui avatar May 30 '24 18:05 linsui

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

licaon-kter avatar May 30 '24 18:05 licaon-kter

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 avatar May 30 '24 20:05 obfusk

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.

obfusk avatar May 30 '24 20:05 obfusk

hmm https://gitlab.com/fdroid/fdroiddata/-/jobs/7729208261#L1695

tried locally with zipalign.py --page-size 16 --pad-like-apksigner --replace unaligned.apk $$OUT$$ but did not help :cry:

licaon-kter avatar Sep 03 '24 14:09 licaon-kter

You're still using my tooling wrong, just trying random things without understanding how it works. Because it works perfectly fine if you don't screw up the padding by using --pad-like-apksigner when the upstream APK doesn't have apksigner padding (which is clearly the case here). The IzzyOnDroid rebuilder had no issues confirming v1.1.3 is reproducible yesterday.

obfusk avatar Sep 03 '24 16:09 obfusk

Thank you, RB expert. :) https://gitlab.com/fdroid/fdroiddata/-/commit/2c466ac3a017605fdeeb86d72c725089ab3e127c

linsui avatar Sep 03 '24 16:09 linsui