-
-
Notifications
You must be signed in to change notification settings - Fork 414
add Gradle and Maven into Java8 + bump aws-lambda-builders #167
Conversation
|
👍 |
mhart
left a comment
There was a problem hiding this 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
java8/build/Dockerfile
Outdated
| 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 && \ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
java8/build/Dockerfile
Outdated
| 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 && \ |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat. Will update.
java8/build/Dockerfile
Outdated
| 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/" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@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. |
|
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
|
Great! Merging now, will let you know when they've all been pushed |
|
All pushed! |
No description provided.