* [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-27 19:52 ` Thomas Petazzoni
` (2 more replies)
2019-01-27 18:59 ` [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS Ricardo Martincoski
` (6 subsequent siblings)
7 siblings, 3 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Commit "c5b85231fb s6-networking: enable SSL if libressl is selected"
actually dropped the dependency on s6-dns and s6 when libressl is
enabled.
Fix this by using += inside the conditional code.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
NOTE: I don't know why the autobuilders didn't caught this.
On current master (f6843a75fe), test-pkg with this config:
BR2_PACKAGE_OPENSSL=y
BR2_PACKAGE_LIBRESSL=y
BR2_PACKAGE_S6_NETWORKING=y
results in:
6 builds, 1 skipped, 5 build failed, 0 legal-info failed
After this patch:
6 builds, 1 skipped, 0 build failed, 0 legal-info failed
---
package/s6-networking/s6-networking.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package/s6-networking/s6-networking.mk b/package/s6-networking/s6-networking.mk
index 7c9f087051..b621eb225d 100644
--- a/package/s6-networking/s6-networking.mk
+++ b/package/s6-networking/s6-networking.mk
@@ -12,33 +12,33 @@ S6_NETWORKING_INSTALL_STAGING = YES
S6_NETWORKING_DEPENDENCIES = s6-dns s6
S6_NETWORKING_CONF_OPTS = \
--prefix=/usr \
--with-sysdeps=$(STAGING_DIR)/usr/lib/skalibs/sysdeps \
--with-include=$(STAGING_DIR)/usr/include \
--with-dynlib=$(STAGING_DIR)/usr/lib \
--with-lib=$(STAGING_DIR)/usr/lib/execline \
--with-lib=$(STAGING_DIR)/usr/lib/s6 \
--with-lib=$(STAGING_DIR)/usr/lib/s6-dns \
--with-lib=$(STAGING_DIR)/usr/lib/skalibs \
$(if $(BR2_STATIC_LIBS),,--disable-allstatic) \
$(SHARED_STATIC_LIBS_OPTS)
ifeq ($(BR2_PACKAGE_LIBRESSL),y)
S6_NETWORKING_CONF_OPTS += --enable-ssl=libressl
-S6_NETWORKING_DEPENDENCIES = libressl
+S6_NETWORKING_DEPENDENCIES += libressl
endif
define S6_NETWORKING_CONFIGURE_CMDS
(cd $(@D); $(TARGET_CONFIGURE_OPTS) ./configure $(S6_NETWORKING_CONF_OPTS))
endef
define S6_NETWORKING_BUILD_CMDS
$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
endef
define S6_NETWORKING_REMOVE_STATIC_LIB_DIR
rm -rf $(TARGET_DIR)/usr/lib/s6-networking
endef
S6_NETWORKING_POST_INSTALL_TARGET_HOOKS += S6_NETWORKING_REMOVE_STATIC_LIB_DIR
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled
2019-01-27 18:59 ` [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled Ricardo Martincoski
@ 2019-01-27 19:52 ` Thomas Petazzoni
2019-01-28 2:26 ` Ricardo Martincoski
2019-01-27 21:15 ` Peter Korsgaard
2019-01-29 21:56 ` Peter Korsgaard
2 siblings, 1 reply; 29+ messages in thread
From: Thomas Petazzoni @ 2019-01-27 19:52 UTC (permalink / raw)
To: buildroot
Hello Ricardo,
On Sun, 27 Jan 2019 16:59:36 -0200
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> Commit "c5b85231fb s6-networking: enable SSL if libressl is selected"
> actually dropped the dependency on s6-dns and s6 when libressl is
> enabled.
> Fix this by using += inside the conditional code.
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Eric Le Bihan <eric.le.bihan.dev@free.fr>
> ---
> NOTE: I don't know why the autobuilders didn't caught this.
Because libressl is selected as part of a choice..endchoice, and such
choices are not randomized by "randpackageconfig". Therefore, only
OpenSSL is tested by the autobuilders, and LibreSSL is never tested.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled
2019-01-27 19:52 ` Thomas Petazzoni
@ 2019-01-28 2:26 ` Ricardo Martincoski
0 siblings, 0 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-28 2:26 UTC (permalink / raw)
To: buildroot
Hello,
On Sun, Jan 27, 2019 at 05:52 PM, Thomas Petazzoni wrote:
> On Sun, 27 Jan 2019 16:59:36 -0200
> Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
>
>> Commit "c5b85231fb s6-networking: enable SSL if libressl is selected"
>> actually dropped the dependency on s6-dns and s6 when libressl is
>> enabled.
>> Fix this by using += inside the conditional code.
>>
>> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> Cc: Eric Le Bihan <eric.le.bihan.dev@free.fr>
>> ---
>> NOTE: I don't know why the autobuilders didn't caught this.
>
> Because libressl is selected as part of a choice..endchoice, and such
> choices are not randomized by "randpackageconfig". Therefore, only
> OpenSSL is tested by the autobuilders, and LibreSSL is never tested.
Right. Thank you for the clarification.
Regards,
Ricardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled
2019-01-27 18:59 ` [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled Ricardo Martincoski
2019-01-27 19:52 ` Thomas Petazzoni
@ 2019-01-27 21:15 ` Peter Korsgaard
2019-01-29 21:56 ` Peter Korsgaard
2 siblings, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-27 21:15 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Commit "c5b85231fb s6-networking: enable SSL if libressl is selected"
> actually dropped the dependency on s6-dns and s6 when libressl is
> enabled.
> Fix this by using += inside the conditional code.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Eric Le Bihan <eric.le.bihan.dev@free.fr>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled
2019-01-27 18:59 ` [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled Ricardo Martincoski
2019-01-27 19:52 ` Thomas Petazzoni
2019-01-27 21:15 ` Peter Korsgaard
@ 2019-01-29 21:56 ` Peter Korsgaard
2 siblings, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-29 21:56 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Commit "c5b85231fb s6-networking: enable SSL if libressl is selected"
> actually dropped the dependency on s6-dns and s6 when libressl is
> enabled.
> Fix this by using += inside the conditional code.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Eric Le Bihan <eric.le.bihan.dev@free.fr>
Committed to 2018.02.x and 2018.11.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
2019-01-27 18:59 ` [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-27 21:16 ` Peter Korsgaard
2019-01-29 21:57 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 3/8] Revert "avrdude: add license information" Ricardo Martincoski
` (5 subsequent siblings)
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Since "57ace26b6c package/sdl_sound: add optional support for
libmodplug" from 2016, optional CONF_OPTS are added but they do not
really take effect because there is an unconditional override below the
conditional append.
Currently this does not cause build failures, but it can lead to wrong
detection of dependencies because many explicit --enable/--disable are
not passed to configure.
Fix this by moving the unconditional code to the top.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
On current master (f6843a75fe), test-pkg with this config:
BR2_PACKAGE_SDL=y
BR2_PACKAGE_SDL_SOUND=y
results in:
6 builds, 0 skipped, 0 build failed, 0 legal-info failed
After this patch, the same result:
6 builds, 0 skipped, 0 build failed, 0 legal-info failed
After running test-pkg I diffed this file between the output of master
and the output after this patch:
br-arm-full/build/sdl_sound-1.0.3/config.status
Following flags are added, result of this patch:
'--disable-flac' '--disable-modplug' '--disable-ogg' '--disable-speex' '--disable-physfs'
On current master (f6843a75fe), test-pkg with this config:
BR2_PACKAGE_FLAC=y
BR2_PACKAGE_SDL=y
BR2_PACKAGE_SDL_SOUND=y
BR2_PACKAGE_SDL_SOUND_PLAYSOUND=y
BR2_PACKAGE_LIBMODPLUG=y
BR2_PACKAGE_LIBVORBIS=y
BR2_PACKAGE_SPEEX=y
BR2_PACKAGE_PHYSFS=y
results in:
6 builds, 0 skipped, 0 build failed, 0 legal-info failed
After this patch, the same result:
6 builds, 0 skipped, 0 build failed, 0 legal-info failed
After running test-pkg I diffed this file between the output of master
and the output after this patch:
br-arm-full/build/sdl_sound-1.0.3/config.status
Following flags are added, result of this patch:
'--enable-flac' '--enable-modplug' '--enable-ogg' '--enable-speex' '--enable-physfs'
---
package/sdl_sound/sdl_sound.mk | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/package/sdl_sound/sdl_sound.mk b/package/sdl_sound/sdl_sound.mk
index 2544b598e9..c6028e7910 100644
--- a/package/sdl_sound/sdl_sound.mk
+++ b/package/sdl_sound/sdl_sound.mk
@@ -1,29 +1,34 @@
################################################################################
#
# sdl_sound
#
################################################################################
SDL_SOUND_VERSION = 1.0.3
SDL_SOUND_SOURCE = SDL_sound-$(SDL_SOUND_VERSION).tar.gz
SDL_SOUND_SITE = http://icculus.org/SDL_sound/downloads
SDL_SOUND_LICENSE = LGPL-2.1+
SDL_SOUND_LICENSE_FILES = COPYING
SDL_SOUND_INSTALL_STAGING = YES
SDL_SOUND_DEPENDENCIES = sdl
+SDL_SOUND_CONF_OPTS = \
+ --with-sdl-prefix=$(STAGING_DIR)/usr \
+ --with-sdl-exec-prefix=$(STAGING_DIR)/usr \
+ --disable-sdltest \
+ --enable-static
ifneq ($(BR2_ENABLE_LOCALE),y)
SDL_SOUND_DEPENDENCIES += libiconv
endif
# optional dependencies
ifeq ($(BR2_PACKAGE_FLAC)$(BR2_PACKAGE_LIBOGG),yy)
SDL_SOUND_CONF_OPTS += --enable-flac
SDL_SOUND_DEPENDENCIES += flac libogg
else
SDL_SOUND_CONF_OPTS += --disable-flac
endif
ifeq ($(BR2_PACKAGE_LIBMODPLUG),y)
SDL_SOUND_CONF_OPTS += --enable-modplug
SDL_SOUND_DEPENDENCIES += libmodplug
@@ -39,37 +44,31 @@ SDL_SOUND_CONF_OPTS += --disable-ogg
endif
ifeq ($(BR2_PACKAGE_SPEEX),y)
SDL_SOUND_CONF_OPTS += --enable-speex
SDL_SOUND_DEPENDENCIES += speex
else
SDL_SOUND_CONF_OPTS += --disable-speex
endif
ifeq ($(BR2_PACKAGE_PHYSFS),y)
SDL_SOUND_CONF_OPTS += --enable-physfs
SDL_SOUND_DEPENDENCIES += physfs
else
SDL_SOUND_CONF_OPTS += --disable-physfs
endif
-SDL_SOUND_CONF_OPTS = \
- --with-sdl-prefix=$(STAGING_DIR)/usr \
- --with-sdl-exec-prefix=$(STAGING_DIR)/usr \
- --disable-sdltest \
- --enable-static
-
ifeq ($(BR2_X86_CPU_HAS_MMX),y)
SDL_SOUND_CONF_OPTS += --enable-mmx
else
SDL_SOUND_CONF_OPTS += --disable-mmx
endif
define SDL_SOUND_REMOVE_PLAYSOUND
rm $(addprefix $(TARGET_DIR)/usr/bin/,playsound playsound_simple)
endef
ifneq ($(BR2_PACKAGE_SDL_SOUND_PLAYSOUND),y)
SDL_SOUND_POST_INSTALL_TARGET_HOOKS += SDL_SOUND_REMOVE_PLAYSOUND
endif
$(eval $(autotools-package))
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS
2019-01-27 18:59 ` [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS Ricardo Martincoski
@ 2019-01-27 21:16 ` Peter Korsgaard
2019-01-29 21:57 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-27 21:16 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Since "57ace26b6c package/sdl_sound: add optional support for
> libmodplug" from 2016, optional CONF_OPTS are added but they do not
> really take effect because there is an unconditional override below the
> conditional append.
> Currently this does not cause build failures, but it can lead to wrong
> detection of dependencies because many explicit --enable/--disable are
> not passed to configure.
> Fix this by moving the unconditional code to the top.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS
2019-01-27 18:59 ` [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS Ricardo Martincoski
2019-01-27 21:16 ` Peter Korsgaard
@ 2019-01-29 21:57 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-29 21:57 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Since "57ace26b6c package/sdl_sound: add optional support for
> libmodplug" from 2016, optional CONF_OPTS are added but they do not
> really take effect because there is an unconditional override below the
> conditional append.
> Currently this does not cause build failures, but it can lead to wrong
> detection of dependencies because many explicit --enable/--disable are
> not passed to configure.
> Fix this by moving the unconditional code to the top.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Committed to 2018.02.x and 2018.11.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 3/8] Revert "avrdude: add license information"
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
2019-01-27 18:59 ` [Buildroot] [PATCH 1/8] package/s6-networking: fix dependency when libressl is enabled Ricardo Martincoski
2019-01-27 18:59 ` [Buildroot] [PATCH 2/8] package/sdl_sound: actually use the optional CONF_OPTS Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-27 21:17 ` Peter Korsgaard
2019-01-29 21:59 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 4/8] package/usb_modeswitch: drop unicode space in comment Ricardo Martincoski
` (4 subsequent siblings)
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
This reverts commit d1f545004bf0533064363d87c2d7c71e7acf7435 from 2014
because the added variables already existed. The real problem at the
time was that one of the pre-existent variables had a typo, fixed in a
later commit.
Currently AVRDUDE_LICENSE and AVRDUDE_LICENSE_FILES are declared twice
with the same values for each one. So remove one of them.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Wojciech M. Zabolotny <wzab01@gmail.com>
---
NOTE: When I first sent the series adding check-package, I reported the
typo. Unfortunately I failed to identify the duplicated variables.
The output for below command is the same in current master (f6843a75fe)
and after this patch:
$ make -s BR2_HAVE_DOT_CONFIG=y printvars VARS=AVRDUDE_LICENSE%
AVRDUDE_LICENSE=GPL-2.0+
AVRDUDE_LICENSE_FILES=avrdude/COPYING
---
package/avrdude/avrdude.mk | 2 --
1 file changed, 2 deletions(-)
diff --git a/package/avrdude/avrdude.mk b/package/avrdude/avrdude.mk
index 0e34076e38..ae378dfbac 100644
--- a/package/avrdude/avrdude.mk
+++ b/package/avrdude/avrdude.mk
@@ -2,34 +2,32 @@
#
# avrdude
#
################################################################################
AVRDUDE_VERSION = f8893ccf0aceeff67c45f95ae4e86bc7fbe342aa
AVRDUDE_SITE = $(call github,kcuzner,avrdude,$(AVRDUDE_VERSION))
AVRDUDE_LICENSE = GPL-2.0+
AVRDUDE_LICENSE_FILES = avrdude/COPYING
AVRDUDE_SUBDIR = avrdude
# Sources coming from git, without generated configure and Makefile.in
# files.
AVRDUDE_AUTORECONF = YES
AVRDUDE_CONF_OPTS = --enable-linuxgpio
AVRDUDE_DEPENDENCIES = elfutils libusb libusb-compat ncurses \
host-flex host-bison
-AVRDUDE_LICENSE = GPL-2.0+
-AVRDUDE_LICENSE_FILES = avrdude/COPYING
ifeq ($(BR2_PACKAGE_LIBFTDI1),y)
AVRDUDE_DEPENDENCIES += libftdi1
else ifeq ($(BR2_PACKAGE_LIBFTDI),y)
AVRDUDE_DEPENDENCIES += libftdi
endif
# if /etc/avrdude.conf exists, the installation process creates a
# backup file, which we do not want in the context of Buildroot.
define AVRDUDE_REMOVE_BACKUP_FILE
$(RM) -f $(TARGET_DIR)/etc/avrdude.conf.bak
endef
AVRDUDE_POST_INSTALL_TARGET_HOOKS += AVRDUDE_REMOVE_BACKUP_FILE
$(eval $(autotools-package))
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 3/8] Revert "avrdude: add license information"
2019-01-27 18:59 ` [Buildroot] [PATCH 3/8] Revert "avrdude: add license information" Ricardo Martincoski
@ 2019-01-27 21:17 ` Peter Korsgaard
2019-01-29 21:59 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-27 21:17 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> This reverts commit d1f545004bf0533064363d87c2d7c71e7acf7435 from 2014
> because the added variables already existed. The real problem at the
> time was that one of the pre-existent variables had a typo, fixed in a
> later commit.
> Currently AVRDUDE_LICENSE and AVRDUDE_LICENSE_FILES are declared twice
> with the same values for each one. So remove one of them.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Wojciech M. Zabolotny <wzab01@gmail.com>
> ---
> NOTE: When I first sent the series adding check-package, I reported the
> typo. Unfortunately I failed to identify the duplicated variables.
> The output for below command is the same in current master (f6843a75fe)
> and after this patch:
> $ make -s BR2_HAVE_DOT_CONFIG=y printvars VARS=AVRDUDE_LICENSE%
> AVRDUDE_LICENSE=GPL-2.0+
> AVRDUDE_LICENSE_FILES=avrdude/COPYING
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 3/8] Revert "avrdude: add license information"
2019-01-27 18:59 ` [Buildroot] [PATCH 3/8] Revert "avrdude: add license information" Ricardo Martincoski
2019-01-27 21:17 ` Peter Korsgaard
@ 2019-01-29 21:59 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-29 21:59 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> This reverts commit d1f545004bf0533064363d87c2d7c71e7acf7435 from 2014
> because the added variables already existed. The real problem at the
> time was that one of the pre-existent variables had a typo, fixed in a
> later commit.
> Currently AVRDUDE_LICENSE and AVRDUDE_LICENSE_FILES are declared twice
> with the same values for each one. So remove one of them.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Wojciech M. Zabolotny <wzab01@gmail.com>
Committed to 2018.02.x and 2018.11.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 4/8] package/usb_modeswitch: drop unicode space in comment
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
` (2 preceding siblings ...)
2019-01-27 18:59 ` [Buildroot] [PATCH 3/8] Revert "avrdude: add license information" Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-27 21:19 ` Peter Korsgaard
2019-01-29 22:00 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables Ricardo Martincoski
` (3 subsequent siblings)
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Commit "a554109af8 package/usb_modeswitch: disable parallel build" added
a unicode space in a comment. Replace it with a normal ASCII space for
consistency with elsewhere.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Before this patch:
$ file package/usb_modeswitch/usb_modeswitch.mk
package/usb_modeswitch/usb_modeswitch.mk: UTF-8 Unicode text
After this patch:
$ file package/usb_modeswitch/usb_modeswitch.mk
package/usb_modeswitch/usb_modeswitch.mk: ASCII text
---
package/usb_modeswitch/usb_modeswitch.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/package/usb_modeswitch/usb_modeswitch.mk b/package/usb_modeswitch/usb_modeswitch.mk
index bb0d6ab1bf..8b93e086e3 100644
--- a/package/usb_modeswitch/usb_modeswitch.mk
+++ b/package/usb_modeswitch/usb_modeswitch.mk
@@ -1,29 +1,29 @@
################################################################################
#
# usb_modeswitch
#
################################################################################
USB_MODESWITCH_VERSION = 2.5.2
USB_MODESWITCH_SOURCE = usb-modeswitch-$(USB_MODESWITCH_VERSION).tar.bz2
USB_MODESWITCH_SITE = http://www.draisberghof.de/usb_modeswitch
USB_MODESWITCH_DEPENDENCIES = libusb
USB_MODESWITCH_LICENSE = GPL-2.0+
USB_MODESWITCH_LICENSE_FILES = COPYING
-#?Package does not build in parallel due to improper make rules
+# Package does not build in parallel due to improper make rules
USB_MODESWITCH_MAKE = $(MAKE1)
USB_MODESWITCH_BUILD_TARGETS = static
USB_MODESWITCH_INSTALL_TARGETS = install-static
ifeq ($(BR2_PACKAGE_TCL)$(BR2_PACKAGE_TCL_SHLIB_ONLY),y)
USB_MODESWITCH_DEPENDENCIES += tcl
USB_MODESWITCH_BUILD_TARGETS = script
USB_MODESWITCH_INSTALL_TARGETS = install-script
endif
# build system of embedded jimtcl doesn't use autotools, but does use
# an old version of gnuconfig which doesn't know all the architectures
# supported by Buildroot, so update config.guess / config.sub like we
# do in pkg-autotools.mk
USB_MODESWITCH_POST_PATCH_HOOKS += UPDATE_CONFIG_HOOK
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
` (3 preceding siblings ...)
2019-01-27 18:59 ` [Buildroot] [PATCH 4/8] package/usb_modeswitch: drop unicode space in comment Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-27 21:28 ` Peter Korsgaard
2019-01-29 22:01 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line Ricardo Martincoski
` (2 subsequent siblings)
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Overriding variables in packages recipes is an error-prone practice.
Current behavior of installing either only as a script or only as a
binary is intended, as describe in the commit log of "d3e4db4e34
usb_modeswitch: bump to version 1.2.6" from 2013.
Rewrite the code to keep the same behavior while replacing variable
override [1] by conditional assignments [2].
[1]
VAR = ...
if ...
VAR = ...
[2]
if ...
VAR = ...
else
VAR = ...
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
The output for below commands is the same in current master (f6843a75fe)
and after this patch:
$ make -s BR2_HAVE_DOT_CONFIG=y BR2_PACKAGE_TCL_SHLIB_ONLY=y printvars \
VARS='USB_MODESWITCH_BUILD_TARGETS USB_MODESWITCH_INSTALL_TARGETS'
USB_MODESWITCH_BUILD_TARGETS=script
USB_MODESWITCH_INSTALL_TARGETS=install-script
$ make -s BR2_HAVE_DOT_CONFIG=y printvars \
VARS='USB_MODESWITCH_BUILD_TARGETS USB_MODESWITCH_INSTALL_TARGETS'
USB_MODESWITCH_BUILD_TARGETS=static
USB_MODESWITCH_INSTALL_TARGETS=install-static
---
package/usb_modeswitch/usb_modeswitch.mk | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/package/usb_modeswitch/usb_modeswitch.mk b/package/usb_modeswitch/usb_modeswitch.mk
index 8b93e086e3..9832d3c808 100644
--- a/package/usb_modeswitch/usb_modeswitch.mk
+++ b/package/usb_modeswitch/usb_modeswitch.mk
@@ -1,38 +1,38 @@
################################################################################
#
# usb_modeswitch
#
################################################################################
USB_MODESWITCH_VERSION = 2.5.2
USB_MODESWITCH_SOURCE = usb-modeswitch-$(USB_MODESWITCH_VERSION).tar.bz2
USB_MODESWITCH_SITE = http://www.draisberghof.de/usb_modeswitch
USB_MODESWITCH_DEPENDENCIES = libusb
USB_MODESWITCH_LICENSE = GPL-2.0+
USB_MODESWITCH_LICENSE_FILES = COPYING
# Package does not build in parallel due to improper make rules
USB_MODESWITCH_MAKE = $(MAKE1)
-USB_MODESWITCH_BUILD_TARGETS = static
-USB_MODESWITCH_INSTALL_TARGETS = install-static
-
ifeq ($(BR2_PACKAGE_TCL)$(BR2_PACKAGE_TCL_SHLIB_ONLY),y)
USB_MODESWITCH_DEPENDENCIES += tcl
USB_MODESWITCH_BUILD_TARGETS = script
USB_MODESWITCH_INSTALL_TARGETS = install-script
+else
+USB_MODESWITCH_BUILD_TARGETS = static
+USB_MODESWITCH_INSTALL_TARGETS = install-static
endif
# build system of embedded jimtcl doesn't use autotools, but does use
# an old version of gnuconfig which doesn't know all the architectures
# supported by Buildroot, so update config.guess / config.sub like we
# do in pkg-autotools.mk
USB_MODESWITCH_POST_PATCH_HOOKS += UPDATE_CONFIG_HOOK
define USB_MODESWITCH_BUILD_CMDS
$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
$(if $(BR2_INSTALL_LIBSTDCPP),,CXX=false) \
CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE -Wall -I." \
JIM_CONFIGURE_OPTS="--host=$(GNU_TARGET_NAME) --build=$(GNU_HOST_NAME)" \
-C $(@D) $(USB_MODESWITCH_BUILD_TARGETS)
endef
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables
2019-01-27 18:59 ` [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables Ricardo Martincoski
@ 2019-01-27 21:28 ` Peter Korsgaard
2019-01-29 22:01 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-27 21:28 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Overriding variables in packages recipes is an error-prone practice.
> Current behavior of installing either only as a script or only as a
> binary is intended, as describe in the commit log of "d3e4db4e34
> usb_modeswitch: bump to version 1.2.6" from 2013.
> Rewrite the code to keep the same behavior while replacing variable
> override [1] by conditional assignments [2].
> [1]
> VAR = ...
> if ...
> VAR = ...
> [2]
> if ...
> VAR = ...
> else
> VAR = ...
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables
2019-01-27 18:59 ` [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables Ricardo Martincoski
2019-01-27 21:28 ` Peter Korsgaard
@ 2019-01-29 22:01 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-29 22:01 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Overriding variables in packages recipes is an error-prone practice.
> Current behavior of installing either only as a script or only as a
> binary is intended, as describe in the commit log of "d3e4db4e34
> usb_modeswitch: bump to version 1.2.6" from 2013.
> Rewrite the code to keep the same behavior while replacing variable
> override [1] by conditional assignments [2].
> [1]
> VAR = ...
> if ...
> VAR = ...
> [2]
> if ...
> VAR = ...
> else
> VAR = ...
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Committed to 2018.02.x and 2018.11.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
` (4 preceding siblings ...)
2019-01-27 18:59 ` [Buildroot] [PATCH 5/8] package/usb_modeswitch: avoid overriding variables Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-28 17:16 ` Arnout Vandecappelle
2019-01-29 15:38 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 7/8] utils/check-package: handle ifdef/ifndef in .mk files Ricardo Martincoski
2019-01-27 18:59 ` [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables Ricardo Martincoski
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Currently any exceptions for a check function need to be coded into the
check-package script itself.
Create a pattern that can be used in a comment to make check-package
ignore one or more warning types in the line immediately below:
# check-package Indent, VariableWithBraces
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
utils/check-package | 4 ++++
utils/checkpackagelib/base.py | 2 ++
2 files changed, 6 insertions(+)
diff --git a/utils/check-package b/utils/check-package
index 26439f08eb..ce1fe98d67 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -119,36 +119,40 @@ def check_file_using_lib(fname):
return nwarnings, nlines
classes = inspect.getmembers(lib, is_a_check_function)
if flags.dry_run:
functions_to_run = [c[0] for c in classes]
print("{}: would run: {}".format(fname, functions_to_run))
return nwarnings, nlines
objects = [c[1](fname, flags.manual_url) for c in classes]
for cf in objects:
nwarnings += print_warnings(cf.before())
if six.PY3:
f = open(fname, "r", errors="surrogateescape")
else:
f = open(fname, "r")
+ lastline = ""
for lineno, text in enumerate(f.readlines()):
nlines += 1
for cf in objects:
+ if cf.disable.search(lastline):
+ continue
nwarnings += print_warnings(cf.check_line(lineno + 1, text))
+ lastline = text
f.close()
for cf in objects:
nwarnings += print_warnings(cf.after())
return nwarnings, nlines
def __main__():
global flags
flags = parse_args()
if flags.intree_only:
# change all paths received to be relative to the base dir
base_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
files_to_check = [os.path.relpath(os.path.abspath(f), base_dir) for f in flags.files]
# move current dir so the script find the files
diff --git a/utils/checkpackagelib/base.py b/utils/checkpackagelib/base.py
index fc09bec9a2..9544a64e5a 100644
--- a/utils/checkpackagelib/base.py
+++ b/utils/checkpackagelib/base.py
@@ -1,16 +1,18 @@
# See utils/checkpackagelib/readme.txt before editing this file.
+import re
class _CheckFunction(object):
def __init__(self, filename, url_to_manual):
self.filename = filename
self.url_to_manual = url_to_manual
+ self.disable = re.compile(r"^\s*# check-package .*\b{}\b".format(self.__class__.__name__))
def before(self):
pass
def check_line(self, lineno, text):
pass
def after(self):
pass
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line
2019-01-27 18:59 ` [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line Ricardo Martincoski
@ 2019-01-28 17:16 ` Arnout Vandecappelle
2019-01-29 15:38 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Arnout Vandecappelle @ 2019-01-28 17:16 UTC (permalink / raw)
To: buildroot
On 27/01/2019 19:59, Ricardo Martincoski wrote:
> Currently any exceptions for a check function need to be coded into the
> check-package script itself.
>
> Create a pattern that can be used in a comment to make check-package
> ignore one or more warning types in the line immediately below:
> # check-package Indent, VariableWithBraces
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
It would be nice to have mentioned that an in-tree user of this feature will be
added in a later patch.
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Regards,
Arnout
> ---
> utils/check-package | 4 ++++
> utils/checkpackagelib/base.py | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/utils/check-package b/utils/check-package
> index 26439f08eb..ce1fe98d67 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -119,36 +119,40 @@ def check_file_using_lib(fname):
> return nwarnings, nlines
> classes = inspect.getmembers(lib, is_a_check_function)
>
> if flags.dry_run:
> functions_to_run = [c[0] for c in classes]
> print("{}: would run: {}".format(fname, functions_to_run))
> return nwarnings, nlines
>
> objects = [c[1](fname, flags.manual_url) for c in classes]
>
> for cf in objects:
> nwarnings += print_warnings(cf.before())
> if six.PY3:
> f = open(fname, "r", errors="surrogateescape")
> else:
> f = open(fname, "r")
> + lastline = ""
> for lineno, text in enumerate(f.readlines()):
> nlines += 1
> for cf in objects:
> + if cf.disable.search(lastline):
> + continue
> nwarnings += print_warnings(cf.check_line(lineno + 1, text))
> + lastline = text
> f.close()
> for cf in objects:
> nwarnings += print_warnings(cf.after())
>
> return nwarnings, nlines
>
>
> def __main__():
> global flags
> flags = parse_args()
>
> if flags.intree_only:
> # change all paths received to be relative to the base dir
> base_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
> files_to_check = [os.path.relpath(os.path.abspath(f), base_dir) for f in flags.files]
> # move current dir so the script find the files
> diff --git a/utils/checkpackagelib/base.py b/utils/checkpackagelib/base.py
> index fc09bec9a2..9544a64e5a 100644
> --- a/utils/checkpackagelib/base.py
> +++ b/utils/checkpackagelib/base.py
> @@ -1,16 +1,18 @@
> # See utils/checkpackagelib/readme.txt before editing this file.
> +import re
>
>
> class _CheckFunction(object):
> def __init__(self, filename, url_to_manual):
> self.filename = filename
> self.url_to_manual = url_to_manual
> + self.disable = re.compile(r"^\s*# check-package .*\b{}\b".format(self.__class__.__name__))
>
> def before(self):
> pass
>
> def check_line(self, lineno, text):
> pass
>
> def after(self):
> pass
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line
2019-01-27 18:59 ` [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line Ricardo Martincoski
2019-01-28 17:16 ` Arnout Vandecappelle
@ 2019-01-29 15:38 ` Peter Korsgaard
1 sibling, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-01-29 15:38 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> Currently any exceptions for a check function need to be coded into the
> check-package script itself.
> Create a pattern that can be used in a comment to make check-package
> ignore one or more warning types in the line immediately below:
> # check-package Indent, VariableWithBraces
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 7/8] utils/check-package: handle ifdef/ifndef in .mk files
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
` (5 preceding siblings ...)
2019-01-27 18:59 ` [Buildroot] [PATCH 6/8] utils/check-package: allow to disable warning for a line Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-01-28 17:07 ` Arnout Vandecappelle
2019-01-29 15:39 ` Peter Korsgaard
2019-01-27 18:59 ` [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables Ricardo Martincoski
7 siblings, 2 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
Currently check-package only knows about ifeq/ifneq.
Add code to handle ifdef/ifndef as well.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
utils/checkpackagelib/lib_mk.py | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index 4387cf7167..51c6577d2c 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -1,34 +1,38 @@
# See utils/checkpackagelib/readme.txt before editing this file.
# There are already dependency checks during the build, so below check
# functions don't need to check for things already checked by exploring the
# menu options using "make menuconfig" and by running "make" with appropriate
# packages enabled.
import re
from checkpackagelib.base import _CheckFunction
from checkpackagelib.lib import ConsecutiveEmptyLines # noqa: F401
from checkpackagelib.lib import EmptyLastLine # noqa: F401
from checkpackagelib.lib import NewlineAtEof # noqa: F401
from checkpackagelib.lib import TrailingSpace # noqa: F401
+# used in more than one check
+start_conditional = ["ifdef", "ifeq", "ifndef", "ifneq"]
+end_conditional = ["endif"]
+
class Indent(_CheckFunction):
COMMENT = re.compile("^\s*#")
- CONDITIONAL = re.compile("^\s*(ifeq|ifneq|endif)\s")
+ CONDITIONAL = re.compile("^\s*({})\s".format("|".join(start_conditional + end_conditional)))
ENDS_WITH_BACKSLASH = re.compile(r"^[^#].*\\$")
END_DEFINE = re.compile("^\s*endef\s")
MAKEFILE_TARGET = re.compile("^[^# \t]+:\s")
START_DEFINE = re.compile("^\s*define\s")
def before(self):
self.define = False
self.backslash = False
self.makefile_target = False
def check_line(self, lineno, text):
if self.START_DEFINE.search(text):
self.define = True
return
if self.END_DEFINE.search(text):
self.define = False
@@ -209,34 +213,34 @@ class TypoInPackageVariable(_CheckFunction):
return
if self.REGEX.search(text) is None:
return ["{}:{}: possible typo: {} -> *{}*"
.format(self.filename, lineno, variable, self.package),
text]
class UselessFlag(_CheckFunction):
DEFAULT_AUTOTOOLS_FLAG = re.compile("^.*{}".format("|".join([
"_AUTORECONF\s*=\s*NO",
"_LIBTOOL_PATCH\s*=\s*YES"])))
DEFAULT_GENERIC_FLAG = re.compile("^.*{}".format("|".join([
"_INSTALL_IMAGES\s*=\s*NO",
"_INSTALL_REDISTRIBUTE\s*=\s*YES",
"_INSTALL_STAGING\s*=\s*NO",
"_INSTALL_TARGET\s*=\s*YES"])))
- END_CONDITIONAL = re.compile("^\s*(endif)")
- START_CONDITIONAL = re.compile("^\s*(ifeq|ifneq)")
+ END_CONDITIONAL = re.compile("^\s*({})".format("|".join(end_conditional)))
+ START_CONDITIONAL = re.compile("^\s*({})".format("|".join(start_conditional)))
def before(self):
self.conditional = 0
def check_line(self, lineno, text):
if self.START_CONDITIONAL.search(text):
self.conditional += 1
return
if self.END_CONDITIONAL.search(text):
self.conditional -= 1
return
# allow non-default conditionally overridden by default
if self.conditional > 0:
return
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables
2019-01-27 18:59 [Buildroot] [PATCH 0/8] Detect and fix overridden variables in .mk files Ricardo Martincoski
` (6 preceding siblings ...)
2019-01-27 18:59 ` [Buildroot] [PATCH 7/8] utils/check-package: handle ifdef/ifndef in .mk files Ricardo Martincoski
@ 2019-01-27 18:59 ` Ricardo Martincoski
2019-02-04 15:15 ` Titouan Christophe
` (2 more replies)
7 siblings, 3 replies; 29+ messages in thread
From: Ricardo Martincoski @ 2019-01-27 18:59 UTC (permalink / raw)
To: buildroot
For the general case, appending values to variables is OK and also a
good practice, like this:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR += value2
or this, when the above is not possible:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR := $(PACKAGE_VAR), value2
But this override is an error:
|PACKAGE_VAR = value1
|PACKAGE_VAR = value2
as well this one:
|ifeq ...
|PACKAGE_VAR += value1
|endif
|PACKAGE_VAR = value2
And this override is error-prone:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR = value2
Create a check function to warn about overridden variables.
Some variables are likely to have a default value that gets overridden
in a conditional, so ignore them. The name of such variables end in
_ARCH, _CPU, _SITE, _SOURCE or _VERSION.
After ignoring these variable names, there are a few exceptions to this
rule in the tree. For them use the comment that disables the check.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Simon Dawson <spdawson@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
package/gcc/gcc-final/gcc-final.mk | 2 +
package/zmqpp/zmqpp.mk | 1 +
utils/checkpackagelib/lib_mk.py | 60 ++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+)
diff --git a/package/gcc/gcc-final/gcc-final.mk b/package/gcc/gcc-final/gcc-final.mk
index 37b645d252..49f16f699e 100644
--- a/package/gcc/gcc-final/gcc-final.mk
+++ b/package/gcc/gcc-final/gcc-final.mk
@@ -55,36 +55,38 @@ endef
# Languages supported by the cross-compiler
GCC_FINAL_CROSS_LANGUAGES-y = c
GCC_FINAL_CROSS_LANGUAGES-$(BR2_INSTALL_LIBSTDCPP) += c++
GCC_FINAL_CROSS_LANGUAGES-$(BR2_TOOLCHAIN_BUILDROOT_FORTRAN) += fortran
GCC_FINAL_CROSS_LANGUAGES = $(subst $(space),$(comma),$(GCC_FINAL_CROSS_LANGUAGES-y))
HOST_GCC_FINAL_CONF_OPTS = \
$(HOST_GCC_COMMON_CONF_OPTS) \
--enable-languages=$(GCC_FINAL_CROSS_LANGUAGES) \
--with-build-time-tools=$(HOST_DIR)/$(GNU_TARGET_NAME)/bin
HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib*
# The kernel wants to use the -m4-nofpu option to make sure that it
# doesn't use floating point operations.
ifeq ($(BR2_sh4)$(BR2_sh4eb),y)
HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4,m4-nofpu"
+# check-package OverriddenVariable
HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
endif
ifeq ($(BR2_sh4a)$(BR2_sh4aeb),y)
HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4a,m4a-nofpu"
+# check-package OverriddenVariable
HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
endif
ifeq ($(BR2_GCC_SUPPORTS_LIBCILKRTS),y)
# libcilkrts does not support v8
ifeq ($(BR2_sparc),y)
HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
endif
# Pthreads are required to build libcilkrts
ifeq ($(BR2_PTHREADS_NONE),y)
HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
endif
ifeq ($(BR2_STATIC_LIBS),y)
diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
index 801766a7d8..ea6b50e826 100644
--- a/package/zmqpp/zmqpp.mk
+++ b/package/zmqpp/zmqpp.mk
@@ -6,32 +6,33 @@
ZMQPP_VERSION = 4.2.0
ZMQPP_SITE = $(call github,zeromq,zmqpp,$(ZMQPP_VERSION))
ZMQPP_INSTALL_STAGING = YES
ZMQPP_DEPENDENCIES = zeromq
ZMQPP_LICENSE = MPL-2.0
ZMQPP_LICENSE_FILES = LICENSE
ZMQPP_MAKE_OPTS = LD="$(TARGET_CXX)" BUILD_PATH=./build PREFIX=/usr
ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
# gcc bug internal compiler error: in merge_overlapping_regs, at
# regrename.c:304. This bug is fixed since gcc 6.
# By setting CONFIG to empty, all optimizations such as -funroll-loops
# -ffast-math -finline-functions -fomit-frame-pointer are disabled
ifeq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
+# check-package OverriddenVariable
ZMQPP_CONFIG =
endif
ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
ZMQPP_LDFLAGS += -latomic
endif
ifeq ($(BR2_PACKAGE_ZMQPP_CLIENT),y)
ZMQPP_DEPENDENCIES += boost
endif
ifeq ($(BR2_STATIC_LIBS),y)
ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=no
else ifeq ($(BR2_SHARED_STATIC_LIBS),y)
ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=yes
else ifeq ($(BR2_SHARED_LIBS),y)
diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index 51c6577d2c..00efeb7fb2 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -60,32 +60,92 @@ class Indent(_CheckFunction):
# comment can be indented or not inside define ... endef, so ignore it
if self.define and self.COMMENT.search(text):
return
if expect_tabs:
if not text.startswith("\t"):
return ["{}:{}: expected indent with tabs"
.format(self.filename, lineno),
text]
else:
if text.startswith("\t"):
return ["{}:{}: unexpected indent with tabs"
.format(self.filename, lineno),
text]
+class OverriddenVariable(_CheckFunction):
+ CONCATENATING = re.compile("^([A-Z0-9_]+)\s*(\+|:|)=\s*\$\(\\1\)")
+ END_CONDITIONAL = re.compile("^\s*({})".format("|".join(end_conditional)))
+ OVERRIDING_ASSIGNMENTS = [':=', "="]
+ START_CONDITIONAL = re.compile("^\s*({})".format("|".join(start_conditional)))
+ VARIABLE = re.compile("^([A-Z0-9_]+)\s*((\+|:|)=)")
+ USUALLY_OVERRIDDEN = re.compile("^[A-Z0-9_]+({})".format("|".join([
+ "_ARCH\s*=\s*",
+ "_CPU\s*=\s*",
+ "_SITE\s*=\s*",
+ "_SOURCE\s*=\s*",
+ "_VERSION\s*=\s*"])))
+
+ def before(self):
+ self.conditional = 0
+ self.unconditionally_set = []
+ self.conditionally_set = []
+
+ def check_line(self, lineno, text):
+ if self.START_CONDITIONAL.search(text):
+ self.conditional += 1
+ return
+ if self.END_CONDITIONAL.search(text):
+ self.conditional -= 1
+ return
+
+ m = self.VARIABLE.search(text)
+ if m is None:
+ return
+ variable, assignment = m.group(1, 2)
+
+ if self.conditional == 0:
+ if variable in self.conditionally_set:
+ self.unconditionally_set.append(variable)
+ if assignment in self.OVERRIDING_ASSIGNMENTS:
+ return ["{}:{}: unconditional override of variable {} previously conditionally set"
+ .format(self.filename, lineno, variable),
+ text]
+
+ if variable not in self.unconditionally_set:
+ self.unconditionally_set.append(variable)
+ return
+ if assignment in self.OVERRIDING_ASSIGNMENTS:
+ return ["{}:{}: unconditional override of variable {}"
+ .format(self.filename, lineno, variable),
+ text]
+ else:
+ if variable not in self.unconditionally_set:
+ self.conditionally_set.append(variable)
+ return
+ if self.CONCATENATING.search(text):
+ return
+ if self.USUALLY_OVERRIDDEN.search(text):
+ return
+ if assignment in self.OVERRIDING_ASSIGNMENTS:
+ return ["{}:{}: conditional override of variable {}"
+ .format(self.filename, lineno, variable),
+ text]
+
+
class PackageHeader(_CheckFunction):
def before(self):
self.skip = False
def check_line(self, lineno, text):
if self.skip or lineno > 6:
return
if lineno in [1, 5]:
if lineno == 1 and text.startswith("include "):
self.skip = True
return
if text.rstrip() != "#" * 80:
return ["{}:{}: should be 80 hashes ({}#writing-rules-mk)"
.format(self.filename, lineno, self.url_to_manual),
text,
--
2.17.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables
2019-01-27 18:59 ` [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables Ricardo Martincoski
@ 2019-02-04 15:15 ` Titouan Christophe
2019-02-05 19:25 ` Peter Korsgaard
2019-05-26 9:20 ` Yann E. MORIN
2 siblings, 0 replies; 29+ messages in thread
From: Titouan Christophe @ 2019-02-04 15:15 UTC (permalink / raw)
To: buildroot
Hi Ricardo and all,
On Sun, 2019-01-27 at 16:59 -0200, Ricardo Martincoski wrote:
> For the general case, appending values to variables is OK and also a
> good practice, like this:
> > PACKAGE_VAR = value1
> > ifeq ...
> > PACKAGE_VAR += value2
>
> or this, when the above is not possible:
> > PACKAGE_VAR = value1
> > ifeq ...
> > PACKAGE_VAR := $(PACKAGE_VAR), value2
>
> But this override is an error:
> > PACKAGE_VAR = value1
> > PACKAGE_VAR = value2
>
> as well this one:
> > ifeq ...
> > PACKAGE_VAR += value1
> > endif
> > PACKAGE_VAR = value2
>
> And this override is error-prone:
> > PACKAGE_VAR = value1
> > ifeq ...
> > PACKAGE_VAR = value2
>
> Create a check function to warn about overridden variables.
>
> Some variables are likely to have a default value that gets
> overridden
> in a conditional, so ignore them. The name of such variables end in
> _ARCH, _CPU, _SITE, _SOURCE or _VERSION.
>
> After ignoring these variable names, there are a few exceptions to
> this
> rule in the tree. For them use the comment that disables the check.
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Tested-by: Titouan Christophe <titouan.christophe@railnova.eu>
[Test: adding an offending line in mosquitto.mk to trigger warning]
> Cc: Simon Dawson <spdawson@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> package/gcc/gcc-final/gcc-final.mk | 2 +
> package/zmqpp/zmqpp.mk | 1 +
> utils/checkpackagelib/lib_mk.py | 60
> ++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
> diff --git a/package/gcc/gcc-final/gcc-final.mk b/package/gcc/gcc-
> final/gcc-final.mk
> index 37b645d252..49f16f699e 100644
> --- a/package/gcc/gcc-final/gcc-final.mk
> +++ b/package/gcc/gcc-final/gcc-final.mk
> @@ -55,36 +55,38 @@ endef
> # Languages supported by the cross-compiler
> GCC_FINAL_CROSS_LANGUAGES-y = c
> GCC_FINAL_CROSS_LANGUAGES-$(BR2_INSTALL_LIBSTDCPP) += c++
> GCC_FINAL_CROSS_LANGUAGES-$(BR2_TOOLCHAIN_BUILDROOT_FORTRAN) +=
> fortran
> GCC_FINAL_CROSS_LANGUAGES = $(subst
> $(space),$(comma),$(GCC_FINAL_CROSS_LANGUAGES-y))
>
> HOST_GCC_FINAL_CONF_OPTS = \
> $(HOST_GCC_COMMON_CONF_OPTS) \
> --enable-languages=$(GCC_FINAL_CROSS_LANGUAGES) \
> --with-build-time-tools=$(HOST_DIR)/$(GNU_TARGET_NAME)/bin
>
> HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib*
> # The kernel wants to use the -m4-nofpu option to make sure that it
> # doesn't use floating point operations.
> ifeq ($(BR2_sh4)$(BR2_sh4eb),y)
> HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4,m4-nofpu"
> +# check-package OverriddenVariable
> HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
> endif
> ifeq ($(BR2_sh4a)$(BR2_sh4aeb),y)
> HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4a,m4a-nofpu"
> +# check-package OverriddenVariable
> HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
> endif
>
> ifeq ($(BR2_GCC_SUPPORTS_LIBCILKRTS),y)
>
> # libcilkrts does not support v8
> ifeq ($(BR2_sparc),y)
> HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
> endif
>
> # Pthreads are required to build libcilkrts
> ifeq ($(BR2_PTHREADS_NONE),y)
> HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
> endif
>
> ifeq ($(BR2_STATIC_LIBS),y)
> diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
> index 801766a7d8..ea6b50e826 100644
> --- a/package/zmqpp/zmqpp.mk
> +++ b/package/zmqpp/zmqpp.mk
> @@ -6,32 +6,33 @@
>
> ZMQPP_VERSION = 4.2.0
> ZMQPP_SITE = $(call github,zeromq,zmqpp,$(ZMQPP_VERSION))
> ZMQPP_INSTALL_STAGING = YES
> ZMQPP_DEPENDENCIES = zeromq
> ZMQPP_LICENSE = MPL-2.0
> ZMQPP_LICENSE_FILES = LICENSE
> ZMQPP_MAKE_OPTS = LD="$(TARGET_CXX)" BUILD_PATH=./build PREFIX=/usr
> ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
> ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
>
> # gcc bug internal compiler error: in merge_overlapping_regs, at
> # regrename.c:304. This bug is fixed since gcc 6.
> # By setting CONFIG to empty, all optimizations such as -funroll-
> loops
> # -ffast-math -finline-functions -fomit-frame-pointer are disabled
> ifeq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
> +# check-package OverriddenVariable
> ZMQPP_CONFIG =
> endif
>
> ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> ZMQPP_LDFLAGS += -latomic
> endif
>
> ifeq ($(BR2_PACKAGE_ZMQPP_CLIENT),y)
> ZMQPP_DEPENDENCIES += boost
> endif
>
> ifeq ($(BR2_STATIC_LIBS),y)
> ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=no
> else ifeq ($(BR2_SHARED_STATIC_LIBS),y)
> ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=yes
> else ifeq ($(BR2_SHARED_LIBS),y)
> diff --git a/utils/checkpackagelib/lib_mk.py
> b/utils/checkpackagelib/lib_mk.py
> index 51c6577d2c..00efeb7fb2 100644
> --- a/utils/checkpackagelib/lib_mk.py
> +++ b/utils/checkpackagelib/lib_mk.py
> @@ -60,32 +60,92 @@ class Indent(_CheckFunction):
> # comment can be indented or not inside define ... endef, so
> ignore it
> if self.define and self.COMMENT.search(text):
> return
>
> if expect_tabs:
> if not text.startswith("\t"):
> return ["{}:{}: expected indent with tabs"
> .format(self.filename, lineno),
> text]
> else:
> if text.startswith("\t"):
> return ["{}:{}: unexpected indent with tabs"
> .format(self.filename, lineno),
> text]
>
>
> +class OverriddenVariable(_CheckFunction):
> + CONCATENATING = re.compile("^([A-Z0-
> 9_]+)\s*(\+|:|)=\s*\$\(\\1\)")
> + END_CONDITIONAL =
> re.compile("^\s*({})".format("|".join(end_conditional)))
> + OVERRIDING_ASSIGNMENTS = [':=', "="]
> + START_CONDITIONAL =
> re.compile("^\s*({})".format("|".join(start_conditional)))
> + VARIABLE = re.compile("^([A-Z0-9_]+)\s*((\+|:|)=)")
> + USUALLY_OVERRIDDEN = re.compile("^[A-Z0-
> 9_]+({})".format("|".join([
> + "_ARCH\s*=\s*",
> + "_CPU\s*=\s*",
> + "_SITE\s*=\s*",
> + "_SOURCE\s*=\s*",
> + "_VERSION\s*=\s*"])))
> +
> + def before(self):
> + self.conditional = 0
> + self.unconditionally_set = []
> + self.conditionally_set = []
> +
> + def check_line(self, lineno, text):
> + if self.START_CONDITIONAL.search(text):
> + self.conditional += 1
> + return
> + if self.END_CONDITIONAL.search(text):
> + self.conditional -= 1
> + return
> +
> + m = self.VARIABLE.search(text)
> + if m is None:
> + return
> + variable, assignment = m.group(1, 2)
> +
> + if self.conditional == 0:
> + if variable in self.conditionally_set:
> + self.unconditionally_set.append(variable)
> + if assignment in self.OVERRIDING_ASSIGNMENTS:
> + return ["{}:{}: unconditional override of
> variable {} previously conditionally set"
> + .format(self.filename, lineno,
> variable),
> + text]
> +
> + if variable not in self.unconditionally_set:
> + self.unconditionally_set.append(variable)
> + return
> + if assignment in self.OVERRIDING_ASSIGNMENTS:
> + return ["{}:{}: unconditional override of variable
> {}"
> + .format(self.filename, lineno, variable),
> + text]
> + else:
> + if variable not in self.unconditionally_set:
> + self.conditionally_set.append(variable)
> + return
> + if self.CONCATENATING.search(text):
> + return
> + if self.USUALLY_OVERRIDDEN.search(text):
> + return
> + if assignment in self.OVERRIDING_ASSIGNMENTS:
> + return ["{}:{}: conditional override of variable {}"
> + .format(self.filename, lineno, variable),
> + text]
> +
> +
> class PackageHeader(_CheckFunction):
> def before(self):
> self.skip = False
>
> def check_line(self, lineno, text):
> if self.skip or lineno > 6:
> return
>
> if lineno in [1, 5]:
> if lineno == 1 and text.startswith("include "):
> self.skip = True
> return
> if text.rstrip() != "#" * 80:
> return ["{}:{}: should be 80 hashes ({}#writing-
> rules-mk)"
> .format(self.filename, lineno,
> self.url_to_manual),
> text,
Thanks !
Titouan
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables
2019-01-27 18:59 ` [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables Ricardo Martincoski
2019-02-04 15:15 ` Titouan Christophe
@ 2019-02-05 19:25 ` Peter Korsgaard
2019-05-26 9:20 ` Yann E. MORIN
2 siblings, 0 replies; 29+ messages in thread
From: Peter Korsgaard @ 2019-02-05 19:25 UTC (permalink / raw)
To: buildroot
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:
> For the general case, appending values to variables is OK and also a
> good practice, like this:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR += value2
> or this, when the above is not possible:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR := $(PACKAGE_VAR), value2
> But this override is an error:
> |PACKAGE_VAR = value1
> |PACKAGE_VAR = value2
> as well this one:
> |ifeq ...
> |PACKAGE_VAR += value1
> |endif
> |PACKAGE_VAR = value2
> And this override is error-prone:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR = value2
> Create a check function to warn about overridden variables.
> Some variables are likely to have a default value that gets overridden
> in a conditional, so ignore them. The name of such variables end in
> _ARCH, _CPU, _SITE, _SOURCE or _VERSION.
> After ignoring these variable names, there are a few exceptions to this
> rule in the tree. For them use the comment that disables the check.
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Simon Dawson <spdawson@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables
2019-01-27 18:59 ` [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables Ricardo Martincoski
2019-02-04 15:15 ` Titouan Christophe
2019-02-05 19:25 ` Peter Korsgaard
@ 2019-05-26 9:20 ` Yann E. MORIN
2019-05-26 11:05 ` Arnout Vandecappelle
2 siblings, 1 reply; 29+ messages in thread
From: Yann E. MORIN @ 2019-05-26 9:20 UTC (permalink / raw)
To: buildroot
Ricardo, All,
On 2019-01-27 16:59 -0200, Ricardo Martincoski spake thusly:
> For the general case, appending values to variables is OK and also a
> good practice, like this:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR += value2
>
> or this, when the above is not possible:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR := $(PACKAGE_VAR), value2
>
> But this override is an error:
> |PACKAGE_VAR = value1
> |PACKAGE_VAR = value2
>
> as well this one:
> |ifeq ...
> |PACKAGE_VAR += value1
> |endif
> |PACKAGE_VAR = value2
>
> And this override is error-prone:
> |PACKAGE_VAR = value1
> |ifeq ...
> |PACKAGE_VAR = value2
>
> Create a check function to warn about overridden variables.
>
> Some variables are likely to have a default value that gets overridden
> in a conditional, so ignore them. The name of such variables end in
> _ARCH, _CPU, _SITE, _SOURCE or _VERSION.
Since I updated my host distribution to Ubuntu 19.04, flake8 now reports
tons of errors:
./utils/checkpackagelib/lib_mk.py:22:7: W605 invalid escape sequence '\s'
This is because Ubuntu has update pycodestyle to 2.4.0, which introduced
this new W605 check. It mandates that regexps be prefixed by an 'r'
(like a binary string be prefixed by a 'b'), like:
r"^\s*#"
However, when I fix all these 82 new errors, and run check-package, I
now get a warning about overriden license lists in the few packages
where we do override the license, like in dtc:
ifeq ($(BR2_PACKAGE_DTC_PROGRAMS),y)
DTC_LICENSE := $(DTC_LICENSE), GPL-2.0+ (programs)
[...]
endif
However, as far as I understand the code, there is an explicit condition
that allows concatenating to a variable (the CONCATENATING regexp), now
line 127.
Do you have an idea what's going on now?
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables
2019-05-26 9:20 ` Yann E. MORIN
@ 2019-05-26 11:05 ` Arnout Vandecappelle
0 siblings, 0 replies; 29+ messages in thread
From: Arnout Vandecappelle @ 2019-05-26 11:05 UTC (permalink / raw)
To: buildroot
On 26/05/2019 11:20, Yann E. MORIN wrote:
[snip]
> However, when I fix all these 82 new errors, and run check-package, I
> now get a warning about overriden license lists in the few packages
> where we do override the license, like in dtc:
>
> ifeq ($(BR2_PACKAGE_DTC_PROGRAMS),y)
> DTC_LICENSE := $(DTC_LICENSE), GPL-2.0+ (programs)
> [...]
> endif
This should not trigger an error (which is actually a bug in check-package, but
that's a different issue). check-package code is:
if self.conditional == 0:
...
else:
if variable not in self.unconditionally_set:
self.conditionally_set.append(variable)
return
i.e. because we're now in a conditional, and the variable was unconditionally
assigned before, it's just added to conditionally_set without further checking.
I don't see how replacing \s by \\s (or putting r in front of the string, which
is equivalent) could change that behaviour. Except maybe the
START_CONDITIONAL/END_CONDITIONAL REs are no longer correct?
> However, as far as I understand the code, there is an explicit condition
> that allows concatenating to a variable (the CONCATENATING regexp), now
> line 127.
Note that if you put an r in front, you have to replace \\ by \ again.
CONCATENATING has \\1...
Regards,
Arnout
>
> Do you have an idea what's going on now?
>
> Regards,
> Yann E. MORIN.
>
^ permalink raw reply [flat|nested] 29+ messages in thread