RFC: JMC-6173: jmc -open requires full path

[Erik Gahlin]

Hi Carmine,

See comments inline.

> Hi Erik,
>
> Thank you for your response.
>
> On Thu, Jun 27, 2019 at 6:05 PM Erik Gahlin <erik.gahlin at oracle.com 
> <mailto:erik.gahlin at oracle.com>> wrote:
>
>     Hi Carmine ,
>
>     Searching the home directory could take a very long time. It could
>     also
>     open the wrong file if several files have the same name. I don't
>     think
>     it is the correct approach.
>
>
> I know, starting from the user home is a little bit extreme and can 
> take a while, therefore instead I'm starting from the current directory
It's the same problem.

On my computer, it could take an hour if I am in the wrong directory or 
a network share. I don't know any other applications that searches for a 
file in subdirectories.

> in the updated patch and searching the subdirectories from there.
> Since JMC doesn't have a specific folder to save the recordings it all 
> depends on the users.
This is primarily meant for command-line users. For example,

$ java -XX:StartFlightRecording -jar my.jar
...
$ jcmd 4711 JFR.dump filename=dump.jfr
$ jmc -open recording.jfr

The cumbersome part here is to switch to a GUI and do various operations 
with the mouse.

> As for files with the same name, there is the same problem, JMC gives 
> the possibility to choose the name for every recordings, therefore to 
> have two files with the same name the user should do it on purpose.
>
>
>     Have you tried Paths.get(".", filename) ?
>
>     It will look in the current working directory. To see if the full
>     path
>     is correct do
>
>        path.normalize().toAbsolutePath()
>
>     if this doesn't work, it could be because of some magic happening
>     in jmc
>     launcher, in which case the current directory have to be extracted in
>     the launcher code.
>
>
> Updated the patch with you suggestions regarding Paths, everything 
> seems to work fine.
Good, have you tried on the most common OSes (Mac, Linux and Windows)?

Thanks
Erik

> At the moment a full path, a path with ~/ and a filename both in the 
> current directory or in a subdirectories of the current directory open 
> a recording.
> Also I made some tests for the function findFile, there are 3 tests 
> right now:
> - EmptyPathTest: tests what happens with an empty string as filename.
> - ValidFileNameTest: tests with a temp file whether findFile can find 
> the correct file.
> - InvalidFileNameTest: tests what happens when the given filename 
> doesn't exists.
> Here the updated patch: 
> http://cr.openjdk.java.net/~aptmac/JMC-6173/webrev.01/ 
> <http://cr.openjdk.java.net/%7Eaptmac/JMC-6173/webrev.01/>
>
> Thanks,
> Carmine
>
>
>     Erik
>     > Hi,
>     >
>     > On Fri, Jun 21, 2019 at 10:39 AM Jie Kang <jkang at redhat.com
>     <mailto:jkang at redhat.com>> wrote:
>     >
>     >> On Fri, Jun 21, 2019 at 8:31 AM Carmine Vincenzo Russo
>     >> <carusso at redhat.com <mailto:carusso at redhat.com>> wrote:
>     >>> Hello,
>     >>>
>     >>> The patch addresses the issue JMC-6173[0].
>     >>> I have added a check to see if the path is full or not. If it
>     falls in
>     >> the
>     >>> second case, starting from the user home it recursively search
>     for the
>     >>> given file name in all the not hidden folder.
>     >>> There was also a problem with relative path like
>     ~/record/test.jfr, now
>     >>> fixed.
>     >>> I tested the patch with full path, relative path, filename
>     only and null
>     >>> value.
>     >>>
>     >>> I'm planning to add few tests, at least for a valid filename,
>     an invalid
>     >>> filename and a null value.
>     >>>
>     >>> See the attached patch bellow and let me know what you think.
>     >> I don't see an attachment unfortunately; was it missed or maybe
>     stripped?
>     >>
>     > Carmine sent me a note saying he attached the patch to the
>     e-mail but it
>     > doesn't appear to show up here. Instead, please find his patch
>     at the
>     > following webrev:
>     >
>     > http://cr.openjdk.java.net/~aptmac/JMC-6173/webrev.00/
>     <http://cr.openjdk.java.net/%7Eaptmac/JMC-6173/webrev.00/>
>     >
>     > I believe he's looking into unit tests at the moment, but wanted
>     feedback
>     > on his implementation before going forward.
>     >
>     >
>     >> Regards,
>     >>
>     >>> Thank you!
>     >>> Carmine
>     >>>
>     >>>
>     >>> [0] https://bugs.openjdk.java.net/browse/JMC-6173
>     >>>
>     >>> --
>     >>> Carmine Vincenzo Russo
>     > Cheers,
>     >
>     > Alex
>
>
>
> -- 
> Carmine Vincenzo Russo