Can I please get a review for this patch which proposes to implement the enhancement request noted in https://bugs.openjdk.java.net/browse/JDK-8173970?
The commit in this PR introduces the `-o` and `--output-dir` option to the `jar` command. The option takes a path to a destination directory as a value and extracts the contents of the jar into that directory. This is an optional option and the changes in the commit continue to maintain backward compatibility where the jar is extracted into current directory, if no `-o` or `--output-dir` option has been specified. As far as I know, there hasn't been any discussion on what the name of this new option should be. I was initially thinking of using `-d` but that is currently used by the `jar` command for a different purpose. So I decided to use `-o` and `--output-dir`. This is of course open for change depending on any suggestions in this PR. The commit in this PR also updates the `jar.properties` file which contains the English version of the jar command's `--help` output. However, no changes have been done to the internationalization files corresponding to this one (for example: `jar_de.properties`), because I don't know what process needs to be followed to have those files updated (if at all they need to be updated). The commit also includes a jtreg testcase which verifies the usage of this new option. ------------- Commit messages: - 8173970: jar tool should have a way to extract to a directory Changes: https://git.openjdk.java.net/jdk/pull/2752/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2752&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8173970 Stats: 268 lines in 4 files changed: 262 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2752.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2752/head:pull/2752 PR: https://git.openjdk.java.net/jdk/pull/2752 |
Hi Jaikiran,
Thank you for the proposed patch.
Assuming there is consensus to add support for this enhancement, I think we need to discuss what is the correct option.
The jar tool borrows -C from tar for creating/updating a jar and the -C option is also a valid option when extracting files from a tar file.
Perhaps keeping symmetry with tar and extend support for -C when extracting a jar file would be a better way forward. Let’s give time for additional input.
I believe this would also warrant a CSR to be created as well as updates to the jar man page.
Best
Lance
p.s. I think it would be useful in the future to start the discussion on core-libs-dev prior to creating a PR (or leave it as a draft PR) for a feature request.
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
Hello Lance,
On 27/02/21 1:17 am, Lance Andersen wrote: > > > p.s. I think it would be useful in the future to start the discussion > on core-libs-dev prior to creating a PR (or leave it as a draft PR) > for a feature request. Thank you for that input, I'll keep that in mind for any similar work in future. -Jaikiran |
In reply to this post by Lance Andersen-3
On 27/02/21 1:17 am, Lance Andersen wrote: > > I believe this would also warrant a CSR to be created as well as > updates to the jar man page. I haven't created a CSR before, so I will need some guidance on that part. Is it usually created after all the implementation details have been decided upon or should it be created now? -Jaikiran |
In reply to this post by Lance Andersen-3
On 26/02/2021 19:47, Lance Andersen wrote:
> Hi Jaikiran, > > Thank you for the proposed patch. > > Assuming there is consensus to add support for this enhancement, I think we need to discuss what is the correct option. > > The jar tool borrows -C from tar for creating/updating a jar and the -C option is also a valid option when extracting files from a tar file. > > Perhaps keeping symmetry with tar and extend support for -C when extracting a jar file would be a better way forward. Let’s give time for additional input. > > I believe this would also warrant a CSR to be created as well as updates to the jar man page. > > Best > Lance > > p.s. I think it would be useful in the future to start the discussion on core-libs-dev prior to creating a PR (or leave it as a draft PR) for a feature request. Yes, the option name will need to be agreed. It would be useful to enumerate the options that the other tools are using to specify the location where to extract. If you see JBS issues mentioning tar -C not supporting chdir when extracting then it might be Solaris tar, which isn't the same as GNU tar which has different options. It might be better to look at more main stream tools, like unzip although jar -d is already taken. It would be nice if there were some consistency with other tools in the JDK that doing extracting (The jmod and jimage extract commands use use --dir for example). There are other discussion points around the behavior when the target directory exists or does not exist, to ensure there is some consistency with main stream tools. Yes, a CSR will be needed. -Alan |
In reply to this post by Jaikiran Pai
HI Jaikiran,
I am more than happy to work with you through the CSR process
Lets get the discussion going regarding possible option names on core-libs-dev and then we can move forward :-)
Have a great weekend
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
In reply to this post by Alan Bateman
Hello Alan,
On 27/02/21 2:23 pm, Alan Bateman wrote: > > Yes, the option name will need to be agreed. It would be useful to > enumerate the options that the other tools are using to specify the > location where to extract. If you see JBS issues mentioning tar -C not > supporting chdir when extracting then it might be Solaris tar, which > isn't the same as GNU tar which has different options. It might be > better to look at more main stream tools, like unzip although jar -d > is already taken. It would be nice if there were some consistency with > other tools in the JDK that doing extracting (The jmod and jimage > extract commands use use --dir for example). I had a look at both tar and unzip commands on MacOS and Linux (CentOS) setup that I had access to. -------------- tar on MacOS: -------------- tar --version bsdtar 3.3.2 - libarchive 3.3.2 zlib/1.2.11 liblzma/5.0.5 bz2lib/1.0.6 The version of this tool has: -C directory, --cd directory, --directory directory In c and r mode, this changes the directory before adding the following files. In x mode, change directories after opening the archive but before extracting entries from the archive. A command like "tar -xzf foo.tar.gz -C /tmp/bar/" works fine and extracts the foo.tar.gz from current directory to a target directory /tmp/bar/ -------------- tar on CentOS: -------------- tar --version tar (GNU tar) 1.26 This version of the tool has: Common options: -C, --directory=DIR change to directory DIR Although the wording isn't clear that, when used with -x, it extracts to the directory specified in -C, it does indeed behave that way. Specifically, a command like "tar -xzf foo.tar.gz -C /tmp/bar/" works fine and extracts the foo.tar.gz from current directory to a target directory /tmp/bar/ ------------------------------- unzip on both MacOS and CentOS: ------------------------------- unzip -v UnZip 6.00 of 20 April 2009, by Info-ZIP. Maintained by C. Spieler. This version of the tool has: [-d exdir] An optional directory to which to extract files. By default, all files and sub- directories are recreated in the current directory; the -d option allows extrac- tion in an arbitrary directory (always assuming one has permission to write to the directory). This option need not appear at the end of the command line; it is also accepted before the zipfile specification (with the normal options), immediately after the zipfile specification, or between the file(s) and the -x option. The option and directory may be concatenated without any white space between them, but note that this may cause normal shell behavior to be sup- pressed. In particular, ``-d ~'' (tilde) is expanded by Unix C shells into the name of the user's home directory, but ``-d~'' is treated as a literal subdirec- tory ``~'' of the current directory. unzip foo.zip -d /tmp/bar/ works fine and extracts the foo.zip from current directory to /tmp/bar/ --------------- jimage and jmod --------------- The jimage and jmod as you note use the --dir option for extracting to that specified directory. Those were the tools I looked at. I think using the -d option with -x for the jar command is ruled out since it already is used for a different purpose, although for a different "main" operation of the jar command. As for using --dir for this new feature, I don't think it alone will be enough. Specifically, I couldn't find a "short form" option for the --dir option in the jimage or jmod commands. For the jar extract feature that we are discussing here, I think having a short form option (in addition to the longer form) is necessary to have it match the usage expectations of similar other options that the jar command exposes. So even if we do choose --dir as the long form option, we would still need a short form for it and since -d is already taken for something else, we would still need to come up with a different one. The short form of this option could be -C (see below). I think reusing the -C option, for this new feature, perhaps is a good thing. The -C is currently used by the update and create "main" operation of the jar command and the man page for this option states: -C dir When creating (c) or updating (u) a JAR file, this option temporarily changes the directory while processing files specified by the file operands. Its operation is intended to be similar to the -C option of the UNIX tar utility.For example, the following command changes to the classes directory and adds the Bar.class file from that directory to my.jar: jar uf my.jar -C classes Bar.class .... Using the -C option would indeed align it with the tar command. For the "long form" of this option, the tar command (both on MacOS and CentOS) uses --directory. For this jar extract feature though, we could perhaps just use --dir to have it align with the jimage and the jmod tools. So I think the combination of -C (short form) and --dir (long form) would perhaps be suitable for this feature. > > There are other discussion points around the behavior when the target > directory exists or does not exist, to ensure there is some > consistency with main stream tools. I'm guessing you mean the behaviour of creating a directory (or a hierarchy of directories) if the target directory is not present? My testing with the tar tool (both on MacOS and CentOS) shows that if the specified target directory doesn't exist, then the extract fails. The tar extract command doesn't create the target directory during extract. On the other hand, the unzip tool, does create the directory if it doesn't exist. However, interestingly, the unzip tool creates only one level of that directory if it doesn't exist. Specifically, if you specify: unzip foo.zip -d /tmp/blah/ and if "blah/" isn't a directory inside /tmp/ directory, then it creates the "blah/" directory inside /tmp/ and then extracts the contents of the zip into it. However, unzip foo.zip -d /tmp/blah/hello/ and if "blah/" isn't a directory inside /tmp/ directory, then this command fails with an error and it doesn't create the hierarchy of the target directories. Coming to the jimage and the jmod commands, both these commands create the entire directory hierarchy if the target directory specified during extract, using --dir, doesn't exist. So a command like: jimage extract --dir /tmp/blah/foo/bar/ jdkmodules will create the blah/foo/bar/ directory hierarchy if blah doesn't exist in /tmp/, while extracting the "jdkmodules" image. From the user point of view, I think this behaviour of creating the directories if the target directory doesn't exist, is probably the most intuitive and useful and if we did decide to use this approach for this new option for jar extract command, then it would align with what we already do in jimage and jmod commands. One another minor detail, while we are at this, is that, IMO we should let the jar extract command to continue to behave the way it currently does when it comes to overwriting existing files. If the jar being extracted contains a file by the same name, in the target directory (hierarchy) then it should continue to overwrite that file. In other words, I don't think we should change the way the jar extract command currently behaves where it overwrites existing files when extracting. -Jaikiran |
Hi Jaikiran,
Thank you for this. I went through this myself yesterday in addition to what you have below here are a few more:
7zip: -o
Info-zip: -d (MacOS version of zip)
Bandzip: -o:{dir}
jpackage: -d —dest
jlink —output
Thinking about this some more, I might suggest supporting
-C
—dir
—directory
Best
Lance
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
Thank you Lance. So I think there's some
level of agreement on using -C and --dir (or --directory) for
the option names. Anymore opinion on the directory creation semantics and any other aspects to consider? -Jaikiran On 28/02/21 5:38 pm, Lance Andersen
wrote:
Hi Jaikiran, |
Hi Jaikiran,
It should not be too difficult to support the options listed below via GNUStyleOptions.
Some other things needed to be defined and agreed upon in order to move forward
Once you have a chance to consider the above, please send your proposal back to the alias to continue the discussion
Best
Lance
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
Hello Lance, On 03/03/21 9:14 pm, Lance Andersen
wrote:
One of my previous reply included the details of how I think it
should behave for 2 of the above cases. I'll paste that here again
for easier visibility. As for how it should behave if the option
is specified more than once, I'll spend some time today to see how
the jar tool currently behaves for some of the other options in
this aspect and send back my response. Thank you for your help so
far. Pasting below my proposal from a previous reply for the other
2 cases:
I'm guessing you mean the behaviour of creating a directory (or a hierarchy of directories) if the target directory is not present? My testing with the tar tool (both on MacOS and CentOS) shows that if the specified target directory doesn't exist, then the extract fails. The tar extract command doesn't create the target directory during extract. On the other hand, the unzip tool, does create the directory if it doesn't exist. However, interestingly, the unzip tool creates only one level of that directory if it doesn't exist. Specifically, if you specify: unzip foo.zip -d /tmp/blah/ and if "blah/" isn't a directory inside /tmp/ directory, then it creates the "blah/" directory inside /tmp/ and then extracts the contents of the zip into it. However, unzip foo.zip -d /tmp/blah/hello/ and if "blah/" isn't a directory inside /tmp/ directory, then this command fails with an error and it doesn't create the hierarchy of the target directories. Coming to the jimage and the jmod commands, both these commands create the entire directory hierarchy if the target directory specified during extract, using --dir, doesn't exist. So a command like: jimage extract --dir /tmp/blah/foo/bar/ jdkmodules will create the blah/foo/bar/ directory hierarchy if blah doesn't exist in /tmp/, while extracting the "jdkmodules" image. From the user point of view, I think this behaviour of creating the directories if the target directory doesn't exist, is probably the most intuitive and useful and if we did decide to use this approach for this new option for jar extract command, then it would align with what we already do in jimage and jmod commands. One another minor detail, while we are at this, is that, IMO we should let the jar extract command to continue to behave the way it currently does when it comes to overwriting existing files. If the jar being extracted contains a file by the same name, in the target directory (hierarchy) then it should continue to overwrite that file. In other words, I don't think we should change the way the jar extract command currently behaves where it overwrites existing files when extracting. -Jaikiran |
Hi Jaikiran
The jar man page includes the following :
The syntax for the
jar command resembles the syntax for the tar command.Because of the above, I feel that we should support:
———
-C
—dir —directory ————
The addition of ‘-dir’ adds support for an option used by some of the other java command line tools.
I would suggest for the next step is flush/write out your proposed syntax as it would be included in an updated version of the jar man page.
Given the other java command line tools create the directory if it does not exist, I think that is reasonable to propose that.
Your change should not modify the longstanding overwrite behavior of jar.
Another thing to think about is whether there should be any additional output when the -v option is specified
In summary, I think you are moving in the right direction. The next step is to make a pass at creating the man page updates so that we can reach consensus on the syntax and behavior.
Thank you again for your efforts on this
Best
Lance
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
In reply to this post by Jaikiran Pai-3
> Can I please get a review for this patch which proposes to implement the enhancement request noted in https://bugs.openjdk.java.net/browse/JDK-8173970?
> > The commit in this PR introduces the `-o` and `--output-dir` option to the `jar` command. The option takes a path to a destination directory as a value and extracts the contents of the jar into that directory. This is an optional option and the changes in the commit continue to maintain backward compatibility where the jar is extracted into current directory, if no `-o` or `--output-dir` option has been specified. > > As far as I know, there hasn't been any discussion on what the name of this new option should be. I was initially thinking of using `-d` but that is currently used by the `jar` command for a different purpose. So I decided to use `-o` and `--output-dir`. This is of course open for change depending on any suggestions in this PR. > > The commit in this PR also updates the `jar.properties` file which contains the English version of the jar command's `--help` output. However, no changes have been done to the internationalization files corresponding to this one (for example: `jar_de.properties`), because I don't know what process needs to be followed to have those files updated (if at all they need to be updated). > > The commit also includes a jtreg testcase which verifies the usage of this new option. Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains one additional commit since the last revision: 8173970: jar tool should have a way to extract to a directory ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2752/files - new: https://git.openjdk.java.net/jdk/pull/2752/files/3a8e329d..a9954240 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2752&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2752&range=00-01 Stats: 62407 lines in 2851 files changed: 39040 ins; 14156 del; 9211 mod Patch: https://git.openjdk.java.net/jdk/pull/2752.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2752/head:pull/2752 PR: https://git.openjdk.java.net/jdk/pull/2752 |
In reply to this post by Jaikiran Pai-3
On Fri, 26 Feb 2021 17:03:11 GMT, Jaikiran Pai <[hidden email]> wrote:
> Can I please get a review for this patch which proposes to implement the enhancement request noted in https://bugs.openjdk.java.net/browse/JDK-8173970? > > The commit in this PR introduces the `-o` and `--output-dir` option to the `jar` command. The option takes a path to a destination directory as a value and extracts the contents of the jar into that directory. This is an optional option and the changes in the commit continue to maintain backward compatibility where the jar is extracted into current directory, if no `-o` or `--output-dir` option has been specified. > > As far as I know, there hasn't been any discussion on what the name of this new option should be. I was initially thinking of using `-d` but that is currently used by the `jar` command for a different purpose. So I decided to use `-o` and `--output-dir`. This is of course open for change depending on any suggestions in this PR. > > The commit in this PR also updates the `jar.properties` file which contains the English version of the jar command's `--help` output. However, no changes have been done to the internationalization files corresponding to this one (for example: `jar_de.properties`), because I don't know what process needs to be followed to have those files updated (if at all they need to be updated). > > The commit also includes a jtreg testcase which verifies the usage of this new option. I think the summary is that we've converged on -C/--dir. We might have to tweak the usage message for -C so that it starts with the existing "Change to the specified directory ..." rather than changing it to start with the extract case. Are you, or Lance, going to create the CSR for this? ------------- PR: https://git.openjdk.java.net/jdk/pull/2752 |
Yes, once we flush everything out, I will work with Jaikiran on the CSR and determine which of us will create the CSR. On the todo list for next week.
Best
Lance
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
In reply to this post by Lance Andersen-3
I am leaning towards an error given this is a new feature when -P and -C/-dir are specified with -x
Best
Lance
![]() Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [hidden email] |
On 28/03/21 9:52 pm, Lance Andersen
wrote:
That sounds reasonable to me. I'll update the PR accordingly to take this into account. -Jaikiran
|
In reply to this post by Jaikiran Pai-3
> Can I please get a review for this patch which proposes to implement the enhancement request noted in https://bugs.openjdk.java.net/browse/JDK-8173970?
> > The commit in this PR introduces the `-o` and `--output-dir` option to the `jar` command. The option takes a path to a destination directory as a value and extracts the contents of the jar into that directory. This is an optional option and the changes in the commit continue to maintain backward compatibility where the jar is extracted into current directory, if no `-o` or `--output-dir` option has been specified. > > As far as I know, there hasn't been any discussion on what the name of this new option should be. I was initially thinking of using `-d` but that is currently used by the `jar` command for a different purpose. So I decided to use `-o` and `--output-dir`. This is of course open for change depending on any suggestions in this PR. > > The commit in this PR also updates the `jar.properties` file which contains the English version of the jar command's `--help` output. However, no changes have been done to the internationalization files corresponding to this one (for example: `jar_de.properties`), because I don't know what process needs to be followed to have those files updated (if at all they need to be updated). > > The commit also includes a jtreg testcase which verifies the usage of this new option. Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision: - Alan's review feedback for -C help text - Keep -xfP backward compatible but don't allow -C/--dir with -xfP ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2752/files - new: https://git.openjdk.java.net/jdk/pull/2752/files/a9954240..3df602d2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2752&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2752&range=01-02 Stats: 88 lines in 3 files changed: 78 ins; 3 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/2752.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2752/head:pull/2752 PR: https://git.openjdk.java.net/jdk/pull/2752 |
Free forum by Nabble | Edit this page |