Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7070: Allow JMC to open compressed recordings with the extension .lz4 #250

Closed
wants to merge 1 commit into from

Conversation

aptmac
Copy link
Member

@aptmac aptmac commented May 20, 2021

This one-worder addresses JMC-7070 [0], in which LZ4 compressed files cannot be opened while having the file extension ".lz4"

JMC-6701 added support for opening jfr files that were compressed using lz4 [0]. The functionality works, however the file association was not updated to include files ending in ".lz4" for flight recording files, so these compressed recordings must still be supplied with the file ending of ".jfr". This isn't ideal, because the default behaviour of lz4 is to append ".lz4" to the end of the supplied file; in my example I have a file called lol.jfr, which when compressed becomes lol.jfr.lz4 and cannot be opened by JMC without manually renaming it.

This one-word fix adds the file extension "lz4" to the extension list in flightrecorder.ui's plugin.xml for JfrEditor, so that supplying a *.lz4 file will result in trying to open a JfrEditor page.

[0] https://bugs.openjdk.java.net/browse/JMC-7070
[1] b6e5bde

Before:

  • LZ4 does not exist as a valid file extension in the open file dialog
    0-before-opening
  • LZ4 does not have a valid file association in the preferences
    1-before-file-associations
  • Dialog pops up complaining that no editor is associated with the file ending "lz4"
    2-before-opening

After:

  • LZ4 is listed as a valid extension in the open file dialog
    2021-05-20-171807_192x239_scrot
  • LZ4 appears as a valid file extension in the preferences
    3-after-file-associations
  • Opening a LZ4 compressed recording works
    4-after-opened

Progress

  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JMC-7070: Allow JMC to open compressed recordings with the extension .lz4

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jmc pull/250/head:pull/250
$ git checkout pull/250

Update a local copy of the PR:
$ git checkout pull/250
$ git pull https://git.openjdk.java.net/jmc pull/250/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 250

View PR using the GUI difftool:
$ git pr show -t 250

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jmc/pull/250.diff

@bridgekeeper
Copy link

bridgekeeper bot commented May 20, 2021

👋 Welcome back aptmac! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label May 20, 2021
@mlbridge
Copy link

mlbridge bot commented May 20, 2021

Webrevs

@egahlin
Copy link
Member

egahlin commented May 21, 2021

Many years ago I looked into using multiple file associations for JMC, but it had to work (without conflicts) when using JMC as an Eclipse plug-in as well. I don't think ".lz4" is used in Eclipse + common plug-ins, but it is an aspect to consider.

@thegreystone
Copy link
Member

I think we should run with it for now. AFAIK no standard plug-in included in the Eclipse IDE distributions will use the lz4 extension. Also, it's becoming less common to run the Eclipse plug-in version of JMC.

@openjdk
Copy link

openjdk bot commented May 21, 2021

@aptmac This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

7070: Allow JMC to open compressed recordings with the extension .lz4

Reviewed-by: hirt

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label May 21, 2021
@aptmac
Copy link
Member Author

aptmac commented May 21, 2021

Many years ago I looked into using multiple file associations for JMC, but it had to work (without conflicts) when using JMC as an Eclipse plug-in as well. I don't think ".lz4" is used in Eclipse + common plug-ins, but it is an aspect to consider.

Thanks for the heads up. I tried using the JMC run configuration for eclipse and plugins, and it seemed to still work okay.

Eclipse:
eclipse-jmc-lz4

@aptmac
Copy link
Member Author

aptmac commented May 21, 2021

/integrate

@openjdk openjdk bot closed this May 21, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels May 21, 2021
@openjdk
Copy link

openjdk bot commented May 21, 2021

@aptmac Pushed as commit 713f93e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@aptmac aptmac deleted the 7070 branch June 25, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants