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

[Roman Yeryomin]

On 2 May 2017 at 10:25, Felix Fietkau <nbd at nbd.name> wrote:
> 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.

I'm talking about board_name, not model. I've added model to the file
only because /tmp/sysinfo/model exists.
You say that including 365 (non-functional) lines is better than 15
(functional) when you only need board_name? I cannot understand
that...
And if you need functions.sh also then it includes board.sh, which is
nothing, compared to functions.sh itself, and nothing is broken.

Regards,
Roman