diff mbox series

[2/3] package/automake: also include autoconf-archive in search paths

Message ID e78e62c9d171941a0c8c8a93ba73d3edade4a9b1.1581261155.git.yann.morin.1998@free.fr
State Rejected
Headers show
Series [1/3] package/libsigrok: drop remnants of autoreconf | expand

Commit Message

Yann E. MORIN Feb. 9, 2020, 3:12 p.m. UTC
Since d255b67972 (autotools: do not overwrite first include path), the
ordering of include paths has changed: the system directories are
specified with explicit options passed to autoreconf, which means that
any directory specified in the package _AUTORECONF_OPTS are no longer
first:

  - in package/autoconf/autoconf.mk, we define AUTORECONF as:
    AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I "$(ACLOCAL_HOST_DIR)"

  - in package/pkg-autotools.mk, we call AUTORECONF with:
    $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)

For a package that needs autoconf-archive, this means that it has to
provide a custom include directive, in its own _AUTORECONF_OPTS. This in
turn means that this directory becomes the first, and goes directly
opposite to what d255b67972 was supposed to accomplish.

However, the path to the autoconf archive macro directory is mnot
searched by default either, so a package has no way to add such an
aclocal include directive.

We can only add it in the global search directory, and we do so only for
those packages that have autoconf-archive in their dependencies.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Michael Walle <michael@walle.cc>
Cc: Heiko Thiery <heiko.thiery@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 package/automake/automake.mk | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Feb. 9, 2020, 3:24 p.m. UTC | #1
All,

On 2020-02-09 16:12 +0100, Yann E. MORIN spake thusly:
> Since d255b67972 (autotools: do not overwrite first include path), the
> ordering of include paths has changed: the system directories are
> specified with explicit options passed to autoreconf, which means that
> any directory specified in the package _AUTORECONF_OPTS are no longer
> first:
> 
>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I "$(ACLOCAL_HOST_DIR)"
> 
>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
> 
> For a package that needs autoconf-archive, this means that it has to
> provide a custom include directive, in its own _AUTORECONF_OPTS. This in
> turn means that this directory becomes the first, and goes directly
> opposite to what d255b67972 was supposed to accomplish.
> 
> However, the path to the autoconf archive macro directory is mnot
> searched by default either, so a package has no way to add such an
> aclocal include directive.
> 
> We can only add it in the global search directory, and we do so only for
> those packages that have autoconf-archive in their dependencies.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Heiko Thiery <heiko.thiery@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  package/automake/automake.mk | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/package/automake/automake.mk b/package/automake/automake.mk
> index 89dcaa1293..238116cb94 100644
> --- a/package/automake/automake.mk
> +++ b/package/automake/automake.mk
> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>  AUTOMAKE = $(HOST_DIR)/bin/automake
>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>  ACLOCAL = $(HOST_DIR)/bin/aclocal
> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
> +	$(ACLOCAL_DIR) \
> +	$(ACLOCAL_HOST_DIR) \
> +	$(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
> +		$(HOST_DIR)/share/autoconf-archive \
> +	) \
> +))

This makes check-package whine because it is not indented. Sigh...
Indenting with a TAB fixes the check-package error, and is still a
working fix.

Regards,
Yann E. MORIN.

>  export ACLOCAL_PATH
> -- 
> 2.20.1
>
Heiko Thiery Feb. 9, 2020, 7:09 p.m. UTC | #2
Hi Yann,

Am So., 9. Feb. 2020 um 16:12 Uhr schrieb Yann E. MORIN
<yann.morin.1998@free.fr>:
>
> Since d255b67972 (autotools: do not overwrite first include path), the
> ordering of include paths has changed: the system directories are
> specified with explicit options passed to autoreconf, which means that
> any directory specified in the package _AUTORECONF_OPTS are no longer
> first:
>
>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I "$(ACLOCAL_HOST_DIR)"
>
>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
>
> For a package that needs autoconf-archive, this means that it has to
> provide a custom include directive, in its own _AUTORECONF_OPTS. This in
> turn means that this directory becomes the first, and goes directly
> opposite to what d255b67972 was supposed to accomplish.
>
> However, the path to the autoconf archive macro directory is mnot
> searched by default either, so a package has no way to add such an
> aclocal include directive.
>
> We can only add it in the global search directory, and we do so only for
> those packages that have autoconf-archive in their dependencies.
>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Heiko Thiery <heiko.thiery@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>

Tested-by: Heiko Thiery <heiko.thiery@gmail.com>



> ---
>  package/automake/automake.mk | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/package/automake/automake.mk b/package/automake/automake.mk
> index 89dcaa1293..238116cb94 100644
> --- a/package/automake/automake.mk
> +++ b/package/automake/automake.mk
> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>  AUTOMAKE = $(HOST_DIR)/bin/automake
>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>  ACLOCAL = $(HOST_DIR)/bin/aclocal
> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
> +       $(ACLOCAL_DIR) \
> +       $(ACLOCAL_HOST_DIR) \
> +       $(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
> +               $(HOST_DIR)/share/autoconf-archive \
> +       ) \
> +))
>  export ACLOCAL_PATH
> --
> 2.20.1
>
Michael Walle Feb. 10, 2020, 1:06 p.m. UTC | #3
Hi Yann,

Am 2020-02-09 16:24, schrieb Yann E. MORIN:
> All,
> 
> On 2020-02-09 16:12 +0100, Yann E. MORIN spake thusly:
>> Since d255b67972 (autotools: do not overwrite first include path), the
>> ordering of include paths has changed: the system directories are
>> specified with explicit options passed to autoreconf, which means that
>> any directory specified in the package _AUTORECONF_OPTS are no longer
>> first:
>> 
>>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I 
>> "$(ACLOCAL_HOST_DIR)"
>> 
>>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
>> 
>> For a package that needs autoconf-archive, this means that it has to
>> provide a custom include directive, in its own _AUTORECONF_OPTS. This 
>> in
>> turn means that this directory becomes the first, and goes directly
>> opposite to what d255b67972 was supposed to accomplish.
>> 
>> However, the path to the autoconf archive macro directory is mnot
>> searched by default either, so a package has no way to add such an
>> aclocal include directive.
>> 
>> We can only add it in the global search directory, and we do so only 
>> for
>> those packages that have autoconf-archive in their dependencies.
>> 
>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>> Cc: Michael Walle <michael@walle.cc>
>> Cc: Heiko Thiery <heiko.thiery@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: Peter Korsgaard <peter@korsgaard.com>
>> ---
>>  package/automake/automake.mk | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/package/automake/automake.mk 
>> b/package/automake/automake.mk
>> index 89dcaa1293..238116cb94 100644
>> --- a/package/automake/automake.mk
>> +++ b/package/automake/automake.mk
>> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>>  AUTOMAKE = $(HOST_DIR)/bin/automake
>>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>>  ACLOCAL = $(HOST_DIR)/bin/aclocal
>> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
>> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
>> +	$(ACLOCAL_DIR) \
>> +	$(ACLOCAL_HOST_DIR) \
>> +	$(if $(filter 
>> host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
>> +		$(HOST_DIR)/share/autoconf-archive \
>> +	) \
>> +))
> 
> This makes check-package whine because it is not indented. Sigh...
> Indenting with a TAB fixes the check-package error, and is still a
> working fix.

This makes me wonder why these are installed into share/autoconf-archive
instead of $(ACLOCAL_HOST_DIR) in the first place. Eg. on my debian 
system
all the macros of autoconf-archive are installed to /usr/share/aclocal.

-michael
Michael Walle Feb. 10, 2020, 1:09 p.m. UTC | #4
Am 2020-02-10 14:06, schrieb Michael Walle:
> Hi Yann,
> 
> Am 2020-02-09 16:24, schrieb Yann E. MORIN:
>> All,
>> 
>> On 2020-02-09 16:12 +0100, Yann E. MORIN spake thusly:
>>> Since d255b67972 (autotools: do not overwrite first include path), 
>>> the
>>> ordering of include paths has changed: the system directories are
>>> specified with explicit options passed to autoreconf, which means 
>>> that
>>> any directory specified in the package _AUTORECONF_OPTS are no longer
>>> first:
>>> 
>>>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>>>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I 
>>> "$(ACLOCAL_HOST_DIR)"
>>> 
>>>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>>>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
>>> 
>>> For a package that needs autoconf-archive, this means that it has to
>>> provide a custom include directive, in its own _AUTORECONF_OPTS. This 
>>> in
>>> turn means that this directory becomes the first, and goes directly
>>> opposite to what d255b67972 was supposed to accomplish.
>>> 
>>> However, the path to the autoconf archive macro directory is mnot
>>> searched by default either, so a package has no way to add such an
>>> aclocal include directive.
>>> 
>>> We can only add it in the global search directory, and we do so only 
>>> for
>>> those packages that have autoconf-archive in their dependencies.
>>> 
>>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>>> Cc: Michael Walle <michael@walle.cc>
>>> Cc: Heiko Thiery <heiko.thiery@gmail.com>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>> Cc: Peter Korsgaard <peter@korsgaard.com>
>>> ---
>>>  package/automake/automake.mk | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/package/automake/automake.mk 
>>> b/package/automake/automake.mk
>>> index 89dcaa1293..238116cb94 100644
>>> --- a/package/automake/automake.mk
>>> +++ b/package/automake/automake.mk
>>> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>>>  AUTOMAKE = $(HOST_DIR)/bin/automake
>>>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>>>  ACLOCAL = $(HOST_DIR)/bin/aclocal
>>> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
>>> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
>>> +	$(ACLOCAL_DIR) \
>>> +	$(ACLOCAL_HOST_DIR) \
>>> +	$(if $(filter 
>>> host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
>>> +		$(HOST_DIR)/share/autoconf-archive \
>>> +	) \
>>> +))
>> 
>> This makes check-package whine because it is not indented. Sigh...
>> Indenting with a TAB fixes the check-package error, and is still a
>> working fix.
> 
> This makes me wonder why these are installed into 
> share/autoconf-archive
> instead of $(ACLOCAL_HOST_DIR) in the first place. Eg. on my debian 
> system
> all the macros of autoconf-archive are installed to /usr/share/aclocal.

Ok, to answer this myself:
  
https://git.buildroot.net/buildroot/commit/?id=2c490f00c8d8a7d34ed6da9130cef8a5e97cef56

Still, maybe this has changed over time and there is no such bug 
anymore?

-michael
Arnout Vandecappelle Feb. 10, 2020, 1:14 p.m. UTC | #5
On 10/02/2020 14:09, Michael Walle wrote:
> Am 2020-02-10 14:06, schrieb Michael Walle:
>> Hi Yann,
>>
>> Am 2020-02-09 16:24, schrieb Yann E. MORIN:
>>> All,
>>>
>>> On 2020-02-09 16:12 +0100, Yann E. MORIN spake thusly:
>>>> Since d255b67972 (autotools: do not overwrite first include path), the
>>>> ordering of include paths has changed: the system directories are
>>>> specified with explicit options passed to autoreconf, which means that
>>>> any directory specified in the package _AUTORECONF_OPTS are no longer
>>>> first:
>>>>
>>>>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>>>>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I
>>>> "$(ACLOCAL_HOST_DIR)"
>>>>
>>>>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>>>>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) $($(PKG)_AUTORECONF_OPTS)
>>>>
>>>> For a package that needs autoconf-archive, this means that it has to
>>>> provide a custom include directive, in its own _AUTORECONF_OPTS. This in
>>>> turn means that this directory becomes the first, and goes directly
>>>> opposite to what d255b67972 was supposed to accomplish.
>>>>
>>>> However, the path to the autoconf archive macro directory is mnot
>>>> searched by default either, so a package has no way to add such an
>>>> aclocal include directive.
>>>>
>>>> We can only add it in the global search directory, and we do so only for
>>>> those packages that have autoconf-archive in their dependencies.
>>>>
>>>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>>>> Cc: Michael Walle <michael@walle.cc>
>>>> Cc: Heiko Thiery <heiko.thiery@gmail.com>
>>>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>>>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>>> Cc: Peter Korsgaard <peter@korsgaard.com>
>>>> ---
>>>>  package/automake/automake.mk | 8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/package/automake/automake.mk b/package/automake/automake.mk
>>>> index 89dcaa1293..238116cb94 100644
>>>> --- a/package/automake/automake.mk
>>>> +++ b/package/automake/automake.mk
>>>> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>>>>  AUTOMAKE = $(HOST_DIR)/bin/automake
>>>>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>>>>  ACLOCAL = $(HOST_DIR)/bin/aclocal
>>>> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
>>>> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
>>>> +    $(ACLOCAL_DIR) \
>>>> +    $(ACLOCAL_HOST_DIR) \
>>>> +    $(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
>>>> +        $(HOST_DIR)/share/autoconf-archive \
>>>> +    ) \
>>>> +))
>>>
>>> This makes check-package whine because it is not indented. Sigh...
>>> Indenting with a TAB fixes the check-package error, and is still a
>>> working fix.
>>
>> This makes me wonder why these are installed into share/autoconf-archive
>> instead of $(ACLOCAL_HOST_DIR) in the first place. Eg. on my debian system
>> all the macros of autoconf-archive are installed to /usr/share/aclocal.
> 
> Ok, to answer this myself:
>  
> https://git.buildroot.net/buildroot/commit/?id=2c490f00c8d8a7d34ed6da9130cef8a5e97cef56
> 
> 
> Still, maybe this has changed over time and there is no such bug anymore?

 No, such issues in packages are unavoidable. autoconf-macros routinely breaks
"API", which is OK because it's assumed to be vendored into the package source code.

 I believe Debian never uses it when a package is built, so they don't have that
problem.


 Regards,
 Arnout
Thomas Petazzoni Feb. 10, 2020, 1:27 p.m. UTC | #6
On Sun,  9 Feb 2020 16:12:41 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> diff --git a/package/automake/automake.mk b/package/automake/automake.mk
> index 89dcaa1293..238116cb94 100644
> --- a/package/automake/automake.mk
> +++ b/package/automake/automake.mk
> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>  AUTOMAKE = $(HOST_DIR)/bin/automake
>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>  ACLOCAL = $(HOST_DIR)/bin/aclocal
> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
> +	$(ACLOCAL_DIR) \
> +	$(ACLOCAL_HOST_DIR) \
> +	$(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
> +		$(HOST_DIR)/share/autoconf-archive \
> +	) \
> +))

Meh :-/ Can we add something more explicit than that in the
infrastructure? Poking around in the package dependencies like this to
second guess which path should be added to ACLOCAL_PATH is not really
great :-/

Thomas
Michael Walle Feb. 10, 2020, 1:50 p.m. UTC | #7
Am 2020-02-10 14:14, schrieb Arnout Vandecappelle:
> On 10/02/2020 14:09, Michael Walle wrote:
>> Am 2020-02-10 14:06, schrieb Michael Walle:
>>> Hi Yann,
>>> 
>>> Am 2020-02-09 16:24, schrieb Yann E. MORIN:
>>>> All,
>>>> 
>>>> On 2020-02-09 16:12 +0100, Yann E. MORIN spake thusly:
>>>>> Since d255b67972 (autotools: do not overwrite first include path), 
>>>>> the
>>>>> ordering of include paths has changed: the system directories are
>>>>> specified with explicit options passed to autoreconf, which means 
>>>>> that
>>>>> any directory specified in the package _AUTORECONF_OPTS are no 
>>>>> longer
>>>>> first:
>>>>> 
>>>>>   - in package/autoconf/autoconf.mk, we define AUTORECONF as:
>>>>>     AUTOCONF = $(HOST_DIR)/bin/autoconf -I "$(ACLOCAL_DIR)" -I
>>>>> "$(ACLOCAL_HOST_DIR)"
>>>>> 
>>>>>   - in package/pkg-autotools.mk, we call AUTORECONF with:
>>>>>     $($(PKG)_AUTORECONF_ENV) $(AUTORECONF) 
>>>>> $($(PKG)_AUTORECONF_OPTS)
>>>>> 
>>>>> For a package that needs autoconf-archive, this means that it has 
>>>>> to
>>>>> provide a custom include directive, in its own _AUTORECONF_OPTS. 
>>>>> This in
>>>>> turn means that this directory becomes the first, and goes directly
>>>>> opposite to what d255b67972 was supposed to accomplish.
>>>>> 
>>>>> However, the path to the autoconf archive macro directory is mnot
>>>>> searched by default either, so a package has no way to add such an
>>>>> aclocal include directive.
>>>>> 
>>>>> We can only add it in the global search directory, and we do so 
>>>>> only for
>>>>> those packages that have autoconf-archive in their dependencies.
>>>>> 
>>>>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>>>>> Cc: Michael Walle <michael@walle.cc>
>>>>> Cc: Heiko Thiery <heiko.thiery@gmail.com>
>>>>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>>>>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>>>> Cc: Peter Korsgaard <peter@korsgaard.com>
>>>>> ---
>>>>>  package/automake/automake.mk | 8 +++++++-
>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/package/automake/automake.mk 
>>>>> b/package/automake/automake.mk
>>>>> index 89dcaa1293..238116cb94 100644
>>>>> --- a/package/automake/automake.mk
>>>>> +++ b/package/automake/automake.mk
>>>>> @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
>>>>>  AUTOMAKE = $(HOST_DIR)/bin/automake
>>>>>  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
>>>>>  ACLOCAL = $(HOST_DIR)/bin/aclocal
>>>>> -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
>>>>> +ACLOCAL_PATH = $(subst $(space),:,$(strip \
>>>>> +    $(ACLOCAL_DIR) \
>>>>> +    $(ACLOCAL_HOST_DIR) \
>>>>> +    $(if $(filter 
>>>>> host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
>>>>> +        $(HOST_DIR)/share/autoconf-archive \
>>>>> +    ) \
>>>>> +))
>>>> 
>>>> This makes check-package whine because it is not indented. Sigh...
>>>> Indenting with a TAB fixes the check-package error, and is still a
>>>> working fix.
>>> 
>>> This makes me wonder why these are installed into 
>>> share/autoconf-archive
>>> instead of $(ACLOCAL_HOST_DIR) in the first place. Eg. on my debian 
>>> system
>>> all the macros of autoconf-archive are installed to 
>>> /usr/share/aclocal.
>> 
>> Ok, to answer this myself:
>>  
>> https://git.buildroot.net/buildroot/commit/?id=2c490f00c8d8a7d34ed6da9130cef8a5e97cef56
>> 
>> 
>> Still, maybe this has changed over time and there is no such bug 
>> anymore?
> 
>  No, such issues in packages are unavoidable. autoconf-macros routinely 
> breaks
> "API", which is OK because it's assumed to be vendored into the
> package source code.
> 
>  I believe Debian never uses it when a package is built, so they don't 
> have that
> problem.

I see, so actually the package should be fixed, correct? OTOH, it might 
also be
useful to support the building of bare packages (eg ones without the 
autoconf
macros inside the package). Wouldn't it then make more sense to have 
some kind of
variable which additional macros a package needs? and according to that 
variable,
the ACLOCAL_PATH is extended? I guess autoconf-archive isn't the only 
package which
can provide additional macros.

-michael
Thomas Petazzoni March 15, 2020, 3:30 p.m. UTC | #8
On Mon, 10 Feb 2020 14:27:21 +0100
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> On Sun,  9 Feb 2020 16:12:41 +0100
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
> > diff --git a/package/automake/automake.mk b/package/automake/automake.mk
> > index 89dcaa1293..238116cb94 100644
> > --- a/package/automake/automake.mk
> > +++ b/package/automake/automake.mk
> > @@ -33,5 +33,11 @@ $(eval $(host-autotools-package))
> >  AUTOMAKE = $(HOST_DIR)/bin/automake
> >  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
> >  ACLOCAL = $(HOST_DIR)/bin/aclocal
> > -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
> > +ACLOCAL_PATH = $(subst $(space),:,$(strip \
> > +	$(ACLOCAL_DIR) \
> > +	$(ACLOCAL_HOST_DIR) \
> > +	$(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
> > +		$(HOST_DIR)/share/autoconf-archive \
> > +	) \
> > +))  
> 
> Meh :-/ Can we add something more explicit than that in the
> infrastructure? Poking around in the package dependencies like this to
> second guess which path should be added to ACLOCAL_PATH is not really
> great :-/

So I've marked PATCH 2/3 and 3/3 as Rejected. We fixed the sdbusplus
issue by re-adding the hook creating the m4 directory. I understand the
idea of having more "automatic", but this approach of looking at the
dependencies of the package felt really odd.

We can always revisit this of course.

Thomas
diff mbox series

Patch

diff --git a/package/automake/automake.mk b/package/automake/automake.mk
index 89dcaa1293..238116cb94 100644
--- a/package/automake/automake.mk
+++ b/package/automake/automake.mk
@@ -33,5 +33,11 @@  $(eval $(host-autotools-package))
 AUTOMAKE = $(HOST_DIR)/bin/automake
 ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
 ACLOCAL = $(HOST_DIR)/bin/aclocal
-ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
+ACLOCAL_PATH = $(subst $(space),:,$(strip \
+	$(ACLOCAL_DIR) \
+	$(ACLOCAL_HOST_DIR) \
+	$(if $(filter host-autoconf-archive,$($(PKG)_FINAL_ALL_DEPENDENCIES)),\
+		$(HOST_DIR)/share/autoconf-archive \
+	) \
+))
 export ACLOCAL_PATH