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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/set_up_cromwell_action/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
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:
Expand All @@ -12,13 +13,13 @@ runs:
#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
- 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
Expand All @@ -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
Expand Up @@ -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: adopt@1.11
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:
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/docker_build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: adopt@1.11
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
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ jobs:
set -e
echo Running test.sh
./src/ci/bin/test.sh
# If a build step fails, activate SSH and idle for 60 minutes
# - name: Setup tmate session
# if: ${{ failure() }}
# uses: mxschmitt/action-tmate@v3
# timeout-minutes: 60
# with:
# limit-access-to-actor: true
# detached: true
# always() is some github magic that forces the following step to run, even when the previous fails.
# Without it, the if statement won't be evaluated on a test failure.
- uses: ravsamhq/notify-slack-action@v2
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/trivy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: adopt@1.11
distribution: 'temurin'
java-version: '17'

# set up SBT cache
- uses: actions/cache@v2
Expand Down
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Cromwell Change Log

## 87 Release Notes

### Java 17

As of this version, a distribution of Java 17 is required to run Cromwell. Cromwell is developed, tested, and
containerized using [Eclipse Temurin](https://adoptium.net/temurin/releases/?version=17).

## 86 Release Notes

### GCP Batch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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-"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@
): WatchKey = throw new UnsupportedOperationException

override def iterator(): java.util.Iterator[Path] =
if (unixPath.isEmpty || unixPath.isRoot) {
if (unixPath.izEmpty || unixPath.isRoot) {

Check warning on line 179 in cloud-nio/cloud-nio-spi/src/main/scala/cloud/nio/spi/CloudNioPath.scala

View check run for this annotation

Codecov / codecov/patch

cloud-nio/cloud-nio-spi/src/main/scala/cloud/nio/spi/CloudNioPath.scala#L179

Added line #L179 was not covered by tests
java.util.Collections.emptyIterator()
} else {
unixPath.split().to(LazyList).map(part => newPath(UnixPath.getPath(part)).asInstanceOf[Path]).iterator.asJava
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@

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

Check warning on line 70 in cloud-nio/cloud-nio-spi/src/main/scala/cloud/nio/spi/UnixPath.scala

View check run for this annotation

Codecov / codecov/patch

cloud-nio/cloud-nio-spi/src/main/scala/cloud/nio/spi/UnixPath.scala#L70

Added line #L70 was not covered by tests

def hasTrailingSeparator: Boolean = UnixPath.hasTrailingSeparator(path)

Expand Down
2 changes: 1 addition & 1 deletion docs/Configuring.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
```

Expand Down
4 changes: 2 additions & 2 deletions docs/Releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ Mac users with Homebrew can also get Cromwell with the command `brew install cro
This documentation frequently refers to a "Cromwell jar" with a name like `cromwell-<version>.jar`.
This is the main artifact in Cromwell releases that contains all executable Cromwell code and default configuration.

A distribution of Java 11 is required to run Cromwell. Cromwell is developed, tested, and containerized using
[AdoptOpenJDK 11 HotSpot](https://adoptopenjdk.net/).
A distribution of Java 17 is required to run Cromwell. Cromwell is developed, tested, and containerized using
[AdoptOpenJDK 17 HotSpot](https://adoptopenjdk.net/).

For users running a Cromwell server [a docker image](https://hub.docker.com/r/broadinstitute/cromwell) has been made available.

Expand Down
6 changes: 3 additions & 3 deletions docs/tutorials/FiveMinuteIntro.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
### Prerequisites:

* A Unix-based operating system (yes, that includes Mac!)
* A Java 11 runtime environment
* A Java 17 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)
* `sdk install java 11.0.16-tem` as of the time of this writing
* We recommend [SDKMAN](https://sdkman.io/install) to install the latest 17 build of [Temurin](https://adoptium.net/temurin/releases/?version=17)
* `sdk install 17.0.9-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!

Expand Down
5 changes: 2 additions & 3 deletions project/Publishing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ object Publishing {
val additionalDockerInstr: Seq[Instruction] = (dockerCustomSettings ?? Nil).value

new Dockerfile {
from("us.gcr.io/broad-dsp-gcr-public/base/jre:11-debian")
from("us.gcr.io/broad-dsp-gcr-public/base/jre:17-debian")
expose(8000)
add(artifact, artifactTargetPath)
runRaw(s"ln -s $artifactTargetPath /app/$projectName.jar")
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion publish/docker-setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions runConfigurations/renderCiResources.run.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
1 change: 0 additions & 1 deletion scripts/docker-compose-mysql/README.md

This file was deleted.

9 changes: 0 additions & 9 deletions scripts/docker-compose-mysql/compose/cromwell/Dockerfile

This file was deleted.

This file was deleted.

This file was deleted.

22 changes: 0 additions & 22 deletions scripts/docker-compose-mysql/docker-compose.yml

This file was deleted.

This file was deleted.

This file was deleted.

Loading
Loading