search

cirruslabs / docker-images-android

Android SDK Docker Images
MIT License
39 stars 18 forks source link

Increase matrix of supported Android CLT and Java versions #74

Open bartekpacia opened 3 weeks ago

bartekpacia commented 3 weeks ago

resolve #73

discovered during process of working on this PR - https://github.com/cirruslabs/cirrus-ci-docs/pull/1288

bartekpacia commented 3 weeks ago

Ughhh... what? :(

@fkorotkov @edigaryev

Screenshot 2024-08-21 at 01 06 41
fkorotkov commented 3 weeks ago

@bartekpacia sorry about that. Unblocked your account. Your first iteration looked too suspicious to our crypto mining detection mechanism.

edigaryev commented 3 weeks ago

Do I understand correctly that this won't produce any new container images and/or tags? Is this intended?

bartekpacia commented 3 weeks ago

This is still WIP, sorry!

bartekpacia commented 3 weeks ago

@fkorotkov Smoke Test is failing. I guess it's because it tries to build sdk/tools/Dockerfile, which now requries 2 arguments, but those args aren't provided. How can I provide those 2 args to the implicit docker_builder building that image?

edigaryev commented 3 weeks ago

How can I provide those 2 args to the implicit docker_builder building that image?

Docker ARG's instruction can accept default values.

bartekpacia commented 3 weeks ago

@edigaryev @fkorotkov I think I got tools Dockerfile matrix done. Now, how should I go about making 34 and 34-ndk dockerfile also have that matrix?

Do I need to create new directories for each combination?

Or, should I parametrize the FROM instruction here:

https://github.com/cirruslabs/docker-images-android/blob/9e83bc20b19e34972bdc0b584d5bcead7701617e/sdk/34/Dockerfile#L1

to be like this:

ARG jdk_version=17

FROM ghcr.io/cirruslabs/android-sdk:tools-${jdk_version}
bartekpacia commented 2 weeks ago

ping @fkorotkov :)

bartekpacia commented 2 weeks ago

@fkorotkov I'm not sure. Your call!

The end result I'd like to see is to have each of the 3 different images in this repo (tools, 34, and 34-ndk), to have 4 JDK version variants.

So that'd give 12 images per release (3 images * 4 variants).

fkorotkov commented 2 weeks ago

I think we need to add the arguments to sdk/34/Dockerfile too. Lastly to preserve backward compatibility ghcr.io/cirruslabs/android-sdk:tools-jdk17 should be tagged as ghcr.io/cirruslabs/android-sdk:tools-jdk17. Alternatively I don't mind if we'll just build ghcr.io/cirruslabs/android-sdk:tools with the defaults from the arguments.

bartekpacia commented 2 weeks ago

@fkorotkov Gotcha. Please see my latest commit, I hope I got it right.

Re: Java version - I think that latest JDK LTS should be the default. See https://github.com/cirruslabs/docker-images-android/pull/71. That said I'm OK with leaving openjdk17 as default.

bartekpacia commented 2 weeks ago

Hm, it looks like we're hitting this problem: docker buildx fails to show result in image list

The answer suggests to add --output type=docker argument, but then we hit https://github.com/docker/buildx/issues/59