All of lore.kernel.org
 help / color / mirror / Atom feed
From: Herve Codina <herve.codina@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package/pecl-dbus: new package
Date: Tue, 5 Oct 2021 18:15:23 +0200	[thread overview]
Message-ID: <20211005181523.085d94b8@bootlin.com> (raw)
In-Reply-To: <20211005155310.7832c4e4@windsurf>

Hi,

On Tue, 5 Oct 2021 15:53:10 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> Hello Hervé,
> 
> Thanks for this contribution!
> 
> On Tue,  5 Oct 2021 15:11:21 +0200
> Herve Codina <herve.codina@bootlin.com> wrote:
> 
> > The pecl-dbus package provides a PHP extension for interaction
> > with D-Bus busses.
> > 
> > https://github.com/derickr/pecl-dbus
> > 
> > Based on initial work from Nicolas Carrier <nicolas.carrier@orolia.com>
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  DEVELOPERS                                    |   6 +
> >  package/Config.in                             |   1 +
> >  package/pecl-dbus/0001-php8-compat.patch      | 161 ++++++++++++++++++
> >  ...lity-with-php8-call_user_function_ex.patch |  31 ++++
> >  package/pecl-dbus/Config.in                   |  15 ++
> >  package/pecl-dbus/pecl-dbus.hash              |   3 +
> >  package/pecl-dbus/pecl-dbus.mk                |  25 +++
> >  .../testing/tests/package/test_pecl_dbus.py   |  32 ++++
> >  8 files changed, 274 insertions(+)  
> 
> Thanks for this new package. I'm just wondering about the package name.
> The upstream project is indeed named pecl-dbus, where pecl stands for
> "PHP Extension Community Library". But all our other PHP packages are
> named "php-<something>". Should we name this package "php-dbus" ?
> "php-pecl-dbus" ?

"php-pecl-dbus" sounds good.
If it's ok for everyone, I will change in v2.

> 
> 
> > diff --git a/package/pecl-dbus/0001-php8-compat.patch b/package/pecl-dbus/0001-php8-compat.patch
> > new file mode 100644
> > index 0000000000..bef9a1fa05
> > --- /dev/null
> > +++ b/package/pecl-dbus/0001-php8-compat.patch
> > @@ -0,0 +1,161 @@
> > +From 7fde4dd4d26d2a4b7957a1010c6ea9e4b940fc62 Mon Sep 17 00:00:00 2001
> > +From: Stefan Saraev <stefan@saraev.ca>
> > +Date: Mon, 18 Jan 2021 23:22:52 +0200
> > +Subject: [PATCH] php8 compat
> > +
> > +This patch was retrieved from the pending upstream pull request
> > +done by Stefan Saraev.
> > +https://github.com/derickr/pecl-dbus/pull/8
> > +
> > +Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Perhaps you could just ping on this pull request, to say that it also
> solved the problem for you?

I did.

> 
> > diff --git a/package/pecl-dbus/0002-Fix-compatibility-with-php8-call_user_function_ex.patch b/package/pecl-dbus/0002-Fix-compatibility-with-php8-call_user_function_ex.patch
> > new file mode 100644
> > index 0000000000..1e579e4272
> > --- /dev/null
> > +++ b/package/pecl-dbus/0002-Fix-compatibility-with-php8-call_user_function_ex.patch
> > @@ -0,0 +1,31 @@
> > +From 8cf66c4b5dd657ca14552b27a42901db0a5e9280 Mon Sep 17 00:00:00 2001
> > +From: Herve Codina <herve.codina@bootlin.com>
> > +Date: Mon, 4 Oct 2021 17:13:45 +0200
> > +Subject: [PATCH] Fix compatibility with php8 - call_user_function_ex
> > +
> > +This patch fix compatibility with php8 replacing
> > +call_user_function_ex by call_user_function.
> > +
> > +Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> And submit this upstream ?

I did too.

> 
> > diff --git a/package/pecl-dbus/Config.in b/package/pecl-dbus/Config.in
> > new file mode 100644
> > index 0000000000..a531397754
> > --- /dev/null
> > +++ b/package/pecl-dbus/Config.in
> > @@ -0,0 +1,15 @@
> > +config BR2_PACKAGE_PECL_DBUS
> > +	bool "pecl-dbus"
> > +	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
> > +	depends on BR2_USE_MMU # dbus
> > +	select BR2_PACKAGE_DBUS
> > +	select BR2_PACKAGE_LIBXML2
> > +	select BR2_PACKAGE_PHP_EXT_LIBXML2 # Runtime dependency  
> 
> I was wondering if you need both BR2_PACKAGE_LIBXML2 and
> BR2_PACKAGE_PHP_EXT_LIBXML2, i.e does pecl-dbus link directly with the
> libxml2 native library ?

libxml2 is needed. In dbus.c source file, we have
  #include "libxml/parser.h"
  #include "libxml/parserInternals.h"
and calls to libxml2 functions such as 'xmlParseMemory()'

> 
> 
> > +PECL_DBUS_VERSION = 315d17558c7614d02e923d898231e51c86a25789
> > +PECL_DBUS_SITE = $(call github,derickr,pecl-dbus,$(PECL_DBUS_VERSION))
> > +PECL_DBUS_LICENSE = PHP-3.01
> > +PECL_DBUS_LICENSE_FILES = LICENSE
> > +PECL_DBUS_DEPENDENCIES = php dbus libxml2 host-autoconf host-pkgconf
> > +
> > +PECL_DBUS_CONF_OPTS = \
> > +	--with-php-config=$(STAGING_DIR)/usr/bin/php-config \
> > +	--enable-dbus  
> 
> pecl-dbus can build without dbus ?!?

Indeed, '--enable-dbus' is not needed and will be removed in v2.

Thanks for the review,

Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2021-10-05 16:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05 13:11 [Buildroot] [PATCH 1/1] package/pecl-dbus: new package Herve Codina
2021-10-05 13:53 ` Thomas Petazzoni
2021-10-05 16:15   ` Herve Codina [this message]
2021-10-08 13:16     ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211005181523.085d94b8@bootlin.com \
    --to=herve.codina@bootlin.com \
    --cc=buildroot@buildroot.org \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.