Sorry again for taking time. Finally, I've spent a reasonable amount of time to deal with the issue. (I've change the subject because "Using system tzdata" keeps people confused that the proposal is to utilize the native Olson binary tzdata on Linux.)

I made the following changes from your proposed changes.

   * The default value of the "sun.zoneinfo.dir" property is "". This
     default value can be overridden at the build time by specifying
     ALT_ZONEINFO_DIR, e.g. "make ALT_ZONEINFO_DIR=/usr/share/javazi".
     This is because Sun's tzupdater tool is confused with the property
     setting if there are valid tzdata files under the directory
     specified by the property. The tool still updates the <JRE>/lib/zi
     area, but the runtime looks up the directory specified by the
     property.
   * The property value is evaluated only once when loading the
     sun.util.calendar.ZoneInfoFile class. This way, we can avoid
     mixing different versions of tzdata files at runtime.
   * "sun.zoneinfo.dir" lookup is under the do-privileged block.

I've tested builds, runtime lookup, and tzupdater. Also I tested the runtime with some broken tzdata files. There's one problem that the lookup code doesn't detect infinite recursions, which problem I need to fix later.

Could you please review the attached diffs? If those changes are OK, I will proceed with some paper work to get interface change (i.e., new property support) approval.

Best regards,
--
Masayoshi Okutsu


On 1/19/2008 5:03 AM, Keith Seitz wrote:
Masayoshi Okutsu wrote:
Therefore, the proposed property needs to be supported for those who want to use another mechanism to update tzdata files.

I don't believe that my proposed change in any way conflicts with the
status quo. If the property is defined, tzdata is read from the value of
the property. If the directory is not valid (or ZoneInfoMapping doesn't
exist) or if the property is otherwise undefined or invalid, it is
simply ignored, and the JDK-supplied tzdata is used instead. [Although I admit, allowing this property to change makes me nervous. I would rather it ALWAYS use the system data or NEVER use it -- rather like how OpenJDK behaves today.]

I need to get approval on the proposed property support because it exposes an interface (even it's internal). To do that, I need to make a decision on the default value.

Default, platform-specific values, no?

I've been having some difficulty to think of default locations for other platforms, especially Windows.

I don't think Microsoft would ever release tzdata for use by Sun's JRE, but I could just be pessimistic. O:-) The only hosts that this really affects are Solaris and Linux.

So I  wondered if it's OK to make the property value undefined by
default. Would that be acceptable for you?

While our (Red Hat) hands are tied, of course, I would not agree with the decision to turn this off by default, effectively ignoring the problem that the patch is attempting to address.

Is there really no solution to this problem?

Keith


diff --git a/make/common/shared/Defs.gmk b/make/common/shared/Defs.gmk
--- a/make/common/shared/Defs.gmk
+++ b/make/common/shared/Defs.gmk
@@ -475,6 +475,13 @@ else
   COPYRIGHT_YEAR = $(shell $(DATE) '+%Y')
 endif
 
+# If ALT_ZONEINFO_DIR is specified, use that directory to look up tzdata files.
+ifdef ALT_ZONEINFO_DIR
+  ZONEINFO_DIR = $(ALT_ZONEINFO_DIR)
+else
+  ZONEINFO_DIR =
+endif
+
 # Absolute path to output directory
 ABS_OUTPUTDIR:=$(call FullPath,$(OUTPUTDIR))
 
diff --git a/make/java/java/Makefile b/make/java/java/Makefile
--- a/make/java/java/Makefile
+++ b/make/java/java/Makefile
@@ -54,6 +54,7 @@ OTHER_CFLAGS += -DJDK_UPDATE_VERSION='"$
 OTHER_CFLAGS += -DJDK_UPDATE_VERSION='"$(JDK_UPDATE_VERSION)"'
 endif
 
+OTHER_CFLAGS += -DZONEINFO_DIR='"$(ZONEINFO_DIR)"'
 
 #
 # Files to compile.
diff --git a/src/share/classes/sun/util/calendar/ZoneInfoFile.java 
b/src/share/classes/sun/util/calendar/ZoneInfoFile.java
--- a/src/share/classes/sun/util/calendar/ZoneInfoFile.java
+++ b/src/share/classes/sun/util/calendar/ZoneInfoFile.java
@@ -372,6 +372,33 @@ public class ZoneInfoFile {
 public class ZoneInfoFile {
 
     /**
+     * The ZoneInfoMappings file name.
+     */
+    public static final String  JAVAZM_FILE_NAME = "ZoneInfoMappings";
+
+    /**
+     * The directory for reading tzdata files. The directory specified
+     * by the sun.zoneinfo.dir property has precedence.
+     */
+    private static final String ziDir;
+    static {
+        String dir = AccessController.doPrivileged(
+                 new 
sun.security.action.GetPropertyAction("sun.zoneinfo.dir"));
+        if (dir != null && dir.length() > 0) {
+            File f = new File(dir, JAVAZM_FILE_NAME);
+            if (!f.exists()) {
+                dir = null;
+            }
+        }
+        if (dir == null) {
+            String homeDir = AccessController.doPrivileged(
+                                   new 
sun.security.action.GetPropertyAction("java.home"));
+            dir = homeDir + File.separator + "lib" + File.separator + "zi";
+        }
+        ziDir = dir;
+    }
+
+    /**
      * The magic number for the ZoneInfo data file format.
      */
     public static final byte[]  JAVAZI_LABEL = {
@@ -420,11 +447,6 @@ public class ZoneInfoFile {
      */
     public static final byte    TAG_GMTOffsetWillChange = 7;
 
-
-    /**
-     * The ZoneInfoMappings file name.
-     */
-    public static final String  JAVAZM_FILE_NAME = "ZoneInfoMappings";
 
     /**
      * The magic number for the ZoneInfoMappings file format.
@@ -1017,17 +1039,13 @@ public class ZoneInfoFile {
      * Reads the specified file under &lt;java.home&gt;/lib/zi into a buffer.
      * @return the buffer, or null if any I/O error occurred.
      */
-    private static byte[] readZoneInfoFile(String fileName) {
+    private static byte[] readZoneInfoFile(final String fileName) {
         byte[] buffer = null;
 
         try {
-            String homeDir = AccessController.doPrivileged(
-                new sun.security.action.GetPropertyAction("java.home"));
-            final String fname = homeDir + File.separator + "lib" + 
File.separator
-                                 + "zi" + File.separator + fileName;
             buffer = (byte[]) AccessController.doPrivileged(new 
PrivilegedExceptionAction() {
                 public Object run() throws IOException {
-                    File file = new File(fname);
+                    File file = new File(ziDir, fileName);
                     if (!file.canRead()) {
                         return null;
                     }
@@ -1038,7 +1056,7 @@ public class ZoneInfoFile {
 
                     if (fis.read(buf) != filesize) {
                         fis.close();
-                        throw new IOException("read error on " + fname);
+                        throw new IOException("read error on " + 
file.getPath());
                     }
                     fis.close();
                     return buf;
diff --git a/src/share/native/java/lang/System.c 
b/src/share/native/java/lang/System.c
--- a/src/share/native/java/lang/System.c
+++ b/src/share/native/java/lang/System.c
@@ -205,6 +205,7 @@ Java_java_lang_System_initProperties(JNI
     PUTPROP_ForPlatformCString(props, "user.home", sprops->user_home);
 
     PUTPROP(props, "user.timezone", sprops->timezone);
+    PUTPROP(props, "sun.zoneinfo.dir", sprops->sun_zoneinfo_dir);
 
     PUTPROP_ForPlatformCString(props, "user.dir", sprops->user_dir);
 
diff --git a/src/share/native/java/lang/java_props.h 
b/src/share/native/java/lang/java_props.h
--- a/src/share/native/java/lang/java_props.h
+++ b/src/share/native/java/lang/java_props.h
@@ -50,6 +50,7 @@ typedef struct {
     char *encoding;
     char *sun_jnu_encoding;
     char *timezone;
+    char *sun_zoneinfo_dir;     /* alternative tzdata directory */
 
     char *printerJob;
     char *graphics_env;
diff --git a/src/solaris/native/java/lang/java_props_md.c 
b/src/solaris/native/java/lang/java_props_md.c
--- a/src/solaris/native/java/lang/java_props_md.c
+++ b/src/solaris/native/java/lang/java_props_md.c
@@ -377,17 +377,18 @@ GetJavaProperties(JNIEnv *env)
         sprops.user_home = pwent ? strdup(pwent->pw_dir) : "?";
     }
 
-    /* User TIMEZONE */
-    {
+    /* time zone related properties */
+    {
+        tzset();        /* for compatibility */
         /*
-         * We defer setting up timezone until it's actually necessary.
-         * Refer to TimeZone.getDefault(). However, the system
-         * property is necessary to be able to be set by the command
-         * line interface -D. Here temporarily set a null string to
-         * timezone.
+         * We defer the "timezone" property setting until it's
+         * actually necessary.  Refer to TimeZone.getDefault().
+         * However, it's necessary that the property can be set using
+         * -D. Here we temporarily set the property to an empty string.
          */
-        tzset();        /* for compatibility */
         sprops.timezone = "";
+       // Directory for reading tzdata files
+        sprops.sun_zoneinfo_dir = ZONEINFO_DIR;
     }
 
     /* Current directory */
diff --git a/src/windows/native/java/lang/java_props_md.c 
b/src/windows/native/java/lang/java_props_md.c
--- a/src/windows/native/java/lang/java_props_md.c
+++ b/src/windows/native/java/lang/java_props_md.c
@@ -878,16 +878,18 @@ GetJavaProperties(JNIEnv* env)
     }
 
     sprops.unicode_encoding = "UnicodeLittle";
-    /* User TIMEZONE */
+
+    /* time zone related properties */
     {
         /*
-         * We defer setting up timezone until it's actually necessary.
-         * Refer to TimeZone.getDefault(). However, the system
-         * property is necessary to be able to be set by the command
-         * line interface -D. Here temporarily set a null string to
-         * timezone.
+         * We defer the "timezone" property setting until it's
+         * actually necessary.  Refer to TimeZone.getDefault().
+         * However, it's necessary that the property can be set using
+         * -D. Here we temporarily set the property to an empty string.
          */
         sprops.timezone = "";
+       // Directory for reading tzdata files
+        sprops.sun_zoneinfo_dir = ZONEINFO_DIR;
     }
 
     /* Current directory */

Reply via email to