[ROO-363] Perform commands do not work in RC3 Created: 10/Nov/09  Updated: 15/Feb/10  Resolved: 15/Nov/09

Status: Closed
Project: Spring Roo
Component/s: BUILD
Affects Version/s: 1.0.0.RC3
Fix Version/s: 1.0.0.RC4

Type: Bug Priority: Major
Reporter: Stefan Ocke Assignee: Joris Kuipers
Resolution: Fixed Votes: 2
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Win XP SP3


Attachments: File MavenCommands.patch    
Issue Links:
Duplicate
is duplicated by ROO-392 perform command not working on Win 7 Closed
Related
is related to ROO-327 Eclipse Tooling: Maven "perform" comm... Closed

 Description   

Just dowloaded RC3.
Perform commands seem not to work at all. Same behavior in sts and in standalone roo shell.

Just looks like this:

roo> perform package
roo>

I have Maven 2.10 in my path.

In previous version (RC2) it did work (besides the known issue in STS ...)



 Comments   
Comment by Scott Murphy [ 11/Nov/09 ]

yeah, no luck with perform eclipse either. Same behavior.

script wedding.roo
perform eclipse

Comment by Stefan Schmidt [ 11/Nov/09 ]

Stefan,

Thanks for raising this!

I just tested this on Linux, Mac and Windows and the problem exists only in Windows (it would be great to mention this in the 'environment' field next time ).

Will need to take a closer look.

Comment by Stefan Schmidt [ 11/Nov/09 ]

Reassigning this bug to Ben.

Ben, in rev 325 you changed MavenCommands:109:

Process p = Runtime.getRuntime().exec(cmd);

to

Process p = Runtime.getRuntime().exec(cmd, new String[0], root);

I am not sure what the background on this is but this causes this bug on Windows. Apparently you want to start the maven process in a seperate process. I tried debugging why it does not work in Windows it got a little weird as the debugger does not present all variables involved. The root file path looks fine to me under Windows though, so the problem must be inside the Runtime.exec method.

When reverting to Runtime.getRuntime().exec(cmd); all works fine but that is obviously not what is desired here.

-Stefan

Comment by Ben Alex [ 11/Nov/09 ]

Regression introduced in ROO-327.

Comment by Joris Kuipers [ 13/Nov/09 ]

You need to change this:
Process p = Runtime.getRuntime().exec(cmd, new String[0], root);
into this:
Process p = Runtime.getRuntime().exec(cmd, null, root);

That's also what exec(cmd) does, and that's what works on Windows. Otherwise the new process won't inherit the environment of the parent process, the path is simply going to be empty then.

BTW, you also shouldn't be using "cmd /c mvn " for Windows, you should be using "mvn.bat " instead. Going through cmd.exe is just going to get you in trouble when you start adding cmd line args that will be processed by the quoting rules used by cmd.exe (which aren't trivial): the reason that "mvn " doesn't work is that without cmd.exe, there's nothing in Windows that will guess that you're really intending mvn.bat when you just specify mvn.
Speaking of cmd line args: you also really should be using the String[] variant of exec() instead of just a String, or you'll run into trouble when you want to pass arguments that contain spaces. Don't fall into the trap of trying to come up with your own quoting algorithm for the various platforms then, just use String[] and everything will work fine on all platforms without any quoting in the code.
Trust me on this, this is coming from a former CruiseControl committer who used to invoke external processes to interact with version control systems using complex cmd lines all the time

Comment by Joris Kuipers [ 13/Nov/09 ]

Here's a patch that also checks the output for errors and warnings to use the appropriate log level for color highlighting of Maven messages.
Since the 'extra' parameter comes in as a single string I haven't used String[] for the cmd here, the user will simply have to make sure to quote args with spaces himself for his OS. Not a big issue, I'd say.

Comment by Joris Kuipers [ 15/Nov/09 ]

Patch committed, also added color output for Maven errors and warnings

Comment by Joris Kuipers [ 15/Nov/09 ]

Note: this was fixed in SVN rev 431. Rev 432 contains an additional fix for recognizing the error msg on Windows emitted when mvn.bat cannot be found.

Comment by Ernest E Vogelsinger [ 20/Nov/09 ]

I hate it to disappoint you but I have the identical problem, running openSUSE 11.1 (see below)

[email protected]:~/roo-test/ten-minutes> cat /etc/SuSE-release
openSUSE 11.1 (i586)
VERSION = 11.1
[email protected]:~/roo-test/ten-minutes> roo
____ ____ ____
/ __ \/ __ \/ __ \
/ /_/ / / / / / / /
/ , _/ // / /_/ /
// ||___/___/ 1.0.0.RC3 [rev 401]

Welcome to Spring Roo. For assistance press TAB or type "hint" then hit ENTER.
roo> perform tests
Error: JAVA_HOME is not defined correctly.
We cannot execute
roo> exit
[email protected]:~/roo-test/ten-minutes> $JAVA_HOME/bin/java -version
java version "1.6.0_17"
Java(TM) SE Runtime Environment (build 1.6.0_17-b04)
Java HotSpot(TM) Server VM (build 14.3-b01, mixed mode)
[email protected]:~/roo-test/ten-minutes>

Comment by Joris Kuipers [ 20/Nov/09 ]

@ Ernest: that's not disappointing, it's expected. The environment is not passed to the process running maven, so JAVA_HOME is not available to maven even though it's defined in your normal environment.
I'm not sure why not more people not on Windows are affected by this bug, but I am sure that the patch I committed will fix this issue for you as well. RC 4 is scheduled for the end of this month I believe, but you can easily build Roo from SVN yourself if you want early access to a release that contains this fix.

Comment by Ernest E Vogelsinger [ 20/Nov/09 ]

Hi Joris, thanks for your comment, I just had a look into your patch (should have done this before ;->) and indeed this should fix the problem.

As this fix only affects the addon.maven jar why not post it here so we don't need to build roo completely?

Generated at Thu Jun 21 06:44:57 UTC 2018 using JIRA 7.9.0#79000-sha1:3ca552e944c2fe83b21589bc06f155b9b428cc2b.