[ovs-dev] [PATCH v2] configure: add configure option to disable building brcompat

[Chris Wright]

* Ben Pfaff (blp at nicira.com) wrote:
> On Tue, Mar 13, 2012 at 12:18:07PM -0700, Chris Wright wrote:
> > This adds ability to do:
> > 
> >   ./configure --disable-brcompat
> > 
> > to disable building userspace and kernel module associated with
> > providing linux bridge compatibility.  Sources should still be
> > distributed w/ make dist.
> > 
> > While there, update comment referring to long removed veth driver
> > which is now relevant for brcompat module.
> > 
> > Signed-off-by: Chris Wright <chrisw at sous-sol.org>
> 
> "git am" says:
> 
>     Applying: configure: add configure option to disable building brcompat
>     /home/blp/ovs/.git/rebase-apply/patch:81: trailing whitespace.
>          [AC_HELP_STRING([--disable-brcompat], 
>     /home/blp/ovs/.git/rebase-apply/patch:72: new blank line at EOF.
>     +
>     warning: 2 lines add whitespace errors.

Thanks, cut'n paste error on the trailing whitespace.  I can submit
another patch to fix that file if you're interested.

> In the OVS_CHECK_BRCOMPAT macro definition, the "yes" case and the
> "not specified" case are different (the "yes" sets BUILD_BRCOMPAT, the
> "missing" case does not), but I do not see why.

Simply a bug, good catch.

>  I'd also be inclined
> to explicitly set BUILD_BRCOMPAT to the empty string in the "no" case

Sure

> I'd be inclined to always clean the brcompatd.8 manpage, regardless of
> whether we build it.  So instead of this:
> 
> > diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk
> > index 8319c93..a8aadd2 100644
> > --- a/vswitchd/automake.mk
> > +++ b/vswitchd/automake.mk
> > @@ -1,10 +1,13 @@
> >  sbin_PROGRAMS += vswitchd/ovs-vswitchd
> > +if BUILD_BRCOMPAT
> > +  ovs_brcompatd_8 = vswitchd/ovs-brcompatd.8
> > +endif
> >  man_MANS += \
> >  	vswitchd/ovs-vswitchd.8 \
> > -	vswitchd/ovs-brcompatd.8
> > +	$(ovs_brcompatd_8)
> >  DISTCLEANFILES += \
> >  	vswitchd/ovs-vswitchd.8 \
> > -	vswitchd/ovs-brcompatd.8
> > +	$(ovs_brcompatd_8)
> >  
> >  vswitchd_ovs_vswitchd_SOURCES = \
> >  	vswitchd/bridge.c \
> 
> more like this:
> 
> man_MANS += vswitchd/ovs-vswitchd.8
> if BUILD_BRCOMPAT
>   man_MANS += vswitch/ovs-brcompatd.8
> endif
> DISTCLEANFILES += \
> 	vswitchd/ovs-vswitchd.8 \
> 	vswitchd/ovs-brcompatd.8
> 
> Not that it matters much either way.

Sure, this works for me.

thanks,
-chris