Based on the inputs so far, I've updated the PR to include the provided
feedback. Since the PR code review hadn't yet started, I decided to do a
force push to the PR so that we can start it afresh.
Command option:
In this current discussion, we seem to have an agreement for using -C
and --dir as the short and long options for this feature. The
implementation in this PR now uses these options. There was also a
suggestion to additionally allow --directory as an option too. I haven't
added that yet, since I wasn't sure if we really want that. It was
suggested that if we do use --directory, we should "hide" --dir. If we
do need the --directory option, I can add that in, in subsequent updates
to this PR.
Directories creation:
There was an agreement in our discussion that if the destination
directory hierarchy isn't present, then we should create that whole
hierarchy and extract the jar into it. The implementation in this PR
matches this decision.
Verbose logging:
During the discussion, there was a question whether this feature should
introduce any new verbose logs during extraction. IMO, it's a good idea
to log the target directory to which the jar is being extracted. So, in
the implementation, I have introduced a new (resource bundle backed)
verbose log message which prints the absolute path to which the jar will
be extracted. Do note that this verbose log message will be printed even
if you don't explicitly specify any target directory. i.e. even when the
default current directory is used, this verbose log message will be
printed if the "-v" option is used.
Repeatability of the newly introduce options:
Unlike in the other main operations of the jar command, the -C option
that we use during the extract main operation, IMO, shouldn't be allowed
to be used more than once. More specifically the destination directory
to which the jar needs to be extracted must only be specified once,
irrespective of whether it's through the use of -C or --dir. The code in
the PR, explicitly throws an error when such repeatition is encountered.
An alternate approach would have been to allow the -C and/or --dir
option to be repeated, but use the last specified value of these
options. However, I decided not to pursue that approach, to keep it
simple as well as to avoid any confusion on the command usage.
Overwriting of contents in existing target directory:
No specific change has been done when it comes to dealing with the
extraction logic itself. Specifically, when the explicitly specified or
the default current directory already has directories/files that belong
to the jar being extracted, those files/dirs will continue to be
overwritten.
Compatibility mode:
The code in this PR also supports the compatibility mode for this
option. Specifically, a command like:
jar -xvf somejar.jar -C /tmp/foo/
will work fine and the jar will be extracted into /tmp/foo directory.
Message/error internationalization:
I have only updated the jar.properties for the English version of the
new output and error messages. I don't know what the process is for
adding this to other languages, if at all that needs to be done in this PR.
jar --help output:
Currently the jar --help output only talks about creation and updation
of the jar. There's no mention of using the tool for extracting the jar
content:
"jar creates an archive for classes and resources, and can manipulate or
restore individual classes or resources from an archive."
It does mention "manipulate" but doesn't specifically say extraction.
The examples in the help command output don't have any examples for
extraction. Should we add an example for extracting the jar file, in
this help output?
Testing:
A new jtreg test has been introduced which tests various aspects of this
feature. It runs most of those tests against both absolute and relative
paths.
A couple of tests in the new introduced test case, check for the
output/error messages. The jar tool uses resource bundles to print out
these messages. I need input on whether I should enforce a specific
locale to run these tests so that I can compare the error/output
messages for expected strings? See testExtractFailWithMultipleDir() or
testHelpOutput() for what I mean.
Man page:
This one I need input on. I have tried to see how these man pages are
generated and from what I can understand it looks like these man pages
are autogenerated during the build process using pandoc. Is that right?
The hints that I see in the Docs.gmk seems to suggest that there are
some markdown source files from which these man pages get generated.
However, I can't seem to locate any such markdown files for this or
other tools, from which the man pages get generated. Any help on how I
should go about editing/updating the man page for the jar tool?
Example usage:
Here are some example usages:
jar -x -f somejar.jar -C /tmp/foo/bar/
This command extracts the somejar.jar file to the /tmp/foo/bar/
directory, creating it if necessary.
jar -x -f somejar.jar --dir /tmp/foo/bar/
Same as above, except uses the long form --dir option
jar -x -f somejar.jar -C /tmp/foo/bar/ f1.txt d1/f2.txt
Assuming somejar.jar contains "f1.txt" (at root), "d1/f2.txt" and other
files, then the above command extracts only "f1.txt" and "d1/f2.txt"
into the /tmp/foo/bar/ directory.
-Jaikiran
On 14/03/21 6:21 pm, Alan Bateman wrote:
On 12/03/2021 12:18, Lance Andersen wrote:
:
I don’t have a strong preference but lean slightly towards
‘-directory’ as it is more descriptive, similar to the other
GNU-style commands jar currently supports . Tar supports ‘—cd’,
‘—directory’ in addition to ‘-C’ which is why I suggested supporting
both GNU-style long options.
Perhaps jpackage should also support —dir/directory in addition to
‘—dest' if we are looking at consistency between java tools.
I do agree that it would be nice to be consistent across the java
tools for options so if we go the ‘-directory’, we should follow your
suggestion and make it the primary and remove ‘—dir’ from the usage
output.
My comment on consistency was limited to the long option to specify
the directory when extracting, didn't mean to suggest doing anything
with the other tools that specify an output/destination directory. In
any case, I think we have enough to make progress on this issue now.
-Alan
