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

have our build plugin dump out the "hidden" dependencies for other tools to use #7717

Open
maxandersen opened this issue Mar 9, 2020 · 14 comments
Labels
kind/enhancement New feature or request

Comments

@maxandersen
Copy link
Member

Description
quarkus has its "shadow" dependencies to deployment that are not captured as maven dependencies hence tools like https://github.com/gnodet/mvnd and https://github.com/vackosar/gitflow-incremental-builder can't really figure out the most effective way to build quarkus extensions.

Idea is to have our maven plugin dump out the extra dependencies in a file or property.

i.e. on how mvnd today do it manually https://github.com/apache/camel-quarkus/blob/master/integration-tests/base64/pom.xml#L33-L39

@maxandersen maxandersen added the kind/enhancement New feature or request label Mar 9, 2020
@famod
Copy link
Member

famod commented Mar 9, 2020

As long as the required *-deployment modules are not somehow part of the reactor, GIB (gitflow-incremental-builder) won't work. Currently, GIB does not add new MavenProject instances to the MavenSession, it just modifies the existing projects/modules (the ones from the Maven reactor).
Extending GIB to parse and add entirely "new" projects to the reactor seems a bit out of scope. But maybe I just have to think about it bit more for some time.

Have you guys ever considered putting those *-deployment dependencies in profiles? Or is it that none of the classes from those *-deployment modules are alllowed to end up on the classpath?

@geoand
Copy link
Contributor

geoand commented Mar 10, 2020

Have you guys ever considered putting those *-deployment dependencies in profiles? Or is it that none of the classes from those *-deployment modules are alllowed to end up on the classpath?

No class from any of these *-deployment modules is ever allowed to be on the runtime classpath

@famod
Copy link
Member

famod commented Mar 10, 2020

I see.

For a integration-test module like the one above for camel-quarkus there might be another approach:

  • add "regular" *-deployment dependencies, maybe even with * exclusions
  • add classpathDependencyExcludes to prevent the *-deployment dependencies from being added to the test runtime classpath

classpathDependencyExcludes does unfortunately not support wildcards, so you have to define every single deployment dependency.

Another variation might be to use classpathDependencyScopeExclude instead of classpathDependencyExcludes.

I am just brainstorming here, but WDYT?

@geoand
Copy link
Contributor

geoand commented Mar 11, 2020

cc @ppalaga

@ppalaga
Copy link
Contributor

ppalaga commented Mar 11, 2020

As I said on the chat, a mojo that has to be called per itest is not what I'd prefer to make mvnd work for Camel Quarkus or Quarkus itself. Analyzing the dependency tree per itest and parsing the jars sounds a bit too heavyweight. I think leveraging the source tree knowledge (extensions live under certain folders) to get the runtime-artifactId:deployment-artifactId mapping once for the whole source tree is much more efficient. Of course, this is not to say that other projects/tool chains cannot benefit from such a mojo.

@maxandersen
Copy link
Member Author

