[Buildroot] [PATCH v2] set simple network setup via the system configuration submenu

[Jeremy Rosen <jeremy.rosen@openwide.fr>]

Yann, all...


> 
> I've meant to review this earlier, but then I always backed off
> because
> I'm not too happy with this kind of stuff, without being able to
> pinpoint the exact underlying reason. Call it gut feelings if you
> want...
> 
> Although I like it when we present user-friendly configuration
> options,
> and this certainly is to some extent, I worry about the option-creep
> we
> are going to have with this kind of options.
> 
> I too am also responsible for this option-creep, so I'm noone to
> really
> blame any one trying for such a change... ;-)
> 
> Still, I would like to see where we going with that kind of setup,
> and
> how far we should go (or rather, where we should stop).
> 
> Certainly, this IPv4-only options are not too intrusive, yet they
> only
> cover a basic setup.
> 

For me it's more a case of being able to quickly test stuff with BR
without having to go through the hassle of adding an ovelray etc...
clone, configure, build, flash. Setting the network is a really
basic thing you always need to do (it's not very different from
setting the hostname, which is already in the system submenu)

It's really a shame that I missed posting that as a subject for 
ELCE, but at least you're opening the real debate :) 

This remarks don't block me working on the patch, it only blocks
merging the patch so i'll repost a v3 when it is ready anyway,
but it would be nice to have this discussion going on...

> Now, for some actual, high-level review (mostly about cosmetics):
> 
>   - multiple spearation lines, or spaces; this breaks the current
>   flow of
>     the "code", and is not in-line with the rest of the file. Please
>     use
>     only one line to separate /sections/, and do not double spaces,
>     especially in the help texts.
> 

will do

>   - I would make all into a single choice, with three entries:
>     - don't generate network settings
>     - use DHCP
>     - use static
> 

sounds good, will do...

>   - Also, we already have a /etc/network/interfaces file in the
>   skeleton,
>     with lo in it; I'd prefer you keep it that way, and not generate
>     the
>     lo entry, and just append the 'eth0' entry.
> 

that's very complicated because skeleton is copied very early in the 
build process whereas my script is called in FINALIZE. So if there
are multiple builds without clean between them, the na?ve approch
would end up with multiple eth0 lines.

I could mark the autogenerated section, cut it out and replace it
but that sounds very complicated/error prone for just the lo line.

I personally think that the most consistant way to do it is to 
remove /etc/network/interfaces from skeleton entirely and move 
the responsability for that file to the script.

* none : generate lo only
* dchp : generate lo and eth0-dhcp
* static :  generate lo and eth0-static

that would be simpler, easier to maintain (all network conf stuff
in one place) and users can still override it easily with overlays

The only problematic case would be users that use a custom skeleton
to override the default one, but they should probably migrate to
overlays anyway.

So, I will probably remove the file from skeleton unless someone
objects to it since that sounds like the best approch to me...
(if the skeleton reorg make it into buildroot first I will also
adapt accordingly)

As usual I will wait a couple of days before a respin 

Thx for the comments

Regards
Jeremy

> Regards,
> Yann E. MORIN.
> 
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> |  conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
> |               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/
> |  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
> |   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
>