Introduce errors for legacy API
cover Introduce errors for legacy API
This patch series introduces a mechanism to give relatively clear error
messages when legacy API is used. The first patch introduces the mechanism, subsequent messages use it to report each of the API changes since 2012.08.
Config options that are no longer supported are moved to Config.in.legacy.
When any of these is selected in an old .config file, a comment will appear to warn the user of it, and it is possible to see which option caused the error in the new 'Check legacy' menu. Unselecting that menuconfig disables all the legacy options in one go.
If any such option remains, 'make' will fail immediately with a message
that legacy options are still selected. However, if BR2_DEPRECATED is selected, that error message is suppressed.
To see this in action, do 'echo BR2_PACKAGE_LIBINTL >> .config' and run
make menuconfig; make.
A new Makefile.in.legacy makes it possible to add make infrastructure
that warns about features that no longer exist. Two examples are the GENTARGETS macro (moved from package/Makefile.in) and the host-pkg-config target.
I've tried to find all the API changes since 2012.08, by looking at the
diffstat. I probably didn't find them all, but it's better than nothing.
There are two changes for which I don't have a solution.
- BR2_INET_RPC: since packages 'depend on' this option, there is really no way to detect that some Config.in is still using it. The result will be that some packages are just not selectable. Hopefully that's enough of a hint for users to find the issue.
- xtensa: since there are patches on the list to re-introduce xtensa, I haven't added it to Config.in.legacy. If these xtensa patches don't make it into 2012.11, then there probably should be a Config.in.legacy for it.
Regards,
Arnout
PATCH 1/5 pkg-infra: introduce errors for legacy API
From: "Arnout Vandecappelle (Essensium/Mind)" <[email protected]>
As discussed in the BR developer days, we want to be more strict about API changes in buildroot. I.e., we want to make it less likely that a user's customizations break down after upgrading buildroot.
A first step is to make sure that the user is warned about API changes. This patch introduces Makefile.legacy and Config.in.legacy, which will issue clear error messages for such situations.
--- Config.in | 2 ++ Config.in.legacy | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 7 ++++++- Makefile.legacy | 13 +++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 Config.in.legacy create mode 100644 Makefile.legacy
Config.in +2 -0
@@ -430,3 +430,5 @@ source "fs/Config.in" source "boot/Config.in" source "linux/Config.in" + +source "Config.in.legacy"
Config.in.legacy new file +51 -0
51 lines not on this page · raw message
Makefile +6 -1
@@ -286,6 +286,12 @@ endif all: world +# Include legacy before the other things, because package .mk files +# may rely on it. +ifneq ($(BR2_DEPRECATED),y) +include Makefile.legacy +endif + include package/Makefile.in include support/dependencies/dependencies.mk
@@ -756,4 +762,3 @@ print-version: include docs/manual/manual.mk .PHONY: $(noconfig_targets) -
Makefile.legacy new file +13 -0
@@ -0,0 +1,13 @@ +# +# Makefile.legacy - support for backward compatibility +# +# This file contains placeholders to detect backward-compatibility problems. +# When a buildroot "API" feature is being deprecated, a rule should be added +# here that issues an error when the old feature is used. +# +# This file is not included if BR2_DEPRECATED is selected, so it is possible +# to bypass the errors (although that's usually a bad idea). + +ifeq ($(BR2_LEGACY),y) +$(error "You have legacy configuration in your .config! Please check your configuration.") +endif
Signature · 1 line
1.7.10.4
PATCH 1/5 pkg-infra: introduce errors for legacy API
On Mon, 12 Nov 2012 21:08:28 +0100, Arnout Vandecappelle
Makefile endif · line 293
Quoted 5 lines
Makefile +5
-0+# Include legacy before the other things, because package .mk files +# may rely on it. +ifneq ($(BR2_DEPRECATED),y) +include Makefile.legacy +endif
BR2_LEGACY here maybe?
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/5 pkg-infra: introduce errors for legacy API
Quoted 10 lines · Thomas Petazzoni
On Mon, 12 Nov 2012 21:08:28 +0100, Arnout Vandecappelle
Makefile endif · line 293
Quoted 5 lines
Makefile +5
-0+# Include legacy before the other things, because package .mk files +# may rely on it. +ifneq ($(BR2_DEPRECATED),y) +include Makefile.legacy +endif
BR2_LEGACYhere maybe?
No that would not make sense - we want to get the error message if BR2_LEGACY
is selected (automatically, by Config.in.legacy).
The idea of the condition is to have an escape path, if you don't want to deal with all the stuff that is misconfigured (not only legacy config symbols but also host-pkg-config etc.), you can just build with BR2_DEPRECATED=y and see how far you get.
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286540 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: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
PATCH 1/5 pkg-infra: introduce errors for legacy API
Quoted 9 lines · Arnout Vandecappelle
Quoted 1 line
BR2_LEGACYhere maybe?No that would not make sense - we want to get the error message if
BR2_LEGACY
is selected (automatically, by Config.in.legacy).The idea of the condition is to have an escape path, if you don't want to deal with all the stuff that is misconfigured (not only legacy config symbols but also host-pkg-config etc.), you can just build with BR2_DEPRECATED=y and see how far you get.
Aaah, ok.
Thanks!
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 2/5 legacy: move old GENTARGETS macros to Makefile.legacy
In reply to Arnout Vandecappelle (Essensium/Mind)From: "Arnout Vandecappelle (Essensium/Mind)" <[email protected]>
--- Makefile.legacy | 7 +++++++ package/Makefile.in | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-)
Makefile.legacy +7 -0
@@ -11,3 +11,10 @@ ifeq ($(BR2_LEGACY),y) $(error "You have legacy configuration in your .config! Please check your configuration.") endif + +# +# Legacy options from 2012.05 +# +GENTARGETS = $$(error The GENTARGETS macro no longer exists; use $$$$(eval $$$$(generic-package)) or $$$$(eval $$$$(host-generic-package))) +AUTOTARGETS = $$(error The AUTOTARGETS macro no longer exists; use $$$$(eval $$$$(autotools-package)) or $$$$(eval $$$$(host-autotools-package))) +CMAKETARGETS = $$(error The CMAKETARGETS macro no longer exists; use $$$$(eval $$$$(cmake-package)) or $$$$(eval $$$$(host-cmake-package)))
package/Makefile.in +0 -6
@@ -305,12 +305,6 @@ else SHARED_STATIC_LIBS_OPTS=--enable-static --enable-shared endif -# Warn if a package uses the deprecated GENTARGETS macros. This can be -# removed again for BR-2012.11. -GENTARGETS = $$(error The GENTARGETS macro no longer exists; use $$$$(eval $$$$(generic-package)) or $$$$(eval $$$$(host-generic-package))) -AUTOTARGETS = $$(error The AUTOTARGETS macro no longer exists; use $$$$(eval $$$$(autotools-package)) or $$$$(eval $$$$(host-autotools-package))) -CMAKETARGETS = $$(error The CMAKETARGETS macro no longer exists; use $$$$(eval $$$$(cmake-package)) or $$$$(eval $$$$(host-cmake-package))) - include package/pkg-utils.mk include package/pkg-download.mk include package/pkg-autotools.mk
Signature · 1 line
1.7.10.4
PATCH 3/5 legacy: add error target for host-pkg-config
In reply to Arnout Vandecappelle (Essensium/Mind)From: "Arnout Vandecappelle (Essensium/Mind)" <[email protected]>
--- Makefile.legacy | 10 ++++++++++ 1 file changed, 10 insertions(+)
Makefile.legacy +10 -0
@@ -13,6 +13,16 @@ $(error "You have legacy configuration in your .config! Please check your config endif # +# Legacy options from 2012.08 +# + +host-pkg-config: + @$(call MESSAGE,host-pkg-config is replaced by host-pkgconf) + @$(call MESSAGE,please update your .mk files) + @false +.PHONY: host-pkg-config + +# # Legacy options from 2012.05 # GENTARGETS = $$(error The GENTARGETS macro no longer exists; use $$$$(eval $$$$(generic-package)) or $$$$(eval $$$$(host-generic-package)))
Signature · 1 line
1.7.10.4
PATCH 4/5 legacy: evtest is dropped from input-tools package
In reply to Arnout Vandecappelle (Essensium/Mind)From: "Arnout Vandecappelle (Essensium/Mind)" <[email protected]>
We select BR2_PACKAGE_EVTEST automatically. This has only limited use:
when the LEGACY_CHECK menu is disabled in menuconfig (or even oldconfig), it will also unselect BR2_PACKAGE_EVTEST again. Still, it can serve as a hint of how to fix things.
--- Config.in.legacy | 7 +++++++ 1 file changed, 7 insertions(+)
Config.in.legacy +7 -0
@@ -48,4 +48,11 @@ if BR2_LEGACY_CHECK_2012_11 # Legacy options from 2012.08 # +config BR2_PACKAGE_INPUT_TOOLS_EVTEST + bool "input-tools evtest is now a separate package evtest" + select BR2_LEGACY + select BR2_PACKAGE_EVTEST + help + The evtest program from input-tools is now a separate package. + endif
Signature · 1 line
1.7.10.4
PATCH 5/5 legacy: BR2_PACKAGE_LIBINTL is replaced by gettext
In reply to Arnout Vandecappelle (Essensium/Mind)From: "Arnout Vandecappelle (Essensium/Mind)" <[email protected]>
--- Config.in.legacy | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
Config.in.legacy +15 -0
@@ -48,6 +48,21 @@ if BR2_LEGACY_CHECK_2012_11 # Legacy options from 2012.08 # +config BR2_PACKAGE_GETTEXT_STATIC + bool "libgettext.a is now selected by BR2_PREFER_STATIC_LIB" + select BR2_LEGACY + help + To build a static gettext library, select BR2_PREFER_STATIC_LIB. + + +config BR2_PACKAGE_LIBINTL + bool "libintl" + select BR2_LEGACY + select BR2_PACKAGE_GETTEXT + help + libintl is now installed by selecting BR2_PACKAGE_GETTEXT. This now + only installs the library, not the executables. + config BR2_PACKAGE_INPUT_TOOLS_EVTEST bool "input-tools evtest is now a separate package evtest" select BR2_LEGACY
Signature · 1 line
1.7.10.4
cover Introduce errors for legacy API
In reply to Arnout Vandecappelle (Essensium/Mind)Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle (Essensium/Mind) <[email protected]> writes:
Arnout> This patch series introduces a mechanism to give relatively Arnout> clear error messages when legacy API is used. The first patch Arnout> introduces the mechanism, subsequent messages use it to report Arnout> each of the API changes since 2012.08.
Cool, thanks!
Arnout> Config options that are no longer supported are moved to Arnout> Config.in.legacy. When any of these is selected in an old Arnout> .config file, a comment will appear to warn the user of it, and Arnout> it is possible to see which option caused the error in the new Arnout> 'Check legacy' menu. Unselecting that menuconfig disables all Arnout> the legacy options in one go.
Is there ever any use case for not checking for legacy stuff?
I like it, the only problem is that the legacy warnings are shown as options when they are really warnings/instructions to the user about what has changed/what needs to get fixed.
I played around with making the options hidden and instead display a comment when it is selected - E.G:
config BR2_PACKAGE_LIBINTL
bool
select BR2_LEGACY
select BR2_PACKAGE_GETTEXT
config BR2_PACKAGE_LIBINTL
bool
select BR2_LEGACY
select BR2_PACKAGE_GETTEXT
if BR2_PACKAGE_LIBINTL
comment "libintl is now installed by selecting" comment "BR2_PACKAGE_GETTEXT. This now only installs the" comment "library, not the executables."
endif
The syntax is not so nice as kconfig doesn't support multi line comments, but OK. More importantly, hidden symbols like BR2_PACKAGE_LIBINTL seems to be ignored when the .config is read (which is what we normally want so they can be disabled again) so this only works when a custom package selects BR2_PACKAGE_LIBINTL, not if the user has manually enabled it.
Anybody with a better idea?
Arnout> If any such option remains, 'make' will fail immediately with Arnout> a message that legacy options are still selected. However, if Arnout> BR2_DEPRECATED is selected, that error message is suppressed.
Would it make sense to still warn (but not halt) when DEPRECATED is enabled?
Arnout> - BR2_INET_RPC: since packages 'depend on' this option, there Arnout> is really no way to detect that some Config.in is still using Arnout> it. The result will be that some packages are just not Arnout> selectable. Hopefully that's enough of a hint for users to find Arnout> the issue.
Arnout> - xtensa: since there are patches on the list to re-introduce Arnout> xtensa, I haven't added it to Config.in.legacy. If these xtensa Arnout> patches don't make it into 2012.11, then there probably should Arnout> be a Config.in.legacy for it.
I'll try to get xtensa in.
Signature · 1 line
Bye, Peter Korsgaard
cover Introduce errors for legacy API
Quoted 1 line · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle (Essensium/Mind)<[email protected]> writes:
[snip]
Quoted 8 lines
Arnout> Config options that are no longer supported are moved to
Arnout> Config.in.legacy. When any of these is selected in an old Arnout> .config file, a comment will appear to warn the user of it, and Arnout> it is possible to see which option caused the error in the new Arnout> 'Check legacy' menu. Unselecting that menuconfig disables all Arnout> the legacy options in one go.Is there ever any use case for not checking for legacy stuff?
Maybe not. Easy enough to remove the condition around the check.
Quoted 23 lines
I like it, the only problem is that the legacy warnings are shown as options when they are really warnings/instructions to the user about what has changed/what needs to get fixed.
I played around with making the options hidden and instead display a comment when it is selected - E.G:
config
BR2_PACKAGE_LIBINTL
bool
selectBR2_LEGACY
selectBR2_PACKAGE_GETTEXTconfig
BR2_PACKAGE_LIBINTL
bool
selectBR2_LEGACY
selectBR2_PACKAGE_GETTEXTif
BR2_PACKAGE_LIBINTL
comment "libintl is now installed by selecting" comment "BR2_PACKAGE_GETTEXT. This now only installs the" comment "library, not the executables."
endif
Quoted 6 lines
The syntax is not so nice as kconfig doesn't support multi line comments, but OK. More importantly, hidden symbols like
BR2_PACKAGE_LIBINTLseems to be ignored when the .config is read (which is what we normally want so they can be disabled again) so this only works when a custom package selectsBR2_PACKAGE_LIBINTL, not if the user has manually enabled it.
I see this as a major problem. I think most use cases in the Config.in
will be for user-selectable options that are no longer available, like BR2_PACKAGE_GETTEXT_STATIC and BR2_PACKAGE_INPUT_TOOLS_EVTEST. BR2_PACKAGE_LIBINTL is one that is normally used in select statements, but that's a bit exceptional.
In addition, if they are blind options, they will just disappear after running
silentoldconfig. The comment will get printed, but who will see that between all the other output?
Quoted 9 lines
Anybody with a better idea?
Arnout> If any such option remains, 'make' will fail immediately with
Arnout> a message that legacy options are still selected. However, if Arnout>BR2_DEPRECATEDis selected, that error message is suppressed.Would it make sense to still warn (but not halt) when DEPRECATED is enabled?
The problem with warnings is that we produce so much output that they'll
just be hidden. They would make sense if an error follows soon after. But in all likelihood, if an error follows, it will be much further down.
[snip]
So, is there still anything that I should change?
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286540 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: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
cover Introduce errors for legacy API
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle <[email protected]> writes:
Hi,
Arnout> The problem with warnings is that we produce so much output Arnout> that they'll just be hidden. They would make sense if an error Arnout> follows soon after. But in all likelihood, if an error follows, Arnout> it will be much further down.
Arnout> [snip]
Arnout> So, is there still anything that I should change?
I'm still not 100% happy with it, but I think this is the best we can do for 2012.11, so I've committed the series, thanks!
Signature · 1 line
Bye, Peter Korsgaard