[RFC,v1,05/14] autotools infrastructure: do the autoreconf as a post patch step

Doing the autoreconf step as a pre-configure hook doesn't work
properly, because the source directory is read-only during the
configure step. And in fact, the autoreconf process modifies the
source code, so it is quite logical to do it as part of the patching
process rather than the configuration process.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-autotools.mk |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 01/21/13 00:52, Thomas Petazzoni wrote:
> Doing the autoreconf step as a pre-configure hook doesn't work
> properly, because the source directory is read-only during the
> configure step. And in fact, the autoreconf process modifies the
> source code, so it is quite logical to do it as part of the patching
> process rather than the configuration process.

  It was in fact moved from POST_PATCH to PRE_CONFIGURE in commit 
89d1ad91fe4b1b65f0e902f94aba99a6cefbf631, and the PRE_CONFIGURE_HOOKS 
were added specifically for this purpose. Unfortunately, the commit gives 
no explanation as to why it was needed.


  Regards,
  Arnout

>
> Signed-off-by: Thomas Petazzoni<thomas.petazzoni@free-electrons.com>
> ---
>   package/pkg-autotools.mk |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index bd87253..ec394fd 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -214,7 +214,7 @@ $(2)_DEPENDENCIES ?= $(filter-out host-automake host-autoconf host-libtool $(1),
>
>
>   ifeq ($$($(2)_AUTORECONF),YES)
> -$(2)_PRE_CONFIGURE_HOOKS += AUTORECONF_HOOK
> +$(2)_POST_PATCH_HOOKS += AUTORECONF_HOOK
>   $(2)_DEPENDENCIES += host-automake host-autoconf host-libtool
>   endif
>

Dear Arnout Vandecappelle,

On Sat, 26 Jan 2013 23:03:21 +0100, Arnout Vandecappelle wrote:
> On 01/21/13 00:52, Thomas Petazzoni wrote:
> > Doing the autoreconf step as a pre-configure hook doesn't work
> > properly, because the source directory is read-only during the
> > configure step. And in fact, the autoreconf process modifies the
> > source code, so it is quite logical to do it as part of the patching
> > process rather than the configuration process.
> 
>   It was in fact moved from POST_PATCH to PRE_CONFIGURE in commit 
> 89d1ad91fe4b1b65f0e902f94aba99a6cefbf631, and the PRE_CONFIGURE_HOOKS 
> were added specifically for this purpose. Unfortunately, the commit
> gives no explanation as to why it was needed.

Hum, right. Interesting. I've looked at the commits around this one,
and also the e-mails around the one through which this patch was
submitted, but it didn't give any clue. The other patches around are
version bumps and other, seemingly unrelated things.

It's even more disappointing that I was amongst the people giving a
Acked-by on this patch...

One reason that might explain this is the lack of ordering guarantees
on post patch hooks. For example, if you have a post patch hook that
applies Debian patches, and a post patch hook to do the autoreconf, you
quite certainly want the Debian patches hook to be executed before the
autoreconf hook. But I have absolutely no idea if this is the problem
that we were trying to fix here.

That said, I still believe that the autoreconf thing belongs to the
patch step. It is really a modification of the source code itself, and
it is common to both the target build and host build. So maybe the
autotools infrastructure needs a special hook in the generic
infrastructure (rather than a normal post patch hook), to ensure that
the autoreconf step gets executed after all post patch hooks?

Thomas

On 27/01/13 17:27, Thomas Petazzoni wrote:
> One reason that might explain this is the lack of ordering guarantees
> on post patch hooks. For example, if you have a post patch hook that
> applies Debian patches, and a post patch hook to do the autoreconf, you
> quite certainly want the Debian patches hook to be executed before the
> autoreconf hook. But I have absolutely no idea if this is the problem
> that we were trying to fix here.

  That sounds like a likely explanation...


> That said, I still believe that the autoreconf thing belongs to the
> patch step. It is really a modification of the source code itself, and
> it is common to both the target build and host build.

  I completely agree.


> So maybe the
> autotools infrastructure needs a special hook in the generic
> infrastructure (rather than a normal post patch hook), to ensure that
> the autoreconf step gets executed after all post patch hooks?

  Or maybe (just thinking out loud here) there could be the possibility 
for the package infrastructure to add additional steps in the dependency 
chain. Something like

ifeq ($$($(2)_AUTORECONF),YES)
$(2)_TARGET_AUTORECONFIGURE = $$($(3)_SRCDIR)/.stamp_autoreconfigured
$(1)-autoreconfigure: $(1)-patch $$($(2)_TARGET_AUTORECONFIGURE)
$(1)-configure: $(1)-autoreconfigure
endif


  Regards,
  Arnout

Dear Arnout Vandecappelle,

On Sun, 27 Jan 2013 23:18:39 +0100, Arnout Vandecappelle wrote:

> > So maybe the
> > autotools infrastructure needs a special hook in the generic
> > infrastructure (rather than a normal post patch hook), to ensure
> > that the autoreconf step gets executed after all post patch hooks?
> 
>   Or maybe (just thinking out loud here) there could be the
> possibility for the package infrastructure to add additional steps in
> the dependency chain. Something like
> 
> ifeq ($$($(2)_AUTORECONF),YES)
> $(2)_TARGET_AUTORECONFIGURE = $$($(3)_SRCDIR)/.stamp_autoreconfigured
> $(1)-autoreconfigure: $(1)-patch $$($(2)_TARGET_AUTORECONFIGURE)
> $(1)-configure: $(1)-autoreconfigure
> endif

Sounds interesting. I'll try to experiment around this idea.

Thanks,

Thomas