Skip to content
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

quarkus-extension manifests and their formats ? #4539

Open
maxandersen opened this issue Oct 14, 2019 · 55 comments
Open

quarkus-extension manifests and their formats ? #4539

maxandersen opened this issue Oct 14, 2019 · 55 comments
Labels
kind/enhancement New feature or request

Comments

@maxandersen
Copy link
Contributor

In #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 ?

@maxandersen maxandersen added the kind/enhancement New feature or request label Oct 14, 2019
@aloubyansky
Copy link
Member

aloubyansky commented Oct 14, 2019

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 14, 2019

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 14, 2019

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 14, 2019

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

FroMage commented Oct 14, 2019

It should be generating the file

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

@FroMage
Copy link
Member

FroMage commented Oct 14, 2019

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
Copy link
Member

Sure.

@loicmathieu
Copy link
Contributor

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
Copy link
Member

+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
Copy link
Member

+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
Copy link
Member

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
Copy link
Contributor Author

maxandersen commented Oct 14, 2019

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
Copy link
Member

emmanuelbernard commented Oct 15, 2019

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:

  • YAML
  • .properties

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
Copy link
Contributor

ia3andy commented Oct 15, 2019

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
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Member

FroMage commented Oct 15, 2019

It even has <name>, just sayin'…

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

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

@maxandersen
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Contributor

ia3andy commented Oct 15, 2019

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
Copy link
Member

FroMage commented Oct 15, 2019

As for who uses those files:

  • quarkus-config-roots.list is used by ExtensionAnnotationProcessor.java to produce it, and ExtensionLoader.java to load it when loading the extension at build time. Also by RunnerJarPhase to ignore the file when building a jar.
  • quarkus-extension.properties is ignored by DevMojo and RunnerJarPhase. It's written by our maven extension plugin and used by our bootstrap maven resolver.
  • quarkus-javadoc.properties is written and read by our APT plugin, and read by ConfigDescriptionBuildStep for build-time loading of configuration documentation. It's ignored by RunnerJarPhase

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

Now, what do they contain?

  • quarkus-config-roots.list contains the list of config roots, but it could be inferred by:
  • quarkus-javadoc.properties which contains the list of config javadoc in the form configroot.property=doc so we can infer the list of config roots from this file
  • 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).

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

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Contributor Author

@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
Copy link
Member

aloubyansky commented Oct 15, 2019

  • 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
Copy link
Member

  • 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).

#4539 (comment)

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Contributor Author

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
Copy link
Member

FroMage commented Oct 15, 2019

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
Copy link
Contributor Author

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

@FroMage
Copy link
Member

FroMage commented Oct 15, 2019

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.

@gsmet
Copy link
Member

gsmet commented Oct 15, 2019 via email

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Oct 15, 2019 via email

@ia3andy
Copy link
Contributor

ia3andy commented Oct 15, 2019

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
Copy link
Contributor

ia3andy commented Oct 15, 2019

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor

ia3andy commented Oct 15, 2019

@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
Copy link
Contributor Author

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

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Oct 15, 2019 via email

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Oct 15, 2019 via email

@maxandersen
Copy link
Contributor Author

Hum read me are really working ? Does it find older versions by tags? People need to be disciplined and on GitHub. Also I tend to put how to build things in readme so that’s another discipline to have.

things that uses readme from your project:

I would say its usage have been wildly tested :)

@gsmet
Copy link
Member

gsmet commented Nov 18, 2019

@maxandersen can we close this one?

@maxandersen
Copy link
Contributor Author

I would say no as we didn't clean anything up nor documented these yet so its still an issue that should be looked at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants