Skip to content
This repository was archived by the owner on Mar 31, 2022. It is now read-only.

[IN-1659] Update openjdk from 8 to 11#311

Merged
fehersanyi-bitrise merged 17 commits intomasterfrom
java-update
Aug 25, 2020
Merged

[IN-1659] Update openjdk from 8 to 11#311
fehersanyi-bitrise merged 17 commits intomasterfrom
java-update

Conversation

@fehersanyi-bitrise
Copy link
Copy Markdown
Contributor

Pull Request Checklist

Check/accept these:

  • The tool I added is stable, and does not require frequent updates. Preinstalled tools on the LTS stacks
    are not updated, so if the tool requires frequent updates you should handle the installation on-demand (see: Install Any Additional Tool - DevCenter for more information)
  • I've updated the version in the Dockerfile
  • I've updated the CHANGELOG.md
  • I've provided a link or explanation below which describes the changes in the tool itself
  • I added a version report line to system_report.sh for the new tool(s) I added

Copy or link the tool's changelog here

Copy link
Copy Markdown
Contributor

@peterheja-bitrise peterheja-bitrise left a comment

Choose a reason for hiding this comment

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

lgtm

lmesz-bitrise
lmesz-bitrise previously approved these changes Aug 19, 2020
Copy link
Copy Markdown
Contributor

@lmesz-bitrise lmesz-bitrise left a comment

Choose a reason for hiding this comment

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

How will we validate this change?

bitrise-tom
bitrise-tom previously approved these changes Aug 19, 2020
Comment thread Dockerfile Outdated
# ------------------------------------------------------
# --- Install Android SDKs and other build packages

ENV SDKMANAGER_PATH /opt/android-sdk-linux/cmdline-tools/tools/bin/sdkmanager
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add it to the PATH? It will be a problem for the customer too if they won't be able to use sdkmanager.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is on path, it is just a make sure it is there because on mac I had to deal with crap so I'm now super explicit kind of movement 🧐

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If it is on the path then this step is unnecessary.
I am afraid if it does not work with PATH the reason is that it is another sdkmanager with different settings which can be resulted to install to not proper places

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, it is possible. At the time of introducing commandline-tools (#300) it needs to be located in the very specific location relative to $ANDROID_HOME.
Even after updating commandline-tools using sdkmanager provided by themselves, the new version was installed in a different location resulting in 2 sdk managers.
Maybe @fehersanyi-bitrise encountered something similar.

Comment thread Dockerfile Outdated
RUN dpkg --add-architecture i386
RUN apt-get update -qq
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y openjdk-8-jdk libc6:i386 libstdc++6:i386 libgcc1:i386 libncurses5:i386 libz1:i386 net-tools
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y openjdk-8-jdk openjdk-11-jdk libc6:i386 libstdc++6:i386 libgcc1:i386 libncurses5:i386 libz1:i386 net-tools
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't we need to remove openjdk-8-jdk?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove it!

Comment thread Dockerfile Outdated
Comment thread Dockerfile Outdated
RUN dpkg --add-architecture i386
RUN apt-get update -qq
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y openjdk-8-jdk libc6:i386 libstdc++6:i386 libgcc1:i386 libncurses5:i386 libz1:i386 net-tools
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y openjdk-8-jdk openjdk-11-jdk libc6:i386 libstdc++6:i386 libgcc1:i386 libncurses5:i386 libz1:i386 net-tools
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove it!

Copy link
Copy Markdown
Contributor

@lmesz-bitrise lmesz-bitrise left a comment

Choose a reason for hiding this comment

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

@fehersanyi-bitrise fehersanyi-bitrise merged commit c2ce84f into master Aug 25, 2020
@fehersanyi-bitrise fehersanyi-bitrise deleted the java-update branch August 25, 2020 09:57
peterheja-bitrise added a commit that referenced this pull request Aug 28, 2020
peterheja-bitrise added a commit that referenced this pull request Aug 28, 2020
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.

6 participants