On Mar 28, 2021, at 9:24 AM, Jaikiran Pai <jai.forums2...@gmail.com<mailto:jai.forums2...@gmail.com>> wrote:
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. Agree 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/ Given the support of -C/-dir is new and -P is there for backwards compatibility then I see the choices are: * Issue an error when -P and -C/-dir are specified together with -x * Silently ignore the -P option * Ignore -C/-dir and the behavior is if -xfP was specified. I am leaning towards an error given this is a new feature when -P and -C/-dir are specified with -x Best Lance 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 [cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home] Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com<mailto:lance.ander...@oracle.com>
