quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.5k stars 2.6k forks source link

quarkus-extension manifests and their formats ? #4539

Open maxandersen opened 4 years ago

maxandersen commented 4 years ago

In https://github.com/quarkusio/quarkus/pull/4423 we are pushing towards a model where instead of a singular manually maintained extensions.json in core we'll have each extension contain a manifest file. Then each manifest file will be used as input to a platform definition - of which one part will be the list of extensions.

For the manifest the current PR suggests a file per extension jar called META-INF/quarkus-extension.json with a content similar to this:

{
  "name": "RESTEasy - Jackson",
  "labels": [
    "resteasy-jackson",
    "jaxrs-json",
    "resteasy-json",
    "resteasy",
    "jaxrs",
    "json",
    "jackson"
  ],
  "groupId": "io.quarkus",
  "artifactId": "quarkus-resteasy-jackson"
}

question is just if json is the "right" format to have ? Would using .properties or yaml be better ?

In addition I see we have a bunch of other files:

quarkus-config-roots.list

io.quarkus.vertx.core.runtime.config.VertxConfiguration

quarkus-extension.properties:

#Generated by extension-descriptor
#Mon Oct 14 11:21:28 CEST 2019
deployment-artifact=io.quarkus\:quarkus-vertx-core-deployment\:999-SNAPSHOT

quarkus-javadoc.properties:

io.quarkus.vertx.core.runtime.config.PfxConfiguration.path=Path to the key file (PFX format)
io.quarkus.vertx.core.runtime.config.EventBusConfiguration.reusePort=Whether or not to reuse the port.
io.quarkus.vertx.core.runtime.config.VertxConfiguration.classpathResolving=Enables or disabled the Vert.x classpath resource resolver.
...
...

Do we need all these to be in ~4 different files with 3 different formats ?

aloubyansky commented 4 years ago

quarkus-extension.properties:

#Generated by extension-descriptor
#Mon Oct 14 11:21:28 CEST 2019
deployment-artifact=io.quarkus\:quarkus-vertx-core-deployment\:999-SNAPSHOT

As I mentioned before, I propose to deprecate this one and simply use something like quarkus-extension-deployment that would contain io.quarkus:quarkus-vertx-core-deployment:999-SNAPSHOT. That way we won't have to parse some formatted file containing other stuff during the build/test setup.

Otherwise a possible categorization could be build-time info and the rest with the idea of getting the build more efficient by avoiding parsing non-build related files.

maxandersen commented 4 years ago

yes, makes sense for that file but does it make sense to also have quarkus-config-roots.list ? not sure when it is used but at the moment we seem to end up what looks like "random" files from a enduser perspective. Not saying need to remove them - but looking to grok why we have them and if it can be improved.

FroMage commented 4 years ago

Aside from the issue of where to store the info, I'd like to open up the question of where it comes from.

Given that we pretty much force people to use Maven for building (our extension plugin has to run), it could make sense to generate that descriptor file from Maven properties, like:

<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>
maxandersen commented 4 years ago

If that is not already there in the plugin then sure. It is already generated by merging pom.xml and whatever file you have.

I would still like to have a consistent format that users can decide to use or implement support for in Other build systems. Personally I remove everything I can from pom.xml as possible and keep in relevant artifacts.

FroMage commented 4 years ago

It is already generated by merging pom.xml and whatever file you have

Not sure what file you refer to?

I'm saying that if name/labels are the only two infos we need to be in this new extension descriptor, I'd rather not require the users to create a file for it. Especially since they have to repeat the groupId:artifactId they already have, and they already must have a pom.xml file.

rsvoboda commented 4 years ago

I have only slightly related comment to this so bare with :) Today I was looking at MicroProfile (MP) stuff Quarkus is covering and it's not easy to find out all guides and extensions related to MP even-though Quarkus and MP relationship is stressed quite often.

https://quarkus.io/guides/ - only few hits with Command/Ctrl+F, an example: OpenTracing doesn't contain MicroProfile keyword. Maybe there could be dedicated MicroProfile section which would aggregate all the guides related to MP

https://code.quarkus.io/ - 6 hits when searching microprofile in Extensions field. smallrye keyword returns 13 hits. People will search for microprofile, smallrye is implementation detail for end user.

maxandersen commented 4 years ago

It is already generated by merging pom.xml and whatever file you have

Not sure what file you refer to?

I'm saying that if name/labels are the only two infos we need to be in this new extension descriptor, I'd rather not require the users to create a file for it. Especially since they have to repeat the groupId:artifactId they already have, and they already must have a pom.xml file.

i'm referring to quarkus-properties.json and there already are a plugin to add the groupid/artifactid - It should also support the other values; if not we for sure should add that.

primarily in this issue interested in cleaning up our current list of "manifests" files, there are 4 of them using 3 different formats ;)

FroMage commented 4 years ago

i'm referring to quarkus-properties.json and there already are a plugin to add the groupid/artifactid

I just noticed that it does not auto-generate the file, I'm a sad panda. I'll comment there, then.

aloubyansky commented 4 years ago

Given that we pretty much force people to use Maven for building (our extension plugin has to run), it could make sense to generate that descriptor file from Maven properties, like:

<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>

That's a nice idea. One thing though, now that we may have extension.json which is an extension descriptor (you can see it in any runtime artifact if you rebase) that could be put in src/resources, you may have two places to define values, which could be confusing. That could be caught by the plugin though and you could be forced to make your choice i.e. provide (the conflicting) values in the json file or using the properties.

aloubyansky commented 4 years ago

i'm referring to quarkus-properties.json and there already are a plugin to add the groupid/artifactid

I just noticed that it does not auto-generate the file, I'm a sad panda. I'll comment there, then.

It should be generating the file. At least with groupId and artifactId. We could add support for other properties with basic defaults.

FroMage commented 4 years ago

It should be generating the file

I meant, as part of the build, by our extension maven plugin. Not manually.

FroMage commented 4 years ago

That's a nice idea. One thing though, now that we may have extension.json which is an extension descriptor (you can see it in any runtime artifact if you rebase) that could be put in src/resources, you may have two places to define values, which could be confusing. That could be caught by the plugin though and you could be forced to make your choice i.e. provide (the conflicting) values in the json file or using the properties.

Cool. Yes indeed it would have to avoid ambiguity, as you say. Should I open a new issue for this, then?

aloubyansky commented 4 years ago

Sure.

loicmathieu commented 4 years ago

I have some concern about extension identifier (feature name from the build step, should be the artifact name without the quarkus prefix) and config root (that should be but is not always the same of the identifier). I think it must be specified somewhere, maybe this should aslo be added in the JSON file:

{
  "name": "RESTEasy - Jackson",
  "identifier":"resteasy-jackson",
  "configRoot":"resteasy.jackson",-- or quarkus.resteasy.jackson ...
  "labels": [
    "resteasy-jackson",
    "jaxrs-json",
    "resteasy-json",
    "resteasy",
    "jaxrs",
    "json",
    "jackson"
  ],
  "groupId": "io.quarkus",
  "artifactId": "quarkus-resteasy-jackson"
}
kenfinnigan commented 4 years ago

+1 to using a plain properties file format for the generated extension metadata. We don't want to require a JSON parser just to read what are a bunch of properties for an extension.

+1 to the content of the file coming from pom.xml properties and not a file that's manually edited in the extension source.

aloubyansky commented 4 years ago

+1 to using a plain properties file format for the generated extension metadata. We don't want to require a JSON parser just to read what are a bunch of properties for an extension.

As long as our values are simple.

+1 to the content of the file coming from pom.xml properties and not a file that's manually edited in the extension source.

I think it's important to take into account that the number of properties may grow and some of them will be relatively long text content, which may be more convenient to keep outside of the pom. For example actual descriptions. @ia3andy wants to have a few kinds of those.

kenfinnigan commented 4 years ago

I'd question any kind of metadata that can't be expressed in a simple manner

Why does it matter how many properties we might have in pom.xml, or how long some of the values might be? It's a natural place to store metadata about the artifact.

If we didn't have or didn't want, there to be a Maven plugin that executes during the build for an extension, then I can understand the argument for a file in the source tree

maxandersen commented 4 years ago

I'm really not talking about files in source tree here - I'm talking about the 4 files we currently have in every extension. We need to document them IMO for how a plugin is written.

How and where you put the info should be up to you - we should make it easy to do from your pom.xml but the pom is not something we can/should rely on during runtime thus how its created is separate from the end result.

Also, I seriously hope we will have native gradle support for extensions eventually.

And to be clear, my first choice is also to use .properties format for these values.

emmanuelbernard commented 4 years ago

If a file is 100% generated and therefore not seen by an extension developer, then I don't care too much even though rationalization of # of files makes sense). If the file might be manually edited, then JSON is really unfriendly. I'd argue two formats are friendly for the kind of list and nested structures we have:

even though .properties is not as great at lists.

And it seems the META-INF/quarkus-extension.json might be something edited by the extension writer.

ia3andy commented 4 years ago

I think we should pick only one format and one possible location for this (to avoid confusion), I don't have any thought on the format and location. As you said we discussed @aloubyansky, it should be a format that let define textual fields (like long and possibly formatted descriptions).

FroMage commented 4 years ago

Well, pom.xml has a <description> element that nobody knews why it's there, we might as well use it for our extension description :)

As for Gradle, I'm pretty sure you'll be able to express those properties in it as well.

FroMage commented 4 years ago

It even has <name>, just sayin'…

FroMage commented 4 years ago

So really we're missing labels and guide, is all.

maxandersen commented 4 years ago

Pom.xml Is not inside the generated artifact. Also do we really want to limit extension writers to maven ?

Could we focus on agreeing on what the end result will be ? We can have plenty of intermediate ways to get to that result - Pom.xml, a yaml file, gradle contents etc.

maxandersen commented 4 years ago

Actually - if noone really seem to care of the resulting format then .properties is the way to go. No extra dependencies and we have ways to map between XML/yaml/JSON to .properties so we can have it in pom.xml or separate file.

Its not pretty but it's simple :)

FroMage commented 4 years ago

Pom.xml Is not inside the generated artifact. Also do we really want to limit extension writers to maven ?

Well, first I think that's wrong because pom.xml ends up in the generated artifact, but I was only saying we should make the extension descriptor generated, so we'd only use the pom.xml file as a source for the info we use to generate it at build time. And I also mentioned it'd be possible to do it in Gradle as well.

As for what type of descriptor gets generated, I don't care if it's generated. It could very well be binary as far as I'm concerned, since it's generated ;)

maxandersen commented 4 years ago

Well I disagree on it being 100% generated as I personally dislike frameworks requiring layers of intermediate formats when it can easily avoided.

Even if pom.xml is inside be artifacts it's not something we want to start parsing at runtime is it ? Nor want to require users to have when using gradle, right?

Anyway sounds like noone cares about the end result inside the artifact so I say we just keep it simple - make it a properties file which (if present) gets read by our maven plugin and injects whatever values exist in pom.xml too. That allows use of resource filtering and even other intermediate formats if one so wishes.

maxandersen commented 4 years ago

That doesn't answer what the other files are for - if anyone knows would be great to know if should stay separate or not.

FroMage commented 4 years ago

Well I disagree on it being 100% generated as I personally dislike frameworks requiring layers of intermediate formats when it can easily avoided.

Even if pom.xml is inside be artifacts it's not something we want to start parsing at runtime is it ? Nor want to require users to have when using gradle, right?

I've said this enough times here that I'm starting to think you haven't read me or I can't express myself correctly ;)

At this point, users must have a pom.xml (or gradle equivalent) to build an extension, and it already has much of the info we want to end up in the extension descriptor, so it doesn't make a lot of sense to require users to hand-write that descriptor. It's unnecessary complexity of adding an additional file to write/maintain.

If they want to write it, fine, we'll use it. But we have #4551 open to make it unnecessary, and to have the file auto-generated by our extension maven plugin during building, based on input from the pom.xml file.

Once more, I haven't checked if we have a gradle extension plugin, but if we do I assume it will be trivial to achieve the same by using the info from the gradle build descriptor instead of pom.xml.

This will generate the extension descriptor during build, so we won't need to parse the pom.xml or gradle build file at run-time.

Clearer?

ia3andy commented 4 years ago

I agree with @FroMage, I think having those info in the pom or equivalent is the simplest for the developer since it already contains some of the extension info. Like you would do with npm or some other similar project, the project configuration file contains those info.

I am against having many different possibilities to configure it (in the pom or in the properties), it makes things more confusing to user (IMHO).

FroMage commented 4 years ago

As for who uses those files:

FroMage commented 4 years ago

Now, what do they contain?

BTW, you forgot quarkus-build-steps.list which contains the list of build step classes to run at build time.

FroMage commented 4 years ago

Now, does it matter that different parts of the build generate different descriptors? Not sure. But they do get mostly generated at different build phases (APT, extension maven plugin) and would be impossible to generate at the same time, so it's justified to have them in separate files when producing them.

Now, if we absolutely must consolidate them, we could consolidate them in the (generated) extension descriptor (that we're discussing here), by aggregating all those generated files into a single one at some point.

Is it useful? Not sure. In general I don't mind what files get generated as long as I never have to edit them myself.

maxandersen commented 4 years ago

@i3andy I must be missing something here - why are everyone happy to force users to use pom.xml to write this kind of info ? Sure it's convenient that you can but require it I don't grok it. Shouldn't we have application.properties specified in pom.xml too ? I'm missing something in this reasoning :)

aloubyansky commented 4 years ago
  • quarkus-extension.properties is ignored by DevMojo and RunnerJarPhase. It's written by our maven extension plugin and used by our bootstrap maven resolver.

Both of those do use the bootstrap maven resolver though.

aloubyansky commented 4 years ago
  • quarkus-extension.properties contains a pointer from the extension runtime jar to its deployment counterpart's GAV. I guess we could store this info in the extension descriptor we generate (the one we're discussing here).

https://github.com/quarkusio/quarkus/issues/4539#issuecomment-541585513

FroMage commented 4 years ago

Both of those do use the bootstrap maven resolver though.

Sorry, I meant "this file is explicitely skipped by DevMojo and RunnerJarPhase", but as you say, lost of stuff depends on the boostrap maven resolver.

FroMage commented 4 years ago

why are everyone happy to force users to use pom.xml to write this kind of info

Because we only need two user inputs for that file: labels and guide. All the rest comes from other places. The GAV, the name, description are already present in pom.xml.

application.properties on the other hand is a (sometimes) run-time thing that can be tweaked to change runtime behaviour.

maxandersen commented 4 years ago

Thanks for the details @fromage.

I'm also fine with separate files if it makes sense and due to the different phases in play at both build and runtimes it probably.makes sense.

Reason why I care about their number and formats is that it's the way I learn how things work. I get annoyed by complex build systems and often to remove any misunderstanding I go look at the generated result to grok how things works. And then seeing multiple files with different formats and naming it's not helping.

Also I was starting to write up what makes as extension and it just gets messy. Hence my interest in cleaning it up where I can get clean before 1.0.

Alexey said he moved the gav outside the metadata info for performance reasons but not sure that applies if we put it in properties and will want to parse the metadata anyways.

Weren't you looking for a place to lookup extension id in your annotation processor too?

FroMage commented 4 years ago

My problem ATM is that I have a set of config root class names and need to find which extensions they belong to, and what those extension names are. Not sure how to solve that, frankly.

But yeah, if we do aggregate all the current info into one file, then I should be able to get that.

maxandersen commented 4 years ago

You don't have these classes package/loader roots ? Should make it possible to locate manifest from there, right ?

FroMage commented 4 years ago

Well, it depends what phase I look this info up. For config we have two phases:

gsmet commented 4 years ago

I don't have a strong opinion about the format but there's one thing we need to support: the ability to define lists - we will need it for the categories as an extension can be attached to several categories.

On Tue, Oct 15, 2019 at 11:38 AM Stéphane Épardaud notifications@github.com wrote:

Well, it depends what phase I look this info up. For config we have two phases:

  • during APT, once per extension, but that's during compilation and it may be in runtime or deployment, so the other descriptor may not be there, probably the wrong phase then
  • during the documentation module building, but it currently doesn't depend on any extension, so I'd have to add a dependency to the extensions. But it seems like the right phase.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/quarkusio/quarkus/issues/4539?email_source=notifications&email_token=AAJYOBP6O4FBEDSICTYUD2TQOWFSZA5CNFSM4JAMWMR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBIDSAY#issuecomment-542128387, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJYOBMEURSKMFFNAHJMXKLQOWFSZANCNFSM4JAMWMRQ .

emmanuelbernard commented 4 years ago

Note that you will need more properties down the line :

Etc

All that can be in a Pom and gradle if it can fit naturally. We would still need a generated version because no way we read a gradle file to find what we are looking for.

On Tue 15 Oct 2019 at 11:23, Stéphane Épardaud notifications@github.com wrote:

why are everyone happy to force users to use pom.xml to write this kind of info

Because we only need two user inputs for that file: labels and guide. All the rest comes from other places. The GAV, the name, description are already present in pom.xml.

application.properties on the other hand is a (sometimes) run-time thing that can be tweaked to change runtime behaviour.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quarkusio/quarkus/issues/4539?email_source=notifications&email_token=AACJNWBUU4KOCELLCQRKDJDQOWDXVA5CNFSM4JAMWMR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBIB75Y#issuecomment-542121975, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACJNWES26KRLOXZ25PVR3LQOWDXVANCNFSM4JAMWMRQ .

ia3andy commented 4 years ago

We also need some kind of manual ordering of the output extension list. I have no idea on how we could make that nicely 😅

ia3andy commented 4 years ago

I need to check if it is authorized to use Markdown in the pom.xml description. I find that the current description we have on most of the extension are not clear enough. This is why I would like the description to really give the user information about what he can expect from the extension and maybe a few hints/examples on how to use it.

What I have in mind is:

  1. abstract is visible in Code Quarkus list (it's I guess what we use as description for now)
  2. full formated (markdown or such) description is quickly available in a tooltip or such
  3. user can click on "guide" to get even more details.

The goal is to avoid the user having to jump from one place to the other many times.

Just to you know, if you are against it, I am ready to fight on this matter 😁

maxandersen commented 4 years ago

@ia3andy If you are looking for markdown to put on a page about the extension then i suggest to take a page from visual code extension playbook. They look up the readme.md from specified github location. Then you can go crazy all you want.

I wouldn’t put such things in this metadata file.

maxandersen commented 4 years ago

@ia3andy by manual ordering what do you mean ? If you mean what kind of order the extension list in the platform is sorted thenthat Is something to handle when generating the platform. Not affecting the core manifests imo.

ia3andy commented 4 years ago

@ia3andy If you are looking for markdown to put on a page about the extension then i suggest to take a page from visual code extension playbook. They look up the readme.md from specified github location. Then you can go crazy all you want. I wouldn’t put such things in this metadata file.

@maxandersen yeah that makes total sense, so maybe we need to add the github location of the extension?

maxandersen commented 4 years ago

@gsmet define lists as in something more advanced than a comma separated list of ids/labels ?