Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 183455 - Multiple Maven Runtimes support
Multiple Maven Runtimes support
Status: RESOLVED FIXED
Product: projects
Classification: Unclassified
Component: Maven
6.x
PC All
: P3 (vote)
: 7.0
Assigned To: johnsonlau
issues@projects
: NETFIX
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-04 16:32 UTC by johnsonlau
Modified: 2010-10-28 02:54 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
multiple maven runtimes support (83.44 KB, patch)
2010-04-04 16:32 UTC, johnsonlau
Details | Diff
Eclipse Maven plugin (57.42 KB, image/png)
2010-04-04 16:34 UTC, johnsonlau
Details
screenshot (enhancements to maven plugin) (79.66 KB, image/png)
2010-04-04 16:35 UTC, johnsonlau
Details
Radio buttons used on Runtime Selector UI (109.22 KB, image/png)
2010-04-25 10:39 UTC, johnsonlau
Details
<no description> (48.86 KB, image/png)
2010-04-25 10:39 UTC, johnsonlau
Details
<no description> (48.18 KB, image/png)
2010-04-25 10:39 UTC, johnsonlau
Details
new patch (118.73 KB, patch)
2010-04-25 10:39 UTC, johnsonlau
Details | Diff
Patch preview using JComboBox (78.99 KB, image/png)
2010-10-20 17:18 UTC, johnsonlau
Details
Patch using JComboBox (118.05 KB, patch)
2010-10-20 17:23 UTC, johnsonlau
Details | Diff
Patch on latest main repo (49.17 KB, patch)
2010-10-25 10:31 UTC, johnsonlau
Details | Diff
Latest patch (114 bytes, patch)
2010-10-26 18:03 UTC, johnsonlau
Details | Diff
JSeparator doen't show up on Mac Aqua L&F (106.26 KB, image/png)
2010-10-26 18:04 UTC, johnsonlau
Details
Patch to AquaSeparatorUI.java brings JSeparator back to screen (106.52 KB, image/png)
2010-10-26 18:06 UTC, johnsonlau
Details
Latest patch (38.01 KB, patch)
2010-10-27 06:25 UTC, johnsonlau
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description johnsonlau 2010-04-04 16:32:40 UTC
Created attachment 96653 [details]
multiple maven runtimes support

Currently Maven plugin only allows one instance of external Maven Runtime.

This might lead to a little painful way when I want to switch between different Maven Runtimes, for some particular reasons, like a restrict Configuration Management requirement.

I redesigned the plugin's option UI and two enhancements introduced.

1) The former External Maven configuration changed to Maven Runtimes now.
You can add as many as your Maven installtions on your computer are,
then choose one as your active runtime.
The Maven Runtime installations are now persisted to a new property named PROP_MAVEN_RUNTIMES, in the form of "DIR1;DIR2;DIR3" (in which ; stands for file.separator).
The active runtime stills stored in the property PROP_COMMANDLINE_PATH.
Codes that now rely on this property still work without any further modification.

2) Global Execution Options changed to a JList to prevent duplicated value to be added.

This work was inspired by the Eclipse Maven plugin.
I've tagged this issue as NETFIX.
Please evaluate this issue.
Comment 1 johnsonlau 2010-04-04 16:34:19 UTC
Created attachment 96654 [details]
Eclipse Maven plugin
Comment 2 johnsonlau 2010-04-04 16:35:43 UTC
Created attachment 96655 [details]
screenshot (enhancements to maven plugin)
Comment 3 Jiri Kovalsky 2010-04-07 13:15:50 UTC
Dafe, can you please review this contribution and provide your comments? Thanks! I assume this could be integrated once 6.9 is branched and trunk is open for new features.
Comment 4 Antonin Nebuzelsky 2010-04-15 12:01:31 UTC
Thanks for the patch! It will be considered for integration in nb.next. Not for 6.9 at this stage, long after feature-freeze.

Milosi, what's your opinion on this implementation?
Comment 5 David Simonek 2010-04-15 12:18:23 UTC
Ouch, I wrote review a week ago but it wasn't delivered somehow! :-(

Anyway, I basically said the same as Antonin. With one complaint - from UI point of view, I would use radio buttons instead of check boxes, as only one runtime can be active at the moment.

Overall my opinion is to integrate soon after the trunk is open for new features, which should be in May according to schedule: http://wiki.netbeans.org/NetBeans_69#Schedule

johnsonlau, thanks a lot.
Comment 6 Milos Kleint 2010-04-15 13:39:51 UTC
with the rise of maven3, the option to choose a different maven runtime for each project might prove useful indeed. 

 I'm not sure how per project configuration shall look like and how such configuration will be shared across users. Shall it? The idea with one maven install is that there is not really any per project configuration necessary and each user can have any maven installed. With a shared choice, you need to persist the choice, create identifier for it, bug people if they don't have the one selected. a non-shared choice is easier.. However since m3 doesn't support profiles.xml files (inheritable per- project user settings), the selection of a given maven install needs to be done for each individual project separately. Which can become cumbersome with 200 projects (not to speak that you might get different results when building from parent aggregator and from module project). 
So maybe easiest solution could be the best.. have multiple choice but on global level only.. which is more like a single choice with history then..hmm..


I agree with dsimonek's comment on checkboxes, we are not using a checkboxed list in netbeans often (at all?). Since only one can be chosen at a time, then a simple list selection should do.
Comment 7 johnsonlau 2010-04-25 10:38:52 UTC
Hi, all.
I updated my patch to reflect your suggestions.

1) Use Radio buttons on the Runtime selector UI.
2) The "Use external Maven" option is changed to "Force to use embedded runtime"
3) Changed warning text on the Java compile properties page when embedded runtime is being used
4) nb will show warnings on embedded runtime when
  a) "Force to use embedded runtime" set to true
  b) An embeded runtime activated in the Runtime selector UI
Comment 8 johnsonlau 2010-04-25 10:39:18 UTC
Created attachment 97967 [details]
Radio buttons used on Runtime Selector UI
Comment 9 johnsonlau 2010-04-25 10:39:30 UTC
Created attachment 97968 [details]
<no description>
Comment 10 johnsonlau 2010-04-25 10:39:41 UTC
Created attachment 97969 [details]
<no description>
Comment 11 johnsonlau 2010-04-25 10:39:52 UTC
Created attachment 97970 [details]
new patch
Comment 12 Jiri Kovalsky 2010-06-02 08:47:13 UTC
Now that trunk is open again, can you please Dafe review and integrate the patch? Thanks!
Comment 13 Jesse Glick 2010-07-28 15:31:53 UTC
Selection in maven-dev branch now works a bit differently. No option to run in-VM; location field never blank, defaults to bundled installation. Patch would need to be reworked.

Rather than the fairly heavyweight list/add/edit/remove UI, I would recommend what comment #6 suggested - just turn the text field into a combo box and retain history of recent selections. This should work about as well for the minority of people who need to frequently switch installations, and consume less visual space for everyone else.
Comment 14 Jiri Kovalsky 2010-09-30 12:05:59 UTC
Zhongcheng, are you willing to rework the patch according to Jesse's recommendation?
Comment 15 johnsonlau 2010-10-20 17:18:33 UTC
Hi all,
Sorry for the delayed response.
I've been working on some enhancements to OpenJDK and finally got some time to continue this issue.

In the latest patch, I changed the JTable to JComboBox as told.
Maven Runtimes are separated into 2 groups - Predefined Runtimes (the embedded one and the runtime found on PATH, above the first JSeparator) and User-Defined Runtimes (added manually by user, under the first JSeparator), in the JComboBox.

Also, two menu-like actions are provided - "Browse..." for browsing and adding a new user-defined Maven Runtime, "Clear User-Defined Maven Runtimes" for clearing all runtimes added manually. These two items can be easily changed to JButton if that was better.

Changes to Global Execution Options are withdrawn since the current NB implementation is good enough to let the user type in any option that isn't provided by the NB options window yet but supported by Maven.
Comment 16 johnsonlau 2010-10-20 17:18:54 UTC
Created attachment 102528 [details]
Patch preview using JComboBox
Comment 17 johnsonlau 2010-10-20 17:23:30 UTC
Created attachment 102529 [details]
Patch using JComboBox
Comment 18 Jesse Glick 2010-10-20 18:16:20 UTC
(In reply to comment #15)
> In the latest patch

This patch does not apply against current trunk; there are dozens of rejects. I cannot locate the supposed parent revision dced82c6595e (another MQ patch?). See the first para of comment #13: you seem to be working against some old version of NB sources, predating the Maven 3 integration.

Also please review your own patch more carefully to catch e.g. the wholesale replacement of line endings in MavenSettings.java (for this particular case see <http://wiki.netbeans.org/HgHowTos#Configuring_Mercurial>). For a minor UI change like this I would expect to see a much smaller patch.

> Maven Runtimes are separated into 2 groups - Predefined Runtimes (the embedded
> one and the runtime found on PATH, above the first JSeparator) and User-Defined
> Runtimes (added manually by user, under the first JSeparator), in the
> JComboBox.

Seems reasonable.

> two menu-like actions are provided - "Browse..." for browsing and adding
> a new user-defined Maven Runtime

Yes, this is pretty standard.

> "Clear User-Defined Maven Runtimes" for clearing all runtimes added manually.

Suggest just deleting this; I don't see a clear use case for it - assuming that the list of user-defined runtimes is an LRU list with a maximum of say 10 elements, and that you automatically clear any file paths which no longer exist.

No need to use the label "Active Maven Runtimes", I think; current "Maven Home" plus a combo box rather than a text field + buttons should be clear enough.

I also don't see any compelling need for tool tips with the Maven version. If you know about a local Maven installation, you probably know what it is just from the file path. Should simplify patch by eliminating need for MavenRuntime and MavenUtil classes.
Comment 19 johnsonlau 2010-10-25 10:29:40 UTC
Sorry for the CRLF problem on the patch and bringing so many inconviniences to you. I just switched from Windows to Mac which got me messed up with my local copy.

Patch was originally worked on community-main repo which becomes obsoleted currently.
The new patch on main repo is posted.
I took your advice to eliminate reduant codes and files, except those relevant to the followings.

> Suggest just deleting this; I don't see a clear use case for it - assuming that
the list of user-defined runtimes is an LRU list with a maximum of say 10
elements, and that you automatically clear any file paths which no longer
exist.
The list of user-defined runtimes is already LRU list which only preserve only 5 elements.
The "Clear" option is intended to give users the oppotunity to clear that LRU, resetting the settings back to the time NB installed, since there is no way to clear them once they are added.
It just acts like "Reset your configuration".

> I also don't see any compelling need for tool tips with the Maven version. If
you know about a local Maven installation, you probably know what it is just
from the file path.
The version label is to give a clear vision on the version detail of selected Maven runtime. It helps users to identify which version they're making it an active Maven Home.

I would like to keep them as these two enhancements improve user experience in my opinion.
But also I wouldn't mind they were removed since that doesn't affect too much actually.
Comment 20 johnsonlau 2010-10-25 10:31:24 UTC
Created attachment 102610 [details]
Patch on latest main repo
Comment 21 Jesse Glick 2010-10-25 21:11:47 UTC
(In reply to comment #19)
> I took your advice to eliminate reduant codes and files, except those relevant
> to the followings.

There is still a lot in the patch with no clear relation to the stated purpose:

1. component.setToolTipText("") -> component.setToolTipText(null)

2. block with "M2_HOME must be filtered when 2.x being used and 3.x on the PATH" (maybe more appropriate for a separate bug report?)

3. gratuitous label changes to other parts of the panel

4. rename of MyJTextField

5. removal of call to MavenSettings.setDefaultOptions from applyValues

6. import com.sun.tools.javac.resources.version

Also the term "embedded" should not be used in either code or UI, since we no longer provide the option to run Maven in embedded mode; to avoid confusion, rather use "bundled" or "default" to describe the Maven installation at $NB_HOME/java/maven. ("Default" is consistent with the UI for Ant settings, which however does not have the equivalent of this issue.)

Careful with ComboBoxRenderer (which ought to be a static nested class BTW); it is not obvious that will work well on all L&Fs. Not sure if extending DefaultListCellRenderer is preferable.

> The "Clear" option is intended to give users the oppotunity to clear that LRU,
> resetting the settings back to the time NB installed, since there is no way to
> clear them once they are added.

I understand what it does, but not why it is there. If you aren't using an old installation, what harm is done by it being in the list? We don't provide any "Clear" option for e.g. File > Open Recent File, for example.

> The version label is to give a clear vision on the version detail of selected
> Maven runtime. It helps users to identify which version they're making it an
> active Maven Home.

Is this not obvious from the directory name in a typical case, such as your attached screenshot? Anyway the code is now simpler so it is harmless to keep this.
Comment 22 johnsonlau 2010-10-26 18:03:07 UTC
Thank you for your review.
I attached the latest patch reflecting your suggestion.

> 1. component.setToolTipText("") -> component.setToolTipText(null)
Filed as new bug report.
http://netbeans.org/bugzilla/show_bug.cgi?id=191375

> 2. block with "M2_HOME must be filtered when 2.x being used and 3.x on the
> PATH" (maybe more appropriate for a separate bug report?)
Filed as new bug report.
http://netbeans.org/bugzilla/show_bug.cgi?id=191374

> 3. gratuitous label changes to other parts of the panel
> 4. rename of MyJTextField
Removed from the patch.

> 5. removal of call to MavenSettings.setDefaultOptions from applyValues
Sorry, a fool mistake. The latest patch keeps this.

> 6. import com.sun.tools.javac.resources.version
Deleted.

> to avoid confusion,
> rather use "bundled" or "default" to describe the Maven installation at
> $NB_HOME/java/maven.
Now changed to use "bundled".

> I understand what it does, but not why it is there. If you aren't using an old
> installation, what harm is done by it being in the list? We don't provide any
> "Clear" option for e.g. File > Open Recent File, for example.
The "Clear" option is also removed now.

> Careful with ComboBoxRenderer (which ought to be a static nested class BTW); > it is not obvious that will work well on all L&Fs. Not sure if extending
> DefaultListCellRenderer is preferable.
I changed ComboBoxRenderer to static and extend from DefaultListCellRenderer currently.
But I found that the JSeparator didn't show up in Aqua L&F on Mac.
I noticed that NB uses its own AquaSeparatorUI for JSeparator on Mac, which simply does nothing except for those JSeparators on a popup menu.
The JSeparator shows normally after I patched the implementation.
Please refer to the attached screenshots.
Is it a bug or for some certain reason? I will file another issue if it is confirmed as a bug.

The patch shouldn't contain other changes except those to this issue.
Please review it again and see what else would need to change.
Comment 23 johnsonlau 2010-10-26 18:03:16 UTC
Created attachment 102651 [details]
Latest patch
Comment 24 johnsonlau 2010-10-26 18:04:39 UTC
Created attachment 102652 [details]
JSeparator doen't show up on Mac Aqua L&F
Comment 25 johnsonlau 2010-10-26 18:06:09 UTC
Created attachment 102653 [details]
Patch to AquaSeparatorUI.java brings JSeparator back to screen
Comment 26 Jesse Glick 2010-10-26 20:24:09 UTC
(In reply to comment #22)
> I attached the latest patch reflecting your suggestion.

Unfortunately this latest attachment was an empty patch...?

> I changed ComboBoxRenderer to static and extend from DefaultListCellRenderer
> currently.

OK, I hope this works as well on Aqua? I know cell renderers are notoriously fickle; various native-type L&Fs make undocumented assumptions about how and where renderers are used.

> Is it a bug or for some certain reason? I will file another issue if it is
> confirmed as a bug.

I'm afraid I have no clue about that (and without a Mac I can't test it); better to file in platform/JDK Problems and someone more knowledgeable (probably saubrecht) can evaluate.
Comment 27 johnsonlau 2010-10-27 06:25:55 UTC
Created attachment 102667 [details]
Latest patch

Correct patch uploaded.

After I patch o.n.swing.plaf/src/org/netbeans/swing/plaf/aqua/AquaSeparatorUI.java for Aqua L&F,
DefaultListCellRenderer works well on my Mac in all Metal, Nimbus and Aqua L&F.

I will file another issue about the Aqua L&F that NetBeans uses currently later.
Comment 28 Jesse Glick 2010-10-27 12:17:00 UTC
(In reply to comment #27)
> Correct patch uploaded.

Thanks, applied in core-main #84598c2ddd05. I had to add core-main #b7ce92e8f16d to prevent

SEVERE [global]
java.lang.NullPointerException
	at java.io.File.<init>(File.java:222)
	at org.netbeans.modules.maven.options.SettingsPanel.setValues(SettingsPanel.java:653)
	at org.netbeans.modules.maven.options.MavenOptionController.update(MavenOptionController.java:82)
...

since on my machine

$ ls -ld `which mvn`
lrwxrwxrwx 1 root root 20 2010-09-16 17:49 /usr/local/bin/mvn -> /space/maven/bin/mvn

BTW getDefaultExternalMavenRuntime might need some tweaks in the future; File.pathSeparator.length() should perhaps be File.separator.length() (though in practice both are always 1), and path.endsWith("bin") might be false if someone tacks on an extra / or \ at the end. Usually preferred to convert to java.io.File and use that for further calculations.

> I will file another issue about the Aqua L&F that NetBeans uses currently
> later.

OK. Make it block bug #66885.
Comment 29 Jiri Kovalsky 2010-10-27 12:21:57 UTC
Cool, thanks a lot Jesse for your valuable comments and integration. And of course thanks Zhongcheng for your patch and persistence!
Comment 30 Quality Engineering 2010-10-28 02:54:36 UTC
Integrated into 'main-golden', will be available in build *201010280000* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/84598c2ddd05
User: johnsonlau@netbeans.org
Log: (#183455) Multiple Maven Runtimes support


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo