Skip to content
This repository was archived by the owner on Jul 5, 2023. It is now read-only.

Conversation

Maychell
Copy link

@Maychell Maychell commented Oct 9, 2017

This pr aims to create a Docker Android 26 image to be used by Gitlab CI.

26/Dockerfile Outdated
&& unzip android-sdk.zip -d ${ANDROID_HOME} \
&& rm -f android-sdk.zip

ENV PATH ${PATH}:${ANDROID_HOME}/tools:${ANDROID_HOME}/tools/bin:${ANDROID_HOME}/platform-tools
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you create a link to the binary instead of override the PATH? (Like you did on line 42)

@Maychell Maychell force-pushed the create-docker-image-for-android-26 branch from 8e80fe4 to 06fde13 Compare October 10, 2017 13:28
Copy link

@paulodiovani paulodiovani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The images misses a CMD and/or ENTRYPOINT, it hard to figure it's purpose without them.

26/Dockerfile Outdated
"platforms;android-26" \
"build-tools;26.0.2"

RUN ln -s ${EMULATOR_HOME}/emulator64-x86 /usr/bin/emulator64-x86 && \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Join all the RUN commands into a single one for a shallow image.

26/Dockerfile Outdated
"build-tools;26.0.2"

RUN ln -s ${EMULATOR_HOME}/emulator64-x86 /usr/bin/emulator64-x86 && \
emulator64-x86 -avd test -no-window -no-audio &

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this command ends with an &? 🤔

Docker images should not spawn process before the CMD entry.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is something required for a posterior CMDentry it should be on the ENTRYPOINT

26/Dockerfile Outdated

# Install required tools
RUN apt-get --quiet update --yes \
&& apt-get --quiet install --yes wget tar unzip lib32stdc++6 lib32z1 --no-install-recommends

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to remove unneeded dependencies in the end (wget, tar, unzip...).

@@ -1,2 +1,31 @@
# docker-ci-android
Docker Android images used by Gitlab CI
# Codeminer42 Android Image for CI builds

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an usage instructions/example to the readme

Copy link

@paulodiovani paulodiovani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still no CMD / ENTRYPOINT

Copy link

@paulodiovani paulodiovani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested, but seems good enough for a first version.

# License is valid for all the standard components in versions installed from this file
# Non-standard components: MIPS system images, preview versions, GDK (Google Glass) and Android Google TV require separate licenses, not accepted there
RUN mkdir -p ${ANDROID_HOME}/licenses \
&& echo 8933bad161af4178b1185d1a37fbf41ea5269c55 > ${ANDROID_HOME}/licenses/android-sdk-license

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this hash? If this is a license itself, it's not safe to add to the image.

We can create ${ANDROID_HOME}/licenses as a volume and force to add license on run (with a mounted volume, env var, etc).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants