Subject: debian-policy: please update example in 4.9.1 (debian/rules and DEB_BUILD_OPTIONS)

From: Niels Thykier <[email-masked]>
Date: Sat, 12 Feb 2011 14:25:40 +0100

Package: debian-policy
Version: 3.9.1.0
Severity: minor

Hey

The example in 4.9.1 suggests to set CFLAGS in a way that completely
overrides values from dpkg-buildpackage/dpkg-buildflags[1]:

CFLAGS = -Wall -g

This will set CFLAGS to "-Wall -g" regardless of what dpkg-buildflags
provides.  Possible alternatives that appears to work are:

CFLAGS := $(CFLAGS) -Wall -g

CFLAGS  = $(shell dpkg-buildflags --get CFLAGS) -Wall -g

While related to #578597, I believe it to be a distinct issue. This was triggered
by this email[2] on debian-mentors.

Thank you in advance,
~Niels

[1] http://www.debian.org/doc/debian-policy/ch-source.html#s-debianrules-options

[2] <[email-masked]>

- -- System Information:
Debian Release: 6.0
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.32-5-686 (SMP w/2 CPU cores)
Locale: LANG=en_DK.UTF-8, LC_CTYPE=en_DK.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

debian-policy depends on no packages.

debian-policy recommends no packages.

Versions of packages debian-policy suggests:
pn  doc-base                      <none>     (no description available)

- -- no debconf information

---

From: Julien Cristau <[email-masked]>
Date: Sat, 12 Feb 2011 22:32:34 +0100

> CFLAGS := $(CFLAGS) -Wall -g
That would be wrong.  A package build shouldn't depend on random env
variables.

> by this email[2] on debian-mentors.
Seems to be the exact same request to me?

Cheers,
Julien

---

From: Niels Thykier <[email-masked]>
Date: Sat, 12 Feb 2011 23:38:46 +0100

> That would be wrong.  A package build shouldn't depend on random env

I believe a lot of packages in the archive actually suffers from this
issue then, if dpkg-build{package,flags} did not sanitize these variables.
  How many packages explicitly sets e.g. LDFLAGS in debian/rules? Almost
none of mine does it (granted most of my packages are Java packages, but
still).

> Seems to be the exact same request to me?

Well, yes, if you insist the "CFLAGS  := $(CFLAGS) somearg" is wrong, it
would appear that my original request can be trivially be reduced to
#578597.
  Though in that case, I think we should change this one to a request to
make the policy be explicit in how a package should handle variables
from the environment (and which variables it should neutralize).

~Niels

---

From: Steve Langasek <[email-masked]>
Date: Sat, 12 Feb 2011 17:37:56 -0800

> That would be wrong.  A package build shouldn't depend on random env

Depend on, or respect?

Why shouldn't we expect packages to honor extra CFLAGS set in the
environment, and require sanitized environments with no $CFLAGS set for
standard package builds?

(Anyway, isn't setting $CFLAGS in the environment exactly what
dpkg-buildpackage + dpkg-buildflags *does*?)

-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
[email-masked]                                     [email-masked]

---

From: Raphael Hertzog <[email-masked]>
Date: Mon, 14 Feb 2011 15:14:04 +0100

Hi,

> environment, and require sanitized environments with no $CFLAGS set for

The way to pass supplementary flags is now defined by dpkg-buildflags as
well (and it's DEB_CFLAGS_{APPEND,SET}).

> dpkg-buildpackage + dpkg-buildflags *does*?)

Yes currently. But dpkg-buildpackage will stop setting the variables and
packages should solely rely on dpkg-buildflags to get the proper values.

That's because most people do not want "debian/rules build" to behave
differently from dpkg-buildpackage.

That said helper tools like "dh" should be free to use dpkg-buildflags
to set environment variables that ./configure and other similar calls
can inspect and use.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)

---

From: Julien Cristau <[email-masked]>
Date: Mon, 14 Feb 2011 15:19:27 +0100

> to set environment variables that ./configure and other similar calls
The usual way to pass CFLAGS to configure is as a command line argument,
not an env var.

Cheers,
Julien

---

From: Jonathan Nieder <[email-masked]>
Date: Tue, 1 Mar 2011 21:38:36 -0600

user [email-masked]
usertags 613046 + informative discussion
quit

Hi,

> Seems to be the exact same request to me?

I believe they are different.  This request seems to be about
informative text (an example), while #578597 is about the normative
part of policy (a policy "should").

Here's a rough patch for discussion's sake.  Points that might be nice
to fit in somehow:

 - warning flags come before the flags read from dpkg-buildflags,
   so they can be overridden with
   DEB_CFLAGS_APPEND="-Wno-all -Wno-extra"

 - important optimization flags such as -fno-strict-aliasing
   come after the flags read from dpkg-buildflags, so they cannot
   be overridden

 - using := instead of = saves us from running dpkg-buildflags
   multiple times if $(CFLAGS) is used more than once.

Thoughts?  Improvements?
---
diff --git a/policy.sgml b/policy.sgml
index 242ac10..5814960 100644
--- a/policy.sgml
+++ b/policy.sgml
@@ -2256,18 +2256,13 @@
 	    massage this example in order to make it work for your
 	    package.
 	    <example compact="compact">
-CFLAGS = -Wall -g
 INSTALL = install
 INSTALL_FILE    = $(INSTALL) -p    -o root -g root  -m  644
 INSTALL_PROGRAM = $(INSTALL) -p    -o root -g root  -m  755
 INSTALL_SCRIPT  = $(INSTALL) -p    -o root -g root  -m  755
 INSTALL_DIR     = $(INSTALL) -p -d -o root -g root  -m  755
 
-ifneq (,$(filter noopt,$(DEB_BUILD_OPTIONS)))
-    CFLAGS += -O0
-else
-    CFLAGS += -O2
-endif
+CFLAGS := -Wall $(shell dpkg-buildflags --get CFLAGS)
 ifeq (,$(filter nostrip,$(DEB_BUILD_OPTIONS)))
     INSTALL_PROGRAM += -s
 endif

---

From: Raphael Hertzog <[email-masked]>
Date: Mon, 30 Jan 2012 15:14:26 +0100

On Fri, 27 Jan 2012, Matthijs Kooijman wrote:
> Wouldn't it be more appropriate to use
> 
>     DEB_CFLAGS_MAINT_PREPEND := -Wall
> 
> here before calling dpkg-buildflags? Possibly the rest of the example
> could be modified to use DEB_CFLAGS_MAINT_APPEND as well.

Well, they are there so that you can tweak the output of dpkg-buildflags
when the call happens in lower layer and that you have no control over the
call and its output.

And you must take care because $(shell dpkg-buildflags ...) will not see the
DEB_CFLAGS_MAINT_PREPEND that you have set in the rules files. Either you
do $(shell DEB_CFLAGS_MAINT_PREPEND=... dpkg-buildflags ...) or you use
/usr/share/dpkg/buildflags.mk which does the right thing for you.

> Not sure if this is really needed, but it seems to me that that is what
> these variables were invented for :-)

Maybe all the samples from Policy should be simplified to use the Makefile
snippets...

Cheers,

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/

---

From: Matthijs Kooijman <[email-masked]>
Date: Fri, 27 Jan 2012 13:28:02 +0100

Hi folks,

just stumbled upon this report, and I have a small suggestion to improve

Gr.

Matthijs

---

From: Russ Allbery <[email-masked]>
Date: Mon, 30 Jan 2012 10:56:09 -0800

Raphael Hertzog <[email-masked]> writes:

> when the call happens in lower layer and that you have no control over
> the call and its output.

> And you must take care because $(shell dpkg-buildflags ...) will not see
> the DEB_CFLAGS_MAINT_PREPEND that you have set in the rules
> files. Either you do $(shell
> DEB_CFLAGS_MAINT_PREPEND=... dpkg-buildflags ...) or you use

Or you just use:

    export DEB_CFLAGS_MAINT_PREPEND = -Wall

-- 
Russ Allbery ([email-masked])               <http://www.eyrie.org/~eagle/>

---

From: Raphael Hertzog <[email-masked]>
Date: Mon, 30 Jan 2012 21:10:24 +0100

>     export DEB_CFLAGS_MAINT_PREPEND = -Wall

No because $(shell ...) only gets environment variable that were existing
at the time make has been invoked. Variables exported by the makefile
itself are not forwarded. :-(

Cheers,

---

From: Russ Allbery <[email-masked]>
Date: Mon, 30 Jan 2012 12:18:07 -0800

Raphael Hertzog <[email-masked]> writes:

> No because $(shell ...) only gets environment variable that were
> existing at the time make has been invoked. Variables exported by the
> makefile itself are not forwarded. :-(

Ah, indeed.  I didn't realize that.

Using `` instead of $(shell) would of course work, but you have to be
careful about quoting.

---

From: Charles Plessy <[email-masked]>
Date: Wed, 1 Feb 2012 09:37:20 +0900

>     DEB_CFLAGS_MAINT_PREPEND := -Wall

Hello everybody,

since §4.9.1's object is to document the DEB_BUILD_OPTIONS, I would find it a
bit dry to only provide an example that works through a tool that is not
mentionned or explained anywhere else in the Policy (see http://bugs.debian.org/578597).
I therfore propose to replace the sentence above it,

  The following makefile snippet is an example of how one may implement the
  build options; you will probably have to massage this example in order to make
  it work for your package.

by:

  In the following example, the build options nocheck, nostrip and parallel are
  implemented directly in make, and noopt is implemented through its support in
  dpkg-buildflags.  You will probably have to massage this example in order to
  make it work for your package.  Note also that other helper tools provide
  support for nocheck, nostrip and parallel.

Alternatively, the example could be repaired as proposed in the original,
message or ‘CFLAGS = -Wall -g’ could be simply removed (it is already
recommended somewhere lese), and a mention of dpkg-buildflag's support for
nooopt could be inserted as part of #578597.  This would be in line with the
other parts of the Policy and of this example, that focus on the action to
take, not on the helper to use.

By the way, I read in the paragraph describing noopt:

  For C programs, it is best to add -O0 to CFLAGS (although this is usually the default).

Isn't that a bit outdated ?  I see -O2 or superior in most of my packages.

Have a nice day, 

-- 
Charles Plessy
Tsurumi, Kanagawa, Japan

---

From: Russ Allbery <[email-masked]>
Date: Tue, 31 Jan 2012 16:42:37 -0800

> Isn't that a bit outdated ?  I see -O2 or superior in most of my

This is specifically in the context of implementing the noopt build
option.  -O0 explicitly disables gcc optimization.  However, gcc does not
optimize by default, so omitting the flag entirely also works.  That's the
point of the parenthetical.

---

From: Jonathan Nieder <[email-masked]>
Date: Tue, 31 Jan 2012 18:43:55 -0600

Hi,

>   support for nocheck, nostrip and parallel.

By now I imagine that dpkg-buildflags is becoming enough of a de facto
standard that it would be reasonable to just recommend it with a policy
"should".  Niels, would you mind if I merge this with bug#578597?

[...]
> Isn't that a bit outdated ?  I see -O2 or superior in most of my packages.

I believe the intended meaning is that in many build systems, if you
override CFLAGS and do not specify an optimization level then no
optimization level is passed to gcc, resulting in no optimization.

Thanks,
Jonathan

---

From: Niels Thykier <[email-masked]>
Date: Wed, 01 Feb 2012 10:34:17 +0100

> "should".  Niels, would you mind if I merge this with bug#578597?

Feel free to merge them. :)

~Niels

---

From: Jonathan Nieder <[email-masked]>
Date: Wed, 1 Feb 2012 10:33:42 -0600

# Niels Thykier wrote:
# > On 2012-02-01 01:43, Jonathan Nieder wrote:
# 
# >> Niels, would you mind if I merge this with bug#578597?
# >
# > Feel free to merge them. :)
#
# Thanks, doing so.

forcemerge 578597 613046