ADR for enhanced metadata generation

[holly-cummins]

See file contents for discussion.
Enables #40306.

[cescoffier]

LGTM, I would like @aloubyansky's opinion.

[cescoffier]

Should be change on merge.

[cescoffier]

Might need an update.

[cescoffier]

ADR number to be updated.

[aloubyansky]

I support the idea behind this. Let's polish some details a bit more though. For example, how a registry (and possibly other tools) will know how to get the metadata: deployment JAR vs yaml artifact, feeding this metadata to registries on release.

[holly-cummins]

Thanks, @aloubyansky. I think I've addressed all your comments in the ADR doc. I left the question of the yaml artifact as a 'we could do either' for further discussion, because I don't really understand about the platform data generation or maven publishing to have a firm opinion about the feasibility/best option.

Intuitively, the yaml archive seems like it should be a little optional extra that might be handy for something. If the tooling is always run locally with a deployment artifact sat next to it, maybe a separate archive is overkill. After all, anything that wants to access the extension's metadata is supposed to ask the registry, not query maven. But maybe there are scenarios where either tooling doesn't want to use the registry, or it would have to pull the binary from maven because the registry hasn't been published yet.

... but this whole area is not one I know, so I'm very happy to take a recommendation!

[aloubyansky]

Given it has a cost, we probably shouldn't support something we don't know exists.

we know they exist - we already multiple times caught issues with the multiversion test action that @ia3andy setup - which i for some reason can't locate now :)

That's not external tools.

[maxandersen]

That's not external tools.

I don't understand what that means? we can't fix them in a release - we would need a timemachine. thus they are as external as intellij IDE or similar are :)

[maxandersen]

The registry is providing that. And if the registry is off, it's limited to the platform metadata atm.

are you saying codestarts are effectively NOT supported outside platform - thats a bug then.

[maxandersen]

The registry will look for it in the deployment first and falls back to look in the runtime artifact.

thats exactly what I keep proposing. Look in deployment first and if not there look up in runtime artifact.

only reason for not using same name is I'm not sure how I would differentiate between them if scanning in a place where I don't have access to GAV info.

[maxandersen]

DevUI is searching for yaml on the classpath, so having it in any one module will be enough.

but it will have to know which one to look for if we allow both.

[aloubyansky]

That's not external tools.

I don't understand what that means? we can't fix them in a release - we would need a timemachine. thus they are as external as intellij IDE or similar are :)

Do you know where those tests are to check the impact?

[maxandersen]

What problem is the second file solving? I'm blanking on why we add ask extension owners to manually add a source file to deployment, if the same file is still going to be present in the runtime source?

I'm talking about adding a file in a unique place and remove the other one when/if it is redundant.

I'm simply not going to call it that we are moving it because the two files does not have the same content and has two different roles.

[aloubyansky]

The registry is providing that. And if the registry is off, it's limited to the platform metadata atm.

are you saying codestarts are effectively NOT supported outside platform - thats a bug then.

It seems like they aren't supported when the registry is off.

[maxandersen]

Do you know where those tests are to check the impact?

I'm trying to find them - andy ran them and shown them fail but i haven't spotted them yet.

[maxandersen]

It seems like they aren't supported when the registry is off.

yes, so thats a bug. we should not design changes based on that bug to stay around :)

[aloubyansky]

DevUI is searching for yaml on the classpath, so having it in any one module will be enough.

but it will have to know which one to look for if we allow both.

Exactly. So using the same name will be an advantage. We just need to make sure it's in the one artifact.

[maxandersen]

Exactly. So using the same name will be an advantage. We just need to make sure it's in the one artifact.

how do you ensure it will pick up the deployment located one that has more info than the one in runtime?

[aloubyansky]

It seems like they aren't supported when the registry is off.

yes, so thats a bug. we should not design changes based on that bug to stay around :)

That wouldn't be an issue anyway. We'd still be looking for quarkus-extension.yaml like the registry does/will do. It just has to be implemented

[aloubyansky]

Exactly. So using the same name will be an advantage. We just need to make sure it's in the one artifact.

how do you ensure it will pick up the deployment located one that has more info than the one in runtime?

Because if the deployment artifact has it then the runtime one wouldn't. We can enforce it in the plugin generating it in the deployment module.

[maxandersen]

Because if the deployment artifact has it then the runtime one wouldn't. We can enforce it in the plugin generating it in the deployment module.

I see how it can work,
but thats assuming devui and any other tool has enough info to know a jar has a certain GAV...is that always the case?

[maxandersen]

Do you know where those tests are to check the impact?

I'm trying to find them - andy ran them and shown them fail but i haven't spotted them yet.

found it - https://github.com/quarkusio/quarkus-devtools-compat

see https://github.com/quarkusio/quarkus-devtools-compat/actions - its intended to be expanded if we find cross-version concerns like this one.

[aloubyansky]

No, DevUI (and I can see some dev tools tests) are just looking for it on the deployment classpath. So these consumers shouldn't be affected.
There are a couple of dev tools tests that assert for quarkus-extension.yaml presence. I'm not sure they are a part of the cross-version test suite though.

[holly-cummins]

What problem is the second file solving? I'm blanking on why we add ask extension owners to manually add a source file to deployment, if the same file is still going to be present in the runtime source?

I'm talking about adding a file in a unique place and remove the other one when/if it is redundant.

I'm simply not going to call it that we are moving it because the two files does not have the same content and has two different roles.

Can you give example content of what you're suggesting go in the deployment/quarkus/quarkus-extension.yaml file? If the runtime/src/main/resources/META-INF/quarkus-extension.yaml file stays, the deployment/quarkus-extension.yaml doesn't need to reproduce any of its content. Obviously, maintenance is easiest if there's no duplicated content between the two files.So the new file is for data that we don't want to put in the 'main' runtime file quarkus-extension.yaml file, but want to be picked up by the extension mojo so it's visible to the registry?

[aloubyansky]

Do you know where those tests are to check the impact?

I'm trying to find them - andy ran them and shown them fail but i haven't spotted them yet.

found it - https://github.com/quarkusio/quarkus-devtools-compat

see https://github.com/quarkusio/quarkus-devtools-compat/actions - its intended to be expanded if we find cross-version concerns like this one.

These tests are not based on quarkus-extension.yaml as the source. They consume JSON provided by the registry, so it looks like they shouldn't be affected.

[maxandersen]

Can you give example content of what you're suggesting go in the deployment/quarkus/quarkus-extension.yaml file? If the runtime/src/main/resources/META-INF/quarkus-extension.yaml file stays, the deployment/quarkus-extension.yaml doesn't need to reproduce any of its content

I'm not saying the file is staying when an extension move to the new and improved mvn extension plugin. the src one could be deleted.

Only thing I haven't considered yet is if any impact on extensions wanting to be able to target both LTS and main development at the same time. Here being able to have the runtime src in place could be useful.

[maxandersen]

These tests are not based on quarkus-extension.yaml as the source. They consume JSON provided by the registry, so it looks like they shouldn't be affected.

just because the test aren't doing it now doesn't mean they should continue relying on the registry.

[aloubyansky]

These tests are not based on quarkus-extension.yaml as the source. They consume JSON provided by the registry, so it looks like they shouldn't be affected.

just because the test aren't doing it now doesn't mean they should continue relying on the registry.

So if we are not breaking them then what's the issue?

[aloubyansky]

Only thing I haven't considered yet is if any impact on extensions wanting to be able to target both LTS and main development at the same time. Here being able to have the runtime src in place could be useful.

For what?

[holly-cummins]

I'm not saying the file is staying when an extension move to the new and improved mvn extension plugin. the src one could be deleted.

Ahh, got it. That's what I meant by 'move'. So we would not expect the deployment and runtime source files to co-exist in the same extension. For backwards compatibility, the extension mojo obviously has to support files in both locations, as shown in the diagrams.

Only thing I haven't considered yet is if any impact on extensions wanting to be able to target both LTS and main development at the same time. Here being able to have the runtime src in place could be useful.

In that case, I'd expect them to just leave the runtime source and not use the new deployment location. Otherwise we risk the files going out of sync. It also has the advantage that when they decide to move to the deployment location, the change history on the file is preserved. If we have a period when there's two copies of the file, and then the 'original' copy is deleted, change history is lost.

[holly-cummins]

To summarise the discussion about what goes into the binary, we're discussing
(a) the best name for the file in the deployment module
(b) whether to require/tolerate/forbid metadata files co-existing in both runtime and deployment of the same extension

Here are some advantages and disadvantages I've pulled from the comments above:

Everything is called quarkus-extension.yaml

• ++ classpath-based discovery can always find something, but ...
• -- even new classpath discovery might find the wrong/old file; we can mitigate this by forbidding metadata in both runtime and deployment modules, but that breaks legacy tooling using explicit lookup
• ++ registry upload works great, once we update the scripts

The file in the deployment archive gets a different name

• ++ explicit lookup can find it (old tooling will find runtime, new tooling will find deployment)
• ++ new classpath-based discovery always gets the right one
• ++ old classpath-based discovery will find something
• -- old classpath-based discovery with a new archive will never find the 'best' file (but it's old tooling, so fair enough, and it probably wouldn't be able to use the new information anyway)
• -- we would want to go for files in both archives, so slightly increased binary size
• ++ registry upload works great, once we update the scripts

[aloubyansky]

To summarise the discussion about what goes into the binary, we're discussing (a) the best name for the file in the deployment module (b) whether to require/tolerate/forbid metadata files co-existing in both runtime and deployment of the same extension

Here are some advantages and disadvantages I've pulled from the comments above:

Everything is called quarkus-extension.yaml

• ++ classpath-based discovery can always find something, but ...
• -- even new classpath discovery might find the wrong/old file; we can mitigate this by forbidding metadata in both runtime and deployment modules, but that breaks legacy tooling using explicit lookup

AFAICT, we've been able to identify any such legacy tooling, besides the registry. Who are we breaking?

• ++ registry upload works great, once we update the scripts

The file in the deployment archive gets a different name

• ++ explicit lookup can find it (old tooling will find runtime, new tooling will find deployment)
• ++ new classpath-based discovery always gets the right one
• ++ old classpath-based discovery will find something
• -- old classpath-based discovery with a new archive will never find the 'best' file (but it's old tooling, so fair enough, and it probably wouldn't be able to use the new information anyway)

This is going to make it more complicated for consumers such as DevUI, since it would have to discover the legacy and new one, and merge or filter out those that describe the same thing.

• -- we would want to go for files in both archives, so slightly increased binary size
• ++ registry upload works great, once we update the scripts

[maxandersen]

This is going to make it more complicated for consumers such as DevUI, since it would have to discover the legacy and new one, and merge or filter out those that describe the same thing.

but if we don't then you expect all extensions to move to the new structure at the same time?

[aloubyansky]

This is going to make it more complicated for consumers such as DevUI, since it would have to discover the legacy and new one, and merge or filter out those that describe the same thing.

but if we don't then you expect all extensions to move to the new structure at the same time?

No, what would be the reason for that?