Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8223334

Additional cleanup in jpackage tool

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P3
    • internal
    • internal
    • tools

    Description

      jpackage.main.Main:

       - Main.run returns -1, which then is used as exit status, -1 is not the usual exit status for a C/Unix main.

      65, the run() method is usually not static and should be re-named to avoid confusion

      92: Implies something should be logged on a failure.

      65: Run (Pw, Pw, args...) doesn't use try ... finally to ensure log is flushed.

      65: 'throws Exception' implies it can throw an exception but is ambiguous as to whether
      it returns an error number or throws on what kinds of errors?

      91: Ambiguous as to whether processArguments() throws or return false!

      CommandLine:
      59: Would be more flexible and powerful to use List<String> consistently...

      67: use arg.startsWith() for cleaner code.

      102: Seems wasteful to parse all the arguments twice.

      jdk.jpackage.internal classes:

      AbstractAppImageBuilder:
       57: The constructor does not need to throw IOException

      60: are .diz files common enough to preemptively exclude (w/o documentation)

      89: Can the mix of old File API and new Path/Files APIs be avoided?

      Adding javadoc to the abstract builder methods will help future maintainers.

      203: is a bit more generous than most CLASSPATH parsing and might lead to non-obvious bugs.
         For example, a path component with a space in it!

      229: There is no mention of debugging support in JEP 343.
        Where is this functionality defined or is it to be identified as undocumented internal implementation

      StandardBundlerParams:
      661-662: escaped is never set to true, so escaping is not supported?

      IOUtils:
       - 262: why the mix of ProcessBuilder and Runtime.exec - stick to ProcessBuilder

      Attachments

        Issue Links

          Activity

            People

              herrick Andy Herrick (Inactive)
              kcr Kevin Rushforth
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: