[ovs-dev] [PATCH 2/2] debian: Remove obsolete manual setting of CFLAGS and warnings from rules.

Ben Pfaff blp at nicira.com
Wed Jun 10 16:20:19 UTC 2015


On Mon, Jun 08, 2015 at 12:34:25AM +0300, Andrey Korolyov wrote:
> On Sun, Jun 7, 2015 at 11:14 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Sun, Jun 07, 2015 at 10:15:29PM +0300, Andrey Korolyov wrote:
> >> On Sun, Jun 7, 2015 at 7:48 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > Setting CFLAGS by hand before invoking dpkg-buildflags is ineffective,
> >> > because dpkg-buildflags overrides it.
> >> >
> >> > Reported-by: Andrey Korolyov <andrey at xdel.ru>
> >> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> >> > ---
> >> >  debian/rules | 9 +--------
> >> >  1 file changed, 1 insertion(+), 8 deletions(-)
> >> >
> >> > diff --git a/debian/rules b/debian/rules
> >> > index 38ecd62..6d1ccec 100755
> >> > --- a/debian/rules
> >> > +++ b/debian/rules
> >> > @@ -22,13 +22,6 @@ PARALLEL =
> >> >  endif
> >> >  MAKEFLAGS += $(PARALLEL)
> >> >
> >> > -CFLAGS += -g
> >> > -ifneq (,$(filter noopt,$(DEB_BUILD_OPTIONS)))
> >> > -CFLAGS += -O0
> >> > -else
> >> > -CFLAGS += -O2
> >> > -endif
> >> > -
> >> >  # Old versions of dpkg-buildflags do not understand --export=configure.
> >> >  # When dpkg-buildflags does not understand an option, it prints its full
> >> >  # --help output on stdout, so we have to avoid that here.
> >> > @@ -45,7 +38,7 @@ configure-stamp:
> >> >         cd _debian && ( \
> >> >                 test -e Makefile || \
> >> >                 ../configure --prefix=/usr --localstatedir=/var --enable-ssl \
> >> > -                       --sysconfdir=/etc --host=$(DEB_HOST_GNU_TYPE) CFLAGS="$(CFLAGS)" \
> >> > +                       --sysconfdir=/etc --host=$(DEB_HOST_GNU_TYPE) \
> >> >                         $(buildflags) $(DATAPATH_CONFIGURE_OPTS))
> >> >         touch configure-stamp
> >>
> >> Hi,
> >>
> >> for a short explanation, this is a quite ugly workaround which relates
> >> to the issue reported three years ago in [1], still unfixed. In other
> >> words, it is pretty problematic to pass custom CFLAGS from rules as
> >> the helpers do ignore the way docs are currently describe, like
> >> setting DEB_CFLAGS_MAINT_PREPEND directly in rules. All other ways,
> >> including setting build flags in rules in a straightforward way, are
> >> going to be overriden by a helper, so I reproduced a "workaround" from
> >> [2]. Also I suppose that the underlying comment about buildflags
> >> should be removed as well, I missed that point initially. Technically
> >> speaking, we should discard this patch and fix the documentation and
> >> the behavior for dpkg-dev helpers, but it should take a bit more time
> >> to pass the fix down to Debian/Ubuntu packages and I am not a person
> >> who can probably make a right decision for a change of helpers`
> >> behavior.
> >>
> >> 1. https://lists.debian.org/debian-policy/2012/01/msg00142.html
> >> 2. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=664508
> >
> > Andrey, thanks for all the details.
> >
> > I am not sure whether this review means that I should apply this patch.
> > Do you support applying it?
> 
> Yes, by means of fixing things. This patch is fixing the issue which
> is currently contradicting its own documentation of this exact part of
> packaging flow, so ideally fix should`ve been issued for dpkg-dev
> instead.
> >
> > When you say that the comment about buildflags should be removed, do you
> > mean the one that starts "Old versions of dpkg-buildflags..."?  I
> > believe that that is still correct, and relevant, so can you explain
> > further?
> 
> Sorry, I`ve thought about an original version of patch there. For this
> patch we should probably put a remark that the buildflags overrides
> within 'rules' will be ineffective and one should always use
> environment variables instead (at least until pointed bug will be
> fixed).

Thanks for the review.  I applied these patches to master.

I don't really understand some of your comments here.  If you could
express them in the form of a patch, then it would make it clearer and
I'd be able to apply it directly.

Thanks,

Ben.



More information about the dev mailing list