Derby
  1. Derby
  2. DERBY-826

sysinfo does not report the version of derby.jar if the class does not explictly contain it.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.2.1.6
    • Fix Version/s: 10.2.1.6
    • Component/s: Tools
    • Labels:
      None

      Description

      derby.jar is in the classpath here indirectly because derbynet.jar includes it through a manifest entry.
      java -cp jars/sane/derbynet.jar org.apache.derby.tools.sysinfo

      ------------------ Java Information ------------------
      Java Version: 1.4.2
      Java Vendor: IBM Corporation
      Java home: C:\Program Files\IBM\Java142\jre
      Java classpath: jars/sane/derbynet.jar
      OS name: Windows XP
      OS architecture: x86
      OS version: 5.1
      Java user name: djd
      Java user home: C:\Documents and Settings\Administrator
      Java user dir: c:_work\svn_clean2\trunk
      java.specification.name: Java Platform API Specification
      java.specification.version: 1.4
      --------- Derby Information --------
      JRE - JDBC: J2SE 1.4.2 - JDBC 3.0
      [C:\_work\svn_clean2\trunk\jars\sane\derbynet.jar] 10.2.0.0 alpha - (370500M)
      ------------------------------------------------------
      ----------------- Locale Information -----------------
      Current Locale : [English/United States [en_US]]
      Found support for locale: [de_DE]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [es]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [fr]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [it]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [ja_JP]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [ko_KR]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [pt_BR]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [zh_CN]
      version: 10.2.0.0 alpha - (370500M)
      Found support for locale: [zh_TW]
      version: 10.2.0.0 alpha - (370500M)
      ------------------------------------------------------

      1. derby-826-v4.diff
        12 kB
        Andrew McIntyre
      2. derby-826-v3.diff
        11 kB
        Bryan Pendleton
      3. derby-826.diff
        11 kB
        Andrew McIntyre
      4. derby-826_v2.diff
        11 kB
        Andrew McIntyre

        Issue Links

          Activity

          Hide
          Andrew McIntyre added a comment -

          This patch attempts to locate the info properties files using getResource() first, and then through the classpath. This allows us to find the info properties files when loaded from mechanisms other than through the classpath, e.g. from the extensions directory, java -jar, the Class-Path attribute, etc. Also added javadoc for the methods that I touched.

          Related to DERBY-668, this fixes the regular (non -cp) part of that issue.

          I will commit this early next week if there are no concerns.

          Show
          Andrew McIntyre added a comment - This patch attempts to locate the info properties files using getResource() first, and then through the classpath. This allows us to find the info properties files when loaded from mechanisms other than through the classpath, e.g. from the extensions directory, java -jar, the Class-Path attribute, etc. Also added javadoc for the methods that I touched. Related to DERBY-668 , this fixes the regular (non -cp) part of that issue. I will commit this early next week if there are no concerns.
          Hide
          Andrew McIntyre added a comment -

          Thanks for the helpful review, Bryan.

          1 - Yes, I removed this and an unused import from an experiment I had done previously.

          2 & 3 - A great suggestion on 3. I pulled out the formatting code and put it into a new method, formatURL. This also normalizes the path separators in the jar/directory locations, so the duplicates are properly removed for 2.

          4 - Added additional javadoc comments to mergeZips.

          Let me know if you think this needs any more refinement, otherwise I'd like to commit it soon.

          Show
          Andrew McIntyre added a comment - Thanks for the helpful review, Bryan. 1 - Yes, I removed this and an unused import from an experiment I had done previously. 2 & 3 - A great suggestion on 3. I pulled out the formatting code and put it into a new method, formatURL. This also normalizes the path separators in the jar/directory locations, so the duplicates are properly removed for 2. 4 - Added additional javadoc comments to mergeZips. Let me know if you think this needs any more refinement, otherwise I'd like to commit it soon.
          Hide
          Bryan Pendleton added a comment -

          Hi Andrew. Thank you for considering all my feedback. Your new patch looks very good to me. I have only one final concern. When I was spot-testing your patch, I noticed that, while the mergeZips method appears to strip duplicates out of zip1 entirely, and it strips out of zip2 any entries that also exist in zip1, it does not strip duplicates which exist twice in zip2.

          For example, try running java -cp derby.jar:db2jcc.jar:db2jcc.jar:derbytools.jar sysinfo. I believe you will see info about db2jcc.jar being printed twice.

          I think this can be fixed by changing the zip2 processing loop in mergeZips so that, when it searches to see if the current entry duplicates an existing entry, it searches the entries in "v", rather than searching the entries in zip1.

          I've attached a proposed version 3 of the patch. The only substantive change between this version and your version is as follows:

          < + for (int k = 0; k < zip1.length; k++)

          > + for (int k = 0; k < v.size(); k++)
          265c265,266
          < + if (zip1[k] == null)

          > + ZipInfoProperties z = (ZipInfoProperties)v.get(k);
          > + if (z == null)
          267c268
          < + if (zip2[j].getLocation().equals(zip1[k].getLocation()))

          > + if (zip2[j].getLocation().equals(z.getLocation()))

          Show
          Bryan Pendleton added a comment - Hi Andrew. Thank you for considering all my feedback. Your new patch looks very good to me. I have only one final concern. When I was spot-testing your patch, I noticed that, while the mergeZips method appears to strip duplicates out of zip1 entirely, and it strips out of zip2 any entries that also exist in zip1, it does not strip duplicates which exist twice in zip2. For example, try running java -cp derby.jar:db2jcc.jar:db2jcc.jar:derbytools.jar sysinfo. I believe you will see info about db2jcc.jar being printed twice. I think this can be fixed by changing the zip2 processing loop in mergeZips so that, when it searches to see if the current entry duplicates an existing entry, it searches the entries in "v", rather than searching the entries in zip1. I've attached a proposed version 3 of the patch. The only substantive change between this version and your version is as follows: < + for (int k = 0; k < zip1.length; k++) — > + for (int k = 0; k < v.size(); k++) 265c265,266 < + if (zip1 [k] == null) — > + ZipInfoProperties z = (ZipInfoProperties)v.get(k); > + if (z == null) 267c268 < + if (zip2 [j] .getLocation().equals(zip1 [k] .getLocation())) — > + if (zip2 [j] .getLocation().equals(z.getLocation()))
          Hide
          Andrew McIntyre added a comment -

          Good catch, Bryan!

          I'm attaching derby-826-v4.diff, which includes your changes plus the following:

          • removes the check for z == null in the second loop of mergeZips(). We already checked that elements being added to the vector are not null in the first loop.
          • catches possible NullPointerExceptions if loadZipFromResource() returns a null array. This is unlikely, but possible, if somehow sysinfo is able to run but is isolated from the info properties files so that it cannot load them using getResourceAsStream() or they don't exist. The corresponding diff in getAllInfo() is:

          + // No info properties files found, but here we are in sysinfo.
          + // Avoid an NPE in mergeZips by creating a ZipInfoProperties array
          + // with the location of the sysinfo that is currently executing.
          + if (zips == null)
          +

          { + zips = new ZipInfoProperties[1]; + ZipInfoProperties zip = new ZipInfoProperties(ProductVersionHolder.getProductVersionHolderFromMyEnv(org.apache.derby.tools.sysinfo.TOOLS)); + zip.setLocation(getFileWhichLoadedClass(new Main().getClass())); + zips[0] = zip; + }

          This is looking pretty airtight. Do you see any other potential problems?

          Show
          Andrew McIntyre added a comment - Good catch, Bryan! I'm attaching derby-826-v4.diff, which includes your changes plus the following: removes the check for z == null in the second loop of mergeZips(). We already checked that elements being added to the vector are not null in the first loop. catches possible NullPointerExceptions if loadZipFromResource() returns a null array. This is unlikely, but possible, if somehow sysinfo is able to run but is isolated from the info properties files so that it cannot load them using getResourceAsStream() or they don't exist. The corresponding diff in getAllInfo() is: + // No info properties files found, but here we are in sysinfo. + // Avoid an NPE in mergeZips by creating a ZipInfoProperties array + // with the location of the sysinfo that is currently executing. + if (zips == null) + { + zips = new ZipInfoProperties[1]; + ZipInfoProperties zip = new ZipInfoProperties(ProductVersionHolder.getProductVersionHolderFromMyEnv(org.apache.derby.tools.sysinfo.TOOLS)); + zip.setLocation(getFileWhichLoadedClass(new Main().getClass())); + zips[0] = zip; + } This is looking pretty airtight. Do you see any other potential problems?
          Hide
          Bryan Pendleton added a comment -

          > Do you see any other potential problems?

          Nope, this patch looks great; please check this puppy in!

          Show
          Bryan Pendleton added a comment - > Do you see any other potential problems? Nope, this patch looks great; please check this puppy in!
          Hide
          Andrew McIntyre added a comment -

          Committed derby-826-v.diff with revision 394334.

          Show
          Andrew McIntyre added a comment - Committed derby-826-v.diff with revision 394334.
          Hide
          Daniel John Debrunner added a comment -

          sysinfo now shows derby.jar information even if it's not explicitly in the classpath

          Show
          Daniel John Debrunner added a comment - sysinfo now shows derby.jar information even if it's not explicitly in the classpath

            People

            • Assignee:
              Andrew McIntyre
              Reporter:
              Daniel John Debrunner
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development