diff mbox

package/autotool infra: create missing m4 dir

Message ID 1345294113-11303-1-git-send-email-yann.morin.1998@free.fr
State Rejected
Headers show

Commit Message

Yann E. MORIN Aug. 18, 2012, 12:48 p.m. UTC
When a package needs to be auto-reconfigured, it may be missing the m4/
sub-dir, especially when we use the package from its VCS.

Grep configure.{ac,in} tp find the location of the m4/ sub-dir (defaulting
to the root of the package), and create it.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---

This is a tentative patch for RFC.
---
 package/pkg-autotools.mk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Comments

Thomas Petazzoni Aug. 21, 2012, 9:33 a.m. UTC | #1
Hello,

Le Sat, 18 Aug 2012 14:48:33 +0200,
"Yann E. MORIN" <yann.morin.1998@free.fr> a écrit :

> When a package needs to be auto-reconfigured, it may be missing the m4/
> sub-dir, especially when we use the package from its VCS.
> 
> Grep configure.{ac,in} tp find the location of the m4/ sub-dir (defaulting
> to the root of the package), and create it.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> ---
> 
> This is a tentative patch for RFC.
> ---
>  package/pkg-autotools.mk |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index 785daab..f199ac6 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -173,6 +173,7 @@ endif
>  #
>  define AUTORECONF_HOOK
>  	@$$(call MESSAGE,"Autoreconfiguring")
> +	M4_DIR=$$$$(sed -r -e '/.*AC_CONFIG_MACRO_DIR\(\[(.*)\]\).*/!d; s//\1/g;' $$($$(PKG)_SRCDIR)configure.*) && mkdir -p "$$($$(PKG)_SRCDIR)$$$${M4_DIR:-m4}"
>  	$(Q)cd $$($$(PKG)_SRCDIR) && $(AUTORECONF) $$($$(PKG)_AUTORECONF_OPT)
>  	$(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES"; then \
>  		for i in `find $$($$(PKG)_SRCDIR) -name ltmain.sh`; do \

Thanks for doing this. I am not yet totally convinced, but it needs
some discussion. I find this approach a bit fragile, and I'm not sure
it will cover all possibilities: I've seen packages doing more funky
things than just creating the AC_CONFIG_MACRO_DIR directory in their
autogen.sh/bootstrap script. For example, calling glibtoolize or
intltoolize, which I'm not sure are automatically being called by
autoreconf (but I may be wrong).

So, I see two alternative approaches to yours (I'm not saying those
alternatives are better, just mentioning them to open the discussion):

 1. Add an explicit <pkg>_AC_CONFIG_MACRO_DIR option, which defaults to
    m4, and which can be overridden by the package .mk file. The
    autotools infrastructure then creates in the source tree a
    directory of this name before doing the autoreconfiguration.

 2. Make the autoreconf command configurable. I.e, by default the
    autotools infrastructure does $(AUTORECONF)
    $$($$(PKG)_AUTORECONF_OPT), but maybe we could use a
    $$($$(PKG)_AUTORECONF_CMDS) instead, so that you could do 'mkdir
    $(@D)/m4; $(@D)/autogen.sh' if you need, etc.

Thoughts?

Best regards,

Thomas
Yann E. MORIN Aug. 21, 2012, 4:08 p.m. UTC | #2
Thomas, All,

On Tuesday 21 August 2012 11:33:12 Thomas Petazzoni wrote:
> Le Sat, 18 Aug 2012 14:48:33 +0200,
> "Yann E. MORIN" <yann.morin.1998@free.fr> a écrit :
> 
> > When a package needs to be auto-reconfigured, it may be missing the m4/
> > sub-dir, especially when we use the package from its VCS.
> > 
> > Grep configure.{ac,in} tp find the location of the m4/ sub-dir (defaulting
> > to the root of the package), and create it.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > ---
> > 
> > This is a tentative patch for RFC.
> > ---
> >  package/pkg-autotools.mk |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> > index 785daab..f199ac6 100644
> > --- a/package/pkg-autotools.mk
> > +++ b/package/pkg-autotools.mk
> > @@ -173,6 +173,7 @@ endif
> >  #
> >  define AUTORECONF_HOOK
> >  	@$$(call MESSAGE,"Autoreconfiguring")
> > +	M4_DIR=$$$$(sed -r -e '/.*AC_CONFIG_MACRO_DIR\(\[(.*)\]\).*/!d; s//\1/g;' $$($$(PKG)_SRCDIR)configure.*) && mkdir -p "$$($$(PKG)_SRCDIR)$$$${M4_DIR:-m4}"
> >  	$(Q)cd $$($$(PKG)_SRCDIR) && $(AUTORECONF) $$($$(PKG)_AUTORECONF_OPT)
> >  	$(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES"; then \
> >  		for i in `find $$($$(PKG)_SRCDIR) -name ltmain.sh`; do \
> 
> Thanks for doing this. I am not yet totally convinced, 

Yep, that's why I advertised it as an RFC. ;-)

> but it needs
> some discussion. I find this approach a bit fragile, and I'm not sure
> it will cover all possibilities: I've seen packages doing more funky
> things than just creating the AC_CONFIG_MACRO_DIR directory in their
> autogen.sh/bootstrap script. For example, calling glibtoolize or
> intltoolize, which I'm not sure are automatically being called by
> autoreconf (but I may be wrong).
> 
> So, I see two alternative approaches to yours (I'm not saying those
> alternatives are better, just mentioning them to open the discussion):
> 
>  1. Add an explicit <pkg>_AC_CONFIG_MACRO_DIR option, which defaults to
>     m4, and which can be overridden by the package .mk file. The
>     autotools infrastructure then creates in the source tree a
>     directory of this name before doing the autoreconfiguration.
> 
>  2. Make the autoreconf command configurable. I.e, by default the
>     autotools infrastructure does $(AUTORECONF)
>     $$($$(PKG)_AUTORECONF_OPT), but maybe we could use a
>     $$($$(PKG)_AUTORECONF_CMDS) instead, so that you could do 'mkdir
>     $(@D)/m4; $(@D)/autogen.sh' if you need, etc.
> 
> Thoughts?

Well...
1) is pretty easy to do, but might not cover all cases, far from it, from
what you said above. But it might be "good enough" it is catters for 90+%
of packages.

2) should be relatively easy too, and is very versatile. OTOH, if one needs
to run the buildroot's standard $(AUTORECONF), it can become a bit more
complex.

So, what is the current state of packages in buildroot?

- 2 packages create a m4/ sub-dir:
    $ grep -c -r -E 'mkdir .*m4' * |grep -v -E ':0$'                                                                                                                        
    libiscsi/libiscsi.mk:1    mine, not yet upstream, missing m4/ dir
    sysprof/sysprof.mk:1      uses POST_PATCH_HOOK

  --> so option 1) would really catter for only two packages

- 3 packages declare one or more PRE_CONFIG hooks:
    $ cd package; grep -c -r -E 'PRE_CONFIGURE_HOOKS[[:space:]]*[+:]?=' . \
      |grep -v -E ':0$'
    cups/cups.mk:1            calls only autoconf, standard autreconf breaks
    libiscsi/libiscsi.mk:1    mine, not yet upstream, missing m4/ dir
    socat/socat.mk:1          does not use automake, needs autoconf only

  --> so option 2) would really catter for only two packages

I guess that's not really important to provide such a mechanism.
Forget about my patch. ;-)

Regards,
Yann E. MORIN.
Arnout Vandecappelle Aug. 26, 2012, 9:24 p.m. UTC | #3
On 08/21/12 18:08, Yann E. MORIN wrote:
> Thomas, All,
>
> On Tuesday 21 August 2012 11:33:12 Thomas Petazzoni wrote:
>> Le Sat, 18 Aug 2012 14:48:33 +0200,
>> "Yann E. MORIN"<yann.morin.1998@free.fr>  a écrit :
>>
>>> When a package needs to be auto-reconfigured, it may be missing the m4/
>>> sub-dir, especially when we use the package from its VCS.
[snip]
>> but it needs
>> some discussion. I find this approach a bit fragile, and I'm not sure
>> it will cover all possibilities: I've seen packages doing more funky
>> things than just creating the AC_CONFIG_MACRO_DIR directory in their
>> autogen.sh/bootstrap script. For example, calling glibtoolize or
>> intltoolize, which I'm not sure are automatically being called by
>> autoreconf (but I may be wrong).
>>
>> So, I see two alternative approaches to yours (I'm not saying those
>> alternatives are better, just mentioning them to open the discussion):
>>
>>   1. Add an explicit<pkg>_AC_CONFIG_MACRO_DIR option, which defaults to
>>      m4, and which can be overridden by the package .mk file. The
>>      autotools infrastructure then creates in the source tree a
>>      directory of this name before doing the autoreconfiguration.
>>
>>   2. Make the autoreconf command configurable. I.e, by default the
>>      autotools infrastructure does $(AUTORECONF)
>>      $$($$(PKG)_AUTORECONF_OPT), but maybe we could use a
>>      $$($$(PKG)_AUTORECONF_CMDS) instead, so that you could do 'mkdir
>>      $(@D)/m4; $(@D)/autogen.sh' if you need, etc.
>>
>> Thoughts?
>
> Well...
> 1) is pretty easy to do, but might not cover all cases, far from it, from
> what you said above. But it might be "good enough" it is catters for 90+%
> of packages.
>
> 2) should be relatively easy too, and is very versatile. OTOH, if one needs
> to run the buildroot's standard $(AUTORECONF), it can become a bit more
> complex.
>
> So, what is the current state of packages in buildroot?
>
> - 2 packages create a m4/ sub-dir:
>      $ grep -c -r -E 'mkdir .*m4' * |grep -v -E ':0$'
>      libiscsi/libiscsi.mk:1    mine, not yet upstream, missing m4/ dir
>      sysprof/sysprof.mk:1      uses POST_PATCH_HOOK
>
>    -->  so option 1) would really catter for only two packages
>
> - 3 packages declare one or more PRE_CONFIG hooks:
>      $ cd package; grep -c -r -E 'PRE_CONFIGURE_HOOKS[[:space:]]*[+:]?=' . \
>        |grep -v -E ':0$'
>      cups/cups.mk:1            calls only autoconf, standard autreconf breaks
>      libiscsi/libiscsi.mk:1    mine, not yet upstream, missing m4/ dir
>      socat/socat.mk:1          does not use automake, needs autoconf only
>
>    -->  so option 2) would really catter for only two packages
>
> I guess that's not really important to provide such a mechanism.
> Forget about my patch. ;-)

  Even so, a "smarter" autoreconf would be less fragile than what we do now -
you'd expect the package's bootstrap.sh to be more complete than just a plain
autoreconf.  For instance, I had a package once that required a Changelog file
to be present.  We could add a $(PKG)_AUTORECONF_SCRIPT variable that defines
the script.

  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
index 785daab..f199ac6 100644
--- a/package/pkg-autotools.mk
+++ b/package/pkg-autotools.mk
@@ -173,6 +173,7 @@  endif
 #
 define AUTORECONF_HOOK
 	@$$(call MESSAGE,"Autoreconfiguring")
+	M4_DIR=$$$$(sed -r -e '/.*AC_CONFIG_MACRO_DIR\(\[(.*)\]\).*/!d; s//\1/g;' $$($$(PKG)_SRCDIR)configure.*) && mkdir -p "$$($$(PKG)_SRCDIR)$$$${M4_DIR:-m4}"
 	$(Q)cd $$($$(PKG)_SRCDIR) && $(AUTORECONF) $$($$(PKG)_AUTORECONF_OPT)
 	$(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES"; then \
 		for i in `find $$($$(PKG)_SRCDIR) -name ltmain.sh`; do \