diff mbox

[v4,03/17] package/pkg-rebar: new infrastructure

Message ID 1418135662-773-4-git-send-email-johan.oudinet@gmail.com
State Superseded
Headers show

Commit Message

Johan Oudinet Dec. 9, 2014, 2:34 p.m. UTC
Ease the development of packages that use the erlang rebar tool as
their build system.

Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com>
[yann.morin.1998@free.fr: split the patch into semantically separated
patches; large rewrites of the rest]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Changes v2 -> v3:
  - introduce two new options: ERLANG_FOO_CONFIGURE to use the autotools
    infra (YES) or the generic infra (NO/empty) and ERLANG_FOO_HAS_REBAR
    to use the package's rebar (YES) or our own (NO/empty)
  - remove the duplicated code from the autotools infra
  - largely re-organise the code and variables definitions
  - move the EI_VSN setting into the elrnag package
  - remove the script used to compute EI_VSN, as it is no longer needed
  - split host-rebar to its own patch
  - add some comments

Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com>
---
 package/Makefile.in  |   1 +
 package/pkg-rebar.mk | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 211 insertions(+)
 create mode 100644 package/pkg-rebar.mk

Comments

Thomas Petazzoni Jan. 4, 2015, 9:23 p.m. UTC | #1
Dear Johan Oudinet,

On Tue,  9 Dec 2014 15:34:08 +0100, Johan Oudinet wrote:

> diff --git a/package/pkg-rebar.mk b/package/pkg-rebar.mk
> new file mode 100644
> index 0000000..c9b15c0
> --- /dev/null
> +++ b/package/pkg-rebar.mk
> @@ -0,0 +1,210 @@
> +################################################################################
> +# rebar package infrastructure
> +#
> +# This file implements an infrastructure that eases development of
> +# package .mk files for rebar packages.  It should be used for all
> +# packages that use rebar as their build system.
> +#
> +# In terms of implementation, this rebar infrastructure requires the
> +# .mk file to only specify metadata information 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 rebar behaviour. The
> +# package can also define some post operation hooks.

Maybe we should mention "Erlang" somewhere in this description.

> +#
> +################################################################################
> +
> +# Directories to store rebar dependencies in.
> +#
> +# These directories actually only contain symbolic links to Erlang
> +# applications in either $(HOST_DIR) or $(STAGING_DIR).  One needs
> +# them to avoid rebar complaining about missing dependencies, as this
> +# infrastructure does NOT tell rebar to download dependencies during
> +# the build stage.

I would rephrase that:

"this infrastructure tells rebar to NOT download dependencies during
the build stage".

> +# Setup a symbolic link in rebar's deps_dir to the actual location
> +# where an Erlang application is installed.
> +#
> +# i.e., define a recipe that creates a symbolic link
> +# from $($(PKG)_REBAR_DEPS_DIR)/$($(PKG)_ERLANG_APP)
> +# to $(1)$($(PKG)_ERLANG_LIBDIR).
> +#
> +# One typically uses this to setup symbolic links from
> +# $(BUILD_DIR)/rebar-deps/<HOST_OR_TARGET>/<ERLANG_APP> to the
> +# appropriate application directory in $(HOST_DIR) or $(STAGING_DIR).
> +# This avoids rebar complaining about missing dependencies, as this
> +# infrastructure does NOT tell rebar to download dependencies during
> +# the build stage.

Ditto. "this infrastructure tells rebar to NOT download dependencies
during the build stage".

> +################################################################################
> +# inner-rebar-package -- defines how the configuration, compilation
> +# and installation of a rebar package should be done, implements a few
> +# hooks to tune the build process according to rebar specifities, and
> +# 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 a HOST_ prefix
> +#             for host packages
> +#  argument 3 is the uppercase package name, without the HOST_ prefix
> +#             for host packages
> +#  argument 4 is the type (target or host)
> +#
> +################################################################################
> +
> +define inner-rebar-package
> +
> +# Extract just the raw package name, lowercase without the leading
> +# erlang- or host- prefix, as this is used by rebar to find the
> +# dependencies a package specifies.
> +#
> +$(2)_ERLANG_APP = $(subst -,_,$(call LOWERCASE,$(patsubst ERLANG_%,%,$(3))))

I think the comment doesn't line up any more with what is done exactly:
we're working on the uppercase version, which never has HOST_, and
doesn't have erlang- but ERLANG_. Too bad that LOWERCASE replaces _ by
-, and that then you have replace again - by _.

Would it be clearer to actually use $(1) ?

$(2)_ERLANG_APP = $(subst -,_,$(patsubst erlang-%,%,$(patsubst host-%,%,$(1))

Not sure that's really better.

> +# Path where to store the package's libs, relative to either $(HOST_DIR)
> +# for host packages, or $(STAGING_DIR) for target packages.
> +#
> +$(2)_ERLANG_LIBDIR = \
> +	/usr/lib/erlang/lib/$$($$(PKG)_ERLANG_APP)-$$($$(PKG)_VERSION)
> +
> +# Whether to use the generic rebar or the package's rebar
> +#
> +ifeq ($$($(2)_HAS_REBAR),YES)
> +$(2)_REBAR = ./rebar
> +else
> +$(2)_REBAR = rebar
> +endif

It's not clear what $(2)_HAS_REBAR is useful for, and this variable is
not documented in PATCH 04/17.

> +# Define the build and install commands
> +#
> +ifeq ($(4),target)
> +
> +ifeq ($$($(2)_CONFIGURE),YES)

What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.

> +$(2)_CONF_ENV += \
> +	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_TARGET_DEPS_DIR)"}' \
> +	ERL_EI_LIBDIR=$$(STAGING_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib
> +endif
> +
> +ifndef $(2)_BUILD_CMDS
> +define $(2)_BUILD_CMDS
> +	cd '$$(@D)'; \

Any reason for the simple quotes here? Also, other package infrastructures do:

	(cd $$($$(PKG)_SRCDIR) && \
	  ....)

Which allows to use the <foo>_SUBDIR mechanism when needed.

> +	CC='$$(TARGET_CC)' \
> +	CFLAGS='$$(TARGET_CFLAGS)' \
> +	LDFLAGS='$$(TARGET_LDFLAGS)' \

Why simple quotes? We use double quotes everywhere in Buildroot. I
guess you look at using $$(TARGET_CONFIGURE_OPTS) instead?

> +	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_TARGET_DEPS_DIR)"}' \
> +	ERL_EI_LIBDIR=$$(STAGING_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib \

This is duplicated from the CONFIGURE == YES case above, which defines
$(2)_CONF_ENV. Maybe this can be factorized somehow?

> +	$$(TARGET_MAKE_ENV) \
> +	$$($$(PKG)_ENV) $$($$(PKG)_REBAR) deps_dir=$$(REBAR_TARGET_DEPS_DIR) compile

Maybe $$(PKG)_ENV should be named $$(PKG)_REBAR_ENV, to be more
explicit, and in line with $$(PKG)_CONF_ENV that you use above, and
that we use in the autotools infra?

> +ifeq ($$($(2)_CONFIGURE),YES)
> +$(2)_CONF_ENV += \
> +	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_HOST_DEPS_DIR)"}' \
> +	ERL_EI_LIBDIR=$$(HOST_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib
> +endif
> +
> +ifndef $(2)_BUILD_CMDS
> +define $(2)_BUILD_CMDS
> +	cd '$$(@D)'; \
> +	CC='$$(HOST_CC)' \
> +	CFLAGS='$$(HOST_CFLAGS)' \
> +	LDFLAGS='$$(HOST_LDFLAGS)' \
> +	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_HOST_DEPS_DIR)"}' \
> +	ERL_EI_LIBDIR=$$(HOST_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib \
> +	$$(HOST_MAKE_ENV) \
> +	$$($$(PKG)_ENV) $$($$(PKG)_REBAR) deps_dir=$$(REBAR_HOST_DEPS_DIR) compile

Same comments as for the target variant.

> +endef
> +endif
> +
> +# We need to double-$ the 'call' because it wants to expand
> +# package-related variables
> +ifndef $(2)_INSTALL_CMDS
> +define $(2)_INSTALL_CMDS
> +	$$(call install-erlang-directories,$$(HOST_DIR),include)
> +	$$(call install-rebar-deps,$$(HOST_DIR),HOST)
> +endef
> +endif
> +
> +endif # !target
> +
> +# The package sub-infra to use
> +#
> +ifeq ($$($(2)_CONFIGURE),YES)
> +$(call inner-autotools-package,$(1),$(2),$(3),$(4))
> +else
> +$(call inner-generic-package,$(1),$(2),$(3),$(4))
> +endif

Wow, this is funky :-) A package infra that inherits from either one
infra or the other! And two-level inheritance in the case of
generic-package -> autotools-package -> rebar-package.

All in all, despite the minor questions/comments above, it looks pretty
good. Care to work to resolve those issues and post an updated version?

Thanks!

Thomas
Yann E. MORIN Jan. 4, 2015, 10:20 p.m. UTC | #2
Thomas, Johan, All,

[comments ellided are being dealt with]

On 2015-01-04 22:23 +0100, Thomas Petazzoni spake thusly:
> On Tue,  9 Dec 2014 15:34:08 +0100, Johan Oudinet wrote:
> > diff --git a/package/pkg-rebar.mk b/package/pkg-rebar.mk
> > new file mode 100644
> > index 0000000..c9b15c0
> > --- /dev/null
> > +++ b/package/pkg-rebar.mk
[--SNIP--]
> > +################################################################################
> > +# inner-rebar-package -- defines how the configuration, compilation
> > +# and installation of a rebar package should be done, implements a few
> > +# hooks to tune the build process according to rebar specifities, and
> > +# 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 a HOST_ prefix
> > +#             for host packages
> > +#  argument 3 is the uppercase package name, without the HOST_ prefix
> > +#             for host packages
> > +#  argument 4 is the type (target or host)
> > +#
> > +################################################################################
> > +
> > +define inner-rebar-package
> > +
> > +# Extract just the raw package name, lowercase without the leading
> > +# erlang- or host- prefix, as this is used by rebar to find the
> > +# dependencies a package specifies.
> > +#
> > +$(2)_ERLANG_APP = $(subst -,_,$(call LOWERCASE,$(patsubst ERLANG_%,%,$(3))))
> 
> I think the comment doesn't line up any more with what is done exactly:
> we're working on the uppercase version, which never has HOST_, and
> doesn't have erlang- but ERLANG_. Too bad that LOWERCASE replaces _ by
> -, and that then you have replace again - by _.
> 
> Would it be clearer to actually use $(1) ?
> 
> $(2)_ERLANG_APP = $(subst -,_,$(patsubst erlang-%,%,$(patsubst host-%,%,$(1))
> 
> Not sure that's really better.

Yet, it is better to deal with the original values, rather than to try
to retro-engineer them. I'll check your proposal, and use it.

> > +# Path where to store the package's libs, relative to either $(HOST_DIR)
> > +# for host packages, or $(STAGING_DIR) for target packages.
> > +#
> > +$(2)_ERLANG_LIBDIR = \
> > +	/usr/lib/erlang/lib/$$($$(PKG)_ERLANG_APP)-$$($$(PKG)_VERSION)
> > +
> > +# Whether to use the generic rebar or the package's rebar
> > +#
> > +ifeq ($$($(2)_HAS_REBAR),YES)
> > +$(2)_REBAR = ./rebar
> > +else
> > +$(2)_REBAR = rebar
> > +endif
> 
> It's not clear what $(2)_HAS_REBAR is useful for, and this variable is
> not documented in PATCH 04/17.

Some packages bundle their own rebar, other do not.

Of the former, some may really require we use their bundled rebar
instead f the one we provide. _HAS_REBAR is for those packages.

But maybe it is not really properly named. It's meaning is: this package
has a bundled rebar, and does not work with the system rebar, so must
use its own bnundled version.

But the appreciated meaning of the variable name is just: this package
has a rebar utility. It does not state that the rebar infra shall use
it. I'll try to come up with a better name.

> > +# Define the build and install commands
> > +#
> > +ifeq ($(4),target)
> > +
> > +ifeq ($$($(2)_CONFIGURE),YES)
> 
> What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.

Some rebar packages use autotools for the configure step, and use rebar
for the build step. Worse, some of them even require being autoreconf-ed.

This variable, if set to YES, means that this package will use the
autotools-package infrastrucutre; otherwise, it will be trated as a
generic-package.

Fortunately, I could not see an Erlang package that uses CMake or
setuptoolsi or distutils... :-)

[--SNIP--]
> > +# The package sub-infra to use
> > +#
> > +ifeq ($$($(2)_CONFIGURE),YES)
> > +$(call inner-autotools-package,$(1),$(2),$(3),$(4))
> > +else
> > +$(call inner-generic-package,$(1),$(2),$(3),$(4))
> > +endif
> 
> Wow, this is funky :-) A package infra that inherits from either one
> infra or the other! And two-level inheritance in the case of
> generic-package -> autotools-package -> rebar-package.

So, are you not too horrified by this? Shall we keep it, or do you want
we copme up with an alternate solution?

> All in all, despite the minor questions/comments above, it looks pretty
> good. Care to work to resolve those issues and post an updated version?

I'm doing a sanitising pass with your comments, now. I'll push that to
my branch so Johan can grab it (I'll poke here when I did the push).

Thanks for the reviews! :-)

/me can't wait to have a Jabber daemin in Buildroot... :-]

Regards,
Yann E. MORIN.
Thomas Petazzoni Jan. 5, 2015, 9:31 a.m. UTC | #3
Hello!

On Sun, 4 Jan 2015 23:20:38 +0100, Yann E. MORIN wrote:

> > Would it be clearer to actually use $(1) ?
> > 
> > $(2)_ERLANG_APP = $(subst -,_,$(patsubst erlang-%,%,$(patsubst host-%,%,$(1))
> > 
> > Not sure that's really better.
> 
> Yet, it is better to deal with the original values, rather than to try
> to retro-engineer them. I'll check your proposal, and use it.

Well, it's a matter of taste really because both expressions are
equally "complicated". So using $(1) as I propose, or $(3) as was in
the patch doesn't make much difference. Maybe only the switch back and
forth between - and _ is a bit weird when using $(3).

> > It's not clear what $(2)_HAS_REBAR is useful for, and this variable is
> > not documented in PATCH 04/17.
> 
> Some packages bundle their own rebar, other do not.
> 
> Of the former, some may really require we use their bundled rebar
> instead f the one we provide. _HAS_REBAR is for those packages.
> 
> But maybe it is not really properly named. It's meaning is: this package
> has a bundled rebar, and does not work with the system rebar, so must
> use its own bnundled version.
> 
> But the appreciated meaning of the variable name is just: this package
> has a rebar utility. It does not state that the rebar infra shall use
> it. I'll try to come up with a better name.

$(2)_USE_BUNDLED_REBAR, default to NO, and overridden by YES for those
packages that should really use their rebar rather than the system one.

or:

$(2)_USE_SYSTEM_REBAR, default to YES, and overridden to NO for those
packages that should really use their rebar rather than the system one.

> > > +# Define the build and install commands
> > > +#
> > > +ifeq ($(4),target)
> > > +
> > > +ifeq ($$($(2)_CONFIGURE),YES)
> > 
> > What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.
> 
> Some rebar packages use autotools for the configure step, and use rebar
> for the build step. Worse, some of them even require being autoreconf-ed.
> 
> This variable, if set to YES, means that this package will use the
> autotools-package infrastrucutre; otherwise, it will be trated as a
> generic-package.

Not sure of a better name for this one. Maybe $(2)_USE_AUTOTOOLS,
defaults to no, can be overridden to YES by the packages having a
configure script? Do they both autoconf and automake, or just autoconf?
I guess the latter, so maybe $(2)_USE_AUTOCONF.

> > Wow, this is funky :-) A package infra that inherits from either one
> > infra or the other! And two-level inheritance in the case of
> > generic-package -> autotools-package -> rebar-package.
> 
> So, are you not too horrified by this? Shall we keep it, or do you want
> we copme up with an alternate solution?

No, I'm not horrified by this at all. It's smart, and elegantly re-uses
the existing infrastructures. It's perfectly fine for me.

> I'm doing a sanitising pass with your comments, now. I'll push that to
> my branch so Johan can grab it (I'll poke here when I did the push).

Great, thanks!

Thomas
Johan Oudinet Jan. 5, 2015, 11:13 a.m. UTC | #4
Thomas, All,

On Mon, Jan 5, 2015 at 10:31 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Sun, 4 Jan 2015 23:20:38 +0100, Yann E. MORIN wrote:
>> > > +# Define the build and install commands
>> > > +#
>> > > +ifeq ($(4),target)
>> > > +
>> > > +ifeq ($$($(2)_CONFIGURE),YES)
>> >
>> > What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.
>>
>> Some rebar packages use autotools for the configure step, and use rebar
>> for the build step. Worse, some of them even require being autoreconf-ed.
>>
>> This variable, if set to YES, means that this package will use the
>> autotools-package infrastrucutre; otherwise, it will be trated as a
>> generic-package.
>
> Not sure of a better name for this one. Maybe $(2)_USE_AUTOTOOLS,
> defaults to no, can be overridden to YES by the packages having a
> configure script? Do they both autoconf and automake, or just autoconf?
> I guess the latter, so maybe $(2)_USE_AUTOCONF.

They use autoconf only (mostly for the rebar.config.in file). So, I
vote for $(2)_USE_AUTOCONF.
Yann E. MORIN Jan. 5, 2015, 9:59 p.m. UTC | #5
Thomas, All,

On 2015-01-05 10:31 +0100, Thomas Petazzoni spake thusly:
> On Sun, 4 Jan 2015 23:20:38 +0100, Yann E. MORIN wrote:
[--SNIP--]
> > Some packages bundle their own rebar, other do not.
> > 
> > Of the former, some may really require we use their bundled rebar
> > instead f the one we provide. _HAS_REBAR is for those packages.
> > 
> > But maybe it is not really properly named. It's meaning is: this package
> > has a bundled rebar, and does not work with the system rebar, so must
> > use its own bnundled version.
> > 
> > But the appreciated meaning of the variable name is just: this package
> > has a rebar utility. It does not state that the rebar infra shall use
> > it. I'll try to come up with a better name.
> 
> $(2)_USE_BUNDLED_REBAR, default to NO, and overridden by YES for those
> packages that should really use their rebar rather than the system one.
> 
> or:
> 
> $(2)_USE_SYSTEM_REBAR, default to YES, and overridden to NO for those
> packages that should really use their rebar rather than the system one.

I have already gone for _USE_BUNDLED_REBAR in the series I pushed to
Johan, although I had pondered the alternative you also suggested.

Great minds think alike! Hehe! :-)

> > > > +# Define the build and install commands
> > > > +#
> > > > +ifeq ($(4),target)
> > > > +
> > > > +ifeq ($$($(2)_CONFIGURE),YES)
> > > 
> > > What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.
> > 
> > Some rebar packages use autotools for the configure step, and use rebar
> > for the build step. Worse, some of them even require being autoreconf-ed.
> > 
> > This variable, if set to YES, means that this package will use the
> > autotools-package infrastrucutre; otherwise, it will be trated as a
> > generic-package.
> 
> Not sure of a better name for this one. Maybe $(2)_USE_AUTOTOOLS,
> defaults to no, can be overridden to YES by the packages having a
> configure script? Do they both autoconf and automake, or just autoconf?
> I guess the latter, so maybe $(2)_USE_AUTOCONF.

Alternatively, I was even thinking of an even more generic solution,
$(2)_PACKAGE_TYPE , which would be set to the underlying type of
packages.

In the end, I did not implement that because it is too much overkill.
Also, I kept _CONFIGURE because it is what people do expect: the package
needs to be configured before being built. But I am not completely
opposed to changing the variable name. _USE_AUTOTOOLS or _USE_AUTOCONF
is equally fit, I guess (although I'd still prefer _AUTOTOOLS, because
it matches the fact that we call back to the autotools infrastructure
underneath.

Johan, are there Erlang packages that use something other than
autotools?

Regards,
Yann E. MORIN.
Yann E. MORIN Jan. 5, 2015, 10:01 p.m. UTC | #6
Johan, All,

On 2015-01-05 12:13 +0100, Johan Oudinet spake thusly:
> On Mon, Jan 5, 2015 at 10:31 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > On Sun, 4 Jan 2015 23:20:38 +0100, Yann E. MORIN wrote:
> >> > > +# Define the build and install commands
> >> > > +#
> >> > > +ifeq ($(4),target)
> >> > > +
> >> > > +ifeq ($$($(2)_CONFIGURE),YES)
> >> >
> >> > What is $(2)_CONFIGURE exactly? It is not documented in PATCH 04/17.
> >>
> >> Some rebar packages use autotools for the configure step, and use rebar
> >> for the build step. Worse, some of them even require being autoreconf-ed.
> >>
> >> This variable, if set to YES, means that this package will use the
> >> autotools-package infrastrucutre; otherwise, it will be trated as a
> >> generic-package.
> >
> > Not sure of a better name for this one. Maybe $(2)_USE_AUTOTOOLS,
> > defaults to no, can be overridden to YES by the packages having a
> > configure script? Do they both autoconf and automake, or just autoconf?
> > I guess the latter, so maybe $(2)_USE_AUTOCONF.
> 
> They use autoconf only (mostly for the rebar.config.in file). So, I
> vote for $(2)_USE_AUTOCONF.

As I said in my reply to Thomas, I believe _CONFIGURE is really fit,
because that's what people expect: the package needs to be configured.

But, also as I said, if Thomas prefers we use an alternative, I'd say we
go for _AUTOTOOLS, as it matches the underlying infra we call to,
"autotools".

Regards,
Yann E. MORIN.
Thomas Petazzoni Jan. 6, 2015, 8:24 a.m. UTC | #7
Dear Yann E. MORIN,

On Mon, 5 Jan 2015 22:59:56 +0100, Yann E. MORIN wrote:

> In the end, I did not implement that because it is too much overkill.
> Also, I kept _CONFIGURE because it is what people do expect: the package
> needs to be configured before being built. But I am not completely
> opposed to changing the variable name. _USE_AUTOTOOLS or _USE_AUTOCONF
> is equally fit, I guess (although I'd still prefer _AUTOTOOLS, because
> it matches the fact that we call back to the autotools infrastructure
> underneath.

Unfortunately, I disagree on this. "Configure" is a very vague term,
which does not necessarily mean "run an autoconf generated ./configure
script". We do have certain generic-package packages that do provide an
implementation for the <pkg>_CONFIGURE_CMDS that aren't calling an
autoconf generated ./configure script.

Moreover, a non-autoconf using rebar package may want to implement its
<pkg>_CONFIGURE_CMDS to do some stuff. But it would have to keep
<pkg>_CONFIGURE set to NO. This is really confusing.

Please, use <pkg>_USE_AUTOTOOLS, <pkg>_AUTOTOOLS or <pkg>_USE_AUTOCONF.
I tend to prefer the latter, because this is really what the user is
seeing: the package is using autoconf. You say you prefer _AUTOTOOLS
because it matches the fact that we're using the autotools
infrastructure underneath, but that's an implementation detail. It's
much better to expose things that make sense for the package developer
that things that relate to implementation details, IMO.

Best regards,

Thomas
Johan Oudinet Jan. 6, 2015, 10:05 a.m. UTC | #8
Thomas, Yann, All,

On Tue, Jan 6, 2015 at 9:24 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Mon, 5 Jan 2015 22:59:56 +0100, Yann E. MORIN wrote:
>
>> In the end, I did not implement that because it is too much overkill.
>> Also, I kept _CONFIGURE because it is what people do expect: the package
>> needs to be configured before being built. But I am not completely
>> opposed to changing the variable name. _USE_AUTOTOOLS or _USE_AUTOCONF
>> is equally fit, I guess (although I'd still prefer _AUTOTOOLS, because
>> it matches the fact that we call back to the autotools infrastructure
>> underneath.
>
> Unfortunately, I disagree on this. "Configure" is a very vague term,
> which does not necessarily mean "run an autoconf generated ./configure
> script". We do have certain generic-package packages that do provide an
> implementation for the <pkg>_CONFIGURE_CMDS that aren't calling an
> autoconf generated ./configure script.
>
> Moreover, a non-autoconf using rebar package may want to implement its
> <pkg>_CONFIGURE_CMDS to do some stuff. But it would have to keep
> <pkg>_CONFIGURE set to NO. This is really confusing.
>
> Please, use <pkg>_USE_AUTOTOOLS, <pkg>_AUTOTOOLS or <pkg>_USE_AUTOCONF.
> I tend to prefer the latter, because this is really what the user is
> seeing: the package is using autoconf. You say you prefer _AUTOTOOLS
> because it matches the fact that we're using the autotools
> infrastructure underneath, but that's an implementation detail. It's
> much better to expose things that make sense for the package developer
> that things that relate to implementation details, IMO.

I tend to agree with Thomas on this point and not only because I've
already modified all erlang packages to the USE_AUTOCONF syntax ;-)
Actually, even though pkg-rebar uses the autotools infrastructure, it
does redefine the BUILD_CMDS. So automake is never called. Using the
term AUTOTOOLS might be confusing to the package developer as he would
expect its Makefile.am to be read.
I'm going to push a new version soon, so you can modify it if you want.
diff mbox

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 36ecf0b..6ef4231 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -407,3 +407,4 @@  include package/pkg-python.mk
 include package/pkg-virtual.mk
 include package/pkg-generic.mk
 include package/pkg-kconfig.mk
+include package/pkg-rebar.mk
diff --git a/package/pkg-rebar.mk b/package/pkg-rebar.mk
new file mode 100644
index 0000000..c9b15c0
--- /dev/null
+++ b/package/pkg-rebar.mk
@@ -0,0 +1,210 @@ 
+################################################################################
+# rebar package infrastructure
+#
+# This file implements an infrastructure that eases development of
+# package .mk files for rebar packages.  It should be used for all
+# packages that use rebar as their build system.
+#
+# In terms of implementation, this rebar infrastructure requires the
+# .mk file to only specify metadata information 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 rebar behaviour. The
+# package can also define some post operation hooks.
+#
+################################################################################
+
+# Directories to store rebar dependencies in.
+#
+# These directories actually only contain symbolic links to Erlang
+# applications in either $(HOST_DIR) or $(STAGING_DIR).  One needs
+# them to avoid rebar complaining about missing dependencies, as this
+# infrastructure does NOT tell rebar to download dependencies during
+# the build stage.
+#
+REBAR_HOST_DEPS_DIR = $(HOST_DIR)/usr/share/rebar/deps
+REBAR_TARGET_DEPS_DIR = $(STAGING_DIR)/usr/share/rebar/deps
+
+################################################################################
+# Helper functions
+################################################################################
+
+# Install an Erlang application from $(@D).
+#
+# i.e., define a recipe that installs the "ebin priv $(2)" directories
+# from $(@D) to $(1)$($(PKG)_ERLANG_LIBDIR).
+#
+#  argument 1 should typically be $(HOST_DIR), $(TARGET_DIR),
+#	      or $(STAGING_DIR).
+#  argument 2 is typically empty when installing in $(TARGET_DIR) and
+#             "include" when installing in $(HOST_DIR) or
+#             $(STAGING_DIR).
+#
+# Note: calling this function must be done with $$(call ...) because it
+# expands package-related variables.
+#
+define install-erlang-directories
+	$(INSTALL) -d $(1)/$($(PKG)_ERLANG_LIBDIR)
+	for dir in ebin priv $(2); do                                   \
+		if test -d $(@D)/$$dir; then                            \
+			cp -r $(@D)/$$dir $(1)$($(PKG)_ERLANG_LIBDIR);  \
+		fi;                                                     \
+	done
+endef
+
+# Setup a symbolic link in rebar's deps_dir to the actual location
+# where an Erlang application is installed.
+#
+# i.e., define a recipe that creates a symbolic link
+# from $($(PKG)_REBAR_DEPS_DIR)/$($(PKG)_ERLANG_APP)
+# to $(1)$($(PKG)_ERLANG_LIBDIR).
+#
+# One typically uses this to setup symbolic links from
+# $(BUILD_DIR)/rebar-deps/<HOST_OR_TARGET>/<ERLANG_APP> to the
+# appropriate application directory in $(HOST_DIR) or $(STAGING_DIR).
+# This avoids rebar complaining about missing dependencies, as this
+# infrastructure does NOT tell rebar to download dependencies during
+# the build stage.
+#
+# Therefore,
+#  argument 1 is $(HOST_DIR) (for host packages) or
+#	      $(STAGING_DIR) (for target packages).
+#
+#  argument 2 is HOST (for host packages) or
+#	      TARGET (for target packages).
+#
+# Note: calling this function must be done with $$(call ...) because it
+# expands package-related variables.
+#
+define install-rebar-deps
+	$(INSTALL) -d $(REBAR_$(2)_DEPS_DIR)
+	ln -f -s $(1)/$($(PKG)_ERLANG_LIBDIR) \
+		$(REBAR_$(2)_DEPS_DIR)/$($(PKG)_ERLANG_APP)
+endef
+
+################################################################################
+# inner-rebar-package -- defines how the configuration, compilation
+# and installation of a rebar package should be done, implements a few
+# hooks to tune the build process according to rebar specifities, and
+# 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 a HOST_ prefix
+#             for host packages
+#  argument 3 is the uppercase package name, without the HOST_ prefix
+#             for host packages
+#  argument 4 is the type (target or host)
+#
+################################################################################
+
+define inner-rebar-package
+
+# Extract just the raw package name, lowercase without the leading
+# erlang- or host- prefix, as this is used by rebar to find the
+# dependencies a package specifies.
+#
+$(2)_ERLANG_APP = $(subst -,_,$(call LOWERCASE,$(patsubst ERLANG_%,%,$(3))))
+
+# Path where to store the package's libs, relative to either $(HOST_DIR)
+# for host packages, or $(STAGING_DIR) for target packages.
+#
+$(2)_ERLANG_LIBDIR = \
+	/usr/lib/erlang/lib/$$($$(PKG)_ERLANG_APP)-$$($$(PKG)_VERSION)
+
+# Whether to use the generic rebar or the package's rebar
+#
+ifeq ($$($(2)_HAS_REBAR),YES)
+$(2)_REBAR = ./rebar
+else
+$(2)_REBAR = rebar
+endif
+
+
+# Define the build and install commands
+#
+ifeq ($(4),target)
+
+ifeq ($$($(2)_CONFIGURE),YES)
+$(2)_CONF_ENV += \
+	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_TARGET_DEPS_DIR)"}' \
+	ERL_EI_LIBDIR=$$(STAGING_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib
+endif
+
+ifndef $(2)_BUILD_CMDS
+define $(2)_BUILD_CMDS
+	cd '$$(@D)'; \
+	CC='$$(TARGET_CC)' \
+	CFLAGS='$$(TARGET_CFLAGS)' \
+	LDFLAGS='$$(TARGET_LDFLAGS)' \
+	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_TARGET_DEPS_DIR)"}' \
+	ERL_EI_LIBDIR=$$(STAGING_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib \
+	$$(TARGET_MAKE_ENV) \
+	$$($$(PKG)_ENV) $$($$(PKG)_REBAR) deps_dir=$$(REBAR_TARGET_DEPS_DIR) compile
+endef
+endif
+
+# We need to double-$ the 'call' because it wants to expand
+# package-related variables
+ifndef $(2)_INSTALL_STAGING_CMDS
+define $(2)_INSTALL_STAGING_CMDS
+	$$(call install-erlang-directories,$$(STAGING_DIR),include)
+	$$(call install-rebar-deps,$$(STAGING_DIR),TARGET)
+endef
+endif
+
+# We need to double-$ the 'call' because it wants to expand
+# package-related variables
+ifndef $(2)_INSTALL_TARGET_CMDS
+define $(2)_INSTALL_TARGET_CMDS
+	$$(call install-erlang-directories,$$(TARGET_DIR))
+endef
+endif
+
+else # !target
+
+ifeq ($$($(2)_CONFIGURE),YES)
+$(2)_CONF_ENV += \
+	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_HOST_DEPS_DIR)"}' \
+	ERL_EI_LIBDIR=$$(HOST_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib
+endif
+
+ifndef $(2)_BUILD_CMDS
+define $(2)_BUILD_CMDS
+	cd '$$(@D)'; \
+	CC='$$(HOST_CC)' \
+	CFLAGS='$$(HOST_CFLAGS)' \
+	LDFLAGS='$$(HOST_LDFLAGS)' \
+	ERL_COMPILER_OPTIONS='{i, "$$(REBAR_HOST_DEPS_DIR)"}' \
+	ERL_EI_LIBDIR=$$(HOST_DIR)/usr/lib/erlang/lib/erl_interface-$$(ERLANG_EI_VSN)/lib \
+	$$(HOST_MAKE_ENV) \
+	$$($$(PKG)_ENV) $$($$(PKG)_REBAR) deps_dir=$$(REBAR_HOST_DEPS_DIR) compile
+endef
+endif
+
+# We need to double-$ the 'call' because it wants to expand
+# package-related variables
+ifndef $(2)_INSTALL_CMDS
+define $(2)_INSTALL_CMDS
+	$$(call install-erlang-directories,$$(HOST_DIR),include)
+	$$(call install-rebar-deps,$$(HOST_DIR),HOST)
+endef
+endif
+
+endif # !target
+
+# The package sub-infra to use
+#
+ifeq ($$($(2)_CONFIGURE),YES)
+$(call inner-autotools-package,$(1),$(2),$(3),$(4))
+else
+$(call inner-generic-package,$(1),$(2),$(3),$(4))
+endif
+
+endef # inner-rebar-package
+
+rebar-package = $(call inner-rebar-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+host-rebar-package = $(call inner-rebar-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)