[Buildroot] [PATCH 1/5] gconf: use $() to reference make variables instead of ${}

[Buildroot] [PATCH 1/5] gconf: use $() to reference make variables instead of ${}

[Ricardo Martincoski]

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/gconf/gconf.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/gconf/gconf.mk b/package/gconf/gconf.mk
index 39df95fbd2..60ced95e18 100644
--- a/package/gconf/gconf.mk
+++ b/package/gconf/gconf.mk
@@ -5,7 +5,7 @@
 ################################################################################
 
 GCONF_VERSION = 3.2.6
-GCONF_SOURCE = GConf-${GCONF_VERSION}.tar.xz
+GCONF_SOURCE = GConf-$(GCONF_VERSION).tar.xz
 GCONF_SITE = http://ftp.gnome.org/pub/gnome/sources/GConf/3.2
 GCONF_CONF_OPTS = --disable-orbit
 GCONF_DEPENDENCIES = dbus dbus-glib libglib2 libxml2 \
-- 
2.17.1

[Buildroot] [PATCH 2/5] json-for-modern-cpp: use $() to reference make variables instead of ${}

[Ricardo Martincoski]

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/json-for-modern-cpp/json-for-modern-cpp.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/json-for-modern-cpp/json-for-modern-cpp.mk b/package/json-for-modern-cpp/json-for-modern-cpp.mk
index 4bcfd46910..e547c07875 100644
--- a/package/json-for-modern-cpp/json-for-modern-cpp.mk
+++ b/package/json-for-modern-cpp/json-for-modern-cpp.mk
@@ -5,7 +5,7 @@
 ################################################################################
 
 JSON_FOR_MODERN_CPP_VERSION = v3.1.2
-JSON_FOR_MODERN_CPP_SOURCE = json-${JSON_FOR_MODERN_CPP_VERSION}.tar.gz
+JSON_FOR_MODERN_CPP_SOURCE = json-$(JSON_FOR_MODERN_CPP_VERSION).tar.gz
 JSON_FOR_MODERN_CPP_SITE = $(call github,nlohmann,json,$(JSON_FOR_MODERN_CPP_VERSION))
 JSON_FOR_MODERN_CPP_LICENSE = MIT
 JSON_FOR_MODERN_CPP_LICENSE_FILES = LICENSE.MIT
-- 
2.17.1

[Buildroot] [PATCH 3/5] libpng: use $() to reference make variables instead of ${}

[Ricardo Martincoski]

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/libpng/libpng.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/libpng/libpng.mk b/package/libpng/libpng.mk
index f8594c808f..ace62cd876 100644
--- a/package/libpng/libpng.mk
+++ b/package/libpng/libpng.mk
@@ -7,7 +7,7 @@
 LIBPNG_VERSION = 1.6.34
 LIBPNG_SERIES = 16
 LIBPNG_SOURCE = libpng-$(LIBPNG_VERSION).tar.xz
-LIBPNG_SITE = http://downloads.sourceforge.net/project/libpng/libpng${LIBPNG_SERIES}/$(LIBPNG_VERSION)
+LIBPNG_SITE = http://downloads.sourceforge.net/project/libpng/libpng$(LIBPNG_SERIES)/$(LIBPNG_VERSION)
 LIBPNG_LICENSE = Libpng
 LIBPNG_LICENSE_FILES = LICENSE
 LIBPNG_INSTALL_STAGING = YES
-- 
2.17.1

[Buildroot] [PATCH 4/5] php: use $() to reference make variables instead of ${}

[Ricardo Martincoski]

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/php/php.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/php/php.mk b/package/php/php.mk
index 4c3a87118e..028fa6fc55 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -137,7 +137,7 @@ endif
 
 ifeq ($(BR2_PACKAGE_PHP_EXT_LIBXML2),y)
 PHP_CONF_ENV += php_cv_libxml_build_works=yes
-PHP_CONF_OPTS += --enable-libxml --with-libxml-dir=${STAGING_DIR}/usr
+PHP_CONF_OPTS += --enable-libxml --with-libxml-dir=$(STAGING_DIR)/usr
 PHP_DEPENDENCIES += libxml2
 endif
 
-- 
2.17.1

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

[Ricardo Martincoski]

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 "$${}";
 - do not use \w as it would give false warnings for this sed contruct
   in mesa3d-headers.mk: 's:@includedir@:${prefix}/include:'.

[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@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@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]
-- 
2.17.1

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

[Arnout Vandecappelle]

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!

 Yann, you introduced this more than 3 years ago in 7468b60e7c7fe7f. 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

 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@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@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@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

[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@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@gmail.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > Cc: Thomas Petazzoni <thomas.petazzoni@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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/5] gconf: use $() to reference make variables instead of ${}

[Arnout Vandecappelle]

On 08-07-18 07:16, Ricardo Martincoski wrote:
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

 Patches 1-4 applied to master, thanks.

 Regards,
 Arnout

> ---
>  package/gconf/gconf.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/gconf/gconf.mk b/package/gconf/gconf.mk
> index 39df95fbd2..60ced95e18 100644
> --- a/package/gconf/gconf.mk
> +++ b/package/gconf/gconf.mk
> @@ -5,7 +5,7 @@
>  ################################################################################
>  
>  GCONF_VERSION = 3.2.6
> -GCONF_SOURCE = GConf-${GCONF_VERSION}.tar.xz
> +GCONF_SOURCE = GConf-$(GCONF_VERSION).tar.xz
>  GCONF_SITE = http://ftp.gnome.org/pub/gnome/sources/GConf/3.2
>  GCONF_CONF_OPTS = --disable-orbit
>  GCONF_DEPENDENCIES = dbus dbus-glib libglib2 libxml2 \
> 

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

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

[Ricardo Martincoski]

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 "$${}".

[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@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes v1 -> v2:
  - use \w instead of [A-Z0-9_] because we do have lowercase make
    variables and macros, and we want to catch them as well;  (suggested
    by Arnout)
  - collect review tag.

check-package job after this patch:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/80256746
---
 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..0e430a2f12 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"^[^#].*[^$]\${\w+}")
+
+    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]
-- 
2.17.1

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

[Thomas Petazzoni]

Hello,

On Sun,  8 Jul 2018 22:56:47 -0300, 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 "$${}".
> 
> [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@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> Changes v1 -> v2:
>   - use \w instead of [A-Z0-9_] because we do have lowercase make
>     variables and macros, and we want to catch them as well;  (suggested
>     by Arnout)
>   - collect review tag.

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com