[Buildroot] [PATCH 5/5] check-package: detect the use of ${} in .mk files

[Yann E. MORIN]

Arnout, Ricardo, All,

On 2018-07-08 11:57 +0200, Arnout Vandecappelle spake thusly:
> On 08-07-18 07:17, Ricardo Martincoski wrote:
> > And warn to use $() instead.
> > For examples see [1] and [2].
> > 
> > In the regexp, search for ${VARIABLE} but:
> >  - ignore comments;
> >  - ignore variables to be expanded by the shell "$${}";
> 
>  Theoretically, we should check for $$${} but I don't think the extra complexity
> is warranted.
> 
> >  - do not use \w as it would give false warnings for this sed contruct
> >    in mesa3d-headers.mk: 's:@includedir@:${prefix}/include:'.
> 
>  This is actually a bug in mesa3d-headers.mk!

Yup, definitely...

>  Yann, you introduced this more than 3 years ago in 7468b60e7c7fe7f.

I hardly even remember what I did three *weeks* ago, and when I do, I
don't really remember *why* I did it. So, three years... ;-]

> It clearly
> can never have worked, since the /usr bit would be missing from the paths. Of
> course, libdir and includedir aren't really needed in the pc file since they're
> the default, but wouldn't the dridriver dir be needed?
> 
>  For the convenience of people trying to make sense of what I'm writing, here's
> the relevant bit of mesa3d-headers.mk:
> 
> define MESA3D_HEADERS_BUILD_DRI_PC
>         sed -e 's:@\(exec_\)\?prefix@:/usr:' \
>             -e 's:@libdir@:${exec_prefix}/lib:' \
>             -e 's:@includedir@:${prefix}/include:' \
>             -e 's:@DRI_DRIVER_INSTALL_DIR@:${libdir}/dri:' \
>             -e 's:@VERSION@:$(MESA3D_HEADERS_VERSION):' \
>             -e 's:@DRI_PC_REQ_PRIV@::' \
>             $(@D)/src/mesa/drivers/dri/dri.pc.in \
>             >$(@D)/src/mesa/drivers/dri/dri.pc
> endef

Yup, bad; not good.

Regards,
Yann E. MORIN.

>  And this is the resulting dri.pc file:
> 
> prefix=/usr
> exec_prefix=/usr
> libdir=/lib
> includedir=/include
> dridriverdir=/dri
> 
> Name: dri
> Description: Direct Rendering Infrastructure
> Version: 18.1.3
> Requires.private:
> Cflags: -I${includedir}
> 
> 
> 
>  I think using \w would actually be better, because we do have lowercase make
> variables and macros, and we want to catch them as well.
> 
> 
>  Otherwise LGTM so you can add my
>  Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout at mind.be>
> once it has the \w.
> 
>  Regards,
>  Arnout
> 
> > 
> > [1] http://lists.busybox.net/pipermail/buildroot/2018-July/225211.html
> > [2] https://github.com/buildroot/buildroot/commit/36305380db1312442623128689fe5067d9058381
> > 
> > Signed-off-by: Ricardo Martincoski <ricardo.martincoski at gmail.com>
> > Cc: Arnout Vandecappelle <arnout at mind.be>
> > Cc: Thomas Petazzoni <thomas.petazzoni at bootlin.com>
> > ---
> > With only this patch applied:
> > https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/80161018
> > ---
> >  utils/checkpackagelib/lib_mk.py | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
> > index 86e9aa2d97..423e592de1 100644
> > --- a/utils/checkpackagelib/lib_mk.py
> > +++ b/utils/checkpackagelib/lib_mk.py
> > @@ -251,3 +251,13 @@ class UselessFlag(_CheckFunction):
> >                      "({}#_infrastructure_for_autotools_based_packages)"
> >                      .format(self.filename, lineno, self.url_to_manual),
> >                      text]
> > +
> > +
> > +class VariableWithBraces(_CheckFunction):
> > +    VARIABLE_WITH_BRACES = re.compile(r"^[^#].*[^$]\${[A-Z0-9_]+}")
> > +
> > +    def check_line(self, lineno, text):
> > +        if self.VARIABLE_WITH_BRACES.match(text.rstrip()):
> > +            return ["{}:{}: use $() to delimit variables, not ${{}}"
> > +                    .format(self.filename, lineno),
> > +                    text]
> > 
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'