Skip to content

Refactor deprecated gradle syntax - #20922

Merged
snnn (snnn) merged 7 commits into
mainfrom
Cjian/dg
Jun 7, 2024
Merged

Refactor deprecated gradle syntax #20922
snnn (snnn) merged 7 commits into
mainfrom
Cjian/dg

Conversation

@jchen351

@jchen351 Jian Chen (jchen351) commented Jun 4, 2024

Copy link
Copy Markdown
Contributor

To replaced deprecated API.
Should verify with the Gradle cmakeCheck step from Windows_Packaging_CPU_x64_default stage from the Zip-Nuge-... pipeline.

@jchen351
Jian Chen (jchen351) requested a review from a team June 4, 2024 21:59
@skottmckay

Copy link
Copy Markdown
Contributor

Adam Pocock (@Craigacp) in case there are any concerns

@Craigacp

Copy link
Copy Markdown
Contributor

I also needed to do this to make this deprecation warning (https://docs.gradle.org/8.5/userguide/upgrading_version_8.html#test_framework_implementation_dependencies) go away:

diff --git a/java/build.gradle b/java/build.gradle
index e09fd01e93..96a8d43a3b 100644
--- a/java/build.gradle
+++ b/java/build.gradle
@@ -174,6 +174,7 @@ if (cmakeBuildDir != null) {
 dependencies {
        testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.2'
        testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.2'
+       testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.10.2'
        testImplementation 'com.google.protobuf:protobuf-java:3.21.7'
 }

It looks like that PR broke header file generation for the JNI binding, not had chance to look deeper in to it yet.

@jchen351

Copy link
Copy Markdown
Contributor Author

I also needed to do this to make this deprecation warning (https://docs.gradle.org/8.5/userguide/upgrading_version_8.html#test_framework_implementation_dependencies) go away:

diff --git a/java/build.gradle b/java/build.gradle
index e09fd01e93..96a8d43a3b 100644
--- a/java/build.gradle
+++ b/java/build.gradle
@@ -174,6 +174,7 @@ if (cmakeBuildDir != null) {
 dependencies {
        testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.2'
        testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.2'
+       testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.10.2'
        testImplementation 'com.google.protobuf:protobuf-java:3.21.7'
 }

It looks like that PR broke header file generation for the JNI binding, not had chance to look deeper in to it yet.

It should have been fixed.

@snnn

Copy link
Copy Markdown
Contributor

Adam Pocock (@Craigacp) , could you please help review it?

@snnn

Copy link
Copy Markdown
Contributor

Right now the Gradle cmakeCheck step fails in "Nuget-CUDA-Packaging-Pipeline" because it tries to run GPU tests on a CPU-only machine. This PR would not address the issue, right? But we need to get it right before the next patch release.

@Craigacp

Copy link
Copy Markdown
Contributor

Right now the Gradle cmakeCheck step fails in "Nuget-CUDA-Packaging-Pipeline" because it tries to run GPU tests on a CPU-only machine. This PR would not address the issue, right? But we need to get it right before the next patch release.

I think fixing that would involve modifying the gradle command invoked by that pipeline. The CUDA GPU tests are automatically enabled if Java tests are run when CUDA is enabled in the build. What things is that pipeline supposed to test (e.g. is it ok if all the Java tests are disabled)?

@Craigacp Adam Pocock (Craigacp) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM but I can't see what the NPM test failures are as it wants me to log in to Azure.

@snnn
snnn (snnn) merged commit d32adb2 into main Jun 7, 2024
@snnn
snnn (snnn) deleted the Cjian/dg branch June 7, 2024 18:08
Chunye Wang@AMD (wcy123) pushed a commit to wcy123/onnxruntime that referenced this pull request Jun 8, 2024
To replaced deprecated API. 
Should verify with the `Gradle cmakeCheck` step from
`Windows_Packaging_CPU_x64_default` stage from the Zip-Nuge-...
pipeline.
@sophies927 Sophie Schoenmeyer (sophies927) added the triage:approved Approved for cherrypicks for release label Jun 11, 2024
Yifan Li (yf711) pushed a commit that referenced this pull request Jun 18, 2024
To replaced deprecated API. 
Should verify with the `Gradle cmakeCheck` step from
`Windows_Packaging_CPU_x64_default` stage from the Zip-Nuge-...
pipeline.
Yifan Li (yf711) pushed a commit that referenced this pull request Jun 18, 2024
To replaced deprecated API. 
Should verify with the `Gradle cmakeCheck` step from
`Windows_Packaging_CPU_x64_default` stage from the Zip-Nuge-...
pipeline.
Baiju Meswani (baijumeswani) pushed a commit that referenced this pull request Jun 20, 2024
To replaced deprecated API. 
Should verify with the `Gradle cmakeCheck` step from
`Windows_Packaging_CPU_x64_default` stage from the Zip-Nuge-...
pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:1.18.1 triage:approved Approved for cherrypicks for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants