Hello, On sekmadienis 19 Birželis 2011 21:20:37 Joey Hess wrote: > Modestas Vainius wrote: > > The upside of this aggressiveness is that sensitive environment > > variables, that might already have been set before debian/rules, will be > > overwritten and won't affect build of the package. I do understand that > > doing the same would break backwards compatibility in debhelper, but > > maybe that's a good idea to do for compat(9) or at least provide a > > dh/dh_auto_* option (e.g. --force- buildflags) to give maintainers a > > choice to enable this. However, I very much prefer the former as in my > > opinion, most package maintainers will want (or assume) this behavior. > > Who would want random shell environment variables to affect package > > build? What's more, "clean environment" is even implied in examples of > > the Debian Policy ifself [1]. > > debhelper cannot ensure a clean environment, it runs at the wrong level > of the build stack. Ie, the rules file has run, and had a chance to > set environment before debhelper runs, and it can't stomp on those > settings. So this is not debhelper's problem.
Yeah, now I realized that...
> > By the way, it's also a bit sad that:
> >
> > export CFLAGS += -Wall
> >
> > will break
>
> debhelper could prepend dpkg-buildflags values to pre-existing
> variables to support this. Assuming that CFLAGS="-O2 -Wall -O3" will
> result in the maintainer's overridden -O3 taking effect.
> I don't know that would be a good idea.
Prepending might work for CFLAGS, but it's probably pretty harmful for LDFLAGS
unless maintainer has a way to disable prepending. As 'export LDFLAGS = '
wouldn't cut it with prepending in effect, a dh/dh_auto_* option would be
needed.
On the other hand, it is really too bad that 'export DEB_LDFLAGS_{APPEND,SET}
= ' will have no effect as long as dpkg-buildpackage keeps modifying
environment. Both LDFLAGS and DEB_LDFLAGS_APPEND would need to be exported (or
LDFLAGS unexported) in order to be compatible with both old agressive dpkg-
buildpackage and envvars-friendly dpkg-buildpackage. But this is not clean in
my book (consider backports etc.).
In order to make `export DEB_*_{APPEND,SET}` work with both dpkg-
buildpackage's, I propose the attached patch (0002). The patch also tweaks
documentation a bit wrt this topic. The patch should be safe but it will
override VAR envvar if either of DEB_VAR_{APPEND,SET} is exported so I don't
know if you want to add this to compat=9 (should be trivial to modify).
> Note that this case won't break until dpkg-buildpackage removes the
> forcing of the variables. I never said that this debhelper change could
> solve the problems caused by dpkg-buildpackage forcing those variables.
I understand that. But at least I no longer have to worry about putting
explicit dpkg-buildflags calls in each rules file :-)
> > > So, all that's needed is for dh (when running override targets, or just
> > > generally) and dh_auto_* (for when not called by dh) to run
> > > dpkg-buildflags --export, parse the shell formatted output, and shove
> > > it into %ENV.
> >
> > I don't understand why you do not use Dpkg::BuildFlags directly. Its API
> > is marked as stable (our $VERSION = "1.00" [2]). I'm CC'ing Raphael in
> > case I'm suggesting something wrong.
>
> Thanks, much better.
My patch had a small bug wrt error handling (very unlikely case of
Dpkg::BuildFlags failing to load). The patch (0001) is attached for the sake
of perfection.
--
Modestas Vainius <mo...@debian.org>
From 5467163ededc958b9159a13dbdb68f946ca3d62d Mon Sep 17 00:00:00 2001
From: Modestas Vainius <mo...@debian.org>
Date: Sun, 19 Jun 2011 22:05:08 +0300
Subject: [PATCH 1/2] In the unlikely case Dpkg::BuildFlags fails, don't do
anything.
---
Debian/Debhelper/Dh_Lib.pm | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index 0c779d1..174970b 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -910,6 +910,7 @@ sub set_buildflags {
eval "use Dpkg::BuildFlags";
if ($@) {
warning "unable to load build flags: $@";
+ return;
}
my $buildflags = Dpkg::BuildFlags->new();
--
1.7.5.4
From 03fd6597fc86bd340f79e093ecf51dc220704804 Mon Sep 17 00:00:00 2001
From: Modestas Vainius <mo...@debian.org>
Date: Sun, 19 Jun 2011 23:53:14 +0300
Subject: [PATCH 2/2] Always respect DEB_${flag}_{APPEND,SET} envvars.
Do that even when dpkg-buildpackage modifies environment variables. Also
document DEB_${flag}_{APPEND,SET} as recommended way to override standard build
flags.
---
Debian/Debhelper/Dh_Lib.pm | 2 +-
debian/changelog | 4 +++-
dh | 12 ++++++++++++
dh_auto_build | 5 +++++
dh_auto_configure | 5 +++++
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Debian/Debhelper/Dh_Lib.pm b/Debian/Debhelper/Dh_Lib.pm
index 174970b..86f729a 100644
--- a/Debian/Debhelper/Dh_Lib.pm
+++ b/Debian/Debhelper/Dh_Lib.pm
@@ -917,7 +917,7 @@ sub set_buildflags {
$buildflags->load_config();
foreach my $flag ($buildflags->list()) {
next unless $flag =~ /^[A-Z]/; # Skip flags starting with lowercase
- if (! exists $ENV{$flag}) {
+ if (! exists $ENV{$flag} || $buildflags->get_origin($flag) eq "env") {
$ENV{$flag} = $buildflags->get($flag);
}
}
diff --git a/debian/changelog b/debian/changelog
index 3be2eda..743daa0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -12,7 +12,9 @@ debhelper (8.1.7) UNRELEASED; urgency=low
in --libexecdir when using autoconf. Closes: #541458
* dh_auto_build, dh_auto_configure, dh: Set environment variables
listed by dpkg-buildflags --export. Any environment variables that
- are already set to other values will not be changed.
+ are already set to other values will not be changed unless
+ appropriate DEB_${flag}_{APPEND,SET} environment variables are also
+ set (see dpkg-buildflags(1)).
Closes: #544844
* Correct docs about multiarch and v9. Closes: #630826
diff --git a/dh b/dh
index aba8c27..5ea6643 100755
--- a/dh
+++ b/dh
@@ -193,6 +193,18 @@ sequence addons like this:
%:
dh $@ --with quilt
+In order to override standard build flags, export appropriate environment
+variables as documented in the L<dpkg-buildflags(1)> manual page. They will be
+preferred over directly exported their counterparts (CFLAGS, CXXFLAGS, LDFLAGS
+etc.). For example, to append -Wall to the standard CFLAGS and CXXFLAGS, use:
+
+ #!/usr/bin/make -f
+ export DEB_CFLAGS_APPEND = -Wall
+ export DEB_CXXFLAGS_APPEND = -Wall
+
+ %:
+ dh $@
+
Here is an example of overriding where the B<dh_auto_>I<*> commands find
the package's source, for a package where the source is located in a
subdirectory.
diff --git a/dh_auto_build b/dh_auto_build
index dccd04a..77b55ee 100755
--- a/dh_auto_build
+++ b/dh_auto_build
@@ -25,6 +25,11 @@ This is intended to work for about 90% of packages. If it doesn't work,
you're encouraged to skip using B<dh_auto_build> at all, and just run the
build process manually.
+In order to override standard build flags, export appropriate environment
+variables as documented in the L<dpkg-buildflags(1)> manual page. They will be
+preferred over directly exported their counterparts (CFLAGS, CXXFLAGS, LDFLAGS
+etc.).
+
=head1 OPTIONS
See L<debhelper(7)/B<BUILD SYSTEM OPTIONS>> for a list of common build
diff --git a/dh_auto_configure b/dh_auto_configure
index daf5ed0..f08c0f6 100755
--- a/dh_auto_configure
+++ b/dh_auto_configure
@@ -28,6 +28,11 @@ This is intended to work for about 90% of packages. If it doesn't work,
you're encouraged to skip using B<dh_auto_configure> at all, and just run
F<./configure> or its equivalent manually.
+In order to override standard build flags, export appropriate environment
+variables as documented in the L<dpkg-buildflags(1)> manual page. They will be
+preferred over directly exported their counterparts (CFLAGS, CXXFLAGS, LDFLAGS
+etc.).
+
=head1 OPTIONS
See L<debhelper(7)/B<BUILD SYSTEM OPTIONS>> for a list of common build
--
1.7.5.4
signature.asc
Description: This is a digitally signed message part.
