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

maven daemon support #3210

Merged
merged 2 commits into from
Jan 10, 2022
Merged

maven daemon support #3210

merged 2 commits into from
Jan 10, 2022

Conversation

mbien
Copy link
Member

@mbien mbien commented Oct 4, 2021

added minimal support for https://github.com/mvndaemon/mvnd

maven home has the following layout:

bin/mvn
lib/*.jar

while mvnd home looks like:

bin/mvnd
mvn/  <- bundled maven home

This PR allows users to set a mvnd home as a maven home, NetBeans will then use mvnd and also be able to query the maven version from the bundled maven home.

did a little bit of manual testing and it seems that everything is working fine.

potentially related jira issue:
https://issues.apache.org/jira/browse/NETBEANS-4746

@ebarboni
Copy link
Contributor

ebarboni commented Oct 5, 2021

Seems related to #3198. Maybe good to synchronize idea

@neilcsmith-net
Copy link
Member

@ebarboni I'm not sure they need synchronizing. I don't think this will conflict having looked at the changes. The mvnw support kicks in on the project level when it's there, so should bypass the code in this PR entirely.

@mbien
Copy link
Member Author

mbien commented Oct 5, 2021

@mbien mbien marked this pull request as ready for review October 6, 2021 16:51
@mbien
Copy link
Member Author

mbien commented Oct 6, 2021

two issues encountered while working with mvnd:

  • mvnd prints a dumb-terminal warning on every run
  • mvnd's output is formatted differently, NB does not hyperlink exceptions properly

@sdedic
Copy link
Member

sdedic commented Oct 8, 2021

What about distributing mvnd instead of maven ;) in NetBeans ?

@mbien
Copy link
Member Author

mbien commented Oct 8, 2021

What about distributing mvnd instead of maven ;) in NetBeans ?

not sure if i would recommend doing this tbh. First of all, there is a chance that mvnd will merge with maven in the next major version of maven (so i heard). Also, if mvnw takes off, it will be just a flag and everything downloads behind the scene anyway - no need to bundle anything.

There is also the issue with mvnd being MT, which will cause build failures on some projects or plugin combinations. So it can't be made default. I tested it on some dusty projects and not all of them built reliably with mvnd.

the best strategy might be to wait a bit longer while supporting a little bit of everything. If mvnd merges with maven and mvnw takes off... things might get simpler, and NB could remove/unify some code.

@jglick
Copy link
Contributor

jglick commented Oct 8, 2021

I have used mvnd in some cases where it made a big difference, though I have also come across weird cases where it seems to produce different results than mvn (even after disabling multithreading), so I would be cautious about switching to it by default.

@sdedic
Copy link
Member

sdedic commented Oct 9, 2021

let me rephrase: as mvnd contains (?) mvn as its subset, we could eventually distribute mvnd (which includes mvn @ 3.8.2 at the moment, an upgrade from the current bundled 3.6.3), and get both: default maven, configurable mvnd

@mbien
Copy link
Member Author

mbien commented Oct 9, 2021

let me rephrase: as mvnd contains (?) mvn as its subset, we could eventually distribute mvnd (which includes mvn @ 3.8.2 at the moment, an upgrade from the current bundled 3.6.3), and get both: default maven, configurable mvnd

sure, I tested this PR with that config. if you set path to mvnd/mvn/ you get maven, if you set path to mvnd/ you get mvnd. I don't have anything against it.
My point simply was that if maven 4 going to be released with mvnd and mvnw in it, all this code would be dropped anyway and there will be hopefully one executable with different flags for different modes + mvnd should not be the NB default.

But if there is an easy way to bundle the mvnd+mvn combo in another PR and set the paths accordingly by default, why not. Just don't put too much effort into it i would say ;)

@mbien
Copy link
Member Author

mbien commented Oct 14, 2021

this might be too risky to be merged for 12.6. I did only basic clean/build/run manual testing with it.

Copy link
Member

@sdedic sdedic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@mbien mbien added the Maven [ci] enable "build tools" tests label Dec 28, 2021
@mbien
Copy link
Member Author

mbien commented Jan 9, 2022

mvnd 0.7.1 seems to be working better, the console output is very similar to regular maven and most simple projects I tested worked fine. (roller selenium tests didn't work with mvnd for example but this is not NB' fault)

mvnd also recently became an apache project

rebased this PR to latest master. All tests are green. I think we could integrate this as minimal/experimental feature.

@mbien mbien merged commit 0890441 into apache:master Jan 10, 2022
@jglick
Copy link
Contributor

jglick commented Jan 10, 2022

Got

java.lang.NullPointerException
	at org.netbeans.modules.maven.execute.CommandLineOutputHandler.trimTree(CommandLineOutputHandler.java:548)
	at org.netbeans.modules.maven.execute.CommandLineOutputHandler.processExecEvent(CommandLineOutputHandler.java:456)
	at org.netbeans.modules.maven.execute.CommandLineOutputHandler.access$300(CommandLineOutputHandler.java:74)
	at org.netbeans.modules.maven.execute.CommandLineOutputHandler$Output.run(CommandLineOutputHandler.java:276)
	at …

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maven [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants