-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Initialize dependency validation for gradle extensions #21856
Initialize dependency validation for gradle extensions #21856
Conversation
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 85aebf3
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
✖
✖
✖
⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
✖
✖
✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: devtools/gradle/gradle-extension-plugin
📦 devtools/gradle/gradle-extension-plugin✖ |
85aebf3
to
460cb56
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 460cb56
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
✖
✖
✖
⚙️ JVM Tests - JDK 11 #- Failing: devtools/gradle/gradle-extension-plugin
📦 devtools/gradle/gradle-extension-plugin✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: devtools/gradle/gradle-extension-plugin
📦 devtools/gradle/gradle-extension-plugin✖ ⚙️ JVM Tests - JDK 17 #- Failing: devtools/gradle/gradle-extension-plugin
📦 devtools/gradle/gradle-extension-plugin✖ |
b1e446f
to
640dfdc
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 640dfdc
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: devtools/gradle/gradle-extension-plugin
📦 devtools/gradle/gradle-extension-plugin✖ |
@glefloch CI doesn't look happy. |
640dfdc
to
6302beb
Compare
looks better now, @aloubyansky could you have a look ? |
@@ -11,6 +11,7 @@ dependencies { | |||
|
|||
quarkusExtension { | |||
deploymentArtifact = "org.acme:ext-f-deployment:1.0-SNAPSHOT" | |||
disableValidation = true |
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.
What was the reason for disabling validation here and in other projects?
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.
In the test module, we have multiple extensions in a multi module project. Dependencies between module are declared using the following syntax project(':ext-b:deployment')
. In the validation task, when looking for deployment dependencies, we look for what was declared in quarkus-extension.properties
files. In this project this differs from project.group:project.name
(this is due to a limitation of having multiple module calls runtime
and deployment
). I was thinking of pushing a commit to improve that later on.
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.
Does it mean that validation wouldn't work for extensions in the same project?
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.
if we have multiple extension in a the same project that depends to each other, the validation will fail yes.
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.
We probably shouldn't enable it by default then until it's supported.
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.
Should I remove the task from the graph ? this would avoid introducing a "breaking change" in further versions.
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.
Yes, I think so.
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.
I pushed a fix, the task must be explicitly called to be executed
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.
So these disableValidation = true
could be removed now?
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.
sure done :)
6302beb
to
3e8a005
Compare
3e8a005
to
6a907d0
Compare
This branch adds a new task that aims to validate extension dependencies. This checks (based on extension detected in the runtime module):
This check can be disabled.
BTW, we start to have some tooling to create extension project. With some this could be externalize and re-used in other gradle tests.