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.

Sign up for GitHub

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

Use the latest available jdk #59788

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+4 −5
Open

Use the latest available jdk #59788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f55fd1
Use the latest available jdk
ViktorHofer Jan 9, 2025
3a3c11b
Specify toolchain in build.gradle files
ViktorHofer Jan 9, 2025
ddd67f0
Try something else
ViktorHofer Jan 9, 2025
6fb3e6b
Update InstallJdk.ps1
ViktorHofer Jan 9, 2025
0ff7224
Remove unused variable `$globalJson` in InstallJdk.ps1
wtgodbe Jan 9, 2025
3b86147
Update Helix.targets
wtgodbe Jan 9, 2025
2e2bd77
Update GsonHubProtocolTest.java
BrennanConroy Jan 10, 2025
fca0103
Update Helix.targets
BrennanConroy Jan 10, 2025
475047d
Add HelixPreCommand for non-Windows, non-Mac queues
wtgodbe Jan 13, 2025
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
3 changes: 1 addition & 2 deletions eng/scripts/InstallJdk.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ $installDir = "$repoRoot\.tools\jdk\win-x64\"
$javacExe = "$installDir\bin\javac.exe"
$tempDir = "$repoRoot\obj"
if (-not $JdkVersion) {
$globalJson = Get-Content "$repoRoot\global.json" | ConvertFrom-Json
$JdkVersion = $globalJson.'native-tools'.jdk
$JdkVersion = "11.0.24"
}

if (Test-Path $javacExe) {
Expand Down
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"xcopy-msbuild": "17.8.5"
},
"native-tools": {
"jdk": "11.0.24"
"jdk": "latest"
ViktorHofer marked this conversation as resolved.
Show resolved Hide resolved
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "10.0.0-beta.25056.1",
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/clients/java/signalr/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ allprojects {
version project.findProperty('packageVersion') ?: "99.99.99-dev"

java {
sourceCompatibility = 1.8
sourceCompatibility = 1.9
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed?

invalid source release: 9

Copy link
Member

Choose a reason for hiding this comment

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

See dotnet/sdk#45822 (comment)

We started hitting an error w/ version 8 when updating to 11.0.25:

EXEC : error : [options] source value 8 is obsolete and will be removed in a future release [D:\a\_work\1\vmr\src\aspnetcore\src\SignalR\clients\java\signalr\core\signalr.client.java.core.javaproj]

I'm not sure how to proceed if the underlying machines are in a mixed state, as they seem to be (some have 11.0.24, which is incompatible w/ 9, and some have 11.0.25, which is incompatible w/ 8)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, never mind, it's Helix, not the build machines. Do you know why non-windows is on JDK 10 & windows is on 11 here? The Helix failure was only on Mac.

aspnetcore/eng/targets/Helix.targets

Lines 3 to 4 in e0e0224

<HelixPreCommand Condition="'$(IsWindowsHelixQueue)' == 'true'" Include="call RunPowershell.cmd InstallJdk.ps1 11.0.3 %25HELIX_CORRELATION_PAYLOAD%25\jdk &amp;&amp; set %22JAVA_HOME=%25HELIX_CORRELATION_PAYLOAD%25\jdk%22" />
<HelixPreCommand Condition="'$(IsWindowsHelixQueue)' != 'true' AND '$(IsMacHelixQueue)' != 'true'" Include="./installjdk.sh 10.0.2 x64 &amp;&amp; if [ &quot;%24JAVA_HOME&quot; = &quot;&quot; ]%3B then export JAVA_HOME=%24PWD/java%3B fi" />

Copy link
Member

Choose a reason for hiding this comment

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

I thought all machines were supposed to have moved to 21.X?

Copy link
Member

Choose a reason for hiding this comment

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

Probably should remove those lines now? Wonder if we used to have to install the jdk ourselves 5 years ago.

Copy link
Member

@wtgodbe wtgodbe Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I totally missed that. So it looks like whatever version of macos-openjdk is pre-installed on the MacOS-13 queue isn't compatible w/ 1.9. @dotnet/dnceng does anyone know how we can check what version is installed on OSX.13.amd64.open? helix.dot.net doesn't list one.

Copy link
Member

Choose a reason for hiding this comment

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

Related to this ask?

Copy link
Member

Choose a reason for hiding this comment

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

dotnet/dnceng#4554

all macOS machines will have OpenJDK 8 and 21 after they are next refreshed

So maybe the machines haven't been refreshed?

Copy link
Member

Choose a reason for hiding this comment

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

Now we have

ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.

On Windows 🙄

Copy link
Member

Choose a reason for hiding this comment

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

Should probably revert 3b86147

}

repositories {
Expand Down
2 changes: 1 addition & 1 deletion ...lR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/GsonHubProtocolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ public void invocationBindingFailureWhenParsingLocalDateTimeWithoutAppropriateTy
assertEquals(HubMessageType.INVOCATION_BINDING_FAILURE, message.getMessageType());
InvocationBindingFailureMessage failureMessage = (InvocationBindingFailureMessage) messages.get(0);

assertEquals("java.lang.IllegalStateException: Expected BEGIN_OBJECT but was STRING at line 1 column 41 path $.arguments[0]", failureMessage.getException().getMessage());
assertEquals("com.google.gson.JsonSyntaxException", failureMessage.getException().getClass().getName());
}

@Test
Expand Down
Loading