Skip to content
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

WX-1409 Java 17 #7342

Merged
merged 17 commits into from
Dec 14, 2023
Merged
16 changes: 8 additions & 8 deletions .github/set_up_cromwell_action/action.yml
Original file line number Diff line number Diff line change
@@ -3,22 +3,23 @@
name: 'Set Up Cromwell Steps'
description: Specific steps that will set up git secrets, java, sbt, and Cromwell on the local machine.
inputs:
cromwell_repo_token: #As an input to this action, you are required to pass in a token that can be used to authenticate while checking out Cromwell.
cromwell_repo_token:
description: "As an input to this action, you are required to pass in a token that can be used to authenticate while checking out Cromwell."
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IntelliJ believes that a description is required, so I put the comment into one.

required: true

runs:
using: "composite" # <-- this allows these steps to be used by other workflows.
steps:
#Allows this github action to use a cache to store stuff like Java and sbt files between runs.
- uses: actions/checkout@v3
name: Checkout Coursier Cache
- uses: coursier/cache-action@v6
name: Enable Coursier Cache
# - uses: actions/checkout@v3
# name: Checkout Coursier Cache
# - uses: coursier/cache-action@v6
# name: Enable Coursier Cache

#Cromwell requires git-secrets be setup. Here, we set up secrets and verify success with a script.
- name: Git secrets setup
run: |
git clone https://github.com/awslabs/git-secrets.git ~/git-secrets
git clone --quiet https://github.com/awslabs/git-secrets.git ~/git-secrets
cd ~/git-secrets
git checkout ad82d68ee924906a0401dfd48de5057731a9bc84
sudo make install
@@ -43,5 +44,4 @@ runs:
uses: actions/setup-java@v3
with:
distribution: temurin
java-version: 11

java-version: 17
5 changes: 3 additions & 2 deletions .github/workflows/chart_update_on_merge.yml
Original file line number Diff line number Diff line change
@@ -23,9 +23,10 @@ jobs:
repository: broadinstitute/cromwell
token: ${{ secrets.BROADBOT_GITHUB_TOKEN }} # Has to be set at checkout AND later when pushing to work
path: cromwell
- uses: olafurpg/setup-scala@v10
- uses: actions/setup-java@v4
with:
java-version: [email protected]
Comment on lines -26 to -28
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This action is unmaintained and does not support our Java 17 distribution.

distribution: 'temurin'
java-version: '17'
- name: Clone Cromwhelm
uses: actions/checkout@v2
with:
5 changes: 3 additions & 2 deletions .github/workflows/docker_build_test.yml
Original file line number Diff line number Diff line change
@@ -25,9 +25,10 @@ jobs:
repository: broadinstitute/cromwell
token: ${{ secrets.BROADBOT_GITHUB_TOKEN }}
path: cromwell
- uses: olafurpg/setup-scala@v10
- uses: actions/setup-java@v4
with:
java-version: [email protected]
distribution: 'temurin'
java-version: '17'
# The following invocation should be as similar as possible to the one in chart_update_on_merge.yml
# To state the obvious: This test should not publish anything. It should simply verify that the build completes.
- name: Build Cromwell Docker
6 changes: 6 additions & 0 deletions .github/workflows/integration_tests.yml
Original file line number Diff line number Diff line change
@@ -90,6 +90,12 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 120
steps:
# - name: Setup tmate session
# uses: mxschmitt/action-tmate@v3
# timeout-minutes: 10
# with:
# limit-access-to-actor: true
# detached: true
- uses: actions/checkout@v3 # checkout the cromwell repo
with:
ref: ${{ inputs.target-branch }}
5 changes: 3 additions & 2 deletions .github/workflows/trivy.yml
Original file line number Diff line number Diff line change
@@ -20,9 +20,10 @@ jobs:
- uses: actions/checkout@v2

# fetch SBT package
- uses: olafurpg/setup-scala@v10
- uses: actions/setup-java@v4
with:
java-version: [email protected]
distribution: 'temurin'
java-version: '17'

# set up SBT cache
- uses: actions/cache@v2
2 changes: 1 addition & 1 deletion .sdkmanrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Enable auto-env through the sdkman_auto_env config
# Add key=value pairs of SDKs to use below
java=11.0.11.hs-adpt
java=17.0.9-tem
Original file line number Diff line number Diff line change
@@ -14,8 +14,7 @@ import java.util.concurrent.atomic.AtomicInteger
* > main thread).
*/
object DaemonizedDefaultThreadFactory extends ThreadFactory {
private val s = System.getSecurityManager
private val group = if (s != null) s.getThreadGroup else Thread.currentThread.getThreadGroup
Comment on lines -17 to -18
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SecurityManager is deprecated in 17. I honestly don't know what its role is here, but it certainly seems like the logic is just fine with the possibility of it being null.

private val group = Thread.currentThread.getThreadGroup
private val threadNumber = new AtomicInteger(1)
private val namePrefix = "daemonpool-thread-"

Original file line number Diff line number Diff line change
@@ -176,7 +176,7 @@ class CloudNioPath(filesystem: CloudNioFileSystem, private[spi] val unixPath: Un
): WatchKey = throw new UnsupportedOperationException

override def iterator(): java.util.Iterator[Path] =
if (unixPath.isEmpty || unixPath.isRoot) {
if (unixPath.izEmpty || unixPath.isRoot) {
java.util.Collections.emptyIterator()
} else {
unixPath.split().to(LazyList).map(part => newPath(UnixPath.getPath(part)).asInstanceOf[Path]).iterator.asJava
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ final private[spi] case class UnixPath(path: String) extends CharSequence {

def isAbsolute: Boolean = UnixPath.isAbsolute(path)

def isEmpty: Boolean = path.isEmpty
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a name collision new in 17.

The initial compile error is that it needs an override. Adding the override results in a second error saying it overrides nothing!

def izEmpty: Boolean = path.isEmpty

def hasTrailingSeparator: Boolean = UnixPath.hasTrailingSeparator(path)

2 changes: 1 addition & 1 deletion docs/Configuring.md
Original file line number Diff line number Diff line change
@@ -689,7 +689,7 @@ This limit may be configured via the configuration value:

```hocon
yaml {
max-depth = 1000
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Depth of 1000 causes a stack overflow in 17.

I've never even heard of anyone using YAML inputs in real life, and I'm pretty confident 100 will be enough if I'm wrong.

max-depth = 100
}
```

2 changes: 1 addition & 1 deletion docs/tutorials/FiveMinuteIntro.md
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@
* A Java 11 runtime environment
* You can see what you have by running `$ java -version` on a terminal.
* If not, consider installing via conda or brew [as explained here](../Releases.md).
* We recommend [SDKMAN](https://sdkman.io/install) to install the latest 11 build of [Temurin](https://adoptium.net/temurin/releases/?version=11)
* We recommend [SDKMAN](https://sdkman.io/install) to install the latest 17 build of [Temurin](https://adoptium.net/temurin/releases/?version=17)
* `sdk install java 11.0.16-tem` as of the time of this writing
* You might need to update the `export JAVA_HOME` in your bash profile to point to your JAVA install location.
* A sense of adventure!
3 changes: 1 addition & 2 deletions project/Publishing.scala
Original file line number Diff line number Diff line change
@@ -163,8 +163,7 @@ object Publishing {
val additionalResolvers = List(
broadArtifactoryResolver,
broadArtifactoryResolverSnap,
Resolver.sonatypeRepo("releases")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was a deprecation

)
) ++ Resolver.sonatypeOssRepos("releases")

private val artifactoryCredentialsFile =
file("target/ci/resources/artifactory_credentials.properties").getAbsoluteFile
2 changes: 1 addition & 1 deletion publish/docker-setup.sh
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ mkdir -p /etc/apt/keyrings
wget -O - https://packages.adoptium.net/artifactory/api/gpg/key/public | tee /etc/apt/keyrings/adoptium.asc
echo "deb [signed-by=/etc/apt/keyrings/adoptium.asc] https://packages.adoptium.net/artifactory/deb $(awk -F= '/^VERSION_CODENAME/{print$2}' /etc/os-release) main" | tee /etc/apt/sources.list.d/adoptium.list
apt update
apt install -y temurin-11-jdk
apt install -y temurin-17-jdk

# Install jq 1.6 to ensure --rawfile is supported
curl -L https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64 -o /usr/bin/jq
4 changes: 2 additions & 2 deletions runConfigurations/renderCiResources.run.xml
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@
</option>
<option name="tasks" value="renderCiResources" />
<option name="useSbtShell" value="false" />
<option name="vmparams" value="-Xms512M -Xmx1024M -Xss1M -XX:+CMSClassUnloadingEnabled" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This option was dropped in 17

<option name="vmparams" value="-Xms512M -Xmx1024M -Xss1M" />
<option name="workingDir" value="$PROJECT_DIR$" />
<method v="2" />
</configuration>
</component>
</component>
2 changes: 1 addition & 1 deletion src/ci/bin/test.inc.sh
Original file line number Diff line number Diff line change
@@ -746,7 +746,7 @@ cromwell::private::login_docker() {

cromwell::private::render_secure_resources() {
# Avoid docker output to sbt's stderr by pulling the image here
docker pull broadinstitute/dsde-toolbox:dev | cat
docker pull --quiet broadinstitute/dsde-toolbox:dev | cat
# Copy the CI resources, then render the secure resources using Vault
sbt -Dsbt.supershell=false --warn renderCiResources \
|| if [[ "${CROMWELL_BUILD_IS_CI}" == "true" ]]; then
2 changes: 1 addition & 1 deletion src/ci/docker-compose/cromwell-test/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ubuntu:focal
FROM ubuntu:latest

WORKDIR /cromwell-test/
COPY docker-setup.sh ./
14 changes: 6 additions & 8 deletions src/ci/docker-compose/cromwell-test/docker-setup.sh
Original file line number Diff line number Diff line change
@@ -25,13 +25,11 @@ apt-get install -y \
sudo \
wget \

# setup install for adoptopenjdk
# https://adoptopenjdk.net/installation.html#linux-pkg-deb
wget -qO - https://adoptopenjdk.jfrog.io/adoptopenjdk/api/gpg/key/public | apt-key add -
echo "deb https://adoptopenjdk.jfrog.io/adoptopenjdk/deb $(
grep UBUNTU_CODENAME /etc/os-release | cut -d = -f 2
) main" |
tee /etc/apt/sources.list.d/adoptopenjdk.list
# setup install for Temurin
# https://adoptium.net/installation/linux/
mkdir -p /etc/apt/keyrings
wget -O - https://packages.adoptium.net/artifactory/api/gpg/key/public | tee /etc/apt/keyrings/adoptium.asc
echo "deb [signed-by=/etc/apt/keyrings/adoptium.asc] https://packages.adoptium.net/artifactory/deb $(awk -F= '/^UBUNTU_CODENAME/{print$2}' /etc/os-release) main" | tee /etc/apt/sources.list.d/adoptium.list

# setup install for gcloud
# https://cloud.google.com/sdk/docs/install#deb
@@ -52,7 +50,7 @@ add-apt-repository \
# install packages that required setup
apt-get update
apt-get install -y \
adoptopenjdk-11-hotspot \
temurin-17-jdk \
containerd.io \
docker-ce \
docker-ce-cli \
Loading