diff mbox series

[OpenWrt-Devel,packages] php7: add package dependency on zoneinfo-core

Message ID 20180624081938.22602-1-zajec5@gmail.com
State Accepted
Delegated to: John Crispin
Headers show
Series [OpenWrt-Devel,packages] php7: add package dependency on zoneinfo-core | expand

Commit Message

Rafał Miłecki June 24, 2018, 8:19 a.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

Having "select PACKAGE_zoneinfo-core" wasn't enough for builds without
php7-cli=y or php7-cgi=y. It didn't result in installing zoneinfo-core
when using "opkg install" (during runtime or when building images with
CONFIG_TARGET_PER_DEVICE_ROOTFS).

Missing zoneinfo results in PHP fatal errors, e.g.:
Fatal error: DateTime::createFromFormat(): Timezone database is corrupt - this should *never* happen!

For years users were told to manually install zoneinfo-core package.
This problem was hidden for some time (including 17.01 release) due to
disabled support for CONFIG_PHP7_SYSTEMTZDATA. It's now back as support
for --with-system-tzdata was enabled again.

The proper solution is to simply make php7 package depend on
zoneinfo-core when PHP7_SYSTEMTZDATA is used.

Fixes: 84e5012e8853 ("php7: re-enable system timezone data usage")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 lang/php7/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Philip Prindeville June 24, 2018, 4:55 p.m. UTC | #1 
LGTM

Sent from my iPhone

> On Jun 24, 2018, at 2:19 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Having "select PACKAGE_zoneinfo-core" wasn't enough for builds without
> php7-cli=y or php7-cgi=y. It didn't result in installing zoneinfo-core
> when using "opkg install" (during runtime or when building images with
> CONFIG_TARGET_PER_DEVICE_ROOTFS).
> 
> Missing zoneinfo results in PHP fatal errors, e.g.:
> Fatal error: DateTime::createFromFormat(): Timezone database is corrupt - this should *never* happen!
> 
> For years users were told to manually install zoneinfo-core package.
> This problem was hidden for some time (including 17.01 release) due to
> disabled support for CONFIG_PHP7_SYSTEMTZDATA. It's now back as support
> for --with-system-tzdata was enabled again.
> 
> The proper solution is to simply make php7 package depend on
> zoneinfo-core when PHP7_SYSTEMTZDATA is used.
> 
> Fixes: 84e5012e8853 ("php7: re-enable system timezone data usage")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> lang/php7/Makefile | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lang/php7/Makefile b/lang/php7/Makefile
> index aa860a95..93348543 100644
> --- a/lang/php7/Makefile
> +++ b/lang/php7/Makefile
> @@ -7,7 +7,7 @@ include $(TOPDIR)/rules.mk
> 
> PKG_NAME:=php
> PKG_VERSION:=7.2.6
> -PKG_RELEASE:=1
> +PKG_RELEASE:=2
> 
> PKG_MAINTAINER:=Michael Heimpold <mhei@heimpold.de>
> 
> @@ -74,7 +74,6 @@ define Package/php7/config
>    config PHP7_SYSTEMTZDATA
>        bool "Use system timezone data instead of php's built-in database"
>        depends on PACKAGE_php7-cli || PACKAGE_php7-cgi
> -        select PACKAGE_zoneinfo-core
>        default y
>        help
>            Enabling this feature automatically selects the zoneinfo-core package
> @@ -86,7 +85,8 @@ define Package/php7
>   $(call Package/php7/Default)
> 
>   DEPENDS:=+libpcre +zlib \
> -           +PHP7_LIBXML:libxml2
> +           +PHP7_LIBXML:libxml2 \
> +           +PHP7_SYSTEMTZDATA:zoneinfo-core
> endef
> 
> define Package/php7/description
> -- 
> 2.13.7
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/listinfo/openwrt-devel
John Crispin June 24, 2018, 5:40 p.m. UTC | #2 
On 24/06/18 10:19, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Having "select PACKAGE_zoneinfo-core" wasn't enough for builds without
> php7-cli=y or php7-cgi=y. It didn't result in installing zoneinfo-core
> when using "opkg install" (during runtime or when building images with
> CONFIG_TARGET_PER_DEVICE_ROOTFS).
>
> Missing zoneinfo results in PHP fatal errors, e.g.:
> Fatal error: DateTime::createFromFormat(): Timezone database is corrupt - this should *never* happen!
>
> For years users were told to manually install zoneinfo-core package.
> This problem was hidden for some time (including 17.01 release) due to
> disabled support for CONFIG_PHP7_SYSTEMTZDATA. It's now back as support
> for --with-system-tzdata was enabled again.
>
> The proper solution is to simply make php7 package depend on
> zoneinfo-core when PHP7_SYSTEMTZDATA is used.
>
> Fixes: 84e5012e8853 ("php7: re-enable system timezone data usage")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

php7 is part of the feed, please post a PR on github. the ML is 
unfortunately only for trunk
     John

> ---
>   lang/php7/Makefile | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lang/php7/Makefile b/lang/php7/Makefile
> index aa860a95..93348543 100644
> --- a/lang/php7/Makefile
> +++ b/lang/php7/Makefile
> @@ -7,7 +7,7 @@ include $(TOPDIR)/rules.mk
>   
>   PKG_NAME:=php
>   PKG_VERSION:=7.2.6
> -PKG_RELEASE:=1
> +PKG_RELEASE:=2
>   
>   PKG_MAINTAINER:=Michael Heimpold <mhei@heimpold.de>
>   
> @@ -74,7 +74,6 @@ define Package/php7/config
>   	config PHP7_SYSTEMTZDATA
>   		bool "Use system timezone data instead of php's built-in database"
>   		depends on PACKAGE_php7-cli || PACKAGE_php7-cgi
> -		select PACKAGE_zoneinfo-core
>   		default y
>   		help
>   			Enabling this feature automatically selects the zoneinfo-core package
> @@ -86,7 +85,8 @@ define Package/php7
>     $(call Package/php7/Default)
>   
>     DEPENDS:=+libpcre +zlib \
> -           +PHP7_LIBXML:libxml2
> +           +PHP7_LIBXML:libxml2 \
> +           +PHP7_SYSTEMTZDATA:zoneinfo-core
>   endef
>   
>   define Package/php7/description
Michael Heimpold June 24, 2018, 7:39 p.m. UTC | #3 
Hi,

Am Sonntag, 24. Juni 2018, 19:40:53 CEST schrieb John Crispin:
> On 24/06/18 10:19, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > Having "select PACKAGE_zoneinfo-core" wasn't enough for builds without
> > php7-cli=y or php7-cgi=y. It didn't result in installing zoneinfo-core
> > when using "opkg install" (during runtime or when building images with
> > CONFIG_TARGET_PER_DEVICE_ROOTFS).
> > 
> > Missing zoneinfo results in PHP fatal errors, e.g.:
> > Fatal error: DateTime::createFromFormat(): Timezone database is corrupt -
> > this should *never* happen!
> > 
> > For years users were told to manually install zoneinfo-core package.
> > This problem was hidden for some time (including 17.01 release) due to
> > disabled support for CONFIG_PHP7_SYSTEMTZDATA. It's now back as support
> > for --with-system-tzdata was enabled again.
> > 
> > The proper solution is to simply make php7 package depend on
> > zoneinfo-core when PHP7_SYSTEMTZDATA is used.
> > 
> > Fixes: 84e5012e8853 ("php7: re-enable system timezone data usage")
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> 
> php7 is part of the feed, please post a PR on github. the ML is
> unfortunately only for trunk

this is true, but since I'm reading this list too and the patch is correct
I picked it up - no need to open a PR - hope this is ok for all :-)

Thanks,
Michael

>      John
> 
> > ---
> > 
> >   lang/php7/Makefile | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lang/php7/Makefile b/lang/php7/Makefile
> > index aa860a95..93348543 100644
> > --- a/lang/php7/Makefile
> > +++ b/lang/php7/Makefile
> > @@ -7,7 +7,7 @@ include $(TOPDIR)/rules.mk
> > 
> >   PKG_NAME:=php
> >   PKG_VERSION:=7.2.6
> > 
> > -PKG_RELEASE:=1
> > +PKG_RELEASE:=2
> > 
> >   PKG_MAINTAINER:=Michael Heimpold <mhei@heimpold.de>
> > 
> > @@ -74,7 +74,6 @@ define Package/php7/config
> > 
> >   	config PHP7_SYSTEMTZDATA
> >   	
> >   		bool "Use system timezone data instead of php's built-in database"
> >   		depends on PACKAGE_php7-cli || PACKAGE_php7-cgi
> > 
> > -		select PACKAGE_zoneinfo-core
> > 
> >   		default y
> >   		help
> >   		
> >   			Enabling this feature automatically selects the zoneinfo-core 
package
> > 
> > @@ -86,7 +85,8 @@ define Package/php7
> > 
> >     $(call Package/php7/Default)
> >     
> >     DEPENDS:=+libpcre +zlib \
> > 
> > -           +PHP7_LIBXML:libxml2
> > +           +PHP7_LIBXML:libxml2 \
> > +           +PHP7_SYSTEMTZDATA:zoneinfo-core
> > 
> >   endef
> >   
> >   define Package/php7/description
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/listinfo/openwrt-devel
Rafał Miłecki June 24, 2018, 7:49 p.m. UTC | #4 
On 2018-06-24 21:39, Michael Heimpold wrote:
> Am Sonntag, 24. Juni 2018, 19:40:53 CEST schrieb John Crispin:
>> On 24/06/18 10:19, Rafał Miłecki wrote:
>> > From: Rafał Miłecki <rafal@milecki.pl>
>> >
>> > Having "select PACKAGE_zoneinfo-core" wasn't enough for builds without
>> > php7-cli=y or php7-cgi=y. It didn't result in installing zoneinfo-core
>> > when using "opkg install" (during runtime or when building images with
>> > CONFIG_TARGET_PER_DEVICE_ROOTFS).
>> >
>> > Missing zoneinfo results in PHP fatal errors, e.g.:
>> > Fatal error: DateTime::createFromFormat(): Timezone database is corrupt -
>> > this should *never* happen!
>> >
>> > For years users were told to manually install zoneinfo-core package.
>> > This problem was hidden for some time (including 17.01 release) due to
>> > disabled support for CONFIG_PHP7_SYSTEMTZDATA. It's now back as support
>> > for --with-system-tzdata was enabled again.
>> >
>> > The proper solution is to simply make php7 package depend on
>> > zoneinfo-core when PHP7_SYSTEMTZDATA is used.
>> >
>> > Fixes: 84e5012e8853 ("php7: re-enable system timezone data usage")
>> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> 
>> php7 is part of the feed, please post a PR on github. the ML is
>> unfortunately only for trunk
> 
> this is true, but since I'm reading this list too and the patch is 
> correct
> I picked it up - no need to open a PR - hope this is ok for all :-)

Thanks!
diff mbox series

Patch

diff --git a/lang/php7/Makefile b/lang/php7/Makefile
index aa860a95..93348543 100644
--- a/lang/php7/Makefile
+++ b/lang/php7/Makefile
@@ -7,7 +7,7 @@  include $(TOPDIR)/rules.mk
 
 PKG_NAME:=php
 PKG_VERSION:=7.2.6
-PKG_RELEASE:=1
+PKG_RELEASE:=2
 
 PKG_MAINTAINER:=Michael Heimpold <mhei@heimpold.de>
 
@@ -74,7 +74,6 @@  define Package/php7/config
 	config PHP7_SYSTEMTZDATA
 		bool "Use system timezone data instead of php's built-in database"
 		depends on PACKAGE_php7-cli || PACKAGE_php7-cgi
-		select PACKAGE_zoneinfo-core
 		default y
 		help
 			Enabling this feature automatically selects the zoneinfo-core package
@@ -86,7 +85,8 @@  define Package/php7
   $(call Package/php7/Default)
 
   DEPENDS:=+libpcre +zlib \
-           +PHP7_LIBXML:libxml2
+           +PHP7_LIBXML:libxml2 \
+           +PHP7_SYSTEMTZDATA:zoneinfo-core
 endef
 
 define Package/php7/description