Bug 201132 - Spaces in paths break Maven NBM profiling
Spaces in paths break Maven NBM profiling
Status: RESOLVED FIXED
Product: apisupport
Classification: Unclassified
Component: Maven
7.0
All All
: P2 with 1 vote (vote)
: 7.2
Assigned To: Milos Kleint
issues@apisupport
: 7.1_WAIVER_APPROVED, SPACE_IN_PATH
: 208780 212434 (view as bug list)
Depends on:
Blocks: 199922
  Show dependency treegraph
 
Reported: 2011-08-19 15:44 UTC by Jesse Glick
Modified: 2012-05-13 23:09 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Patch for testing (5.21 KB, patch)
2011-12-03 00:08 UTC, Jesse Glick
Details | Diff
A new exploratory patch (1.77 KB, patch)
2012-03-01 21:46 UTC, Jesse Glick
Details | Diff
patch exploring the double quote edge case (1.52 KB, patch)
2012-03-06 10:43 UTC, Milos Kleint
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2011-08-19 15:44:50 UTC
See bug #199922 comment #13.
Comment 1 Petr Cyhelsky 2011-11-29 10:01:23 UTC
Imho this is at least P2 because on windows(majority of NB users) where almost any path has spaces in it the maven profiling is thus completely broken.
Comment 2 Jesse Glick 2011-12-03 00:03:22 UTC
By carefully parsing, processing, and re-escaping everything the run hook gets, I can produce what seems like the right command sent to Maven:

cd C:\Documents and Settings\Jesse Glick\Local Settings\Temp\mavenproject10\application; "JAVA_HOME=C:\\Program Files\\Java\\jdk1.7.0_01" "\"F:\\tmp\\netbeans with spaces\\java\\maven\\bin\\mvn.bat\"" "-Dnetbeans.run.params.ide=-J-XX:+HeapDumpOnOutOfMemoryError \\\"-J-XX:HeapDumpPath=C:\\\\Documents and Settings\\\\Jesse Glick\\\\.netbeans\\\\dev\\\\var\\\\cache\\\\mavencachedirs\\\\1618297522\\\\org-netbeans-modules-profiler\\\" \\\"-J-agentpath:F:/tmp/netbeans with spaces/profiler/lib/deployed/jdk16/windows/profilerinterface.dll=F:/tmp/netbeans with spaces/profiler/lib,5140,10\\\" --jdkhome \\\"C:\\\\Program Files\\\\Java\\\\jdk1.7.0_01\\\"" -Dprofiler.action=profile --debug install nbm:run-platform

but this gets mangled by the time Maven core sees it:

netbeans.run.params.ide=-J-XX:+HeapDumpOnOutOfMemoryError "-J-XX:HeapDumpPath=C:\\Documents and Settings\\Jesse Glick\\.netbeans\\dev\\var\\cache\\mavencachedirs\\1618297522\\org-netbeans-modules-profiler" "-J-agentpath:F:/tmp/netbeans with spaces/profiler/lib/deployed/jdk16/windows/profilerinterface.dll=F:/tmp/netbeans with spaces/profiler/lib,5140,10" --jdkhome "C:\\Program Files\\Java\\jdk1.7.0_01

Note the missing final quote, so:

Failed to execute goal org.codehaus.mojo:nbm-maven-plugin:3.6:run-platform (default-cli) on project mavenproject10-app: Failed executing NetBeans: unbalanced quotes in -J-XX:+HeapDumpOnOutOfMemoryError "-J-XX:HeapDumpPath=C:\\Documents and Settings\\Jesse Glick\\.netbeans\\dev\\var\\cache\\mavencachedirs\\1618297522\\org-netbeans-modules-profiler" "-J-agentpath:F:/tmp/netbeans with spaces/profiler/lib/deployed/jdk16/windows/profilerinterface.dll=F:/tmp/netbeans with spaces/profiler/lib,5140,10" --jdkhome "C:\\Program Files\\Java\\jdk1.7.0_01 -> [Help 1]

I have no explanation for this other than a possible bug in mvn.bat, or maybe it is just not possible to get quoting right on Windows.
Comment 3 Jesse Glick 2011-12-03 00:08:44 UTC
Created attachment 113784 [details]
Patch for testing
Comment 4 Jesse Glick 2011-12-29 14:37:42 UTC
I committed what I have so far, but have no further ideas for a Windows fix; perhaps someone with Windows batch script expertise would know what is going on. Workaround is, as always, to avoid paths with spaces in them.
Comment 5 Quality Engineering 2011-12-31 15:42:53 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/2fb63284cbc7
User: Jesse Glick <jglick@netbeans.org>
Log: #201132: spaces in paths break Maven NBM profiling.
Current attempt at a fix, which seems beneficial on Unix and does not make the
situation on Windows any better; there seems to be some bug either in mvn.bat or
in the Windows batch script system which causes quoting of the JDK path to fail.
Comment 6 Jesse Glick 2012-02-08 16:58:46 UTC
Not a patch candidate since there is no known fix. (Miloš any clue?)
Comment 7 Milos Kleint 2012-02-08 17:49:20 UTC
what about working on the nbm:run-platform goal. maybe different format of parameters, special goal for profiler and/or running via commons-exec rather than maven's own plexus-utils..
Comment 8 Jesse Glick 2012-02-14 18:21:24 UTC
But I do not see anything wrong with the command the plugin has prepared; the problem appears to be happening in the Maven launcher.
Comment 9 Milos Kleint 2012-02-15 12:01:26 UTC
(In reply to comment #8)
> But I do not see anything wrong with the command the plugin has prepared; the
> problem appears to be happening in the Maven launcher.

Yes and  by simplifying what gets passed to the maven launcher we could avoid translation problems. Eg. if we pass just -Dnetbeans.profiler.agentPath=<path with spaces>
Comment 10 Jesse Glick 2012-02-15 17:17:52 UTC
Hardcoding this kind of information in the nbm:run-platform goal seems wrong. Better IMHO to figure out why what we have now does not work and see if there is a straightforward fix.
Comment 11 Petr Cyhelsky 2012-02-27 10:25:17 UTC
*** Bug 208780 has been marked as a duplicate of this bug. ***
Comment 12 Jesse Glick 2012-02-28 00:26:08 UTC
Certainly not a P1. There is a simple workaround: use reasonable path names, without spaces in them.
Comment 13 Milos Kleint 2012-02-28 12:56:45 UTC
a special agent path parameter would only only be useful for profiling, but also agents like jrebel or javeleon, right?

in general I think avoiding the parameter in parameter wrapping helps also the user experience, can be better documented and so on.
Comment 14 Milos Kleint 2012-02-28 15:10:59 UTC
the final quote is NOT missing when I add "aaa" to the property in the Actions/profile project mapping.
Interestingly the various quoting is done at multiple places.

1. RunCheckerImpl.profilerArgs performs Utilities.escapeParameters()
2. MavenCommandLineExecutor.createMavenExecutionCommand() performs String.replace("\"", "\\\"")
3. java's ProcessImpl() will check if the entry with spaces starts with " and when not, wrap the entry in "" quotes.

my head still hurts from even thinking about it, but I believe the 3. wrapping (which is happening for the troubled parameter) causes problems as the eventual existing "" characters are not escaped while wrapping into a new set..
Comment 15 Jesse Glick 2012-02-28 19:47:30 UTC
(In reply to comment #13)
> a special agent path parameter would only only be useful for profiling, but
> also agents like jrebel or javeleon, right?

Yes, at least Javeleon for 7.2 uses issue #206196 to add agent args; I suppose that might suffer from this bug as well.

(In reply to comment #14)
> 2. MavenCommandLineExecutor.createMavenExecutionCommand() performs
> String.replace("\"", "\\\"")

This seems suspicious. The args are being passed to ProcessBuilder as a List<String> so quoting their contents should be redundant. At least that should be true on Unix AFAIK.
Comment 16 Milos Kleint 2012-02-29 08:01:07 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > 2. MavenCommandLineExecutor.createMavenExecutionCommand() performs
> > String.replace("\"", "\\\"")
> 
> This seems suspicious. The args are being passed to ProcessBuilder as a
> List<String> so quoting their contents should be redundant. At least that
> should be true on Unix AFAIK.

yes, but it's parameter for mvn execution  that contains parameters to the jvm executed by a goal, and some of these contains spaces in their values. Without the escaping, the processbuilder would still add quotes at start and end, but the unescaped quotes would interfere then and chop the parameter in parts. Somehow it's happening anyway in the setup we have, but in my experiment with appending aaa et the end of the run.args property it was not happening. 

a few random searches on stackoverflow suggest that "" means an escape for " 
eg. http://stackoverflow.com/questions/130698/how-do-i-protect-quotes-in-a-batch-file

that would explain the breaking after the --jdkhome \\\"C:\\\\Program
Files\\\\Java\\\\jdk1.7.0_01\\\"" in your example, as that's the only place with 2 quotes. why my experiment with appending aaa works also supports this claim as then there are no "" in place.
Comment 17 Jesse Glick 2012-03-01 21:46:02 UTC
Created attachment 116264 [details]
A new exploratory patch

Interesting lead, but I did not manage to get any variant of this to work.
Comment 18 Milos Kleint 2012-03-06 10:43:35 UTC
Created attachment 116380 [details]
patch exploring the double quote edge case

attached patch seems to handle the case described in the issue, I'm able to profile an nbm application afterwards. I'm (still, not sure it it was the case before though) having problems now profiling simple j2se apps now.
Comment 19 Milos Kleint 2012-04-27 08:27:01 UTC
http://hg.netbeans.org/core-main/rev/b7675d27e408

applied my latest patch which was tested by jbachorik and appears to be working..
Comment 20 Quality Engineering 2012-05-01 09:56:42 UTC
Integrated into 'main-golden', will be available in build *201205010400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/b7675d27e408
User: Milos Kleint <mkleint@netbeans.org>
Log: #201132 on windows append a space after trailing doublequotes..see issue for more details
Comment 21 J Bachorik 2012-05-13 23:09:51 UTC
*** Bug 212434 has been marked as a duplicate of this bug. ***


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