diff mbox series

[RFC,1/1] package/pkg-meson.mk: handle possibly non existing compilers

Message ID 20220816094829.377-1-guillaume.bressaix@gmail.com
State Superseded
Headers show
Series [RFC,1/1] package/pkg-meson.mk: handle possibly non existing compilers | expand

Commit Message

Guillaume Bres Aug. 16, 2022, 9:48 a.m. UTC
To avoid populating the cross-file with non existing compilers,
we tie them to /bin/false

Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
---
I only managed the CXX and FC case,
assuming all the other ones are always there ?
---
 package/pkg-meson.mk | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni Aug. 16, 2022, 10:01 a.m. UTC | #1
On Tue, 16 Aug 2022 11:48:29 +0200
"Guillaume W. Bres" <guillaume.bressaix@gmail.com> wrote:

> To avoid populating the cross-file with non existing compilers,
> we tie them to /bin/false
> 
> Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
> ---
> I only managed the CXX and FC case,
> assuming all the other ones are always there ?
> ---
>  package/pkg-meson.mk | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
> index 0632ab21cf..f246a126d1 100644
> --- a/package/pkg-meson.mk
> +++ b/package/pkg-meson.mk
> @@ -68,15 +68,29 @@ else
>  PKG_MESON_TARGET_CPU_FAMILY = $(ARCH)
>  endif
>  
> +# To avoid populating the cross-file with non existing compilers,
> +# we tie them to /bin/false
> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT_CXX),y)

This is not correct as this boolean only makes sense when using an
internal toolchain. You should use BR2_INSTALL_LIBSTDCPP (yes, I know
the name is crappy, but it's historical).

> +PKG_MESON_TARGET_CXX = $(TARGET_CXX)
> +else
> +PKG_MESON_TARGET_CXX = /bin/false
> +endif
> +
> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT_FORTRAN),y)

This should be BR2_TOOLCHAIN_HAS_FORTRAN

> +PKG_MESON_TARGET_FC = $(TARGET_FC)
> +else
> +PKG_MESON_TARGET_FC = /bin/false
> +endif

Now, the annoying question: should this be done in pkg-meson.mk, or
should we do it more globally, i.e define TARGET_CXX and TARGET_FC to
correct values only when C++ or Fortran are supported.

Yann, what do you think? Did we ever try to have TARGET_CXX=/bin/false
and TARGET_FC=/bin/false when there is no C++/Fortran support?

Best regards,

Thomas
Yann E. MORIN Aug. 16, 2022, 10:12 a.m. UTC | #2
Thomas, Guillaume, All,

On 2022-08-16 12:01 +0200, Thomas Petazzoni via buildroot spake thusly:
> On Tue, 16 Aug 2022 11:48:29 +0200
> "Guillaume W. Bres" <guillaume.bressaix@gmail.com> wrote:
> > To avoid populating the cross-file with non existing compilers,
> > we tie them to /bin/false
[--SNIP--]
> Now, the annoying question: should this be done in pkg-meson.mk, or
> should we do it more globally, i.e define TARGET_CXX and TARGET_FC to
> correct values only when C++ or Fortran are supported.
> 
> Yann, what do you think? Did we ever try to have TARGET_CXX=/bin/false
> and TARGET_FC=/bin/false when there is no C++/Fortran support?

I think my non-work identity would also remember commit bd39d11d2eaa
(core/infra: fix build on toolchain without C++), and the mess it
caused, which was fixed by commit 4cd1ab15886a (core: alternate solution
to disable C++).

So, yes, we tried for CXX, and no, we do not want to do it again. For
consistency, we should not try for other languages either, I believe.

Regards,
Yann E. MORIN.
Thomas Petazzoni Aug. 16, 2022, 11:53 a.m. UTC | #3
On Tue, 16 Aug 2022 12:12:49 +0200
<yann.morin@orange.com> wrote:

> I think my non-work identity would also remember commit bd39d11d2eaa
> (core/infra: fix build on toolchain without C++), and the mess it
> caused, which was fixed by commit 4cd1ab15886a (core: alternate solution
> to disable C++).

Thanks for the historical details on this one!

> So, yes, we tried for CXX, and no, we do not want to do it again. For
> consistency, we should not try for other languages either, I believe.

So, the Meson-specific solution proposed by Guillaume is relevant?

Thomas
Yann E. MORIN Aug. 16, 2022, 12:04 p.m. UTC | #4
Thomas, All,

On 2022-08-16 13:53 +0200, Thomas Petazzoni spake thusly:
> On Tue, 16 Aug 2022 12:12:49 +0200
> <yann.morin@orange.com> wrote:
[--SNIP--]
> > So, yes, we tried for CXX, and no, we do not want to do it again. For
> > consistency, we should not try for other languages either, I believe.
> So, the Meson-specific solution proposed by Guillaume is relevant?

Yes, I believe so. Disabling use/detection of tools should be a
per-infra solution: the meson solution (use /bin/false) is incompatible
with the autotools (which break with /bin/false).

As for why using /bin/false in meson, see:
    https://lore.kernel.org/buildroot/20220809162534.GE3168@scaer/

Regards,
Yann E. MORIN.
Guillaume Bres Aug. 16, 2022, 12:53 p.m. UTC | #5
Hello,

>> So, yes, we tried for CXX, and no, we do not want to do it again. For
>> consistency, we should not try for other languages either, I believe.
>So, the Meson-specific solution proposed by Guillaume is relevant

that was my understanding from Yann's clarifications this morning. I
submitted a V1 (non RFC) with your corrections, please disregard the RFC
from now on

Guillaume W. Bres
Software engineer
<guillaume.bressaix@gmail.com>


Le mar. 16 août 2022 à 14:04, <yann.morin@orange.com> a écrit :

> Thomas, All,
>
> On 2022-08-16 13:53 +0200, Thomas Petazzoni spake thusly:
> > On Tue, 16 Aug 2022 12:12:49 +0200
> > <yann.morin@orange.com> wrote:
> [--SNIP--]
> > > So, yes, we tried for CXX, and no, we do not want to do it again. For
> > > consistency, we should not try for other languages either, I believe.
> > So, the Meson-specific solution proposed by Guillaume is relevant?
>
> Yes, I believe so. Disabling use/detection of tools should be a
> per-infra solution: the meson solution (use /bin/false) is incompatible
> with the autotools (which break with /bin/false).
>
> As for why using /bin/false in meson, see:
>     https://lore.kernel.org/buildroot/20220809162534.GE3168@scaer/
>
> Regards,
> Yann E. MORIN.
>
> --
>                                         ____________
> .-----------------.--------------------:       _    :------------------.
> |  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
> | +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
> | +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
> |      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
> '--------------------------------------:______/_____:------------------'
>
>
>
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez
> recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
> electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou
> falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged
> information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and
> delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.
>
>
Arnout Vandecappelle Aug. 16, 2022, 8:59 p.m. UTC | #6
On 16/08/2022 12:12, yann.morin@orange.com wrote:
> Thomas, Guillaume, All,
> 
> On 2022-08-16 12:01 +0200, Thomas Petazzoni via buildroot spake thusly:
>> On Tue, 16 Aug 2022 11:48:29 +0200
>> "Guillaume W. Bres" <guillaume.bressaix@gmail.com> wrote:
>>> To avoid populating the cross-file with non existing compilers,
>>> we tie them to /bin/false
> [--SNIP--]
>> Now, the annoying question: should this be done in pkg-meson.mk, or
>> should we do it more globally, i.e define TARGET_CXX and TARGET_FC to
>> correct values only when C++ or Fortran are supported.
>>
>> Yann, what do you think? Did we ever try to have TARGET_CXX=/bin/false
>> and TARGET_FC=/bin/false when there is no C++/Fortran support?
> 
> I think my non-work identity would also remember commit bd39d11d2eaa
> (core/infra: fix build on toolchain without C++), and the mess it
> caused, which was fixed by commit 4cd1ab15886a (core: alternate solution
> to disable C++).
> 
> So, yes, we tried for CXX, and no, we do not want to do it again. For
> consistency, we should not try for other languages either, I believe.

  I think it's the other way round: /bin/false should work fine on sane build 
systems. It's just autotools that is special. And we have a special case for 
that (commit 4cd1ab15886a doesn't go away), so it will keep working.

  Well, assuming that cmake and others will behave correctly with CXX=/bin/false.

  Regards,
  Arnout

> 
> Regards,
> Yann E. MORIN.
>
diff mbox series

Patch

diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
index 0632ab21cf..f246a126d1 100644
--- a/package/pkg-meson.mk
+++ b/package/pkg-meson.mk
@@ -68,15 +68,29 @@  else
 PKG_MESON_TARGET_CPU_FAMILY = $(ARCH)
 endif
 
+# To avoid populating the cross-file with non existing compilers,
+# we tie them to /bin/false
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT_CXX),y)
+PKG_MESON_TARGET_CXX = $(TARGET_CXX)
+else
+PKG_MESON_TARGET_CXX = /bin/false
+endif
+
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT_FORTRAN),y)
+PKG_MESON_TARGET_FC = $(TARGET_FC)
+else
+PKG_MESON_TARGET_FC = /bin/false
+endif
+
 # Generates sed patterns for patching the cross-compilation.conf template,
 # since Flags might contain commas the arguments are passed indirectly by
 # variable name (stripped to deal with whitespaces).
 # Arguments are variable containing cflags, cxxflags, ldflags, fcflags
 define PKG_MESON_CROSSCONFIG_SED
         -e "s%@TARGET_CC@%$(TARGET_CC)%g" \
-        -e "s%@TARGET_CXX@%$(TARGET_CXX)%g" \
+        -e "s%@TARGET_CXX@%$(PKG_MESON_TARGET_CXX)%g" \
         -e "s%@TARGET_AR@%$(TARGET_AR)%g" \
-        -e "s%@TARGET_FC@%$(TARGET_FC)%g" \
+        -e "s%@TARGET_FC@%$(PKG_MESON_TARGET_FC)%g" \
         -e "s%@TARGET_STRIP@%$(TARGET_STRIP)%g" \
         -e "s%@TARGET_ARCH@%$(PKG_MESON_TARGET_CPU_FAMILY)%g" \
         -e "s%@TARGET_CPU@%$(GCC_TARGET_CPU)%g" \