[Buildroot] [pkg-perl infra V2 01/12] perl: new infrastructure

François Perrad francois.perrad at gadz.org
Tue Feb 4 07:29:08 UTC 2014


2014-02-03 Yann E. MORIN <yann.morin.1998 at free.fr>:
> François, All,
>
> Thank you for this contribution! :-)
>
> Arnout and I are now doing a review of this series.  Sorry to come back
> to you so late...
>
>> perl: new infrastructure
>
> Could you please add a bit more description about this new
> infrastructure?
> You are doing quite some complex changes, and some of them seem
> unrelated
> to actually adding the infrastructure.
>
> For example it seems the PERLIB -> PERL5LIB could be split to a separate
> patch.
>
> On 2013-11-21 13:35 +0100, Francois Perrad spake thusly:
>> Signed-off-by: Francois Perrad <francois.perrad at gadz.org>
>> ---
>>  package/Makefile.in          |   11 ++-
>>  package/intltool/intltool.mk |    4 +-
>>  package/pkg-perl.mk          |  220 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 229 insertions(+), 6 deletions(-)
>>  create mode 100644 package/pkg-perl.mk
>>
>> diff --git a/package/Makefile.in b/package/Makefile.in
>> index 612f3c7..34ce1ec 100644
>> --- a/package/Makefile.in
>> +++ b/package/Makefile.in
>> @@ -206,6 +206,8 @@ HOST_PATH=$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(PATH)
>>  HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
>>       sed -n 's/^.* \([0-9]*\)\.\([0-9]*\)\.\([0-9]*\)[ ]*.*$$/\1\2\3/p')
>>
>> +HOST_PERL_ARCHNAME = $(shell perl -MConfig -e "print Config->{archname}")
>
> This should use ':=', not '='. (Spawning a shell is really costly, so we
> want to evaluate this variable only once.)
>
>>               AS="$(TARGET_AS)" \
>> @@ -241,11 +243,11 @@ TARGET_CONFIGURE_OPTS=PATH=$(TARGET_PATH) \
>>               LDFLAGS="$(TARGET_LDFLAGS)" \
>>               FCFLAGS="$(TARGET_FCFLAGS)" \
>>               PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
>> -             PERLLIB="$(HOST_DIR)/usr/lib/perl" \
>> +             PERL5LIB=$(HOST_DIR)/usr/lib/perl5/$(HOST_PERL_ARCHNAME):$(HOST_DIR)/usr/lib/perl5 \
>
> Separate patch, please (not repeated for the following instances).
>
> [--SNIP--]
>> diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
>> new file mode 100644
>> index 0000000..893501c
>> --- /dev/null
>> +++ b/package/pkg-perl.mk
>> @@ -0,0 +1,220 @@
>> +################################################################################
>> +# Perl package infrastructure
>> +#
>> +# This file implements an infrastructure that eases development of
>> +# package .mk files for Perl packages.
>> +#
>> +# See the Buildroot documentation for details on the usage of this
>> +# infrastructure
>> +#
>> +# In terms of implementation, this perl infrastructure requires
>> +# the .mk file to only specify metadata informations about the
>> +# package: name, version, download URL, etc.
>> +#
>> +# We still allow the package .mk file to override what the different
>> +# steps are doing, if needed. For example, if <PKG>_BUILD_CMDS is
>> +# already defined, it is used as the list of commands to perform to
>> +# build the package, instead of the default perl behaviour. The
>> +# package can also define some post operation hooks.
>> +#
>> +################################################################################
>> +
>> +_PERL_ARCHNAME       = $(ARCH)-linux
>> +
>> +_HOST_PERL_ARCHNAME  = $(shell perl -MConfig -e "print Config->{archname}")
>> +_HOST_PERL_INSTALL_BASE      = $$(HOST_DIR)/usr
>> +_HOST_PERL5LIB               = $(_HOST_PERL_INSTALL_BASE)/lib/perl5/$(_HOST_PERL_ARCHNAME):$(_HOST_PERL_INSTALL_BASE)/lib/perl5
>
> Did you have some search-and-replace issue here? Those variables are
> prefixed with an underscore. If that was on purpose, can you explain
> why?
>
> Also, it seems you're repeating the HOST_PERL_INSTALL_BASE which is
> computed above (maybe it really belongs here, in fact?)
>
>> +################################################################################
>> +# inner-perl-package -- defines how the configuration, compilation and
>> +# installation of an autotools package should be done, implements a
>
> s/autotools/perl/
>
>> +# few hooks to tune the build process for autotools specifities and
>
> Ditto.
>
>> +# calls the generic package infrastructure to generate the necessary
>> +# make targets
>> +#
>> +#  argument 1 is the lowercase package name
>> +#  argument 2 is the uppercase package name, including an HOST_ prefix
>> +#             for host packages
>> +#  argument 3 is the uppercase package name, without the HOST_ prefix
>> +#             for host packages
>> +#  argument 4 is the package directory prefix
>> +#  argument 5 is the type (target or host)
>> +################################################################################
>> +
>> +define inner-perl-package
>> +
>> +$(2)_CONF_OPT                        ?=
>> +$(2)_BUILD_OPT                       ?=
>> +$(2)_INSTALL_OPT             ?=
>> +$(2)_INSTALL_TARGET_OPT              ?=
>> +$(2)_CLEAN_OPT                       ?=
>> +
>> +#
>> +# Configure step. Only define it if not already defined by the package
>> +# .mk file. And take care of the differences between host and target
>> +# packages.
>> +#
>> +ifndef $(2)_CONFIGURE_CMDS
>> +ifeq ($(5),target)
>> +
>> +# Configure package for target
>> +define $(2)_CONFIGURE_CMDS
>> +     cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>> +             PERL5LIB=$(_HOST_PERL5LIB) \
>> +             perl Build.PL \
>> +                     --config ar="$(TARGET_AR)" \
>> +                     --config full_ar="$(TARGET_AR)" \
>> +                     --config cc="$(TARGET_CC)" \
>> +                     --config ccflags="$(TARGET_CFLAGS)" \
>> +                     --config ld="$(TARGET_CC)" \
>> +                     --config lddlflags="-shared $(TARGET_LDFLAGS)" \
>> +                     --config ldflags="$(TARGET_LDFLAGS)" \
>> +                     --include_dirs $$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$(_PERL_ARCHNAME)/CORE \
>> +                     --destdir $$(TARGET_DIR) \
>
> How is this going to play when/if we have to install a perl package to
> staging/ as well as in target/ ? Is that never going to happen?
>
>> +                     --installdirs vendor \
>> +                     --install_path lib=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
>> +                     --install_path arch=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(_PERL_ARCHNAME) \
>> +                     --install_path bin=/usr/bin \
>> +                     --install_path script=/usr/bin \
>> +                     --install_path bindoc=/usr/share/man/man1 \
>> +                     --install_path libdoc=/usr/share/man/man3 \
>> +                     $$($(2)_CONF_OPT); \
>> +     else \
>> +             PERL5LIB=$(_HOST_PERL5LIB) \
>> +             PERL_AUTOINSTALL=--skipdeps \
>> +             perl Makefile.PL \
>> +                     AR="$(TARGET_AR)" \
>> +                     FULL_AR="$(TARGET_AR)" \
>> +                     CC="$(TARGET_CC)" \
>> +                     CCFLAGS="$(TARGET_CFLAGS)" \
>> +                     LD="$(TARGET_CC)" \
>> +                     LDDLFLAGS="-shared $(TARGET_LDFLAGS)" \
>> +                     LDFLAGS="$(TARGET_LDFLAGS)" \
>> +                     DESTDIR=$$(TARGET_DIR) \
>
> Ditto.
>
>> +                     INSTALLDIRS=vendor \
>> +                     INSTALLVENDORLIB=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
>> +                     INSTALLVENDORARCH=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(_PERL_ARCHNAME) \
>> +                     INSTALLVENDORBIN=/usr/bin \
>> +                     INSTALLVENDORSCRIPT=/usr/bin \
>> +                     INSTALLVENDORMAN1DIR=/usr/share/man/man1 \
>> +                     INSTALLVENDORMAN3DIR=/usr/share/man/man3 \
>> +                     $$($(2)_CONF_OPT); \
>> +     fi
>
> Is this automatic detection based on Build.PL really bullet-proof?
> If there is the slighest chance this is not guaranteed to be true, then
> we would prefer to have an explicit package variable (like the python
> infrastrucuture does for distutils vs. setup-tools), like
>    PERLFOO_SETUP_TYPE = {Build.PL|Makefile.PL}
>
> See the nightly-build of the manual:
>     http://nightly.buildroot.org/#_infrastructure_for_python_packages
>
> Also see the python infra:
>     package/pkg-python.mk
>
> [--SNIP--]
>> +#
>> +# Clean step. Only define it if not already defined by
>> +# the package .mk file.
>> +#
>> +ifndef $(2)_CLEAN_CMDS
>> +define $(2)_CLEAN_CMDS
>> +     cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>> +             PERL5LIB=$(_HOST_PERL5LIB) \
>> +             perl Build $$($(2)_CLEAN_OPT) clean; \
>> +     else \
>> +             PERL5LIB=$(_HOST_PERL5LIB) \
>> +             $(MAKE1) $$($(2)_CLEAN_OPT) clean; \
>> +     fi
>> +endef
>> +endif
>
> We removed the 'clean' targets some time ago (after you posted your
> series), so no need to define them any more.
>
> We'll continue the review tomorrow for the rest of the series.
>

I am afraid that you review the V2 series instead of V3.

François

> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot


More information about the buildroot mailing list