-
Notifications
You must be signed in to change notification settings - Fork 53
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
Replace Ammonite with Scala-CLI #1165
Conversation
Scala-CLI is the new command-line runner for the Scala language, intended to replace Ammonite.
The ODKFull image defines the COURSIER_CACHE environment variable to /tools/.coursier-cache, so Ammonite and Scala-CLI expect to be able to store their downloaded dependencies in there. If we run under a non-privileged user, we need to ensure that, if the directory does not exist, it is created and given to the odkuser. (If the directory already exists, it means a host directory has been explicitly bound to it, in which case permissions will already be correct, and we have nothing to do.)
Now that we have Scala-CLI, there should not be any need for Ammonite anymore.
if [ ! -d /tools/.coursier-cache ] ; then | ||
mkdir /tools/.coursier-cache | ||
chown odkuser:odkuser /tools/.coursier-cache | ||
fi |
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.
You removed Ammonite, but build logic to handle coursier cache? Is /tools/.coursier-cache
used for anything other than ammonite
?
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.
Scala-CLI uses it as well. :)
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.
Apart from a little non-breaking comment LGTM
@balhoff I would appreciate your input on this PR before merging it. In particular, do you think we can remove Ammonite immediately? You said on Slack that switching to Scala-CLI would require Scala users to adapt their scripts – if the migration is thought to be a difficult one, I am open to the idea of having both Ammonite and Scala-CLI co-existing in ODK 1.6, and removing Ammonite only in ODK 1.7. |
Thanks for doing this @gouttegd! I haven't run it but I think it looks good. If you can have both in one release, then drop Ammonite, that would be amazing. |
OK, then I will merge only the first two commits of that PR (adding Scala-CLI, but leaving Ammonite in place). We won’t remove Ammonite until ODK 1.7. |
Closing in favour of #1166, which does not include the Ammonite removal. |
This PR adds the Scala-CLI runner to the ODKFull image, as a replacement for Ammonite.
closes #1161
closes #1162