[arch-projects] [PATCH 2/2] network: deprecate net-tools, add iproute2 support

[Dave Reisner]

On Sun, May 22, 2011 at 05:50:18PM +0200, Tom Gundersen wrote:
> On Sun, May 22, 2011 at 4:43 PM, Dave Reisner <d at falconindy.com> wrote:
> > Provide large warnings when net-tools functionality is used. Add
> > documentation in rc.conf for the new iproute2 based config.
> 
> Functionality looks great (though untested). Nitpicks below.
> 
> > diff --git a/PKGBUILD b/PKGBUILD
> > index c5f2acd..5c9764c 100644
> > --- a/PKGBUILD
> > +++ b/PKGBUILD
> > @@ -9,9 +9,10 @@ groups=('base')
> >  conflicts=('initscripts')
> >  provides=('initscripts=9999')
> >  backup=(etc/inittab etc/rc.conf etc/rc.local etc/rc.local.shutdown)
> > -depends=('glibc' 'bash' 'grep' 'coreutils' 'udev>=139-1'
> > -         'net-tools' 'ncurses' 'kbd' 'findutils' 'sysvinit')
> > -optdepends=('bridge-utils: Network bridging support'
> > +depends=('glibc' 'bash' 'grep' 'coreutils' 'udev>=139-1' 'iproute2'
> > +         'ncurses' 'kbd' 'findutils' 'sysvinit')
> > +optdepends=('net-tools: legacy networking support'
> > +            'bridge-utils: Network bridging support'
> >             'dhcpcd: DHCP network configuration'
> >             'wireless_tools: Wireless networking')
> 
> Not important, but the above hunk could easily have been made +/- 4
> rather than +/- 7 (to make it easier to review :) ).
> 

Yeah, carelessness on my part. and now optdepends isn't sorted anymore.

/me panics

> > diff --git a/network b/network
> > index 9cd6109..7350149 100755
> > --- a/network
> > +++ b/network
> > @@ -7,6 +7,49 @@ for s in wireless bonding bridges dhcpcd; do
> >        [[ -f /etc/conf.d/$s ]] && . "/etc/conf.d/$s"
> >  done
> >
> > +# helper function to determine if legacy network support is needed
> > +need_legacy() {
> > +       if [[ -z $interface ]]; then
> > +               return 0 # need legacy
> > +       fi
> > +
> > +       return 1 # enough present for iproute2 support
> > +}
> 
> Isn't the logic here inverted: 1=false and 0=true? Maybe just the name
> needs to change?

Looks fine to me. bash evaluates a command as "true" when it returns a 0
(no-error) status.

> 
> > +deprecated() {
> > +       printf "${C_FAIL}Warning:${C_CLEAR} This functionality is deprecated.\n"
> > +       printf "  Please refer to /etc/rc.conf on how to define a single wired\n"
> > +       printf "  connection, or use a utility such as netcfg.\n"
> > +}
> 
> I guess the people who will see this are the ones who have not updated
> their rc.conf's and hence might not have the new comments either, so
> maybe it would be better to put a reference to the wiki, and add the
> explanation there too? Just a suggestion.
> 

Certainly, updating the wiki would be in order. That said, we need to
assume that our users are following the upgrade paths we set forth and
properly merging their pacnews.

d