All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jussi Kukkonen <jussi.kukkonen@intel.com>
To: "Patel, Vedang" <vedang.patel@intel.com>
Cc: "openembedded-core@lists.openembedded.org"
	<openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] libxslt: Add PACKAGECONFIG support
Date: Thu, 23 Mar 2017 09:37:58 +0200	[thread overview]
Message-ID: <CAHiDW_GZuxuq9JHmsQ4aHM8M6iiFbz3-5yg9fhimD+LmC=xq1Q@mail.gmail.com> (raw)
In-Reply-To: <1490213829.15509.26.camel@intel.com>

[-- Attachment #1: Type: text/plain, Size: 4317 bytes --]

On 22 March 2017 at 22:17, Patel, Vedang <vedang.patel@intel.com> 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 <vedang.patel@intel.com<mailto:
vedang.patel@intel.com>> 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 <vedang.patel@intel.com<mailto:
vedang.patel@intel.com>>
> ---
>  meta/recipes-support/libxslt/libxslt_1.1.29.bb<http://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=<ver> --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<mailto:
Openembedded-core@lists.openembedded.org>
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>

[-- Attachment #2: Type: text/html, Size: 6036 bytes --]

  parent reply	other threads:[~2017-03-23  7:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21  0:31 [PATCH] libxslt: Add PACKAGECONFIG support Vedang Patel
2017-03-21  8:36 ` Jussi Kukkonen
2017-03-22 20:17   ` Patel, Vedang
2017-03-22 20:19     ` Burton, Ross
2017-03-22 22:16       ` Patel, Vedang
2017-03-22 22:18         ` Burton, Ross
2017-03-22 23:33           ` Patel, Vedang
2017-03-23  1:05             ` [PATCH v2] libxslt: Add PACKAGECONFIG support for python Vedang Patel
2017-03-23 19:55               ` Randy Witt
2017-03-23 21:56                 ` Patel, Vedang
2017-03-24  0:00                   ` [PATCH] " Vedang Patel
2017-03-24  1:39                     ` Andre McCurdy
2017-03-29  0:29                       ` Patel, Vedang
2017-03-23  7:37     ` Jussi Kukkonen [this message]
2017-03-23  7:41       ` [PATCH] libxslt: Add PACKAGECONFIG support Jussi Kukkonen

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='CAHiDW_GZuxuq9JHmsQ4aHM8M6iiFbz3-5yg9fhimD+LmC=xq1Q@mail.gmail.com' \
    --to=jussi.kukkonen@intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=vedang.patel@intel.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.