[Buildroot] [PATCH for-2012.11 0/5] Introduce errors for legacy API

[Buildroot] [PATCH for-2012.11 0/5] Introduce errors for legacy API

[Arnout Vandecappelle]

 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

[Buildroot] [PATCH for-2012.11 1/5] pkg-infra: introduce errors for legacy API

[Arnout Vandecappelle]

From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

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.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 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

diff --git a/Config.in b/Config.in
index 68190a5..71b811b 100644
--- a/Config.in
+++ b/Config.in
@@ -430,3 +430,5 @@ source "fs/Config.in"
 source "boot/Config.in"
 
 source "linux/Config.in"
+
+source "Config.in.legacy"
diff --git a/Config.in.legacy b/Config.in.legacy
new file mode 100644
index 0000000..4bc8784
--- /dev/null
+++ b/Config.in.legacy
@@ -0,0 +1,51 @@
+#
+# Config.in.legacy - support for backward compatibility
+#
+# When an existing Config.in symbol is removed, it should be added again in this
+# file, and take appropriate action to approximate backward compatibility. If
+# there is an equivalent (set of) new symbols, these can just be select'ed by
+# the old symbol. This makes sure that running 'make oldconfig' will make things
+# "just work" when upgrading to a new buildroot version. If the change is too
+# fundamental and cannot be fixed by a simple select, then the old symbol should
+# select BR2_LEGACY.  If that symbol is set, the build will issue an error.
+#
+# When adding legacy symbols to this file, add them to the front. The oldest
+# symbols will be removed again after about two years.
+#
+# The symbol should be copied as-is from the place where it was previously
+# defined, but the help text should be removed or replaced with something that
+# explains how to fix it.
+
+config BR2_LEGACY
+	bool
+	help
+	  This option is selected automatically when your old .config uses an
+	  option that no longer exists in current buildroot. In that case, the
+	  build will fail. Look for config options which are selected in the
+	  menu below: they no longer exist and should be replaced by something
+	  else.
+
+# This comment fits exactly in a 80-column display
+comment "Legacy detected: check the content of the menu below"
+	depends on BR2_LEGACY
+
+# This option should get a new name with every buildroot release, so it defaults
+# to y again for people who upgrade.
+menuconfig BR2_LEGACY_CHECK_2012_11
+	bool "Check for legacy config options"
+	default y
+	help
+	  Select this option to see the config options that are present in your
+	  current .config but are no longer supported by buildroot. If any of
+	  the options in this menu is selected, they should be replaced with
+	  something else. As long as they stay selected, the build will fail.
+	  Just de-select this option to automatically remove all the legacy
+	  configuration.
+
+if BR2_LEGACY_CHECK_2012_11
+
+#
+# Legacy options from 2012.08
+#
+
+endif
diff --git a/Makefile b/Makefile
index c526fe1..300a0eb 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
-
diff --git a/Makefile.legacy b/Makefile.legacy
new file mode 100644
index 0000000..2e015a1
--- /dev/null
+++ b/Makefile.legacy
@@ -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
-- 
1.7.10.4

[Buildroot] [PATCH for-2012.11 2/5] legacy: move old GENTARGETS macros to Makefile.legacy

[Arnout Vandecappelle]

From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile.legacy     |    7 +++++++
 package/Makefile.in |    6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Makefile.legacy b/Makefile.legacy
index 2e015a1..c8b568d 100644
--- a/Makefile.legacy
+++ b/Makefile.legacy
@@ -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)))
diff --git a/package/Makefile.in b/package/Makefile.in
index 9fdc745..2dd81c7 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -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
-- 
1.7.10.4

[Buildroot] [PATCH for-2012.11 3/5] legacy: add error target for host-pkg-config

[Arnout Vandecappelle]

From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile.legacy |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Makefile.legacy b/Makefile.legacy
index c8b568d..e0b7ec2 100644
--- a/Makefile.legacy
+++ b/Makefile.legacy
@@ -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)))
-- 
1.7.10.4

[Buildroot] [PATCH for-2012.11 4/5] legacy: evtest is dropped from input-tools package

[Arnout Vandecappelle]

From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

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.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Config.in.legacy |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Config.in.legacy b/Config.in.legacy
index 4bc8784..04ccb92 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -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
-- 
1.7.10.4

[Buildroot] [PATCH for-2012.11 5/5] legacy: BR2_PACKAGE_LIBINTL is replaced by gettext

[Arnout Vandecappelle]

From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Config.in.legacy |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Config.in.legacy b/Config.in.legacy
index 04ccb92..7ddc6ea 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -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
-- 
1.7.10.4

[Buildroot] [PATCH for-2012.11 1/5] pkg-infra: introduce errors for legacy API

[Thomas Petazzoni]

On Mon, 12 Nov 2012 21:08:28 +0100, Arnout Vandecappelle

> +# 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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH for-2012.11 1/5] pkg-infra: introduce errors for legacy API

[Arnout Vandecappelle]

On 11/12/12 22:05, Thomas Petazzoni wrote:
>
> On Mon, 12 Nov 2012 21:08:28 +0100, Arnout Vandecappelle
>
>> +# 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
-- 
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

[Buildroot] [PATCH for-2012.11 1/5] pkg-infra: introduce errors for legacy API

[Thomas Petazzoni]

On Mon, 12 Nov 2012 22:18:30 +0100, Arnout Vandecappelle wrote:

> > 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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH for-2012.11 0/5] Introduce errors for legacy API

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> 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.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH for-2012.11 0/5] Introduce errors for legacy API

[Arnout Vandecappelle]

On 15/11/12 12:23, Peter Korsgaard wrote:
> >>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind)<arnout@mind.be>  writes:
[snip]
>   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.

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

  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?

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

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

[Buildroot] [PATCH for-2012.11 0/5] Introduce errors for legacy API

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> 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!

-- 
Bye, Peter Korsgaard