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

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

[Jonathan Borne via buildroot]

Hi,
I stumbled upon a suspicious line in binutils.mk.

In BINUTILS_INSTALL_TARGET_CMDS,
libopcode and libbfd DESTDIR are both set to $(TARGET_DIR)
and libiberty is the only one set to $(STAGING_DIR).

Is there a reason behind this?
Sorry if this is an obvious question, I am quite new to buildroot.
I assume I am probably missing something.
I though it was still better to ask...

Thanks
Best regards.
Jonathan.


Jonathan Borne(1):
  Fix binutils/libiberty target install directory.

 package/binutils/binutils.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Jonathan Borne via buildroot]

Change libiberty target installation directory to $(TARGET_DIR).

Signed-off-by: Jonathan Borne <jborne@kalray.eu>
---
 package/binutils/binutils.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/binutils/binutils.mk b/package/binutils/binutils.mk
index 13f61be035..d2119e67cc 100644
--- a/package/binutils/binutils.mk
+++ b/package/binutils/binutils.mk
@@ -106,7 +106,7 @@ 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
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/libiberty DESTDIR=$(TARGET_DIR) install
 endef
 endif
 
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Baruch Siach via buildroot]

Hi Jonathan,

On Mon, Jan 24 2022, Jonathan Borne via buildroot wrote:
> Hi,
> I stumbled upon a suspicious line in binutils.mk.
>
> In BINUTILS_INSTALL_TARGET_CMDS,
> libopcode and libbfd DESTDIR are both set to $(TARGET_DIR)
> and libiberty is the only one set to $(STAGING_DIR).
>
> Is there a reason behind this?

As I understand that's because libiberty is static only. Buildroot does
not copy static libraries to $(TARGET_DIR) because they are only used at
build time, but not at run-time.

baruch

> Sorry if this is an obvious question, I am quite new to buildroot.
> I assume I am probably missing something.
> I though it was still better to ask...
>
> Thanks
> Best regards.
> Jonathan.
>
>
> Jonathan Borne(1):
>   Fix binutils/libiberty target install directory.
>
>  package/binutils/binutils.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Romain Naour]

Hello,

Le 24/01/2022 à 18:44, Baruch Siach via buildroot a écrit :
> Hi Jonathan,
> 
> On Mon, Jan 24 2022, Jonathan Borne via buildroot wrote:
>> Hi,
>> I stumbled upon a suspicious line in binutils.mk.
>>
>> In BINUTILS_INSTALL_TARGET_CMDS,
>> libopcode and libbfd DESTDIR are both set to $(TARGET_DIR)
>> and libiberty is the only one set to $(STAGING_DIR).
>>
>> Is there a reason behind this?
> 
> As I understand that's because libiberty is static only. Buildroot does
> not copy static libraries to $(TARGET_DIR) because they are only used at
> build time, but not at run-time.

Maybe we can just remove libiberty (static) from BINUTILS_INSTALL_TARGET_CMDS
because it's already installed in staging by BINUTILS_INSTALL_STAGING_CMDS

Best regards,
Romain


> 
> baruch
> 
>> Sorry if this is an obvious question, I am quite new to buildroot.
>> I assume I am probably missing something.
>> I though it was still better to ask...
>>
>> Thanks
>> Best regards.
>> Jonathan.
>>
>>
>> Jonathan Borne(1):
>>   Fix binutils/libiberty target install directory.
>>
>>  package/binutils/binutils.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni]

On Mon, 24 Jan 2022 19:44:29 +0200
Baruch Siach via buildroot <buildroot@buildroot.org> wrote:

> > Is there a reason behind this?  
> 
> As I understand that's because libiberty is static only. Buildroot does
> not copy static libraries to $(TARGET_DIR) because they are only used at
> build time, but not at run-time.

True, but it doesn't make sense to do that in
BINUTILS_INSTALL_TARGET_CMDS. It should go in
BINUTILS_INSTALL_STAGING_CMDS, where it is already present, as Romain
pointed out.

So basically:

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

needs to be dropped from BINUTILS_INSTALL_TARGET_CMDS, if indeed
libiberty is only installed as a static library.

However, taking a step back:

# 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.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Thomas, All,

On 2022-01-24 22:02 +0100, Thomas Petazzoni spake thusly:
> On Mon, 24 Jan 2022 19:44:29 +0200
> Baruch Siach via buildroot <buildroot@buildroot.org> wrote:
> 
> > > Is there a reason behind this?  
> > 
> > As I understand that's because libiberty is static only. Buildroot does
> > not copy static libraries to $(TARGET_DIR) because they are only used at
> > build time, but not at run-time.
> 
> True, but it doesn't make sense to do that in
> BINUTILS_INSTALL_TARGET_CMDS. It should go in
> BINUTILS_INSTALL_STAGING_CMDS, where it is already present, as Romain
> pointed out.
> 
> So basically:
> 
>         $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/libiberty DESTDIR=$(STAGING_DIR) install
> 
> needs to be dropped from BINUTILS_INSTALL_TARGET_CMDS, if indeed
> libiberty is only installed as a static library.
> 
> However, taking a step back:
> 
> # 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.

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.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni]

On Mon, 24 Jan 2022 22:21:49 +0100
"Yann E. MORIN" <yann.morin.1998@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.

Aah, indeed, the condition is:

ifneq ($(BR2_PACKAGE_BINUTILS_TARGET),y)

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

Correct!

Thanks for pointing me at my own mistake :-)

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Jonathan Borne via buildroot]

Hi again, 
thank you very much for your answers!

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

> On Mon, 24 Jan 2022 22:21:49 +0100
> "Yann E. MORIN" <yann.morin.1998@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.




_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni]

On Wed, 26 Jan 2022 16:30:44 +0100 (CET)
Jonathan Borne <jborne@kalray.eu> wrote:

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

This is never going to be true, as disabled options have an empty
value. To work, it should be:

ifeq ($(BR2_PACKAGE_BINUTILS_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.

Yes!

Thanks,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Jonathan, All,

On 2022-01-26 16:30 +0100, Jonathan Borne via buildroot spake thusly:
> ----- On Jan 24, 2022, at 10:53 PM, Thomas Petazzoni thomas.petazzoni@bootlin.com wrote:
> > On Mon, 24 Jan 2022 22:21:49 +0100
> > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> >> 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.

Hehe... ;-)

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

Yes, but to avoid any confusion, please also add a comment above the
staging installation commands that states that libiberty is static only,
as Baruch pointed out earlier.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot