On Mon, Jun 08, 2015 at 12:34:25AM +0300, Andrey Korolyov wrote: > On Sun, Jun 7, 2015 at 11:14 PM, Ben Pfaff <b...@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 <b...@nicira.com> wrote: > >> > Setting CFLAGS by hand before invoking dpkg-buildflags is ineffective, > >> > because dpkg-buildflags overrides it. > >> > > >> > Reported-by: Andrey Korolyov <and...@xdel.ru> > >> > Signed-off-by: Ben Pfaff <b...@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
