| Message ID | 1426270934-15499-2-git-send-email-gustavo@zacarias.com.ar |
|---|---|
| State | Superseded |
| Headers | show |
Gustavo, All, On 2015-03-13 15:22 -0300, Gustavo Zacarias spake thusly: > Take 'm' as parameter in $2 (buildroot option name) in > check_uclibc_feature to indicate that a toolchain option is mandatory, > and bail out with an appropiate message if that's the case. I had a hard time figuring out what you were doing here... What about: toolchain/helpers: add check for mandatory uClibc options We currently only check that the Buildroot configuration matches what is available in the toolchain. Since we're going to remove the check for LFS and make it a mandatory feature, we will lose the corresponding Buildroot option, so we won't be able to use check_uclibc_feature as-is. Introduce a special, magic value passed as the Buildroot option name to recognise checks for mandatory uClibc options that do not have a corresponding option in Buldroot. If the Buildroot option name is 'm', then the check is against a mandatory uClibc option. If a mandatory uClibc option is missing, we reject the toolchain as being unusable by Buildroot. However, I don't think 'm' is the most appropriate. That's what somehow confused me: wtf are we concerned about tristates? What about making the check against an empty Buildroot option name, instead? No Buildroot option name means there's no correlation to be made, and hence is a mandatory uClibc option... > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> > --- > toolchain/helpers.mk | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk > index 3121da4..ef657ee 100644 > --- a/toolchain/helpers.mk > +++ b/toolchain/helpers.mk > @@ -238,6 +238,7 @@ check_musl = \ > # Check the conformity of Buildroot configuration with regard to the > # uClibc configuration of the external toolchain, for a particular > # feature. > +# If 'Buildroot option name' ($2) = 'm' it means a mandatory option. > # > # $1: uClibc macro name > # $2: Buildroot option name > @@ -246,6 +247,10 @@ check_musl = \ > # > check_uclibc_feature = \ > IS_IN_LIBC=`grep -q "\#define $(1) 1" $(3) && echo y` ; \ > + if [ "$(2)" = "m" -a "$${IS_IN_LIBC}" != "y" ] ; then \ With my proposal, that'd read: if [ -z "$(2)" -a "$${IS_IN_LIBC}" != "y" ] ; then \ Otherwise, code looks good. :-) Regards, Yann E. MORIN. > + echo "$(4) not available in C library, toolchain unsuitable for Buildroot" ; \ > + exit 1 ; \ > + fi ; \ > if [ "$($(2))" != "y" -a "$${IS_IN_LIBC}" = "y" ] ; then \ > echo "$(4) available in C library, please enable $(2)" ; \ > exit 1 ; \ > -- > 2.0.5 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On 03/14/2015 07:18 PM, Yann E. MORIN wrote: > Gustavo, All, > > On 2015-03-13 15:22 -0300, Gustavo Zacarias spake thusly: >> Take 'm' as parameter in $2 (buildroot option name) in >> check_uclibc_feature to indicate that a toolchain option is mandatory, >> and bail out with an appropiate message if that's the case. > > I had a hard time figuring out what you were doing here... > > What about: > > toolchain/helpers: add check for mandatory uClibc options > > We currently only check that the Buildroot configuration matches > what is available in the toolchain. > > Since we're going to remove the check for LFS and make it a > mandatory feature, we will lose the corresponding Buildroot > option, so we won't be able to use check_uclibc_feature as-is. > > Introduce a special, magic value passed as the Buildroot option > name to recognise checks for mandatory uClibc options that do not > have a corresponding option in Buldroot. > > If the Buildroot option name is 'm', then the check is against > a mandatory uClibc option. > > If a mandatory uClibc option is missing, we reject the toolchain > as being unusable by Buildroot. > > However, I don't think 'm' is the most appropriate. That's what somehow > confused me: wtf are we concerned about tristates? What about making > the check against an empty Buildroot option name, instead? No Buildroot > option name means there's no correlation to be made, and hence is a > mandatory uClibc option... Actually i used 'm' as shortcut for 'mandatory', it's purely casual that it matches tristate. Initially i was going to duplicate functionality (make another function) for mandatory toolchain options, used like: $(call check_uclibc_feature,__ARCH_USE_MMU__,BR2_USE_MMU,$${UCLIBC_CONFIG_FILE},MMU support) ;\ (original) $(call check_uclibc_feature_mandatory,__UCLIBC_HAS_LFS__,$${UCLIBC_CONFIG_FILE},Large file support) ;\ I'm not a big friend of magical strings but decided to throw the 'm' and see what other said. Still, i'd prefer the extra function for clarity. Regards.
On 03/14/2015 07:18 PM, Yann E. MORIN wrote: >> check_uclibc_feature = \ >> IS_IN_LIBC=`grep -q "\#define $(1) 1" $(3) && echo y` ; \ >> + if [ "$(2)" = "m" -a "$${IS_IN_LIBC}" != "y" ] ; then \ > > With my proposal, that'd read: > > if [ -z "$(2)" -a "$${IS_IN_LIBC}" != "y" ] ; then \ > > Otherwise, code looks good. :-) > > Regards, > Yann E. MORIN. Actually there's another scenario here, truth table! If mandatory is true and the function is in libc the flow will go on, hence doing the two other checks. Problem is if parameter $2 is != 'y' and IS_IN_LIBC = 'y' then we've got a problem (external toolchain with largefile and uclibc for example). So resulting in: ----- if [ -z "$(2)" ] ; then \ if [ "$${IS_IN_LIBC}" != "y" ] ; then \ echo "$(4) not available in C library, toolchain unsuitable for Buildroot" ; exit 1 ; \ fi ; \ else \ if [ "$($(2))" != "y" -a "$${IS_IN_LIBC}" = "y" ] ; then \ echo "$(4) available in C library, please enable $(2)" ; \ exit 1 ; \ fi ; \ if [ "$($(2))" = "y" -a "$${IS_IN_LIBC}" != "y" ] ; then \ echo "$(4) not available in C library, please disable $(2)" ; \ exit 1 ; \ fi ; \ fi ----- Happens for not testing an uclibc lfs-enabled external toolchain :) Regards.
diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk index 3121da4..ef657ee 100644 --- a/toolchain/helpers.mk +++ b/toolchain/helpers.mk @@ -238,6 +238,7 @@ check_musl = \ # Check the conformity of Buildroot configuration with regard to the # uClibc configuration of the external toolchain, for a particular # feature. +# If 'Buildroot option name' ($2) = 'm' it means a mandatory option. # # $1: uClibc macro name # $2: Buildroot option name @@ -246,6 +247,10 @@ check_musl = \ # check_uclibc_feature = \ IS_IN_LIBC=`grep -q "\#define $(1) 1" $(3) && echo y` ; \ + if [ "$(2)" = "m" -a "$${IS_IN_LIBC}" != "y" ] ; then \ + echo "$(4) not available in C library, toolchain unsuitable for Buildroot" ; \ + exit 1 ; \ + fi ; \ if [ "$($(2))" != "y" -a "$${IS_IN_LIBC}" = "y" ] ; then \ echo "$(4) available in C library, please enable $(2)" ; \ exit 1 ; \
Take 'm' as parameter in $2 (buildroot option name) in check_uclibc_feature to indicate that a toolchain option is mandatory, and bail out with an appropiate message if that's the case. Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> --- toolchain/helpers.mk | 5 +++++ 1 file changed, 5 insertions(+)