repaint-io / maven-tiles

Injecting maven configurations by composition rather than inheritance
155 stars 32 forks source link

Removing Distribution Management URL From POM Causes NullPointerException #116

Closed danielshiplett closed 3 years ago

danielshiplett commented 3 years ago

When configuring a project to distribute its artifacts to a Nexus repository, it is common to have a distributionManagement section which defines the repositories. It is often common to have the URLs of the target Nexus repositories in this section. However, Maven supports the use of properties 'altReleaseDeploymentRepository' and 'altSnapshotDeploymentRepository' for defining the URL for the target Nexus repositories.

When using this feature of Maven, either by putting the alt properties on the command line or in the .m2/settings.xml, the Tiles Plugin will fail to build the Maven model and cause a NullPointerException.

danielshiplett commented 3 years ago

I'd like to also point out that I attempted to put an empty string in the URL to get past this. However, that seems to cause Maven to not take the alt properties into account. So it is not a viable solution.

talios commented 3 years ago

I think this is a deeper issue where properties referred to/used in a tile, need to be declared in the tile.

In our own release tile I have:

<properties>
  <altReleaseDeploymentRepository>smx-releases::default::https://nexus.somewhere/nexus/content/repositories/releases/</altReleaseDeploymentRepository>

and the release plugin configured in the same tile. This gives us a default deployment location, which can be overridden in the artifact by setting the altReleaseDeploymentRepository property either in the pom.xml or command line.

You don't by any chance have the NPE stack trace handy to tack into the issue?

danielshiplett commented 3 years ago

Oh dang. You're right. I put a placeholder URL into my distribution management section and then replace the property on the command line with '-DaltSnapshotDeploymentRepository' and it built the model with no problems.

In our case, we are trying to keep these types of properties out of code completely since we have to build on two completely separate networks. That's why we are putting them on the command line (or in our settings.xml that is injected into our build system).

So, I guess at this point, reject my PR. I still don't know what would be best in terms of handling the NPE other than perhaps better documentation or a better warning on the failed model build. Thoughts?

Here's the stack trace you asked for (let me know if you need debug output on):

[INFO] Scanning for projects... [INFO] --- tiles-maven-plugin: Injecting 1 tiles as intermediary parent artifacts for org.example:example... [INFO] Mixed 'org.example:example:1.0-SNAPSHOT' with tile 'org.example:example-tile:1.0-SNAPSHOT' as its new parent. [INFO] Mixed 'org.example:example-tile:1.0-SNAPSHOT' with original parent '(no parent)' as its new top level parent. [INFO] [ERROR] Internal error: java.lang.NullPointerException -> [Help 1] org.apache.maven.InternalErrorException: Internal error: java.lang.NullPointerException at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:120) at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289) at org.apache.maven.cli.MavenCli.main (MavenCli.java:193) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62) at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke (Method.java:566) at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282) at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225) at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406) at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347) Caused by: java.lang.NullPointerException at org.apache.maven.artifact.repository.MavenArtifactRepository.protocol (MavenArtifactRepository.java:228) at org.apache.maven.artifact.repository.MavenArtifactRepository. (MavenArtifactRepository.java:84) at org.apache.maven.repository.legacy.repository.DefaultArtifactRepositoryFactory.createArtifactRepository (DefaultArtifactRepositoryFactory.java:129) at org.apache.maven.repository.legacy.repository.DefaultArtifactRepositoryFactory.createDeploymentArtifactRepository (DefaultArtifactRepositoryFactory.java:78) at org.apache.maven.artifact.repository.DefaultArtifactRepositoryFactory.createDeploymentArtifactRepository (DefaultArtifactRepositoryFactory.java:67) at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.discoverAndSetDistributionManagementArtifactoryRepositoriesIfTheyExist (TilesMavenLifecycleParticipant.groovy:346) at io.repaint.maven.tiles.TilesMavenLifecycleParticipant.afterProjectsRead (TilesMavenLifecycleParticipant.groovy:326) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:264) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192) at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105) at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289) at org.apache.maven.cli.MavenCli.main (MavenCli.java:193) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62) at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke (Method.java:566) at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282) at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225) at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406) at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)

notiriel commented 3 years ago

While looking at this problem (All our snapshot builds fail with a NPE when building with 2.21), I found the issue for that particular problem. It was a messed up ternary expression leading to an always null value for the snapshor repository url. Here's the proposed change for it: https://review.gerrithub.io/c/repaint-io/maven-tiles/+/516954 I laso took the freedom of removing the unnecessary null check in for the release case.

rvowles commented 3 years ago

Yeah, this has hit me as well

On Wed, May 19, 2021 at 6:12 AM Stefan Riesen @.***> wrote:

While looking at this problem (All our snapshot builds fail with a NPE when building with 2.21), I found the issue for that particular problem. It was a messed up ternary expression leading to an always null value for the snapshor repository url. Here's the proposed change for it: https://review.gerrithub.io/c/repaint-io/maven-tiles/+/516954 I laso took the freedom of removing the unnecessary null check in for the release case.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/repaint-io/maven-tiles/issues/116#issuecomment-843414188, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAANTWCL5HW5ND2YIIWWU2DTOKUYXANCNFSM4U6GJH4Q .

--

Richard Vowles, Full stack - from Kubernetes, through Node & Java, Web and Mobile development in Flutter - software developer for hire!

ph: +64275467747

rvowles commented 3 years ago

@notiriel could you please resubmit this as a PR here and tie it to this issue please? We are in the process of removing Gerrit from the process.

notiriel commented 3 years ago

Sure, done. You might want to update the contribution document, that's what sent me down the gerrit path.

rvowles commented 3 years ago

Definitely going to update that :-) Thats for the contribution!