[U-Boot] [PATCH 1/2] configs: Resync with savedefconfig

[Stephen Warren]

On 09/11/2016 08:24 PM, Masahiro Yamada wrote:
> 2016-09-10 7:25 GMT+09:00 Stephen Warren <swarren at wwwdotorg.org>:
>> On 09/09/2016 03:14 PM, Tom Rini wrote:
>>>
>>> On Fri, Sep 09, 2016 at 03:06:01PM -0600, Stephen Warren wrote:
>>>>
>>>> On 09/09/2016 01:11 PM, Tom Rini wrote:
>>>>>
>>>>> On Fri, Sep 09, 2016 at 01:09:43PM -0600, Stephen Warren wrote:
>>>>>>
>>>>>> On 09/09/2016 01:02 PM, Tom Rini wrote:
>>>>>>>
>>>>>>> On Fri, Sep 09, 2016 at 10:21:45AM -0600, Stephen Warren wrote:
>>>>>>>>
>>>>>>>> On 09/08/2016 07:19 PM, Tom Rini wrote:
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tom Rini <trini at konsulko.com>
>>>>>>>
>>>>>>> /bin/bash: ess: command not found
>>>>>>>>>
>>>>>>>>> diff --git a/configs/harmony_defconfig b/configs/harmony_defconfig
>>>>>>>>
>>>>>>>>
>>>>>>>>> -CONFIG_USE_PRIVATE_LIBGCC=y
>>>>>>>>
>>>>>>>>
>>>>>>>> I assume that's because =y is the default for that now?
>>>>>>>
>>>>>>>
>>>>>>> Yes.
>>>>>>
>>>>>>
>>>>>>>>> diff --git a/configs/p2771-0000-500_defconfig
>>>>>>>>> b/configs/p2771-0000-500_defconfig
>>>>>>>>
>>>>>>>>
>>>>>>>>> -CONFIG_TARGET_P2771_0000=y
>>>>>>>>
>>>>>>>>
>>>>>>>> I think we need to keep those two. Those two defconfigs are slightly
>>>>>>>> different configurations of U-Boot's p2771-0000 board/target.
>>>>>>>
>>>>>>>
>>>>>>> OK, then you need to submit a patch to fix the underlying problem, if
>>>>>>> there is one, really.  Keep in mind that what this is, is re-running
>>>>>>> savedefconfig for each target.  So if something odd drops out that
>>>>>>> means
>>>>>>> that it wasn't having any effect before.
>>>>>>
>>>>>>
>>>>>> I don't believe there is any underlying problem. The Kconfig option
>>>>>> in question is defined in arch/arm/mach-tegra/tegra186/Kconfig, when
>>>>>> building for p2771-0000-000 the option makes it into .config just
>>>>>> fine, and the value is used by board/nvidia/p2771-0000/Kconfig.
>>>>>> Isn't this a bug in savedefconfig? I'm CC'ing Masahiro to get some
>>>>>> insight.
>>>>>
>>>>>
>>>>> It is the default value then and isn't saved is I believe the answer.
>>>>
>>>>
>>>> I don't think it's the default; nothing selects that option and it
>>>> has no "default y".
>>>>
>>>> Or maybe since the value is inside a choice, and is the only entry
>>>> that's there, that does make it the default? If so, that seems a
>>>> little fragile; what if someone comes along and adds a new board
>>>> into the list, and puts it before the existing entry (e.g. to
>>>> maintain alphabetical sorting). That would changing the meaning of
>>>> the current defconfig file if the first entry is picked as default.
>>>> If so, I'm surprised if nobody has had that issue yet.
>>>
>>>
>>> It's all correct and I think we have had this hit once or twice when
>>> adding or changing the order.  It is a side effect of how Kconfig just
>>> works.
>>
>>
>> I'm not convinced. If I apply your patch, and also the following:
>>
>>> diff --git a/arch/arm/mach-tegra/tegra186/Kconfig
>>> b/arch/arm/mach-tegra/tegra186/Kconfig
>>> index 97cf23f31f80..05f691ed3ede 100644
>>> --- a/arch/arm/mach-tegra/tegra186/Kconfig
>>> +++ b/arch/arm/mach-tegra/tegra186/Kconfig
>>> @@ -7,6 +7,14 @@ if TEGRA186
>>>  choice
>>>         prompt "Tegra186 board select"
>>>
>>> +config TARGET_E9999_1234
>>> +       bool "NVIDIA Tegra186 E9999-1234 board"
>>> +       help
>>> +         P2771-0000 is a P3310 CPU board married to a P2597 I/O board.
>>> The
>>> +         combination contains SoC, DRAM, eMMC, SD card slot, HDMI, USB
>>> +         micro-B port, Ethernet, USB3 host port, SATA, PCIe, and two GPIO
>>> +         expansion headers.
>>> +
>>>  config TARGET_P2771_0000
>>>         bool "NVIDIA Tegra186 P2771-0000 board"
>>>         help
>>
>> ... then I get a build failure for p2771-0000-000_defconfig since .config
>> ends up containing CONFIG_TARGET_E9999_1234 rather than
>> CONFIG_TARGET_P2771_0000. I think we want to avoid that don't we? If we
>> don't, savedefconfig in the face of choice options seems rather too fragile,
>> or we need to force people to update *_defconfig any time any Kconfig choice
>> is edited.
>
> We have discussed this problem several times before.
>
>   - The first entry in a "choice" is the default, unless otherwise specified.
>   - "make savedefconfig" drops all the defaults to create a minimum
> set of configs.
>
> So, we need to be careful when we insert a new entry at the first
> or delete the first entry in a "choice".
>
> We are supposed to not re-sync defconfigs in Linux Kernel, so we are not hit
> by this issue much there, but we still be careful when we edit the
> first entry of a choice.

Really? In practice, it happens all the time. It's rather critical 
whenever adding new options to the defconfig, to separate out the 
/actual/ changes you're making from the simple act of "savedefconfig" 
itself making changes even without edits to the defconfig file.