Fix mvn

[gnodet]

No description provided.

[ppalaga]

Most of the changes look good, I have just a single concern:

People will generally have mvn already when installing mvnd. Our mvn script may clash with the stock mvn. I do not think I would want that myself.

What is the purpose of our mvn script actually?

[gnodet]

The purpose is to make maven embedded work. They way it has been written is the following : it runs the script located inside $MAVEN_HOME/bin/mvn. Not having the script will cause archetypes tests to fail for example. But this is true for everything any plugin using the Maven Invoker. I think the reason is that the maven.home property is set to the mvnd home : https://github.com/mvndaemon/mvnd/blob/156ab7e3232aece8d21a42d564e30a4bc5eec5ff/daemon/src/main/java/org/apache/maven/cli/DaemonMavenCli.java#L205-L208

[ppalaga]

The purpose is to make maven embedded work. They way it has been written is the following : it runs the script located inside $MAVEN_HOME/bin/mvn. Not having the script will cause archetypes tests to fail for example. But this is true for everything any plugin using the Maven Invoker.

Hm... that's a valid point. Let me try which mvn gets picked when I install this PR via sdkman. It surely depends on ordering and it perhaps can be done somehow that both kinds of users are happy: the ones with pre-installed mvn and the ones expecting mvn embedded to work within mvnd.

[ppalaga]

If both mvnd and mvn are installed via sdkman, then the following happens:

which mvn
~/.sdkman/candidates/mvnd/current/bin/mvn

which is a bad news, IMO.

[ppalaga]

echo $MAVEN_HOME
/home/ppalaga/.sdkman/candidates/maven/current

[ppalaga]

mvn -v
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /home/ppalaga/.sdkman/candidates/mvnd/current
Java version: 11.0.8, vendor: AdoptOpenJDK, runtime: /home/ppalaga/.sdkman/candidates/java/11.0.8.hs-adpt
Default locale: en_IE, platform encoding: UTF-8
OS name: "linux", version: "5.8.13-100.fc31.x86_64", arch: "amd64", family: "unix"

[ppalaga]

I wonder what can we do about that.

A. Making our mvn delegate to stock mvn if MAVEN_HOME is set (or if other mvn binaries can be found in the PATH) sounds dirty and dangerous.

B. Maybe we could try to play with the Java code on our side so that mvn is not required for the embedded scenarios. I am afraid this could turn out to be a lot of work.

C. We could advertise our distro as a Maven distro, so that providing our own mvn executable is expected and so that it is not possible (at least with sdkman) to have both stock Maven and mvnd active at the same time. I would not like this personally. I perfer not to be the first level support for all issues happening with mvn.

Other ideas?

[gnodet]

I wonder what can we do about that.

A. Making our mvn delegate to stock mvn if MAVEN_HOME is set (or if other mvn binaries can be found in the PATH) sounds dirty and dangerous.

B. Maybe we could try to play with the Java code on our side so that mvn is not required for the embedded scenarios. I am afraid this could turn out to be a lot of work.

C. We could advertise our distro as a Maven distro, so that providing our own mvn executable is expected and so that it is not possible (at least with sdkman) to have both stock Maven and mvnd active at the same time. I would not like this personally. I perfer not to be the first level support for all issues happening with mvn.

Other ideas?

I'd rather go for C. I don't really understand the problem with having our own mvn script. I'm not a heavy user of sdkman, but I do choose the executables using the ordering of the $PATH variable when needed. In all cases, we do embed the jars from maven, so if anything is problematic in maven or if we want to upgrade the internal maven, we'd have to do another release. There may be subtle changes between the stock mvn and our own, but we could either resolve those, or advertise them more.

Also, that was the case until a few weeks ago 9224031#diff-9cff1802f6c575e8a1147f0f4b8d2673b0c2ac9b26bfc27b0012e2ead4795fdeR23

[ppalaga]

Asked on SDKMAN sdkman/sdkman-cli#809

[ppalaga]

Asked on SDKMAN sdkman/sdkman-cli#809

Marco says

We could introduce a post-install hook that removes executable rights from your mvn executable

I assume that would help us, right? We need to have the mvn script executable for the embedded use cases to work, don't we?

[ppalaga]

Romain Manni-Bucau proposed the option D.:

relayout the distribution: mvnd/bin does not contain mvn but set maven.home to a folder not in path with mvn script.

We should try this (a follow up would be fine). I'd prefer this option if it turns out to work well.

That actually brings another follow up: We should have a test for some scenario using mvn.