Extension metadata and generation of extensions.json #4423

aloubyansky · 2019-10-07T21:43:09Z

Enhanced the quarkus-bootstrap:extension-descriptor mojo to also generate META-INF/quarkus-extension.json with the extension descr/props;
Added quarkus:generate-extensions-json mojo that generates extensions.json file (and attaches it as a Maven artifact to the project) for a BOM by collecting all the META-INF/quarkus-extension.json files from its managed deps;
Added devtools/extensions-json module that generates extensions.json file for Quarkus Core (which for now isn't used yet as it needs refinement).

I decided not to re-use META-INF/quarkus-extension.properties for this, I think we should deprecate it and have a simpler META-INF/quarkus-extension-deployment-gav or something to make it easier/faster to resolve the deployment artifact at build time.

FYI @maxandersen, @paulrobinson, @quintesse, @gastaldi

gastaldi

LGTM, added a few minor comments

devtools/maven/src/main/java/io/quarkus/maven/GenerateExtensionsJsonMojo.java

independent-projects/bootstrap/pom.xml

maxandersen · 2019-10-08T12:56:00Z

Weren't we going to put these in quarkus-extension.properties rather than introduce another marker file ?

aloubyansky · 2019-10-08T13:16:24Z

@maxandersen i mentioned my reasons in the summary of the PR.

aloubyansky · 2019-10-08T13:46:09Z

I added a few changed since @gastaldi has approved it, specifically I added the BOM GAV from which the JSON was generated to the resulting JSON. So now it's an object with attributes: bom and extensions which is an array of extensions with the usual attributes.
So now this PR needs a new approval to get merged.

gsmet · 2019-10-08T16:13:07Z

extensions/agroal/runtime/src/main/resources/META-INF/quarkus-extension.json

+
+{
+    "name": "Agroal - Database connection pool",
+    "labels": [


I'm wondering if we should include at least part of the info we have on the extensions.yaml file from the website?

See https://github.com/quarkusio/quarkusio.github.io/blob/develop/_data/extensions.yaml .

I'm thinking about:

the description

a key defining the category (I would keep the list of category key <-> name mapping and the order in the website - or on code.quarkus.io).

If we do that, I wouldn't have to add the extensions on the website.

Note that I haven't followed all the discussions so maybe it's a stupid idea :).

Sure. Here I'm taking what's in the current extensions.json as a start.

+1 on getting the minimal version in first.
Next step there efter is to add the additional features such as description, category, etc.

aloubyansky · 2019-10-08T16:50:51Z

Please, don't merge it yet. I'm looking into a build issue in Eclipse.

maxandersen · 2019-10-08T18:32:33Z

I decided not to re-use META-INF/quarkus-extension.properties for this, I think we should deprecate it and have a simpler META-INF/quarkus-extension-deployment-gav or something to make it easier/faster to resolve the deployment artifact at build time.

I'm not following why we wouldn't want to reuse the existing properites, nor why there should be -gav info at the end. Can you elaborate ?

aloubyansky · 2019-10-08T19:56:20Z

The properties file has been used so far to store the GAV of the corresponding deployment artifact. While I did have in mind to possibly extend this file with more info (primarily provisioning related), the way it is evolving now is not looking to be effective to me.

We need to discover what the corresponding deployment artifact coords are during the build and test setup in the most efficient manner. Checking for a specific JAR entry containing only what we need it is a lot more efficient then loading a potentially large properties file looking for a single key.
It seems like what we need to have at the end is content in a JSON format, it is easier to provide it in this format in the first place and avoid transforming it into properties syntax and then back.

aloubyansky · 2019-10-09T11:33:11Z

Please, don't merge it yet. I'm looking into a build issue in Eclipse.

That's related to mojo parameter initialization. E.g. we've had a parameter deployment with some defaultValue properly initialized. If I add another parameter initialized the same way with the same value, this parameter will remain null when Eclipse is building the workspace. Yet, the code that I change in the plugin is executed, so it's not like it's picking the wrong impl of the plugin. Perhaps, the wrong metadata somehow. Perhaps, it's also related to the fact that I'm testing a SNAPSHOT version. This is too weird, I'm done with it.
The conclusion is building from Eclipse is not reliable. Not a surprise, I guess.

maxandersen · 2019-10-10T13:10:13Z

It is intentional we are committing the marker files for each extension or do we plan to have our own extension using the mojo's ?

also if we are introducing META-INF/quarkus-extension.json as the official marker file I do think it would be better to either do META-INF/quarkus-extension.yaml or at least support both formats.

This was one of the reasons I was liking more META-INF/quarkus-extension.properties to start :)

aloubyansky · 2019-10-10T13:40:10Z

It is intentional we are committing the marker files for each extension or do we plan to have our own extension using the mojo's ?

Not sure I understand your question. Every extension is supposed to include the description. Whether it's generated or written maintained manually.

also if we are introducing META-INF/quarkus-extension.json as the official marker file I do think it would be better to either do META-INF/quarkus-extension.yaml or at least support both formats.

Why?

This was one of the reasons I was liking more META-INF/quarkus-extension.properties to start :)

To later support 3 formats?

maxandersen · 2019-10-10T18:02:39Z

Not sure I understand your question. Every extension is supposed to include the description. Whether it's generated or written maintained manually.

just curious as thought you said you wanted the extensions in core to have the maven plugin generate it.

maxandersen · 2019-10-10T18:03:18Z

also if we are introducing META-INF/quarkus-extension.json as the official marker file I do think it would be better to either do META-INF/quarkus-extension.yaml or at least support both formats.

Why?

yaml is better for humans - json is for machines ;)

maxandersen · 2019-10-10T18:04:03Z

This was one of the reasons I was liking more META-INF/quarkus-extension.properties to start :)

To later support 3 formats?

no, to avoid having to deal with anything but built-in to java parsing/formats :)

maxandersen · 2019-10-14T08:03:35Z

devtools/maven/src/main/java/io/quarkus/maven/GenerateExtensionsJsonMojo.java

+                    processMetaInfDir(artifact, artifactFs.getPath(BootstrapConstants.META_INF), extJsonBuilder);
+                }
+            }
+        } catch (Throwable t) {


shouldn't this not just catch (Exception e) ?

Thanks, I am actually throwing a wrong exception here. Copy-pasted from another class.

devtools/maven/pom.xml

devtools/maven/src/main/java/io/quarkus/maven/GenerateExtensionsJsonMojo.java

...-projects/bootstrap/maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java

maxandersen

There are few minor things in this but I approve this as its generally what we want to get to and I would rather have generation of extensions.json in core now than wait.

The other things we can fix in follow-up PR's as we haven't yet removed core extensions.json thus not breaking anything.

maxandersen · 2019-10-14T08:57:46Z

just noticed that the generated quarkus-extension.json in some cases ends up with:

{
  "groupId": null,
  "artifactId": null
}

i.e. ./vertx-core/runtime/target/classes/META-INF/quarkus-extension.json

others have no GAV at all:
./jgit/runtime/src/main/resources/META-INF/quarkus-extension.json:

{
    "name": "JGit",
    "labels": [
        "git"
    ]
}

and finally also when existing data you get:

./flyway/runtime/target/classes/META-INF/quarkus-extension.json

{
  "name": "Flyway",
  "labels": [
    "flyway",
    "database",
    "data"
  ],
  "guide": "https://quarkus.io/guides/flyway-guide",
  "groupId": null,
  "artifactId": null
}

Shouldn't there at least be a groupid and artifactid in here ?

gsmet · 2019-10-14T09:13:34Z

@maxandersen I wasn't able to reproduce your last issue with groupId and artifactId being null. Everything is OK here.

gsmet

I added a few questions.

It needs a rebase and once it's done, if you could run a full build with an empty repo and check that:

it works
we don't have these null groupIds and artifactIds issues

Once you have checked that, I would say let's commit it and make incremental changes on top of it.

devtools/maven/pom.xml

gsmet · 2019-10-14T08:46:00Z

independent-projects/bootstrap/maven-plugin/pom.xml

@@ -61,6 +61,10 @@
            <groupId>org.apache.maven</groupId>
            <artifactId>maven-core</artifactId>
        </dependency>
+        <dependency>
+            <groupId>com.eclipsesource.minimal-json</groupId>
+            <artifactId>minimal-json</artifactId>


Why do we use this here and not the regular JSON dependency we use elsewhere?

(Not blocking, just a question)

I was originally using the glassfish one but then George suggested me to use this impl as a smaller and faster one. Given that it's an independent project, I didn't mind changing. I won't mind changing back neither.

...-projects/bootstrap/maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java

maxandersen · 2019-10-14T09:18:03Z

I wasn't able to reproduce your last issue with groupId and artifactId being null. Everything is OK here.

shouldn't mvn package be sufficient ?

gsmet · 2019-10-14T09:19:40Z

Ah, that's a good question for Alexey. With Quarkus, I usually do mvn clean install

maxandersen · 2019-10-14T09:25:52Z

I did a new run with mvn clean package and now I have groupId/artifactId filled out where they before was null.

Still have neither in ./jgit/runtime/src/main/resources/META-INF/quarkus-extension.json:

{
    "name": "JGit",
    "labels": [
        "git"
    ]
}

aloubyansky · 2019-10-14T09:26:09Z

also if we are introducing META-INF/quarkus-extension.json as the official marker file I do think it would be better to either do META-INF/quarkus-extension.yaml or at least support both formats.

Why?
yaml is better for humans - json is for machines ;)

I can do that. But I'll keep only one format. The aggregated extensions list will still be in JSON though, I guess.

aloubyansky · 2019-10-14T09:36:00Z

just noticed that the generated quarkus-extension.json in some cases ends up with:
{
  "groupId": null,
  "artifactId": null
}
i.e. ./vertx-core/runtime/target/classes/META-INF/quarkus-extension.json

others have no GAV at all:
./jgit/runtime/src/main/resources/META-INF/quarkus-extension.json:
{
    "name": "JGit",
    "labels": [
        "git"
    ]
}
and finally also when existing data you get:

./flyway/runtime/target/classes/META-INF/quarkus-extension.json
{
  "name": "Flyway",
  "labels": [
    "flyway",
    "database",
    "data"
  ],
  "guide": "https://quarkus.io/guides/flyway-guide",
  "groupId": null,
  "artifactId": null
}
Shouldn't there at least be a groupid and artifactid in here ?

That's probably your IDE building it? There is a comment somewhere above where I summarized my struggling figuring out why Eclipse (in my case) didn't properly initialize Mojo parameters.

aloubyansky · 2019-10-14T09:37:23Z

I did a new run with mvn clean package and now I have groupId/artifactId filled out where they before was null.

Still have neither in ./jgit/runtime/src/main/resources/META-INF/quarkus-extension.json:
{
    "name": "JGit",
    "labels": [
        "git"
    ]
}

Yes, but that's src dir. It's a template. You should be checking the target one.

gsmet · 2019-10-14T09:38:09Z

The target one is perfectly OK for me.

aloubyansky · 2019-10-14T09:40:25Z

I wasn't able to reproduce your last issue with groupId and artifactId being null. Everything is OK here.

shouldn't mvn package be sufficient ?

Worked for me.

…nerate META-INF/quarkus-extension.json with the extension descr/props; * Added quarkus:generate-extensions-json mojo that generates extensions.json file (and attaches it as a Maven artifact to the project) for a BOM by collecting all the META-INF/quarkus-extension.json files from its managed deps; * Added devtools/core-extensions-json module that generates extensions.json file for Quarkus Core (which for now isn't used yet as it needs refinement).

maxandersen · 2019-10-14T10:10:44Z

That's probably your IDE building it? There is a comment somewhere above where I summarized my struggling figuring out why Eclipse (in my case) didn't properly initialize Mojo parameters.

could be - I had code open that uses m2e behind the scenes.
Lets not block this on that issue but I opened #4542 as it would affect extension writers so would be good to have a way to handle/workaround this.

FroMage · 2019-10-14T12:42:55Z

Damnit, I only noticed this issue now. I would prefer to generate that quarkus-extension.json file from our existing maven extension plugin if the following properties are defined (and the file does not exist already):

<project>
 ...
 <properties>
  <quarkus.extension.name>RESTEasy - Jackson</quarkus.extension.name>
  <quarkus.extension.labels>resteasy-jackson,jaxrs-json,resteasy-json,resteasy,jaxrs,json,jackson</quarkus.extension.labels>
 </properties>
...
</project>

FroMage · 2019-10-14T12:52:42Z

...-projects/bootstrap/maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java

@@ -78,5 +95,37 @@ public void execute() throws MojoExecutionException {
        } catch(IOException e) {
            throw new MojoExecutionException("Failed to persist extension descriptor " + output.resolve(BootstrapConstants.DESCRIPTOR_FILE_NAME), e);
        }
+


@aloubyansky I don't understand this code: it appears to read the file if it exists, and then writes it back. Surely I'm reading this wrong?

Not entirely wrong :) At this point, the file that's actually on the disk is missing groupId and artifactId, which makes it more like a template that needs to be complete by the plugin. However, you're right, it could originally be complete, in which case persisting it is useless.

But you're reading it from the output folder. Shouldn't it be read from the src folder?

I am reading it from the output folder to support filtering of resources, just in case.

OK, if it's done on purpose then fine :)

ia3andy · 2019-10-14T13:45:15Z

Please, don't merge it yet. I'm looking into a build issue in Eclipse.

We should introduce the WIP bot in here?

maxandersen · 2019-10-14T22:37:21Z

We should introduce the WIP bot in here?

isn't that superseeded by github now directly supporting drafts pull-requests ?

aloubyansky added the area/platform Issues related to definition and interaction with Quarkus Platform label Oct 7, 2019

gastaldi approved these changes Oct 7, 2019

View reviewed changes

devtools/maven/src/main/java/io/quarkus/maven/GenerateExtensionsJsonMojo.java Outdated Show resolved Hide resolved

independent-projects/bootstrap/pom.xml Outdated Show resolved Hide resolved

aloubyansky force-pushed the extensions-json-generator branch from 796c951 to 2706925 Compare October 8, 2019 08:55

aloubyansky added this to the 0.24.0 milestone Oct 8, 2019

aloubyansky force-pushed the extensions-json-generator branch from 2706925 to ad0e573 Compare October 8, 2019 12:40

gsmet reviewed Oct 8, 2019

View reviewed changes

aloubyansky added the coding-in-progress label Oct 8, 2019

gsmet modified the milestones: 0.24.0, 0.25.0 Oct 9, 2019

aloubyansky force-pushed the extensions-json-generator branch from ad0e573 to 29ecf77 Compare October 9, 2019 11:26

aloubyansky removed the coding-in-progress label Oct 9, 2019

maxandersen reviewed Oct 14, 2019

View reviewed changes

devtools/maven/pom.xml Show resolved Hide resolved

maxandersen reviewed Oct 14, 2019

View reviewed changes

devtools/maven/src/main/java/io/quarkus/maven/GenerateExtensionsJsonMojo.java Show resolved Hide resolved

maxandersen reviewed Oct 14, 2019

View reviewed changes

...-projects/bootstrap/maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java Show resolved Hide resolved

maxandersen approved these changes Oct 14, 2019

View reviewed changes

gsmet added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Oct 14, 2019

gsmet reviewed Oct 14, 2019

View reviewed changes

maxandersen mentioned this pull request Oct 14, 2019

quarkus-extension manifests and their formats ? #4539

Open

aloubyansky force-pushed the extensions-json-generator branch from 29ecf77 to 1901746 Compare October 14, 2019 09:55

maxandersen mentioned this pull request Oct 14, 2019

issue with extension generator nulls when m2e involved #4542

Closed

machi1990 removed the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Oct 14, 2019

aloubyansky merged commit 51c3476 into quarkusio:master Oct 14, 2019

FroMage mentioned this pull request Oct 14, 2019

Config all docs: temporary titles, sorted, faster #4522

Merged

FroMage reviewed Oct 14, 2019

View reviewed changes

ia3andy mentioned this pull request Oct 14, 2019

Make the extensions.json consistant quarkusio/code.quarkus.io#40

Closed

paulrobinson mentioned this pull request Oct 15, 2019

Platform & Tools MVP #4566

Closed

18 tasks

Extension metadata and generation of extensions.json #4423

Extension metadata and generation of extensions.json #4423

Conversation

aloubyansky commented Oct 7, 2019

gastaldi left a comment

Choose a reason for hiding this comment

maxandersen commented Oct 8, 2019

aloubyansky commented Oct 8, 2019

aloubyansky commented Oct 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aloubyansky commented Oct 8, 2019

maxandersen commented Oct 8, 2019

aloubyansky commented Oct 8, 2019

aloubyansky commented Oct 9, 2019

maxandersen commented Oct 10, 2019

aloubyansky commented Oct 10, 2019

maxandersen commented Oct 10, 2019

maxandersen commented Oct 10, 2019

maxandersen commented Oct 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxandersen left a comment

Choose a reason for hiding this comment

maxandersen commented Oct 14, 2019

gsmet commented Oct 14, 2019

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxandersen commented Oct 14, 2019

gsmet commented Oct 14, 2019

maxandersen commented Oct 14, 2019

aloubyansky commented Oct 14, 2019

aloubyansky commented Oct 14, 2019

aloubyansky commented Oct 14, 2019

gsmet commented Oct 14, 2019

aloubyansky commented Oct 14, 2019

maxandersen commented Oct 14, 2019

FroMage commented Oct 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ia3andy commented Oct 14, 2019

maxandersen commented Oct 14, 2019