Thread: OS timezone files support

This patch brings possibility to switch from default build-in timezone
to another timezone source - typically to OS timezone location. This
enhancement helps to packagers easy solve integration problem with
systems Timezones.

If --with-tzdir=/usr/share/zoneinfo is specified as ./configure
parameter, postgres will use timezone files from /usr/share/zoneinfo
directory and build-in timezone files will not be installed by make
install command.


It was discussed few weeks ago:

http://archives.postgresql.org/pgsql-hackers/2007-03/msg00784.php



    With regards Zdenek


Index: configure.in
===================================================================
RCS file: /projects/cvsroot/pgsql/configure.in,v
retrieving revision 1.503
diff -c -r1.503 configure.in
*** configure.in    21 Mar 2007 14:39:23 -0000    1.503
--- configure.in    22 Mar 2007 16:01:51 -0000
***************
*** 121,126 ****
--- 121,133 ----


  #
+ # Directory where timezone files are stored
+ #
+ PGAC_ARG_REQ(with, tzdir, [  --with-tzdir=DIR        timezone files directory location],
+     [tzdir=$withval])
+ AC_SUBST(tzdir)
+
+ #
  # Add non-standard directories to the include path
  #
  PGAC_ARG_REQ(with, includes, [  --with-includes=DIRS    look for additional header files in DIRS])
Index: src/Makefile.global.in
===================================================================
RCS file: /projects/cvsroot/pgsql/src/Makefile.global.in,v
retrieving revision 1.233
diff -c -r1.233 Makefile.global.in
*** src/Makefile.global.in    9 Feb 2007 15:55:57 -0000    1.233
--- src/Makefile.global.in    22 Mar 2007 16:01:51 -0000
***************
*** 77,82 ****
--- 77,84 ----
  endif
  endif

+ tzdir := @tzdir@
+
  sysconfdir := @sysconfdir@
  ifeq "$(findstring pgsql, $(sysconfdir))" ""
  ifeq "$(findstring postgres, $(sysconfdir))" ""
***************
*** 122,127 ****
--- 124,130 ----

  bindir := $(shell $(PG_CONFIG) --bindir)
  datadir := $(shell $(PG_CONFIG) --sharedir)
+ tzdir := $(shell $(PG_CONFIG) --timezonedir)
  sysconfdir := $(shell $(PG_CONFIG) --sysconfdir)
  libdir := $(shell $(PG_CONFIG) --libdir)
  pkglibdir := $(shell $(PG_CONFIG) --pkglibdir)
Index: src/bin/pg_config/pg_config.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_config/pg_config.c,v
retrieving revision 1.24
diff -c -r1.24 pg_config.c
*** src/bin/pg_config/pg_config.c    7 Feb 2007 00:28:54 -0000    1.24
--- src/bin/pg_config/pg_config.c    22 Mar 2007 16:01:51 -0000
***************
*** 194,199 ****
--- 194,211 ----
  }

  static void
+ show_timezonedir(bool all)
+ {
+     char        path[MAXPGPATH];
+
+     if (all)
+         printf("TIMEZONEDIR = ");
+     get_timezone_path(mypath, path);
+     cleanup_path(path);
+     printf("%s\n", path);
+ }
+
+ static void
  show_sysconfdir(bool all)
  {
      char        path[MAXPGPATH];
***************
*** 377,382 ****
--- 389,395 ----
      {"--localedir", show_localedir},
      {"--mandir", show_mandir},
      {"--sharedir", show_sharedir},
+     {"--timezonedir", show_timezonedir},
      {"--sysconfdir", show_sysconfdir},
      {"--pgxs", show_pgxs},
      {"--configure", show_configure},
Index: src/include/port.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/port.h,v
retrieving revision 1.110
diff -c -r1.110 port.h
*** src/include/port.h    7 Feb 2007 00:28:55 -0000    1.110
--- src/include/port.h    22 Mar 2007 16:01:51 -0000
***************
*** 34,39 ****
--- 34,40 ----
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
  extern const char *get_progname(const char *argv0);
  extern void get_share_path(const char *my_exec_path, char *ret_path);
+ extern void get_timezone_path(const char *my_exec_path, char *ret_path);
  extern void get_etc_path(const char *my_exec_path, char *ret_path);
  extern void get_include_path(const char *my_exec_path, char *ret_path);
  extern void get_pkginclude_path(const char *my_exec_path, char *ret_path);
Index: src/port/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/port/Makefile,v
retrieving revision 1.34
diff -c -r1.34 Makefile
*** src/port/Makefile    9 Feb 2007 15:56:00 -0000    1.34
--- src/port/Makefile    22 Mar 2007 16:01:51 -0000
***************
*** 82,87 ****
--- 82,88 ----
      echo "#define LOCALEDIR \"$(localedir)\"" >>$@
      echo "#define DOCDIR \"$(docdir)\"" >>$@
      echo "#define MANDIR \"$(mandir)\"" >>$@
+     echo "#define TZDIR \"$(tzdir)\"" >>$@

  clean distclean maintainer-clean:
      rm -f libpgport.a libpgport_srv.a $(LIBOBJS) $(LIBOBJS_SRV) pg_config_paths.h
Index: src/port/path.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/port/path.c,v
retrieving revision 1.71
diff -c -r1.71 path.c
*** src/port/path.c    5 Jan 2007 22:20:02 -0000    1.71
--- src/port/path.c    22 Mar 2007 16:01:51 -0000
***************
*** 528,533 ****
--- 528,553 ----
  }

  /*
+  *    get_timezone_path
+  *
+  *  Note: PostgreSQL delivers its own timezone files by default. They are stored
+  *        in {share}/timezone directory. However, in case when packagers and integrators
+  *        prefere usage of OS timezone files, TZDIR is set by ./configure and it
+  *        contains direct path to the system timezone information directory.
+  */
+ void
+ get_timezone_path(const char *my_exec_path, char *ret_path)
+ {
+     if( TZDIR[0] == 0 ) {
+     make_relative_path(ret_path, PGSHAREDIR, PGBINDIR, my_exec_path);
+     strlcpy(ret_path + strlen(ret_path), "/timezone", MAXPGPATH - strlen(ret_path));
+     }
+     else {
+     strlcpy(ret_path, TZDIR, MAXPGPATH);
+     }
+ }
+
+ /*
   *    get_etc_path
   */
  void
Index: src/timezone/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/timezone/Makefile,v
retrieving revision 1.26
diff -c -r1.26 Makefile
*** src/timezone/Makefile    14 Mar 2007 17:38:06 -0000    1.26
--- src/timezone/Makefile    22 Mar 2007 16:01:51 -0000
***************
*** 36,49 ****
--- 36,53 ----
      $(CC) $(CFLAGS) $(ZICOBJS) $(LDFLAGS) $(LIBS) -o $@$(X)

  install: all installdirs
+ ifndef tzdir
      ./zic -d '$(DESTDIR)$(datadir)/timezone' -p '$(POSIXRULES)' $(TZDATAFILES)
+ endif
      $(MAKE) -C tznames $@

  installdirs:
      $(mkinstalldirs) '$(DESTDIR)$(datadir)'

  uninstall:
+ ifndef tzdir
      rm -rf '$(DESTDIR)$(datadir)/timezone'
+ endif
      $(MAKE) -C tznames $@

  clean distclean maintainer-clean:
Index: src/timezone/pgtz.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/timezone/pgtz.c,v
retrieving revision 1.50
diff -c -r1.50 pgtz.c
*** src/timezone/pgtz.c    10 Feb 2007 14:58:55 -0000    1.50
--- src/timezone/pgtz.c    22 Mar 2007 16:01:52 -0000
***************
*** 51,58 ****
      if (done_tzdir)
          return tzdir;

!     get_share_path(my_exec_path, tzdir);
!     strlcpy(tzdir + strlen(tzdir), "/timezone", MAXPGPATH - strlen(tzdir));

      done_tzdir = true;
      return tzdir;
--- 51,57 ----
      if (done_tzdir)
          return tzdir;

!     get_timezone_path(my_exec_path, tzdir);

      done_tzdir = true;
      return tzdir;

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> This patch brings possibility to switch from default build-in timezone
> to another timezone source - typically to OS timezone location.

> It was discussed few weeks ago:
> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00784.php

AFAIR, the conclusion of that discussion was we didn't want it.

I certainly don't see the point of the implementation as you have it
--- it adds a great deal of unnecessary infrastructure compared to just
installing a symlink at share/postgresql/timezone.

            regards, tom lane

Tom Lane wrote:
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>> This patch brings possibility to switch from default build-in timezone
>> to another timezone source - typically to OS timezone location.
>
>> It was discussed few weeks ago:
>> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00784.php
>
> AFAIR, the conclusion of that discussion was we didn't want it.
>
> I certainly don't see the point of the implementation as you have it
> --- it adds a great deal of unnecessary infrastructure compared to just
> installing a symlink at share/postgresql/timezone.

And if you wanted it in the backend instead of a symlink, shouldn't it
at least try to verify that the files in the timezone directory are
compatible before blindly accepting it?

//Magnus

Tom Lane wrote:
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>> This patch brings possibility to switch from default build-in timezone
>> to another timezone source - typically to OS timezone location.
>
>> It was discussed few weeks ago:
>> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00784.php
>
> AFAIR, the conclusion of that discussion was we didn't want it.

I'm sorry, but I don't think that the thread has clear conclusion.

> I certainly don't see the point of the implementation as you have it
> --- it adds a great deal of unnecessary infrastructure compared to just
> installing a symlink at share/postgresql/timezone.

The point of my solution is that all packagers who interested in use
only configure switch instead of playing with link integration. In this
case, Packager also must cleanup build-in timezones after make install.
This is not only about add one line into spec file.

There is also big risk that new version of package which will delivery
timezones can replace system file zone information and it damages a system.

I discussed it with gatekeepers which are responsible for patch
preparation and they afraid about future problems with link solution
(based on experience with another sw integration).


    with regards Zdenek

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> Tom Lane wrote:
>> I certainly don't see the point of the implementation as you have it
>> --- it adds a great deal of unnecessary infrastructure compared to just
>> installing a symlink at share/postgresql/timezone.

> The point of my solution is that all packagers who interested in use
> only configure switch instead of playing with link integration. In this
> case, Packager also must cleanup build-in timezones after make install.
> This is not only about add one line into spec file.

No, it's two lines (rm -rf, ln -s).  It'll take many more lines than
that to do it in the Postgres configure/build system, even using the
simpler symlink approach.  And quite aside from the code addition, what
of documentation?  How much text will it take to make clear what this
switch is good for and when it's safe to use?

I just don't see the value of supporting this option in our
configuration infrastructure.  Anyone who is competent to determine
whether it's a safe thing to use is more than competent to alter the
installation that way for themselves.

            regards, tom lane

Magnus Hagander wrote:
> Tom Lane wrote:
>> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>>> This patch brings possibility to switch from default build-in timezone
>>> to another timezone source - typically to OS timezone location.
>>> It was discussed few weeks ago:
>>> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00784.php
>> AFAIR, the conclusion of that discussion was we didn't want it.
>>
>> I certainly don't see the point of the implementation as you have it
>> --- it adds a great deal of unnecessary infrastructure compared to just
>> installing a symlink at share/postgresql/timezone.
>
> And if you wanted it in the backend instead of a symlink, shouldn't it
> at least try to verify that the files in the timezone directory are
> compatible before blindly accepting it?

I think that packager is responsible for verification his integration.
It is same in both cases (configure or link solution). I'm not sure if
it is necessary make check of timezone validity during build time. Build
environment should be different from target system.

If you talk about checking timezone files on runtime, I hope Postgres
verify timezone files every time.

        Zdenek

Tom Lane wrote:
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>> Tom Lane wrote:
>>> I certainly don't see the point of the implementation as you have it
>>> --- it adds a great deal of unnecessary infrastructure compared to just
>>> installing a symlink at share/postgresql/timezone.
>
>> The point of my solution is that all packagers who interested in use
>> only configure switch instead of playing with link integration. In this
>> case, Packager also must cleanup build-in timezones after make install.
>> This is not only about add one line into spec file.
>
> No, it's two lines (rm -rf, ln -s).  It'll take many more lines than
> that to do it in the Postgres configure/build system, even using the
> simpler symlink approach.  And quite aside from the code addition, what
> of documentation?  How much text will it take to make clear what this
> switch is good for and when it's safe to use?
>
> I just don't see the value of supporting this option in our
> configuration infrastructure.  Anyone who is competent to determine
> whether it's a safe thing to use is more than competent to alter the
> installation that way for themselves.


I thought about it and You are right. It is really better to keep
solution on packagers, than extend infrastructure and give "machine gun"
to everybody :-).

    Thanks for your time

        Zdenek