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

Use muti-release jar to fallback mvnd-client to original maven #722

Merged
merged 4 commits into from
Dec 14, 2022

Conversation

gzm55
Copy link
Contributor

@gzm55 gzm55 commented Oct 18, 2022

The mvnd-client is built to a muti-release jar. The default version of DefaultClient is compiled against the same target version (1.7) as the embedded maven which just invokes the MavenCli.main(). The java-11 version is the full qualified mvnd-client.

To keep the embedded runnable undre JDK 1.7, we have to restore maven-resolver to 1.6.3 and remove DaemonNamedLockFactorySelector.java.

The mvnd-client is built to a muti-release jar. The default version of
DefaultClient is compiled against the same target version as the
embedded maven which only invoke the MavenCli.main(). The java-11
version is the full qualified mvnd-client.
@ppalaga
Copy link
Contributor

ppalaga commented Oct 18, 2022

Thanks for the contribution, this looks very interesting, @gzm55.

Do I understand correctly that this is intended to provide an automatic fallback to the embedded vanilla Maven on platforms for which we (1) do not have a GraalVM native executable and (2) they have java <11?

There is no change in behavior on systems where the mvnd native executable works (because that one is inherrently Java 11), right?

I wonder what are the advantages against running Maven in the traditional way on those systems?

I wonder how portable the fallback is: we forward all command line parameters, some of which won't work with the embedded Maven.

1. build SimpleAppender for JDK 1.7 since it is the log appender only
   for the embedded maven
2. restore maven-resolver to 1.6.3 for JDK 1.7
3. remove DaemonNamedLockFactorySelector.java, it is for resolver 1.7.3,
   and not compatible to the latest resolver
@gzm55
Copy link
Contributor Author

gzm55 commented Oct 19, 2022

Do I understand correctly that this is intended to provide an automatic fallback to the embedded vanilla Maven on platforms for which we (1) do not have a GraalVM native executable and (2) they have java <11?

There is no change in behavior on systems where the mvnd native executable works (because that one is inherrently Java 11), right?

Right.

I wonder what are the advantages against running Maven in the traditional way on those systems?

Improve the usability, and developers can only install mvnd package for a faster and more reliable maven.

I wonder how portable the fallback is: we forward all command line parameters, some of which won't work with the embedded Maven.

Now only few flags break the original maven.

  1. Some of which should break the invokes, e.g. --completion|--purge|--status|--stop, they have special effect only for mvnd.
  2. Others, like -1|--serial|--raw-streams, could be removed in the fallback main() without changing the invoke purpose. Now this fix is not implemented to let the fallback codes clean to review.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 19, 2022

Do I understand correctly that this is intended to provide an automatic fallback to the embedded vanilla Maven on platforms for which we (1) do not have a GraalVM native executable and (2) they have java <11?
There is no change in behavior on systems where the mvnd native executable works (because that one is inherrently Java 11), right?

Right.

Thanks for confirming.

I wonder what are the advantages against running Maven in the traditional way on those systems?

Improve the usability, and developers can only install mvnd package for a faster and more reliable maven.

Install only one package sounds like a useful thing but I still wonder whether the silent fallback to the embedded Maven is a good idea. Won't the users be misled that they use daemon when they actually don't?

Just to explain: this kinda clashes with how I saw mvnd so far. For me, mvnd was a better and faster Maven-like tool that works great on supported platforms but does not necessarily have to stick with all Maven legacy. Now we are smuggling the old slow Maven into mvnd even in situations when users may not know that this even happens. I am not sure this is a good thing. Maybe we should make it opt in by default? - have a new useDaemon parameter with three values yes (default), no, yes-if-possible? What do others think?

I wonder how portable the fallback is: we forward all command line parameters, some of which won't work with the embedded Maven.

Now only few flags break the original maven.

  1. Some of which should break the invokes, e.g. --completion|--purge|--status|--stop, they have special effect only for mvnd.
  2. Others, like -1|--serial|--raw-streams, could be removed in the fallback main() without changing the invoke purpose. Now this fix is not implemented to let the fallback codes clean to review.

Thanks for the precise listing and investigating the impact! All you say sounds right.

@gzm55
Copy link
Contributor Author

gzm55 commented Oct 19, 2022

Maybe we should make it opt in by default? - have a new useDaemon parameter with three values yes (default), no, yes-if-possible? What do others think?

Yes, the user should be able to select what he want. Does useDaemon=no mean to always run the original maven even the native binary or JDK is OK?

Just to explain: this kinda clashes with how I saw mvnd so far. For me, mvnd was a better and faster Maven-like tool that works great on supported platforms but does not necessarily have to stick with all Maven legacy.

As a mvnd user, I would like (or hope) to see mvnd as a blind replace to the original one to get faster experience like gradle.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 20, 2022

Maybe we should make it opt in by default? - have a new useDaemon parameter with three values yes (default), no, yes-if-possible? What do others think?

Yes, the user should be able to select what he want. Does useDaemon=no mean to always run the original maven even the native binary or JDK is OK?

Yes, that's the idea.

Note that I do not insist on the useDaemon particular name nor on naming the 3 values in this particular way. We may want o find better names.

Also note that we already have -Dmvnd.noDaemon=<boolean> which we may deprecate after introducing the new 3-value option.

Just to explain: this kinda clashes with how I saw mvnd so far. For me, mvnd was a better and faster Maven-like tool that works great on supported platforms but does not necessarily have to stick with all Maven legacy.

As a mvnd user, I would like (or hope) to see mvnd as a blind replace to the original one to get faster experience like gradle.

I see. Would the opt-in explicit fallback to embedded Maven be a viable option for you?

@gzm55
Copy link
Contributor Author

gzm55 commented Oct 20, 2022

I see. Would the opt-in explicit fallback to embedded Maven be a viable option for you?

I'm OK to the switch, and it is reasonable for a package manager to always to call the native image. In this pr, we can focus on providing the fallback function, keeping the most entry behavior. I think the opt switch could be implemented in #717 .

@ppalaga
Copy link
Contributor

ppalaga commented Oct 21, 2022

I'm OK to the switch, and it is reasonable for a package manager to always to call the native image. In this pr, we can focus on providing the fallback function, keeping the most entry behavior. I think the opt switch could be implemented in #717 .

Sounds good, thanks!

# Conflicts:
#	client/pom.xml
#	common/pom.xml
#	daemon/pom.xml
#	daemon/src/main/java/org/mvndaemon/mvnd/syncontext/DaemonNamedLockFactorySelector.java
#	pom.xml
@gnodet gnodet added this to the 1.0.0-m1 milestone Dec 14, 2022
@gnodet gnodet merged commit f8d047b into apache:master Dec 14, 2022
gnodet pushed a commit that referenced this pull request Jan 6, 2023
* Use muti-release jar to fallback mvnd-client to original maven

The mvnd-client is built to a muti-release jar. The default version of
    DefaultClient is compiled against the same target version as the
   embedded maven (4.x, so JDK 1.8) which only invoke the MavenCli.main().
The java-11 version is the full qualified mvnd-client.

* update cmd scripts

* embedded maven now works under JDK 1.8

Build SimpleAppender for JDK 1.8 since it is the log appender only
   for the embedded maven

Co-authored-by: Guillaume Nodet <gnodet@gmail.com>
# Conflicts:
#	pom.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants