gconf: use $() to reference make variables instead of ${}

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 <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
With only this patch applied:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/80161018

---
 utils/checkpackagelib/lib_mk.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

utils/checkpackagelib/lib_mk.py +10 -0

index 86e9aa2d97..423e592de1 100644
@@ -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]

Quoted 6 lines

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.

Quoted 2 lines

- 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) <[email protected]>

once it has the \w.

Regards,
Arnout

Quoted 33 lines

[1] http://lists.busybox.net/pipermail/buildroot/2018-July/225211.html
[2] https://github.com/buildroot/buildroot/commit/36305380db1312442623128689fe5067d9058381

Signed-off-by Ricardo Martincoski <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
With only this patch applied:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/80161018

---
 utils/checkpackagelib/lib_mk.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

utils/checkpackagelib/lib_mk.py +10 -0

index 86e9aa2d97..423e592de1 100644
@@ -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]

Signature · 6 lines

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

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 <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Yann E. MORIN <[email protected]>

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

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

Signed-off-by Ricardo Martincoski <[email protected]>

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

package/gconf/gconf.mk +1 -1

index 39df95fbd2..60ced95e18 100644
@@ -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 \