Lance ran some tests against the proposed patch and that has exposed an
area which we haven't yet taken into account for this feature.
The jar tool has a (hidden) option "-P" which can be used with the "c"
(create), "u" (update) and "x" (extract) main operations. When this -P
option is used, the jar tool will preserve/won't strip leading slash and
".." component from file name. So imagine a jar created with the -P
option as follows:
jar -cfP foo.jar /tmp/blah.txt
This will add /tmp/blah.txt with the the leading / preserved, so the
contents of the jar will be:
jar -tf foo.jar
META-INF/
META-INF/MANIFEST.MF
/tmp/blah.txt
Consider being in /home/me/ directory and running the jar -xfP command
against this jar. When you do that, the /tmp/blah.txt will get extracted
to the /tmp/blah.txt absolute path and the META-INF and the other
entries get extracted inside the /home/me/ directory. This is how the
jar tool currently behaves when the leading slashes (and ..) are
involved with the -P option.
Now coming to this new feature we are talking about, IMO, we cannot
break this existing behaviour. So when the user continues to use:
jar -xfP foo.jar
without any explicit -C or --dir option, then IMO, the extract should
continue to work just like it does now and continue to extract the
/tmp/blah.txt to that absolute location. Now when the user explicitly
specifies the new -C or --dir option with the -P option for extract,
something like:
jar -xfP foo.jar -C /tmp/hello/
I think we should continue to extract the /tmp/blah.txt to that absolute
location instead of making it relative to the /tmp/hello/ directory.
Given that -P is a hidden option, I am not sure if this should be
documented in some manner (other than maybe code comments), but I wanted
to bring this up so that we can come to a decision and have the proposed
implementation work in that manner.
-Jaikiran
On 24/03/21 4:10 pm, Jaikiran Pai wrote:
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
