[Buildroot] [PATCH 0/2] package: pkg-autotools: New $(PKG)_AUTOGEN variable

[Buildroot] [PATCH 0/2] package: pkg-autotools: New $(PKG)_AUTOGEN variable

[Vadim Kochan]

Add possibility to automatically call autogen.sh by setting $(PKG)_AUTOGEN = YES
variable. This will simplify a bit packages which should call autogen.sh to generate
'configure' script.

Vadim Kochan (2):
  package: pkg-autotools: Add option to run autogen.sh
  package: Use $(PKG)_AUTOGEN instead of calling autogen.sh manually

 docs/manual/adding-packages-autotools.txt |  4 ++++
 package/faifa/faifa.mk                    | 10 +---------
 package/gpm/gpm.mk                        |  8 +-------
 package/pkg-autotools.mk                  | 20 ++++++++++++++++++++
 package/sdl/sdl.mk                        | 10 +---------
 5 files changed, 27 insertions(+), 25 deletions(-)

-- 
2.14.1

[Buildroot] [PATCH 1/2] package: pkg-autotools: Add option to run autogen.sh

[Vadim Kochan]

Add $(PKG)_AUTOGEN variable which allows to execute autogen.sh script
on pre-configure stage.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 docs/manual/adding-packages-autotools.txt |  4 ++++
 package/pkg-autotools.mk                  | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/docs/manual/adding-packages-autotools.txt b/docs/manual/adding-packages-autotools.txt
index a041d91eb6..105c038961 100644
--- a/docs/manual/adding-packages-autotools.txt
+++ b/docs/manual/adding-packages-autotools.txt
@@ -159,6 +159,10 @@ cases, typical packages will therefore only use a few of them.
   value is correct for most autotools packages, but it is still possible
   to override it if needed.
 
+* +LIBFOO_AUTOGEN+, tells whether the autogen.sh script should be
+  executed to generate 'configure' script. Valid values are +YES+ and +NO+.
+  By default, the value is +NO+
+
 With the autotools infrastructure, all the steps required to build
 and install the packages are already defined, and they generally work
 well for most autotools-based packages. However, when required, it is
diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
index 45de99356f..4f0f93d71c 100644
--- a/package/pkg-autotools.mk
+++ b/package/pkg-autotools.mk
@@ -103,6 +103,14 @@ define AUTORECONF_HOOK
 	$(Q)cd $($(PKG)_SRCDIR) && $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
 endef
 
+#
+# Hook to call autogen.sh if needed
+#
+define AUTOGEN_HOOK
+	@$(call MESSAGE,"Running autogen.sh")
+	$(Q)cd $($(PKG)_SRCDIR) && PATH=$(BR_PATH) ./autogen.sh
+endef
+
 ################################################################################
 # inner-autotools-package -- defines how the configuration, compilation and
 # installation of an autotools package should be done, implements a
@@ -144,6 +152,14 @@ ifndef $(2)_AUTORECONF
  endif
 endif
 
+ifndef $(2)_AUTOGEN
+ ifdef $(3)_AUTOGEN
+  $(2)_AUTOGEN = $$($(3)_AUTOGEN)
+ else
+  $(2)_AUTOGEN ?= NO
+ endif
+endif
+
 ifndef $(2)_GETTEXTIZE
  ifdef $(3)_GETTEXTIZE
   $(2)_GETTEXTIZE = $$($(3)_GETTEXTIZE)
@@ -241,6 +257,10 @@ endif
 
 $(2)_POST_PATCH_HOOKS += UPDATE_CONFIG_HOOK
 
+ifeq ($$($(2)_AUTOGEN),YES)
+$(2)_PRE_CONFIGURE_HOOKS += AUTOGEN_HOOK
+endif
+
 ifeq ($$($(2)_AUTORECONF),YES)
 
 # This has to come before autoreconf
-- 
2.14.1

[Buildroot] [PATCH 2/2] package: Use $(PKG)_AUTOGEN instead of calling autogen.sh manually

[Vadim Kochan]

Use $(PKG)_AUTOGEN variable whenever it is possible instead of
custom pre-configure hooks which calls autogen.sh manually.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 package/faifa/faifa.mk | 10 +---------
 package/gpm/gpm.mk     |  8 +-------
 package/sdl/sdl.mk     | 10 +---------
 3 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/package/faifa/faifa.mk b/package/faifa/faifa.mk
index af7a1741ff..0af87c841f 100644
--- a/package/faifa/faifa.mk
+++ b/package/faifa/faifa.mk
@@ -10,18 +10,10 @@ FAIFA_INSTALL_STAGING = YES
 FAIFA_DEPENDENCIES = libpcap host-autoconf
 FAIFA_LICENSE = BSD-3-Clause
 FAIFA_LICENSE_FILES = COPYING
+FAIFA_AUTOGEN = YES
 
 FAIFA_MAKE_OPTS += GIT_REV=$(FAIFA_VERSION)
 
-# This package uses autoconf, but not automake, so we need to call
-# their special autogen.sh script, and have custom target and staging
-# installation commands.
-
-define FAIFA_RUN_AUTOGEN
-	cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
-endef
-FAIFA_PRE_CONFIGURE_HOOKS += FAIFA_RUN_AUTOGEN
-
 define FAIFA_INSTALL_TARGET_CMDS
 	$(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
 		PREFIX=/usr \
diff --git a/package/gpm/gpm.mk b/package/gpm/gpm.mk
index f59d059a41..ebe210b908 100644
--- a/package/gpm/gpm.mk
+++ b/package/gpm/gpm.mk
@@ -11,6 +11,7 @@ GPM_LICENSE = GPL-2.0+
 GPM_LICENSE_FILES = COPYING
 GPM_INSTALL_STAGING = YES
 GPM_DEPENDENCIES = host-bison
+GPM_AUTOGEN = YES
 
 # if not already installed in staging dir, gpm Makefile may fail to find some
 # of the headers needed to generate build dependencies, the first time it is
@@ -34,13 +35,6 @@ endif
 # http://invisible-island.net/ncurses/ncurses.faq.html#using_gpm_lib
 GPM_CONF_OPTS = --without-curses
 
-# configure is missing but gpm seems not compatible with our autoreconf
-# mechanism so we have to do it manually instead of using GPM_AUTORECONF = YES
-define GPM_RUN_AUTOGEN
-	cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
-endef
-GPM_PRE_CONFIGURE_HOOKS += GPM_RUN_AUTOGEN
-
 GPM_DEPENDENCIES += host-automake host-autoconf host-libtool
 
 # gpm tries to build/install .info doc even if makeinfo isn't installed on the
diff --git a/package/sdl/sdl.mk b/package/sdl/sdl.mk
index 0a6a7de139..a0fa69d6df 100644
--- a/package/sdl/sdl.mk
+++ b/package/sdl/sdl.mk
@@ -10,15 +10,7 @@ SDL_SITE = http://www.libsdl.org/release
 SDL_LICENSE = LGPL-2.1+
 SDL_LICENSE_FILES = COPYING
 SDL_INSTALL_STAGING = YES
-
-# we're patching configure.in, but package cannot autoreconf with our version of
-# autotools, so we have to do it manually instead of setting SDL_AUTORECONF = YES
-define SDL_RUN_AUTOGEN
-	cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
-endef
-
-SDL_PRE_CONFIGURE_HOOKS += SDL_RUN_AUTOGEN
-HOST_SDL_PRE_CONFIGURE_HOOKS += SDL_RUN_AUTOGEN
+SDL_AUTOGEN = YES
 
 SDL_DEPENDENCIES += host-automake host-autoconf host-libtool
 HOST_SDL_DEPENDENCIES += host-automake host-autoconf host-libtool
-- 
2.14.1

[Buildroot] [PATCH 1/2] package: pkg-autotools: Add option to run autogen.sh

[Yann E. MORIN]

Vadim., All

On 2018-12-24 00:37 +0200, Vadim Kochan spake thusly:
> Add $(PKG)_AUTOGEN variable which allows to execute autogen.sh script
> on pre-configure stage.

I'm not sure we want to have this in the infra. IIRC, we already spoke
about it in the past, more than once, and the conclusion had always been
that this was not demed useful.

First, we only have three autotools packages that need that. This is a
bit on the short lead to turn it into the infra.

Second, we don't want to make it easy to use autogen.sh. We prefer
developpers to use AUTORECONF=YES instead. Using autogen.sh should be
a last-resort option.

Thirdly, and lastly, not all packages have autogen.sh, some have it as
bootstrap.sh, for example Asterisk:
    http://git.asterisk.org/gitweb/?p=asterisk/asterisk.git;a=blob;f=bootstrap.sh

(Note that we don't need to call Asterisk's bootstrap.sh in Buildroot,
but that is just an example that such scripts are not always named
autogen.sh.)

So, my opinion is that we should not have it in the infra.

Regards,
Yann E. MORIN.

> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  docs/manual/adding-packages-autotools.txt |  4 ++++
>  package/pkg-autotools.mk                  | 20 ++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/docs/manual/adding-packages-autotools.txt b/docs/manual/adding-packages-autotools.txt
> index a041d91eb6..105c038961 100644
> --- a/docs/manual/adding-packages-autotools.txt
> +++ b/docs/manual/adding-packages-autotools.txt
> @@ -159,6 +159,10 @@ cases, typical packages will therefore only use a few of them.
>    value is correct for most autotools packages, but it is still possible
>    to override it if needed.
>  
> +* +LIBFOO_AUTOGEN+, tells whether the autogen.sh script should be
> +  executed to generate 'configure' script. Valid values are +YES+ and +NO+.
> +  By default, the value is +NO+
> +
>  With the autotools infrastructure, all the steps required to build
>  and install the packages are already defined, and they generally work
>  well for most autotools-based packages. However, when required, it is
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index 45de99356f..4f0f93d71c 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -103,6 +103,14 @@ define AUTORECONF_HOOK
>  	$(Q)cd $($(PKG)_SRCDIR) && $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
>  endef
>  
> +#
> +# Hook to call autogen.sh if needed
> +#
> +define AUTOGEN_HOOK
> +	@$(call MESSAGE,"Running autogen.sh")
> +	$(Q)cd $($(PKG)_SRCDIR) && PATH=$(BR_PATH) ./autogen.sh
> +endef
> +
>  ################################################################################
>  # inner-autotools-package -- defines how the configuration, compilation and
>  # installation of an autotools package should be done, implements a
> @@ -144,6 +152,14 @@ ifndef $(2)_AUTORECONF
>   endif
>  endif
>  
> +ifndef $(2)_AUTOGEN
> + ifdef $(3)_AUTOGEN
> +  $(2)_AUTOGEN = $$($(3)_AUTOGEN)
> + else
> +  $(2)_AUTOGEN ?= NO
> + endif
> +endif
> +
>  ifndef $(2)_GETTEXTIZE
>   ifdef $(3)_GETTEXTIZE
>    $(2)_GETTEXTIZE = $$($(3)_GETTEXTIZE)
> @@ -241,6 +257,10 @@ endif
>  
>  $(2)_POST_PATCH_HOOKS += UPDATE_CONFIG_HOOK
>  
> +ifeq ($$($(2)_AUTOGEN),YES)
> +$(2)_PRE_CONFIGURE_HOOKS += AUTOGEN_HOOK
> +endif
> +
>  ifeq ($$($(2)_AUTORECONF),YES)
>  
>  # This has to come before autoreconf
> -- 
> 2.14.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH 1/2] package: pkg-autotools: Add option to run autogen.sh

[Thomas Petazzoni]

Hello,

On Mon, 24 Dec 2018 10:21:59 +0100, Yann E. MORIN wrote:

> On 2018-12-24 00:37 +0200, Vadim Kochan spake thusly:
> > Add $(PKG)_AUTOGEN variable which allows to execute autogen.sh script
> > on pre-configure stage.  
> 
> I'm not sure we want to have this in the infra. IIRC, we already spoke
> about it in the past, more than once, and the conclusion had always been
> that this was not demed useful.
> 
> First, we only have three autotools packages that need that. This is a
> bit on the short lead to turn it into the infra.
> 
> Second, we don't want to make it easy to use autogen.sh. We prefer
> developpers to use AUTORECONF=YES instead. Using autogen.sh should be
> a last-resort option.
> 
> Thirdly, and lastly, not all packages have autogen.sh, some have it as
> bootstrap.sh, for example Asterisk:
>     http://git.asterisk.org/gitweb/?p=asterisk/asterisk.git;a=blob;f=bootstrap.sh
> 
> (Note that we don't need to call Asterisk's bootstrap.sh in Buildroot,
> but that is just an example that such scripts are not always named
> autogen.sh.)
> 
> So, my opinion is that we should not have it in the infra.

Overall, I agree with you. If we wanted to make this more flexible than
what we have today, I would suggest to allow customizing the command
that it used for autoreconfiguring.

Something like this:

$(2)_AUTORECONF_CMD ?= $(AUTORECONF)

define AUTORECONF_HOOK
        @$(call MESSAGE,"Autoreconfiguring")
        $(Q)cd $($(PKG)_SRCDIR) && $($(PKG)_AUTORECONF_ENV) $($(PKG)_AUTORECONF_CMD) $($(PKG)_AUTORECONF_OPTS)
endef

One drawback is that having a CMD variable doesn't really match our
typical CMDS pattern.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/2] package: pkg-autotools: Add option to run autogen.sh

[Yann E. MORIN]

Thomas, All,

On 2018-12-24 10:59 +0100, Thomas Petazzoni spake thusly:
> On Mon, 24 Dec 2018 10:21:59 +0100, Yann E. MORIN wrote:
> > On 2018-12-24 00:37 +0200, Vadim Kochan spake thusly:
> > > Add $(PKG)_AUTOGEN variable which allows to execute autogen.sh script
> > > on pre-configure stage.  
[--SNIP--]
> > So, my opinion is that we should not have it in the infra.
> 
> Overall, I agree with you. If we wanted to make this more flexible than
> what we have today, I would suggest to allow customizing the command
> that it used for autoreconfiguring.
> 
> Something like this:
> 
> $(2)_AUTORECONF_CMD ?= $(AUTORECONF)
> 
> define AUTORECONF_HOOK
>         @$(call MESSAGE,"Autoreconfiguring")
>         $(Q)cd $($(PKG)_SRCDIR) && $($(PKG)_AUTORECONF_ENV) $($(PKG)_AUTORECONF_CMD) $($(PKG)_AUTORECONF_OPTS)
> endef
> 
> One drawback is that having a CMD variable doesn't really match our
> typical CMDS pattern.

Indeed no, and besides, there are so many different commands being run
before running configure. You can get a peek at it and see the various
sneaky things we do:

    $ grep -E '_PRE_CONFIGURE_HOOKS' $(git grep -l -E '\$\(eval \$\((host-)?autotools-package\)\)')

One of the funniest being libtool, for which we need to carefully touch
some files to make configure believe they are up-to-date.

There are a bunch of packages running just 'autoconf' because they are
not in fact real autotools packages; they are just using autoconf, not
automake, so they do not autoreconf nicely.

Then we have at least one package that needs to chmod +x configure.

And all the various 'mkdir m4' we have here and there...

So, in the end, I stand by my position: no need to make that part of the
infra, the variance is too large.

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.  |
'------------------------------^-------^------------------^--------------------'