@ppalaga I think there are a few things at play here:

  1. find a common way to express this extra info (wether inside the pom.xml or an external file

  2. make tooling understand that info (i.e. mvnd and gitflow-incremental-builder)

  3. have ways to generate/maintain what is in Switch to the Maven distributed copy of the SubstrateVM annotations #1
    and here I see it as you can benefit from both:

    • one is that the quarkus build plugin simply just dump this out when its building it anyways
    • second is some other tools that can do a quick run through based on some assumptions it makes

in both cases you'll still need a first build anyways,right ?

@ppalaga
Copy link
Contributor

ppalaga commented Mar 11, 2020

@ppalaga I think there are a few things at play here:

  1. find a common way to express this extra info (wether inside the pom.xml or an external file

If that info is supposed to be 100% generated, then an external file seems to be easier to maintain.

  1. make tooling understand that info (i.e. mvnd and gitflow-incremental-builder)

mvnd being on a very early stage, this is probably doable (cc @gnodet )

  1. have ways to generate/maintain what is in Switch to the Maven distributed copy of the SubstrateVM annotations #1
    and here I see it as you can benefit from both:

    • one is that the quarkus build plugin simply just dump this out when its building it anyways

Sounds like a chicken-egg situation. I cannot use mvnd to run the plugin, because mvnd needs that info upfront. Having to care for committing the changes may be seen as annoying by contributors.

in both cases you'll still need a first build anyways,right ?

Yeah, you probably mean running a sequential mvn build before the real mvnd build. As I said, mvnd now has an option to call a groovy script to gather the additional rules. That call is once per the whole source tree. That allows mvnd to run always properly without any preparational builds.

So I wish you could provide a way mvnd (and other tools) could call your code to gather the hidden dep info. Maybe mvnd could have an SPI and you'd provide a jar?

@maxandersen
Copy link
Member Author

Sounds like a chicken-egg situation. I cannot use mvnd to run the plugin, because mvnd needs that info upfront. Having to care for committing the changes may be seen as annoying by contributors.

your current solution in mvnd requires users to commit the files too - is there much difference here ?
the beneift of having these files generated by the plugin is that they won't care or be affected by the dir layout.

Yeah, you probably mean running a sequential mvn build before the real mvnd build. As I said, mvnd now has an option to call a groovy script to gather the additional rules. That call is once per the whole source tree. That allows mvnd to run always properly without any preparational builds.

yes, but it relies on assumptions on dir layout.

We can probably make a command that does "mvn quarkus:dumpbuilddepinfo" or something that would just do the resolving needed to generate this info.

Your mvnd approach will probably still be faster as it sound like it just relies on some very explicit rules that works exactly for quarkus tree structure.

@ppalaga
Copy link
Contributor

ppalaga commented Mar 12, 2020

your current solution in mvnd requires users to commit the files too - is there much difference here ?

Yes, committing is required and we prefer to get away from that.

the beneift of having these files generated by the plugin is that they won't care or be affected by the dir layout.

Yes, I agree it is a benefit. But the performance and scalability matter too. We aim at ~300 extensions in our repo. Getting the hidden deps info should ideally take less than a second for the whole source tree.

Yeah, you probably mean running a sequential mvn build before the real mvnd build. As I said, mvnd now has an option to call a groovy script to gather the additional rules. That call is once per the whole source tree. That allows mvnd to run always properly without any preparational builds.

yes, but it relies on assumptions on dir layout.

Yes, I admit this kind solution is hard to generalize so that it works for any repo hosting Quarkus extensions.

Let's brainstorm about how we could do the following for any GAV, in a performat way without assuming anything about the given source tree:

  • Decide whether the GAV is a runtime module of a Quarkus extension
  • Get the GAV of the corresponding deployment module

IIRC, the current approach of the Quarkus plugin requires

  1. Resolving all current module's deps via aether and
  2. It opens each JAR and reads quarkus-extension.properties

Both 1. and 2. make it slow. Can't we avoid them somehow?

@maxandersen
Copy link
Member Author

@aloubyansky is the man for this.

What i'm saying is that while we wait on that to be fixed (I don't think it can be easily with current "lazy resolve") we have a way to make it fast for 90+% of all use cases here so i think it is worth enabling this no matter what.

@aloubyansky
Copy link
Member

There is quarkus-bootstrap:build-tree

@maxandersen
Copy link
Member Author

how do you use that ? just errors instantly for me with non-existent prefix error.

@aloubyansky
Copy link
Member

io.quarkus:quarkus-bootstrap-maven-plugin, it's normally isn't added to the project poms.

@ppalaga
Copy link
Contributor

ppalaga commented Mar 13, 2020

we have a way to make it fast for 90+% of all use cases here so i think it is worth enabling this no matter what.

Of course, go ahead!

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

5 participants