From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw0-f181.google.com (mail-yw0-f181.google.com [209.85.161.181]) by mail.openembedded.org (Postfix) with ESMTP id DDBF072F7B for ; Thu, 23 Mar 2017 07:38:27 +0000 (UTC) Received: by mail-yw0-f181.google.com with SMTP id d191so14160030ywe.2 for ; Thu, 23 Mar 2017 00:38:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=aB/sdACNMvc46fchpkjz+6mj/xrlyEODyH0z3jS7Zto=; b=oXXtrSby8CldiZ0YfJL+bEde6ZAkDvEtXxFyEaRGeahCevaHWBF5C5/EkGXHAdwC1W fESB0EyycHJfeEZwDwSqrLyjXfC7OEZUJbZLHp9zx7z1qXCwBmH0BH+QOIM90uVEQoLk 9N2WWMJvHLgAGSAUOAWNGVMu1Z4Fxar1NikZd5PBrKwX3yf863ypodUmSrS9owbpiwQw KHf6ID6zf0leoQKv6vksZjjdzoYUes43L2p3zEW15f3eFSnuzODmhNSqIZumQ/vHdTKu fXwXhqWB+hp4q8bfymCRob1Nl9WjEucZGmrCple6qyE34uXW8Phw5fhkRvyZ9TdO67El OOmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=aB/sdACNMvc46fchpkjz+6mj/xrlyEODyH0z3jS7Zto=; b=Dw77xJfPyny5pEc2uOI97t5NH8KSLkH0/E4l2MYmIrgj3ZugXXnY4YPrNJBc7+3SgY psL5asakG+5Ve7tY7Oqxh9iHYLZI3QEIbpG1Xj8qcQd3NhiZVm/oW0uYfNFwct+xehu8 7Az1LKt1b2emudKKNRMbhLmMQIHo+qN0Df90jjv1q4lZUA6GUASbHACyvy6o8TdogT0b vLaU3NerfCi8/pt2IyllDPpY6KNfz0L5+6MnA2sj5oMs9a93G5UlTaiA5CiXKIO7mxy0 W1AbKs+AbPi9A9jb2U9CA9xU/KFz0lOK4ofCpBz2NFmUz3IAKM9Z9ejT+aunxk1wxTdp ATHA== X-Gm-Message-State: AFeK/H3cdbWiLW5IBC9qXXLl7MQPn2QrP6U9pyi/fbV6FuP6dzhIGLFwirYEybSA1wRJt1c8IInqGNmO+THFHXcE X-Received: by 10.129.87.132 with SMTP id l126mr772273ywb.167.1490254708585; Thu, 23 Mar 2017 00:38:28 -0700 (PDT) MIME-Version: 1.0 Received: by 10.13.206.131 with HTTP; Thu, 23 Mar 2017 00:37:58 -0700 (PDT) In-Reply-To: <1490213829.15509.26.camel@intel.com> References: <1490056286-13515-1-git-send-email-vedang.patel@intel.com> <1490213829.15509.26.camel@intel.com> From: Jussi Kukkonen Date: Thu, 23 Mar 2017 09:37:58 +0200 Message-ID: To: "Patel, Vedang" Cc: "openembedded-core@lists.openembedded.org" Subject: Re: [PATCH] libxslt: Add PACKAGECONFIG support X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Mar 2017 07:38:29 -0000 Content-Type: multipart/alternative; boundary=001a1146269a87cbe1054b60f666 --001a1146269a87cbe1054b60f666 Content-Type: text/plain; charset=UTF-8 On 22 March 2017 at 22:17, Patel, Vedang wrote: > > Hi Jussi, > Hi Vedang, If you can make your email client use ">" for quoting that would be great (so it's possible to see who's talking even in plain text). PACKAGECONFIG comments below. > On Tue, 2017-03-21 at 08:36 +0000, Kukkonen, Jussi wrote: > On 21 March 2017 at 02:31, Vedang Patel > wrote: > Some options like python bindings, debug support, crypto are hardcoded > inside the recipe. Change that to make those option configurable using > PACKAGECONFIG. > > Signed-off-by: Vedang Patel > > --- > meta/recipes-support/libxslt/libxslt_1.1.29.bb | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/meta/recipes-support/libxslt/libxslt_1.1.29.bb< http://libxslt_1.1.29.bb> b/meta/recipes-support/libxslt/libxslt_1.1.29.bb< http://libxslt_1.1.29.bb> > index be747e608d9d..d362118aa307 100644 > --- a/meta/recipes-support/libxslt/libxslt_1.1.29.bb< http://libxslt_1.1.29.bb> > +++ b/meta/recipes-support/libxslt/libxslt_1.1.29.bb< http://libxslt_1.1.29.bb> > @@ -22,7 +22,7 @@ S = "${WORKDIR}/libxslt-${PV}" > > BINCONFIG = "${bindir}/xslt-config" > > -inherit autotools pkgconfig binconfig-disabled lib_package > +inherit autotools pkgconfig binconfig-disabled lib_package distutils-common-base > > # We don't DEPEND on binutils for ansidecl.h so ensure we don't use the header > do_configure_prepend () { > @@ -33,7 +33,12 @@ do_configure_prepend () { > touch ${S}/doc/xsltproc.1 > } > > -EXTRA_OECONF = "--without-python --without-debug --without-mem-debug --without-crypto" > +PACKAGECONFIG ??= "python libxslt-debug libxslt-mem-debug libxslt-crypto" > > > You change all the defaults, is this on purpose? It should be noted in the commit message in any case. > > Can you elabore on your concern? I am not exactly following it. > > the configs won't be enabled unless the corresponding features are explicitly enabled in the image. I am using libxslt- prefix to make sure someone does not inadvertently enables the features by using the generic flags (python, debug, ... ). >There seems to be some confusion here. I think there's some confusion here about PACKAGECONFIG and image features: PACKAGECONFIG is completely recipe specific -- it only controls this recipe. There's no need to use a naming prefix as these variables are only visible inside the recipe. Changing them from outside (like local.conf) is possible but requires a syntax like PACKAGECONFIG_pn-libxslt = "python crypto" so there's no need to fear namespace mixups. Second, this line: PACKAGECONFIG ??= "python libxslt-debug libxslt-mem-debug libxslt-crypto" sets he default value of libxslt PACKAGECONFIG. Meaning that these options will be given to configure: "--with-python= --with-debug --with-mem-debug --with-crypto" Currently the EXTRA_OECONF line explicitly disables all these so this looks like a change in defaults. If in doubt (or I didn't make sense) you should run "bitbake -cconfigure libxslt" with both versions and check what ends up in the configure options in $WORKDIR/temp/log.do_configure. Cheers, Jussi > I do see a bug here, In the first line, python should be replaced with libxslt-python. I will fix it in the next version of this patch. > > Thanks, > Vedang Patel > Software Engineer > Intel Corporation > > +PACKAGECONFIG[libxslt-python] = "--with-python=${PYTHON_BASE_VERSION}, --without-python" > +PACKAGECONFIG[libxslt-debug] = "--with-debug, --without-debug" > +PACKAGECONFIG[libxslt-mem-debug] = "--with-mem-debug, --without-mem-debug" > +PACKAGECONFIG[libxslt-crypto] = "--with-crypto, --without-crypto" > + > # older versions of this recipe had ${PN}-utils > RPROVIDES_${PN}-bin += "${PN}-utils" > RCONFLICTS_${PN}-bin += "${PN}-utils" > -- > 2.7.3 > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > --001a1146269a87cbe1054b60f666 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On 22 March 2017 at 22:17, Patel, Vedang <vedang.patel@intel.com> wrote:
><= br>> Hi Jussi,
>

Hi Vedang,

If you can make your email client use ">" for quoting th= at would be great (so it's possible to see who's talking even in pl= ain text).

PACKAGECONFIG comments below.

> On Tue, 2017-03-21 at 08:36 +0000, Kukkonen, Jussi wrote:
>= On 21 March 2017 at 02:31, Vedang Patel <vedang.patel@intel.com<mailto:vedang.patel@intel.com>> wrote:
> Some opti= ons like python bindings, debug support, crypto are hardcoded
> insid= e the recipe. Change that to make those option configurable using
> P= ACKAGECONFIG.
>
> Signed-off-by: Vedang Patel <vedang.patel@intel.com<mailto:vedang.patel@intel.com>>
&g= t; ---
> =C2=A0meta/recipes-support/libxslt/libxslt_1.1.29.bb<h= ttp://libxslt_1.1.29.bb> | 9 +++++++--
> =C2=A01 file changed,= 7 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-= support/libxslt/libxslt_1.1.29.bb&= lt;http://libxslt_1.1.29.bb> b/= meta/recipes-support/libxslt/libxslt_1= .1.29.bb<http://libxslt_1.1.29.= bb>
> index be747e608d9d..d362118aa307 100644
> --- a/me= ta/recipes-support/libxslt/libxslt_1.1= .29.bb<http://libxslt_1.1.29.bb= >
> +++ b/meta/recipes-support/libxslt/libxslt_1.1.29.bb<http://libxslt_1.1.29.bb>
> @@ -22,7 +22,7 @@ S =3D "${W= ORKDIR}/libxslt-${PV}"
>
> =C2=A0BINCONFIG =3D "${bin= dir}/xslt-config"
>
> -inherit autotools pkgconfig binconf= ig-disabled lib_package
> +inherit autotools pkgconfig binconfig-disa= bled lib_package distutils-common-base
>
> =C2=A0# We don't= DEPEND on binutils for ansidecl.h so ensure we don't use the header> =C2=A0do_configure_prepend () {
> @@ -33,7 +33,12 @@ do_configu= re_prepend () {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 touch ${S}/doc/xsltproc= .1
> =C2=A0}
>
> -EXTRA_OECONF =3D "--without-python= --without-debug --without-mem-debug --without-crypto"
> +PACKAG= ECONFIG ??=3D "python libxslt-debug libxslt-mem-debug libxslt-crypto&q= uot;
>
>
> You change all the defaults, is this on purpos= e? It should be noted in the commit message in any case.
>
> Ca= n you elabore on your concern? I am not exactly following it.
>
&g= t; the configs won't be enabled unless the corresponding features are e= xplicitly enabled in the image. I am using libxslt- prefix to make sure som= eone does not inadvertently enables the features by using the generic flags= (python, debug, ... ).
>There seems to be some confusion here.

I think there's some confusion here about PACKA= GECONFIG and image features: PACKAGECONFIG is completely recipe specific --= it only controls this recipe. There's no need to use a naming prefix a= s these variables are only visible inside the recipe. Changing them from ou= tside (like local.conf) is possible but requires a syntax like
= =C2=A0=C2=A0 =C2=A0PACKAGECONFIG_pn-libxslt =3D "python crypto"= =C2=A0
so there's no need to fear namespace mixups.

Second, this line:=C2=A0
=C2=A0 =C2=A0 PACKAGECON= FIG ??=3D "python libxslt-debug libxslt-mem-debug libxslt-crypto"=
sets he default value of libxslt PACKAGECONFIG. Meaning that= these options will be given to configure:=C2=A0
=C2=A0 =C2=A0 &q= uot;--with-python=3D<ver> --with-debug --with-mem-debug --with-crypto= "
Currently the EXTRA_OECONF line explicitly disables all th= ese so this looks like a change in defaults.

If in= doubt (or I didn't make sense) you should run "bitbake -cconfigur= e libxslt" with both versions and check what ends up in the configure = options in $WORKDIR/temp/log.do_configure.

Cheers,=
=C2=A0 Jussi



<= div>> I do see a bug here, In the first line, python should be replaced = with libxslt-python. I will fix it in the next version of this patch.
=
>
> Thanks,
> Vedang Patel
> Software = Engineer
> Intel Corporation
>
> +PACKAGECONFIG[libxslt-p= ython] =3D "--with-python=3D${PYTHON_BASE_VERSION}, --without-python&q= uot;
> +PACKAGECONFIG[libxslt-debug] =3D "--with-debug, --withou= t-debug"
> +PACKAGECONFIG[libxslt-mem-debug] =3D "--with-me= m-debug, --without-mem-debug"
> +PACKAGECONFIG[libxslt-crypto] = =3D "--with-crypto, --without-crypto"
> +
> =C2=A0# o= lder versions of this recipe had ${PN}-utils
> =C2=A0RPROVIDES_${PN}-= bin +=3D "${PN}-utils"
> =C2=A0RCONFLICTS_${PN}-bin +=3D &q= uot;${PN}-utils"
> --
> 2.7.3
>
> --
> _= ______________________________________________
> Openembedded-core ma= iling list
> Openembedded-core@lists.openembedded.org<mailto:Openembedded-core@lists.openemb= edded.org>
> http://lists.openembedded.org/mailman/listinfo/= openembedded-core
>
>
--001a1146269a87cbe1054b60f666--