Validation of codestart artifacts configured in extension descriptors

[aloubyansky]

• Adds an option to the extension metadata generator to validate the codestart artifact, in case it's configured, by resolving it.
• Completes the coordinates of the codestart, in case the configured coordinates were not complete, i.e. missing the version.
• Changed previously pre-configured io.quarkus:quarkus-descriptor-json to io.quarkus:quarkus-platform-descriptor-json that actually exists.
• Support loading codestarts referenced from extension metadata.

[aloubyansky]

Unfortunately, I did not isolate the artifact id fix, so this is not good for backport.

[aloubyansky]

@famod so, I went ahead and removed all the dependencies from the quarkus-platform-descriptor-json (which is not a descriptor json at all and now is renamed to quarkus-core-codestarts). Can you think of consequences on the incremental build?

The other issue introduced by this PR is that each extension (in this repo) may reference a codestart artifact, which typically will be quarkus-core-codestarts. The extension descriptor generating plugin (configured for every runtime artifact) is by default validating that the referenced codestart artifact is resolvable. But this dependency isn't really a Maven dependency. So we need to somehow make sure the build order is correct. I was thinking about adding a dependecy on quarkus-core-codestarts:pom from the quarkus-extensions-parent, this doesn't look nice though.
Also maven and gradle plugins appear to directly depend on the codestarts artifact which looks like another clean up candidate.

[aloubyansky]

Actually, I guess, the codestart artifact doesn't actually need to be resolvable (i.e. built) before the extension descriptor is generated. I was thinking I could simply check whether either the project producing the codestart artifact is found in the reactor or, if it isn't, it is resolvable. This could avoid the hard build ordering. But it would still have to be present in the reactor.

[ia3andy]

@aloubyansky maybe we should make another PR to be backported (for the wrong artifacts)?

[aloubyansky]

We could extract the fix, yes.

[ia3andy]

@aloubyansky will it be enough to have the artifact correct without the new code which add the classifier & version?

[famod]

@aloubyansky

@famod so, I went ahead and removed all the dependencies from the quarkus-platform-descriptor-json (which is not a descriptor json at all and now is renamed to quarkus-core-codestarts). Can you think of consequences on the incremental build?

Well, it seems we now don't need that incremental profile anymore in devtools/bom-descriptor-json:

mvn -pl devtools/bom-descriptor-json -amd validate
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Quarkus - BOM - Descriptor JSON                                    [pom]
[INFO] Quarkus - Integration Tests - Maven tooling                        [jar]
[INFO]

So, for incremental builds, we do not have to "disable" all the dependencies from bom-descriptor-json to all the extensions anymore (to stop running all ITs in case just a single extension was changed).
Now, whether or not it is correct that changes to bom-descriptor-json itself (or any of it's non-extension upstream modules) do not trigger any ITs anymore (apart from "Maven tooling") is something that I cannot fully answer.

I can move those dependencies out of the profile via a separate PR or you can do it in this PR. If you do, please also remove this bit:
https://github.com/quarkusio/quarkus/blob/main/.github/workflows/ci-actions-incremental.yml#L94-L95
and please switch -Pincremental to -Dincremental.

[famod]

@aloubyansky and about this:

So we need to somehow make sure the build order is correct. I was thinking about adding a dependecy on quarkus-core-codestarts:pom from the quarkus-extensions-parent, this doesn't look nice though.

This would mean that a change to quarkus-core-codestarts would trigger all extensions to be built and in turn all their ITs!
Doesn't seem like a good idea...unless we introduce such an incremental profile workaround as in bom-descriptor-json.

PS: Such a profile would mean that we need to retain that -P vs. -D workaround in the workflow file (see previous comment), I guess. 🤔

[aloubyansky]

Right. We just need the devtools IT (maven, gradle, devtools) to depend on the codestarts artifact.

[aloubyansky]

Actually, the json descriptor artifact has the same requirement as the codestarts above from testing perspective.

[aloubyansky]

@famod I think there is an issue with the integration-tests/gradle/update-dependencies.sh I am trying to add a dependency on the pom of quarkus-project-core-extension-codestarts (previously quarkus-core-codestarts) with

 dependencies {
+    testImplementation "io.quarkus:quarkus-project-core-extension-codestarts:${version}@pom"

in the build.gradle and

+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-project-core-extension-codestarts</artifactId>
+            <version>${project.version}</version>
+            <type>pom</type>
+            <scope>test</scope>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>
+                    <artifactId>*</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>

in the pom.xml. But if I launch the script it changes the pom to

+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-project-core-extension-codestarts</artifactId>
+            <version>${project.version}</version>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-resteasy</artifactId>
@@ -154,6 +159,19 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-project-core-extension-codestarts-deployment</artifactId>
+            <version>${project.version}</version>
+            <type>pom</type>
+            <scope>test</scope>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>
+                    <artifactId>*</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>

and then fails to resolve the deployment, of course. It shouldn't be assuming that this pom dependency pattern is reserved for extension artifacts. Would that be difficult to fix? Thanks.

[famod]

@aloubyansky
Just add something like -e '/-extension-codestarts/d' here: https://github.com/quarkusio/quarkus/blob/main/integration-tests/gradle/update-dependencies.sh#L58

But it will still add a non-deployment dependency (without exclusions) and I think this is ok. Those deps won't be really used anyway.

So best just adjust the script but don't touch the pom and run the script.

[aloubyansky]

I could just add that in place of the pom one, right?

[famod]

I could just add that in place of the pom one, right?

If you want to do it manually you can of course do it. Or you can just watch what happens when you run the script. 😉

There is also another approach:
By adding the mentioned deletion expression here, the script would ignore that gradle dep entirely:
https://github.com/quarkusio/quarkus/blob/main/integration-tests/gradle/update-dependencies.sh#L38
You would then have to add it manually to the pom (similar to io.quarkus.gradle.plugin).

[aloubyansky]

If I leave the "runtime" one, it still adds the deployment one apparently.

[famod]

Have you added -e '/-extension-codestarts/d' to the script?

[aloubyansky]

No, i thought it wouldn't be necessary.

[famod]

You must tell the script to either ignore it only for the deployment part or entirely (manual addition is then required).

[famod]

Rationale: The script is extracting all Quarkus artifact ids from all gradle build files, applies a filter (=line deletions), creates "runtime" dep entries and then applies another filter and adds deployment dep entries.

[aloubyansky]

Why is adding the -deployment dep for

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-project-core-extension-codestarts</artifactId>
            <version>${project.version}</version>
        </dependency>

though?

[famod]

Because it is not yet filtered (unless you added the line deletion expression already).
See also:

• https://github.com/quarkusio/quarkus/blob/main/integration-tests/gradle/update-dependencies.sh#L55-L56
• https://github.com/quarkusio/quarkus/blob/main/integration-tests/gradle/update-dependencies.sh#L58

[aloubyansky]

Ok, thanks!

[aloubyansky]

I should just remove it from build.gradle

[famod]

If it's not required, neither for the gradle test execution nor for the maven build order, then it should go.

[aloubyansky]

The artifact must be built for the gradle testsuite.

[aloubyansky]

Hm, the script simply removes it unless it's in build.gradle.

[famod]

Hm, the script simply removes it unless it's in build.gradle.

It won't if you put it before

<!-- START update-dependencies.sh -->

or after

<!-- END update-dependencies.sh -->

[ia3andy]

Looking great!

We will still need to move the codestarts/quarkus/core and codestarts/quarkus-jbang codestarts to devtools-common so we only keep the extensions codestarts in quarkus-project-core-extension-codestarts. The reason is also that those codestarts are more tied to the api so better bring them alongside it..