This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
| Summary: | Multiple Maven Runtimes support | ||
|---|---|---|---|
| Product: | projects | Reporter: | johnsonlau <johnsonlau> |
| Component: | Maven | Assignee: | johnsonlau <johnsonlau> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | anebuzelsky, jglick, jkovalsky, mkleint |
| Priority: | P3 | Keywords: | NETFIX |
| Version: | 6.x | ||
| Hardware: | PC | ||
| OS: | All | ||
| Issue Type: | ENHANCEMENT | Exception Reporter: | |
| Attachments: |
multiple maven runtimes support
Eclipse Maven plugin screenshot (enhancements to maven plugin) Radio buttons used on Runtime Selector UI <no description> <no description> new patch Patch preview using JComboBox Patch using JComboBox Patch on latest main repo Latest patch JSeparator doen't show up on Mac Aqua L&F Patch to AquaSeparatorUI.java brings JSeparator back to screen Latest patch |
||
Created attachment 96654 [details]
Eclipse Maven plugin
Created attachment 96655 [details]
screenshot (enhancements to maven plugin)
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. 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? 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. 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. 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 Created attachment 97967 [details]
Radio buttons used on Runtime Selector UI
Created attachment 97968 [details]
<no description>
Created attachment 97969 [details]
<no description>
Created attachment 97970 [details]
new patch
Now that trunk is open again, can you please Dafe review and integrate the patch? Thanks! 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. Zhongcheng, are you willing to rework the patch according to Jesse's recommendation? 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. Created attachment 102528 [details]
Patch preview using JComboBox
Created attachment 102529 [details]
Patch using JComboBox
(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. 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. Created attachment 102610 [details]
Patch on latest main repo
(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. 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. Created attachment 102651 [details]
Latest patch
Created attachment 102652 [details]
JSeparator doen't show up on Mac Aqua L&F
Created attachment 102653 [details]
Patch to AquaSeparatorUI.java brings JSeparator back to screen
(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. 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.
(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. Cool, thanks a lot Jesse for your valuable comments and integration. And of course thanks Zhongcheng for your patch and persistence! 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 |
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.