Skip to contentSkip to conversation

Introduce errors for legacy API

PATCH New Arnout Vandecappelle (Essensium/Mind) 3 participants 12 messages Nov 12, 2012 – Nov 30, 2012 Kbuild

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

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
index 68190a5..71b811b 100644
@@ -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
index c526fe1..300a0eb 100644
@@ -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
new file mode 100644
index 0000000..2e015a1
@@ -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
#2 lore raw

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
#3 lore raw

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_LEGACY here 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
#4 lore raw

PATCH 1/5 pkg-infra: introduce errors for legacy API

Quoted 9 lines · Arnout Vandecappelle
Quoted 1 line

BR2_LEGACY here 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
#5 lore raw

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
index 2e015a1..c8b568d 100644
@@ -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
index 9fdc745..2dd81c7 100644
@@ -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
#6 lore raw

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
index c8b568d..e0b7ec2 100644
@@ -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
#7 lore raw

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
index 4bc8784..04ccb92 100644
@@ -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
#8 lore raw

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
index 04ccb92..7ddc6ea 100644
@@ -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
#9 lore raw

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
#10 lore raw

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

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

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_DEPRECATED is 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
#11 lore raw

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