| Message ID | da9c19eca655653a9e82.1378152470@argentina |
|---|---|
| State | Accepted |
| Headers | show |
Dear Thomas De Schampheleire, On Mon, 02 Sep 2013 22:07:50 +0200, Thomas De Schampheleire wrote: > The existing comments in Config.in.legacy are not entirely in-line with > current practice. The comments implies that BR2_LEGACY should not be set when > the conversion from old-to-new symbol can be done automatically using the > appropriate 'select' statements. However, none of the existing legacy options > does it this way. Moreover, I think it's intentional that the user is notified > of the change, so that the removal of the legacy options in later buildroot > versions no longer poses a problem. Does what we are doing today really makes sense? For example, option BR2_PACKAGE_FOO_BAR is renamed to BR2_PACKAGE_FOO_BARBLEH. BR2_PACKAGE_FOO_BAR is added to Config.in.legacy, with a message like "Option bar for package foo has been renamed barbleh". So the user goes into the sub-options of package "foo"... and discovers that barbleh is already enabled. So he kind of wonders what sort of drug Buildroot developers are taking, no? When there is a direct 1:1 mapping between the old option and the new option, is it really necessary to select BR2_LEGACY ? Shouldn't be select BR2_LEGACY only when there is really a change in behavior (like a package has been entirely removed, or a feature like toolchain on target has been removed) ? Best regards, Thomas
Hi Thomas, all, On Mon, Sep 2, 2013 at 10:54 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Thomas De Schampheleire, > > On Mon, 02 Sep 2013 22:07:50 +0200, Thomas De Schampheleire wrote: > >> The existing comments in Config.in.legacy are not entirely in-line with >> current practice. The comments implies that BR2_LEGACY should not be set when >> the conversion from old-to-new symbol can be done automatically using the >> appropriate 'select' statements. However, none of the existing legacy options >> does it this way. Moreover, I think it's intentional that the user is notified >> of the change, so that the removal of the legacy options in later buildroot >> versions no longer poses a problem. > > Does what we are doing today really makes sense? For example, option > BR2_PACKAGE_FOO_BAR is renamed to BR2_PACKAGE_FOO_BARBLEH. > BR2_PACKAGE_FOO_BAR is added to Config.in.legacy, with a message like > "Option bar for package foo has been renamed barbleh". So the user goes > into the sub-options of package "foo"... and discovers that barbleh is > already enabled. So he kind of wonders what sort of drug Buildroot > developers are taking, no? :-D > > When there is a direct 1:1 mapping between the old option and the new > option, is it really necessary to select BR2_LEGACY ? > > Shouldn't be select BR2_LEGACY only when there is really a change in > behavior (like a package has been entirely removed, or a feature like > toolchain on target has been removed) ? I think there are only a few options: 1. Don't ever rename or remove symbols (so no legacy problems ever). I don't think this is feasible/desirable. 2. Allow symbols to be renamed/removed if there is a good reason for it, but only mention it in the README or Changelog when there is a new release (no legacy menu). In this case, the user is completely on his own when migrating to another buildroot release, every action is manual. This was the situation before the legacy menu was introduced. 3. Use a legacy menu for renamed/removed symbols, indicating which symbols are now legacy. Don't do any automatic propagation of old to new symbols: simply warn the user and let him/her review and update the configuration. This is the simplest to implement, there are also no problems with weird kconfig behavior. By disabling the legacy option, the user simply acknowledges that he has seen the legacy messages, and the rest of the configuration remains untouched. 4. Use a legacy menu as in 3., but additionally perform automatic propagation of old to new symbols where possible and appropriate. This means 'select'ing the right symbols, and also introducing the following kconfig behavior: if you disable the legacy option that automatically selected the new symbol, the new symbol will be disabled too if you did not save your configuration yet (see my other patch and the resulting discussion). This is what is done at the moment, and this is the same path my patch series was continuing on. 5. Use a legacy menu only for symbols that cannot be automatically propagated. For all other legacy symbols, do the automatic propagation but do not inform the user. Here you will still have an old symbol 'select'ing the new one, and the old symbol should remain hidden (I think, to not confuse the user). I haven't tried this yet, but in my understanding the old symbol will remain selected, and will also be part of the defconfig. This would be confusing. Since in this case there is no communication to the user, I think there can be a problem with migration of defconfigs. Consider the following: a user starts with buildroot version A, saves his project's defconfig. Then he migrates to version B, in which some symbols have been marked as legacy, but automatically propagated. The user does not see a reason to update his defconfig as everything still works and he doesn't know better. Then, he migrates to buildroot version C, where the legacy symbols have suddenly been removed. Since his defconfig was not changed, there is no reference to the new alternatives of the legacy symbols, and his configuration is suddenly dysfunctional. I think this is not very nice. If the user would have been warned between the A->B transition, he would have realized he needed to save a new defconfig, and the transition from B->C would be successful. As it stands, I'm leaning towards 3 or 4, but am open for discussion. Best regards, Thomas
On 09/03/13 14:13, Thomas De Schampheleire wrote: > Hi Thomas, all, > > On Mon, Sep 2, 2013 at 10:54 PM, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: [snip] >> When there is a direct 1:1 mapping between the old option and the new >> option, is it really necessary to select BR2_LEGACY ? >> >> Shouldn't be select BR2_LEGACY only when there is really a change in >> behavior (like a package has been entirely removed, or a feature like >> toolchain on target has been removed) ? > > I think there are only a few options: Nice analysis! > 1. Don't ever rename or remove symbols (so no legacy problems ever). I > don't think this is feasible/desirable. This was actually more or less the intention when the Config.in.legacy was introduced. It was not really intended for giving Kconfig symbols more consistent names. Instead, it was intended to have a path forward in a situation as for gettext/libintl. There used to be a BR2_PACKAGE_LIBINTL symbol, which actually enabled an option in the gettext package instead of selecting the libintl package. Peter's point of view was/is that the Kconfig symbols are not visible to the user, so the user doesn't care about inconsistencies there. However, if a symbol is renamed, it suddenly does become visible to the user (even with the current legacy approach), so that should be avoided. > 2. Allow symbols to be renamed/removed if there is a good reason for > it, but only mention it in the README or Changelog when there is a new > release (no legacy menu). In this case, the user is completely on his > own when migrating to another buildroot release, every action is > manual. This was the situation before the legacy menu was introduced. > > 3. Use a legacy menu for renamed/removed symbols, indicating which > symbols are now legacy. Don't do any automatic propagation of old to > new symbols: simply warn the user and let him/her review and update > the configuration. This is the simplest to implement, there are also > no problems with weird kconfig behavior. By disabling the legacy > option, the user simply acknowledges that he has seen the legacy > messages, and the rest of the configuration remains untouched. > > 4. Use a legacy menu as in 3., but additionally perform automatic > propagation of old to new symbols where possible and appropriate. This > means 'select'ing the right symbols, and also introducing the > following kconfig behavior: if you disable the legacy option that > automatically selected the new symbol, the new symbol will be disabled > too if you did not save your configuration yet (see my other patch and > the resulting discussion). This is what is done at the moment, and > this is the same path my patch series was continuing on. > > 5. Use a legacy menu only for symbols that cannot be automatically > propagated. For all other legacy symbols, do the automatic propagation > but do not inform the user. Here you will still have an old symbol > 'select'ing the new one, and the old symbol should remain hidden (I > think, to not confuse the user). I haven't tried this yet, but in my > understanding the old symbol will remain selected, and will also be > part of the defconfig. This would be confusing. It's even worse. The option _must_ remain visible, otherwise it is stripped from the .config when it is read in. Also, when a defconfig is saved, it will be saved under the old name and the new name will not appear in the defconfig. So if the legacy part is removed a couple of years down the line, users who rely on defconfigs are screwed. IMHO, Kconfig is just too limited to do all the things we want to do. > Since in this case there is no communication to the user, I think > there can be a problem with migration of defconfigs. Consider the > following: a user starts with buildroot version A, saves his project's > defconfig. Then he migrates to version B, in which some symbols have > been marked as legacy, but automatically propagated. The user does not > see a reason to update his defconfig as everything still works and he > doesn't know better. Then, he migrates to buildroot version C, where > the legacy symbols have suddenly been removed. Since his defconfig was > not changed, there is no reference to the new alternatives of the > legacy symbols, and his configuration is suddenly dysfunctional. I > think this is not very nice. > If the user would have been warned between the A->B transition, he > would have realized he needed to save a new defconfig, and the > transition from B->C would be successful. > > > As it stands, I'm leaning towards 3 or 4, but am open for discussion. I prefer to keep things as they are, i.e. option 4. I'm also in favour of minimizing the number of simple renames. For instance, I think the rename of BR2_PACKAGE_DOSFSTOOLS_DOSFSCK was unnecessary. Regards, Arnout > > Best regards, > Thomas > >
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> writes: Hi, > The existing comments in Config.in.legacy are not entirely in-line with > current practice. The comments implies that BR2_LEGACY should not be set when > the conversion from old-to-new symbol can be done automatically using the > appropriate 'select' statements. However, none of the existing legacy options > does it this way. Moreover, I think it's intentional that the user is notified > of the change, so that the removal of the legacy options in later buildroot > versions no longer poses a problem. > Additionally, the comments now describe how to handle string options. > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> (Finally) committed series with some minor fixups to get them to apply, thanks!
On Sun, Oct 27, 2013 at 8:24 AM, Peter Korsgaard <jacmet@uclibc.org> wrote: >>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> writes: > > Hi, > > > The existing comments in Config.in.legacy are not entirely in-line with > > current practice. The comments implies that BR2_LEGACY should not be set when > > the conversion from old-to-new symbol can be done automatically using the > > appropriate 'select' statements. However, none of the existing legacy options > > does it this way. Moreover, I think it's intentional that the user is notified > > of the change, so that the removal of the legacy options in later buildroot > > versions no longer poses a problem. > > Additionally, the comments now describe how to handle string options. > > > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > > Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > (Finally) committed series with some minor fixups to get them to apply, > thanks! Thanks Peter! (and also thanks to the other fabulous people present at the Buildroot Developer Days :-) )
diff --git a/Config.in.legacy b/Config.in.legacy --- a/Config.in.legacy +++ b/Config.in.legacy @@ -1,13 +1,9 @@ # # 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 an existing Config.in symbol is removed, it should be added again in +# this file, and take appropriate action to approximate backward compatibility. +# This will make the transition for the user more convenient. # # When adding legacy symbols to this file, add them to the front. The oldest # symbols will be removed again after about two years. @@ -15,6 +11,46 @@ # 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. +# +# For bool options, the old symbol should select BR2_LEGACY, so that the user +# is informed at build-time about selected legacy options. +# If there is an equivalent (set of) new symbols, these should be select'ed by +# the old symbol for backwards compatibility. +# +# For string options, it is not possible to directly select another symbol. In +# this case, a hidden wrap bool option has to be added, that defaults to y if +# the old string is not set at its default value. The wrap symbol should select +# BR2_LEGACY. +# If the original symbol has been renamed, the new symbol should use the value +# of the old symbol as default. This requires a change outside of +# Config.in.legacy, and this should be clearly marked as such below, so that +# removal of legacy options also include the removal of these external +# references. +# +# [Example: renaming a string option from FOO to BAR] +# original symbol: +# config BR2_FOO_STRING +# string "Some foo string" +# +# becomes: +# config BR2_BAR_STRING +# string "Some bar string" +# default BR2_FOO_STRING if BR2_FOO_STRING != "" # legacy +# +# and in Config.in.legacy: +# config BR2_FOO_STRING +# string "The foo string has been renamed" +# help +# <suitable help text> +# +# config BR2_FOO_STRING_WRAP +# bool +# default y if BR2_FOO_STRING != "" +# select BR2_LEGACY +# +# # Note: BR2_FOO_STRING is still referenced from package/foo/Config.in +# +# [End of example] config BR2_LEGACY bool