[LEDE-DEV] [PATCH 1/5] base-files: introduce /lib/functions/board.sh

[Felix Fietkau]

On 2017-05-02 00:50, Roman Yeryomin wrote:
> On 30 April 2017 at 12:46, Felix Fietkau <nbd at nbd.name> wrote:
>> On 2017-04-28 22:51, Roman Yeryomin wrote:
>>> On 28 April 2017 at 20:12, Felix Fietkau <nbd at nbd.name> wrote:
>>>> On 2017-04-28 14:56, Roman Yeryomin wrote:
>>>>> Signed-off-by: Roman Yeryomin <roman at advem.lv>
>>>>> ---
>>>>>  package/base-files/files/lib/functions/board.sh | 17 +++++++++++++++++
>>>>>  1 file changed, 17 insertions(+)
>>>>>  create mode 100644 package/base-files/files/lib/functions/board.sh
>>>>>
>>>>> diff --git a/package/base-files/files/lib/functions/board.sh b/package/base-files/files/lib/functions/board.sh
>>>>> new file mode 100644
>>>>> index 0000000..28f3bad
>>>>> --- /dev/null
>>>>> +++ b/package/base-files/files/lib/functions/board.sh
>>>>> @@ -0,0 +1,17 @@
>>>>> +#!/bin/sh
>>>>> +
>>>>> +sysinfo()
>>>>> +{
>>>>> +     local file=$1
>>>>> +     [ -e /tmp/sysinfo/$file ] && cat /tmp/sysinfo/$file || echo "generic"
>>>>> +}
>>>>> +
>>>>> +board_name()
>>>>> +{
>>>>> +     sysinfo board_name
>>>>> +}
>>>>> +
>>>>> +board_model()
>>>>> +{
>>>>> +     sysinfo model
>>>>> +}
>>>> Do we really need board_model() at all? I think it's better to leave the
>>>> board_name() function where it is instead of adding this extra include file.
>>>>
>>>
>>> If we don't need it why it exists then?
>> It exists in various files because of random copy&paste madness. It
>> hasn't been used in years, so it does not make any sense to carry it
>> forward.
> 
> This is another reason why it should be kept generic and changed in
> one place if needed.
Something not having been used in years and simply carried forward by
mindless copy&paste is a good reason to keep it generic?
I'm sorry, this makes no sense at all. Please just delete this nonsense.

>>> I think it's better to leave as separate file because it's mostly used
>>> separately, sourcing platform file, but for functions.sh it's
>>> negligible change.
>> The function is so small that a separate file does not make any sense.
>> If you don't want it in functions.sh, you could try to find a different
>> one where it fits better. But please don't add a new file for this tiny
>> piece of code.
> 
> This tiny file will nuke a lot of lines from target scripts. So it's
> about optimizing the code actually.
> I don't see a better place for it because, like I've noticed before,
> the users mostly only need board name when they include platform
> files.
board_name already exists, and I've already converted e.g. the lantiq
target to use it directly from functions.sh.
board_model is not used, neither is the model function from any of the
messy copy&paste target files. Which means you're arguing for having a
separate source file for a function that in its current form only has
one single line of code.

In my opinion the best course of action is to nuke your patches 1-3
completely, rework patch 4 to adjust the generic code as proposed, and
adjust patch 5 to simply use the board_name() from /lib/functions.sh

Just ignore the board_model nonsense, it does not need to be part of the
shell 'API' at all.

- Felix