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 dev mode failes, if maven packaging is defined by a variable (property) #9727

Closed
dirkweil opened this issue Jun 2, 2020 · 15 comments · Fixed by #15342
Closed

Quarkus dev mode failes, if maven packaging is defined by a variable (property) #9727

dirkweil opened this issue Jun 2, 2020 · 15 comments · Fixed by #15342
Assignees
Labels
Milestone

Comments

@dirkweil
Copy link

dirkweil commented Jun 2, 2020

If maven packaging is defined by a property, dev mode hangs on startup.

The bug is described in https://github.com/GEDOPLAN/quarkus-devmode-fails-for-variable-packaging.

<tl;dr>
<packaging>${packaging.type}</packaging>

<properties>
<packaging.type>jar</packaging.type>
</properties>

mvn quarkus:dev hangs in application int.

@dirkweil dirkweil added the kind/bug Something isn't working label Jun 2, 2020
@geoand
Copy link
Contributor

geoand commented Jun 2, 2020

Using Quarkus from master, we see this error:

Exception in thread "main" java.lang.RuntimeException: io.quarkus.bootstrap.BootstrapException: Failed to create the application model for null
        at io.quarkus.deployment.dev.DevModeMain.start(DevModeMain.java:130)
        at io.quarkus.deployment.dev.DevModeMain.main(DevModeMain.java:56)
Caused by: io.quarkus.bootstrap.BootstrapException: Failed to create the application model for null
        at io.quarkus.bootstrap.BootstrapAppModelFactory.resolveAppModel(BootstrapAppModelFactory.java:301)
        at io.quarkus.bootstrap.app.QuarkusBootstrap.bootstrap(QuarkusBootstrap.java:151)
        at io.quarkus.deployment.dev.DevModeMain.start(DevModeMain.java:125)
        ... 1 more
Caused by: io.quarkus.bootstrap.resolver.AppModelResolverException: Failed to resolve artifact de.gedoplan:quarkus-devmode-fails-for-variable-packaging:${packaging.type}:1.0-SNAPSHOT
        at io.quarkus.bootstrap.resolver.maven.MavenArtifactResolver.resolveInternal(MavenArtifactResolver.java:166)
        at io.quarkus.bootstrap.resolver.maven.MavenArtifactResolver.resolve(MavenArtifactResolver.java:155)
        at io.quarkus.bootstrap.resolver.BootstrapAppModelResolver.doResolveModel(BootstrapAppModelResolver.java:171)
        at io.quarkus.bootstrap.resolver.BootstrapAppModelResolver.resolveManagedModel(BootstrapAppModelResolver.java:144)
        at io.quarkus.bootstrap.BootstrapAppModelFactory.resolveAppModel(BootstrapAppModelFactory.java:287)
        ... 3 more
Caused by: org.eclipse.aether.resolution.ArtifactResolutionException: Could not find artifact de.gedoplan:quarkus-devmode-fails-for-variable-packaging:${packaging.type}:1.0-SNAPSHOT
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:424)
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:229)
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact(DefaultArtifactResolver.java:207)
        at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveArtifact(DefaultRepositorySystem.java:262)
        at io.quarkus.bootstrap.resolver.maven.MavenArtifactResolver.resolveInternal(MavenArtifactResolver.java:161)
        ... 7 more
Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact de.gedoplan:quarkus-devmode-fails-for-variable-packaging:${packaging.type}:1.0-SNAPSHOT
        at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:414)
        ... 11 more

@geoand
Copy link
Contributor

geoand commented Jun 2, 2020

This is a real corner case. Basically what happens is that ${packaging.type} is not being resolved, and frankly it would be really hard to resolve since we would need to know the profile.

cc @aloubyansky

@aloubyansky
Copy link
Member

We do support profiles in dev mode though for things like dependencies. It can be fixed.

@geoand
Copy link
Contributor

geoand commented Jun 2, 2020

Oh, I didn't know that

@geoand
Copy link
Contributor

geoand commented Jun 2, 2020

I probably won't be able to get to this for another week, so if anyone else wants to take a look, feel free

@dinabogdan
Copy link

There is any news on this? I'm facing the same issue :)

@dinabogdan
Copy link

Is there a possibility to find some details about solving this bug? I want to do it by a PR.

@aloubyansky aloubyansky self-assigned this Sep 22, 2020
@aloubyansky
Copy link
Member

Just assigned it to myself, but actually @dinabogdan did you want to work on this issue?

@aloubyansky
Copy link
Member

It's not a difficult one. Let me know.

@dinabogdan
Copy link

It's not a difficult one. Let me know.

Hi @aloubyansky ! I will be delighted to work on this issue :) If you are available, please provide me some details about it :)

@aloubyansky
Copy link
Member

Ok, so it's kind of easy. It's not gonna be a 100% proper fix but it'll do for now. This line here [1] is simply passing the raw packaging value. Instead it should be checking whether it includes an expression. Given that it's "packaging", it's unlikely it's gonna be anything sophisticated. So, I would probably go for a simple check instead of proper property replacer including nested properties.

  1. I would check the length of the packaging if it's <= 3, it should be good enough (no further checks);
  2. check whether packaging.indexOf("${") is negative and if it is no further checks necessary;
  3. look for the index of the closing } (again, in this case, not checking for escape characters, etc), if not found - no further checks (that'll be weird but let it fail the way it would have);
  4. check whether the property name you detected is found in the current project's rawModel properties, if it's not found then check its getLocalParent() and all the way up until the parent is null.
  5. if the property was resolved successfully then replace the original packaging value with the property replaced;
  6. if the property couldn't be resolved then probably don't do anything (it will fail anyway if the packaging is not supported).
  7. It'll be good to add a little test or two (for single and multimodule projects) to [2]

Let me know if you have further questions. Thanks for your efforts!

[1] https://github.com/quarkusio/quarkus/blob/master/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/LocalProject.java#L366
[2] https://github.com/quarkusio/quarkus/blob/master/independent-projects/bootstrap/maven-resolver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java

@famod
Copy link
Member

famod commented Sep 22, 2020

@aloubyansky

So, I would probably go for a simple check instead of proper property replacer including nested properties.

Why not re-using (with a small adjustment) what we already have for the "Maven CI friendly versions"?
https://github.com/quarkusio/quarkus/blob/master/independent-projects/bootstrap/maven-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/ModelUtils.java#L228

@aloubyansky
Copy link
Member

The regex there is pretty specialized though. It would have to be re-done in a generic way. Which is definitely possible, if somebody is up for it. If we can avoid regex that'd be great too. But a simpler fix in this specific case would work as well, imo.

@famod
Copy link
Member

famod commented Sep 22, 2020

It's late and I might be missing something but the following Pattern should do:

Pattern.compile(Pattern.quote("${") + propertyNameRegex + Pattern.quote("}"))
  • propertyNameRegex CI friendly : "(revision|sha1|changelist)"
  • propertyNameRegex generic (for packaging): "[^}]+"

@aloubyansky
Copy link
Member

#15342 introduced a config option to enable Quarkus workspace initialization based on effective POMs instead of the "raw" ones, which can be enabled by setting system property quarkus.bootstrap.effective-model-builder to true (e.g. in the surefire and/or quarkus maven plugin config).
The option is not enabled by default because effective POMs take significantly more time to resolve. Which depending on the number of modules in the project and the number of tests, may significantly increase testing time.
Until we find a more efficient alternative though, projects that require it, should enable this option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants