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

[Carmine Vincenzo Russo]

Hi Erik,

I want to point out  a couple of things before going forward.

In the issue description it says to use $ jmc -open recording.jfr instead
of $ jmc -open /Demo/recording.jfr
That's why I assumed the subdirectories where needed. While I get from your
words that we are working in the directory /Demo and from the command line
it needs the full path even if the file is in the current directory. Please
correct me if I am wrong.

Another thing is that I tried with the sequence of command you suggested as
use case, when I run
$ jcmd 4711 JFR.dump filename=dump.jfr
It saves the recording in the application directory and not in my current
working directory, therefore everything should be executed in the
application directory for your use case,
or in alternative we should use a full path, but since it is in a different
directory the issue shouldn't apply to it.

To summarize,
if the issue is about opening the recording in the current directory, it
already works for me without any patch (tested on Fedora).
If is about opening the recording from somewhere else, than can you give me
more details about the issue? It's not really clear.

Thanks,

Carmine



On Mon, Jul 8, 2019 at 10:47 PM Erik Gahlin <erik.gahlin at oracle.com> wrote:

> 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>
> 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/
>
> 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
>
>
>

-- 
Carmine Vincenzo Russo