-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sample Java buildpack #3
Conversation
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.
Just some comments and questions. But as a sample, looks good.
java-buildpack/bin/build
Outdated
mkdir -p $launch_dir/jdk/.profile.d | ||
cat << EOF > $launch_dir/jdk/.profile.d/jdk.sh | ||
export JAVA_HOME=$launch_dir/jdk | ||
export PATH=\$JAVA_HOME/bin:\$PATH |
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.
Interesting. I hadn't thought about this need, but it makes sense. The updates to the path can't happen until after the layer has been created.
java-buildpack/bin/build
Outdated
|
||
echo "---> Installing Maven" | ||
|
||
mkdir -p $cache_dir/maven/.m2 |
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.
Granted that we don't do builds, but if we did I'd absolutely require the use of mvnw
and gradlew
to avoid any conflicts here.
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.
for sure, in the next interation. i plan to support both (although i might kill the default mvn install).
java-buildpack/bin/build
Outdated
|
||
echo "---> Running Maven" | ||
|
||
MAVEN_OPTS="${MAVEN_OPTS:-"-Xmx1024m -Duser.home=$cache_dir -Dmaven.repo.local=$cache_dir/.m2/repository"}" |
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.
Traditionally we've symlinked a cached directory to $HOME/.m2
because there's no combination of environment variables and system properties that will move all caches (say the mvnw
download cache). @sclevine would there be any problems doing that, knowing that the symlink would disappear when the container was disposed and that'd be fine?
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.
Should work great. /home/pack
($HOME
) is intended for this purpose -- an ephemeral home directory that is not preserved.
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.
@nebhale in the Heroku Java buildpack we let it do whatever in /app
and then move it to the cache after the build runs. I'm just being lazy here (and was expecting this to change when i support mvnw
).
@sclevine I'm not sure how I feel about relying on $HOME
. I don't think we've made that a part of the API, and today on Heroku $HOME
is actually the /app
dir.
If it's ok with y'all, I'll merge this as is, so I have a baseline to iterate from.
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.
Interesting point about $HOME
. CF is super weird with this: it's /home/vcap
at build, but /home/vcap/app
(symlinked to /app
) at launch. I think it should be the same in both cases, and I think I have a slight preference for /home/pack
, because it prevents auto-generated settings/cache from various CLI tools from persisting (ex. ~/.bundle
).
@nebhale any thoughts about this?
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.
@jkutner I'm totally okay with you adding any samples to this repo directly 😄
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.
I’d agree with definitely ditching the CF insanity, and also lean towards home being somewhere other than the app directory. My anecdotal experience tells me that there are enough packages that write to $HOME
that it’s a vector for vulnerabilities. Choosing a “safer” location seems like a prudent safety measure, but I am sensitive to compatibility.
java-buildpack/bin/build
Outdated
mkdir -p $launch_dir/jdk | ||
jdk_url="https://cdn.azul.com/zulu/bin/zulu8.28.0.1-jdk8.0.163-linux_x64.tar.gz" | ||
curl -sfL "$jdk_url" | tar pxz -C $launch_dir/jdk --strip-components=1 | ||
mkdir -p $launch_dir/jdk/.profile.d |
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.
@jkutner this should be profile.d
instead of .profile.d
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.
Could we define the openjdk version to be installed such as the distro ?
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.
@cmoulliard yea, absolutely. I only grabbed Zulu 8u163 to get something working.
I don't know anything about Java anymore, but could this eventually use something like Jib to produce images which can be built incrementally very quickly, with unchanged layers cached in the registry? |
@imjasonh yea, definitely possible. Although I think it might make more sense as a |
Maybe such files could help us to define the build and run steps by providing env var to configure the JVM, ... run : https://github.com/fabric8io-images/s2i/blob/master/java/images/centos/s2i/run |
@cmoulliard can you elaborate on what those scripts do? They look useful, but i'm a little fuzzy on the intent. |
e910d84
to
94bb5f9
Compare
I merged this and create issues for the ideas @imjasonh and @cmoulliard brought up. |
A very basic Java buildpack that runs Maven to compile an app.