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

Conversation

@jfuss
Copy link
Contributor

@jfuss jfuss commented Feb 15, 2019

No description provided.

@sriram-mv
Copy link
Contributor

👍

Copy link
Member

@mhart mhart left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for doing this! Just some minor notes on the gradle/maven installation

curl https://lambci.s3.amazonaws.com/fs/java8.tgz | tar -zx -C / && \
yum install -y --releasever=latest java-1.8.0-openjdk-devel-1.8.0.181
yum install -y --releasever=latest java-1.8.0-openjdk-devel-1.8.0.181 && \
mkdir /opt/gradle && curl -L -o gradle.zip https://services.gradle.org/distributions/gradle-5.2-bin.zip && \
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use /usr/local – or similar. As /opt is used by layers, people often want to use the build images to build/install things in there, and then zip everything up. Best to keep /opt just for layers I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh good point. I just followed what Gradles docs suggested. Will move to /usr/local.

yum install -y --releasever=latest java-1.8.0-openjdk-devel-1.8.0.181 && \
mkdir /opt/gradle && curl -L -o gradle.zip https://services.gradle.org/distributions/gradle-5.2-bin.zip && \
unzip -d /opt/gradle gradle.zip && rm gradle.zip && mkdir /opt/maven && \
curl -L http://mirror.metrocast.net/apache/maven/maven-3/3.6.0/binaries/apache-maven-3.6.0-bin.tar.gz -o maven.tar.gz && \
Copy link
Member

Choose a reason for hiding this comment

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

Can just be one command:

curl -L http://mirror.metrocast.net/apache/maven/maven-3/3.6.0/binaries/apache-maven-3.6.0-bin.tar.gz | \
  tar -zx -C /usr/local/maven

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neat. Will update.

curl -L http://mirror.metrocast.net/apache/maven/maven-3/3.6.0/binaries/apache-maven-3.6.0-bin.tar.gz -o maven.tar.gz && \
tar -C /opt/maven -zxf maven.tar.gz && rm maven.tar.gz

ENV PATH="${PATH}:/opt/gradle/gradle-5.2/bin:/opt/maven/apache-maven-3.6.0/bin/"
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to put them at the start of the path? I feel like that would lead to less issues if Lambda installed a maven or gradle, for example.

And minor nit: .../bin, not .../bin/ for the maven piece.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There wasn't really a reason. I can flip the order.

@jfuss
Copy link
Contributor Author

jfuss commented Feb 15, 2019

@mhart Would you prefer the gradle and maven to be its own layer (a different RUN command) instead of lumping everything into one? This might be helpful when gradle or maven needs updates but not the runtime itself.

@mhart
Copy link
Member

mhart commented Feb 15, 2019

Nah, not too fussed about that – maven and gradle are updated pretty rarely – probably more rarely than the Lambda base OS layer itself is updated – and when that happens we have to rebuild all layers anyway.

Details:
* installation into /usr/local instead of /opt
* update PATH with gradle and maven first
* simplify maven commands
@mhart
Copy link
Member

mhart commented Feb 15, 2019

Great! Merging now, will let you know when they've all been pushed

@mhart mhart merged commit 16019e2 into lambci:master Feb 15, 2019
@mhart
Copy link
Member

mhart commented Feb 15, 2019

All pushed!

@mhart mhart mentioned this pull request Mar 8, 2019
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