package: pkg-autotools: New $(PKG)_AUTOGEN variable

PATCH New Vadim Kochan 3 participants 6 messages Dec 23, 2018 – Dec 24, 2018

Share & tools

Vadim Kochan

author

2018-12-23 22:37 UTC #0 lore raw

cover package: pkg-autotools: New $(PKG)_AUTOGEN variable

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(-)

Signature · 1 line

2.14.1

Vadim Kochan

author

2018-12-23 22:37 UTC #1 lore raw

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

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

Signed-off-by Vadim Kochan <[email protected]>

---
 docs/manual/adding-packages-autotools.txt |  4 ++++
 package/pkg-autotools.mk                  | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

docs/manual/adding-packages-autotools.txt +4 -0

index a041d91eb6..105c038961 100644
@@ -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

package/pkg-autotools.mk +20 -0

index 45de99356f..4f0f93d71c 100644
@@ -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

Signature · 1 line

2.14.1

Yann E. MORIN

2018-12-24 09:21 UTC #2 lore raw

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

Vadim., All

On 2018-12-24 00:37 +0200, Vadim Kochan spake thusly:

Quoted 2 lines

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.

Quoted 73 lines

Signed-off-by Vadim Kochan <[email protected]>

---
 docs/manual/adding-packages-autotools.txt |  4 ++++
 package/pkg-autotools.mk                  | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

docs/manual/adding-packages-autotools.txt +4 -0

index a041d91eb6..105c038961 100644
@@ -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

package/pkg-autotools.mk +20 -0

index 45de99356f..4f0f93d71c 100644
@@ -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

Signature · 6 lines

2.14.1

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

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

Thomas Petazzoni

2018-12-24 09:59 UTC #3 lore raw

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

Hello,

Quoted 19 lines · Yann E. MORIN

On 2018-12-24 00:37 +0200, Vadim Kochan spake thusly:
Quoted 2 lines
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

Quoted 5 lines

(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

Signature · 3 lines

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

Yann E. MORIN

2018-12-24 11:22 UTC #4 lore raw

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

Thomas, All,

On 2018-12-24 10:59 +0100, Thomas Petazzoni spake thusly:

Quoted 4 lines · Yann E. MORIN

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

[--SNIP--]

Quoted 10 lines

Quoted 1 line
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)

Quoted 7 lines

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.

Signature · 6 lines

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

Vadim Kochan

author

2018-12-23 22:37 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 package/faifa/faifa.mk | 10 +---------
 package/gpm/gpm.mk     |  8 +-------
 package/sdl/sdl.mk     | 10 +---------
 3 files changed, 3 insertions(+), 25 deletions(-)

package/faifa/faifa.mk +1 -9

index af7a1741ff..0af87c841f 100644
@@ -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 \

package/gpm/gpm.mk +1 -7

index f59d059a41..ebe210b908 100644
@@ -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

package/sdl/sdl.mk +1 -9

index 0a6a7de139..a0fa69d6df 100644
@@ -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

Signature · 1 line

2.14.1