[Buildroot] [RFC PATCH 0/1] Fix binutils/libiberty target install directory

[Jonathan Borne]

Hi again, 
thank you very much for your answers!

----- On Jan 24, 2022, at 10:53 PM, Thomas Petazzoni thomas.petazzoni at bootlin.com wrote:

> On Mon, 24 Jan 2022 22:21:49 +0100
> "Yann E. MORIN" <yann.morin.1998 at free.fr> wrote:
> 
>> > # We just want libbfd, libiberty and libopcodes,
>> > # not the full-blown binutils in staging
>> > define BINUTILS_INSTALL_STAGING_CMDS
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/bfd DESTDIR=$(STAGING_DIR) install
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/opcodes DESTDIR=$(STAGING_DIR) install
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/libiberty DESTDIR=$(STAGING_DIR) install
>> > endef
>> > 
>> > # If we don't want full binutils on target
>> > ifneq ($(BR2_PACKAGE_BINUTILS_TARGET),y)
>> > define BINUTILS_INSTALL_TARGET_CMDS
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/bfd DESTDIR=$(TARGET_DIR) install
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/opcodes DESTDIR=$(TARGET_DIR) install
>> >         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/libiberty DESTDIR=$(STAGING_DIR) install
>> > endef
>> > endif
>> > 
>> > This is *extremely* suspicious. Indeed, we get the libraries
>> > unconditionally installed in staging, so any other library or program
>> > that we build after binutils can potentially link against thosze
>> > libraries.
>> > 
>> > But we only install them conditionally in $(TARGET_DIR) if
>> > BR2_PACKAGE_BINUTILS_TARGET=y.
>> > 
>> > This seems very bad to me. Both target and staging installations should
>> > be done under the same condition.
>> 
>> Not really. You missed the fact that binutils is an autotools package.
>> 
>> As such, when BR2_PACKAGE_BINUTILS_TARGET=n, then the full binutils are
>> instaleld in target, so the libs will be installed in target.

I think you made a typo here... 
The full binutils install to target is made when BR2_PACKAGE_BINUTILS_TARGET=y.

> 
> Aah, indeed, the condition is:
> 
> ifneq ($(BR2_PACKAGE_BINUTILS_TARGET),y)
> 

Double negation is a bit confusing indeed, 
maybe the message could be changed to 

# If we don't want full binutils on target
ifeq ($(BR2_PACKAGE_BINUTILS_TARGET),n)

>> So whatever gets in staging is present in target, but target can be a
>> superset of what is in staging, so whatever links to a binutils lib from
>> staging will find it in target.

Agreed! So if I understand correctly, as was suggested by Romain and Thomas,
we could indeed get rid of

$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/libiberty DESTDIR=$(STAGING_DIR) install

in BINUTILS_INSTALL_TARGET_CMDS.

Thanks again
Best Regards.
Jonathan.