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

[Carmine Vincenzo Russo]

Hi Erik,

Thank you for your response.

On Thu, Jun 27, 2019 at 6:05 PM Erik Gahlin <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 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.
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.
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/

Thanks,
Carmine


> Erik
> > Hi,
> >
> > On Fri, Jun 21, 2019 at 10:39 AM Jie Kang <jkang at redhat.com> wrote:
> >
> >> On Fri, Jun 21, 2019 at 8:31 AM Carmine Vincenzo Russo
> >> <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/
> >
> > 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