Hi!

On Fri, 2018-12-07 at 15:05:39 +0000, Iain Lane wrote:
> Package: libdpkg-perl
> Version: 1.19.1
> Severity: normal
> Tags: patch

> In Ubuntu we recently got a version of dpkg including
> d5374bc618310917557daa9c9ac2f4930515a0b2 for the first time, and I
> noticed that our ppc64el -O3 build flag wasn't being used - -O2 was
> being passed instead.
> 
> See the attached commit for my proposed fix - please let me know what
> you think. There are other valid approaches such as having Debian.pm
> check the value doesn't contain `-g -O\d' before appending it.

On Fri, 2018-12-07 at 11:02:49 -0700, Adam Conrad wrote:
> In Ubuntu, the attached patch was applied to achieve the following:
> 
>   * scripts/Dpkg/Vendor/Ubuntu.pm: Instead of running the Debian hooks after
>     the Ubuntu buildflags are set up, run them first, and then strip/prepend
>     the bits we need to change. This fixes compiler optimisation on ppc64el.
> 
> This is vaguely similar to Laney's patch, but produces more pleasant output,
> and is also what I uploaded to Ubuntu just now to solve the problem.

Right, sorry for missing this! And thanks for the patches. As I
mentioned to Adam on IRC, I went with a solution I think should be
more future proof, and in addition also added a unit test to detect
similar failures in the future, see the attached patch, which is
queued for merging.

Iain: Should your mail address really be @debian or perhaps it should
have been either @canonical or @ubuntu instead? For better © tracking.

Thanks,
Guillem
From eb27f9167602a4ca18d409164f0500f3977e1b8d Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Sat, 8 Dec 2018 00:28:57 +0100
Subject: [PATCH] Dpkg::Vendor::Ubuntu: Fix buildflags override after default
 setting move

The default buildflags got moved from the Dpkg::BuildFlags module to
the Dpkg::Vendor::Debian, but this module was not addapted to match.

Instead of running the Debian hooks after the Ubuntu buildflags are set
up, run them first, and then modify/prepend the bits we need to change.
This fixes compiler optimization on ppc64el, and makes setting it more
future proof.

Co-Author: Iain Lane <la...@debian.org>
Co-Author: Adam Conrad <adcon...@ubuntu.com>
Fixes: commit d5374bc618310917557daa9c9ac2f4930515a0b2
Closes: #915881
---
 scripts/Dpkg/Vendor/Ubuntu.pm      | 12 ++++---
 scripts/Makefile.am                |  1 +
 scripts/t/Dpkg_BuildFlags_Ubuntu.t | 57 ++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 5 deletions(-)
 create mode 100644 scripts/t/Dpkg_BuildFlags_Ubuntu.t

diff --git a/scripts/Dpkg/Vendor/Ubuntu.pm b/scripts/Dpkg/Vendor/Ubuntu.pm
index eb2dffefe..e6335c204 100644
--- a/scripts/Dpkg/Vendor/Ubuntu.pm
+++ b/scripts/Dpkg/Vendor/Ubuntu.pm
@@ -98,6 +98,9 @@ sub run_hook {
     } elsif ($hook eq 'update-buildflags') {
 	my $flags = shift @params;
 
+        # Run the Debian hook to add hardening flags
+        $self->SUPER::run_hook($hook, $flags);
+
         require Dpkg::BuildOptions;
 
 	my $build_opts = Dpkg::BuildOptions->new();
@@ -109,15 +112,14 @@ sub run_hook {
             if (Dpkg::Arch::debarch_eq($arch, 'ppc64el')) {
 		for my $flag (qw(CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS
 		                 FFLAGS FCFLAGS)) {
-		    $flags->set($flag, '-g -O3', 'vendor');
+                    my $value = $flags->get($flag);
+                    $value =~ s/-O[0-9]/-O3/;
+                    $flags->set($flag, $value);
 		}
 	    }
 	}
 	# Per https://wiki.ubuntu.com/DistCompilerFlags
-	$flags->set('LDFLAGS', '-Wl,-Bsymbolic-functions', 'vendor');
-
-	# Run the Debian hook to add hardening flags
-	$self->SUPER::run_hook($hook, $flags);
+        $flags->prepend('LDFLAGS', '-Wl,-Bsymbolic-functions');
     } else {
         return $self->SUPER::run_hook($hook, @params);
     }
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index d7bd8d2f7..31df4d8df 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -219,6 +219,7 @@ test_scripts = \
 	t/Dpkg_Shlibs_Cppfilt.t \
 	t/Dpkg_Shlibs.t \
 	t/Dpkg_BuildFlags.t \
+	t/Dpkg_BuildFlags_Ubuntu.t \
 	t/Dpkg_BuildOptions.t \
 	t/Dpkg_BuildProfiles.t \
 	t/Dpkg_Build_Env.t \
diff --git a/scripts/t/Dpkg_BuildFlags_Ubuntu.t b/scripts/t/Dpkg_BuildFlags_Ubuntu.t
new file mode 100644
index 000000000..3cfdb268f
--- /dev/null
+++ b/scripts/t/Dpkg_BuildFlags_Ubuntu.t
@@ -0,0 +1,57 @@
+#!/usr/bin/perl
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+use strict;
+use warnings;
+
+use Test::More tests => 16;
+
+BEGIN {
+    use_ok('Dpkg::BuildFlags');
+}
+
+sub test_optflag
+{
+    my ($bf, $optflag) = @_;
+
+    foreach my $flag (qw(CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS GCJFLAGS
+                         FFLAGS FCFLAGS)) {
+        my $value = $bf->get($flag);
+        ok($value =~ m/$optflag/, "$flag contains $optflag: $value");
+    }
+}
+
+my $bf;
+
+# Force loading the Dpkg::Vendor::Ubuntu module.
+$ENV{DEB_VENDOR} = 'Ubuntu';
+
+# Test the optimization flag inherited from the Dpkg::Vendor::Debian module.
+$ENV{DEB_HOST_ARCH} = 'amd64';
+$bf = Dpkg::BuildFlags->new();
+
+test_optflag($bf, '-O2');
+
+# Test the overlaid Ubuntu-specific linker flag.
+ok($bf->get('LDFLAGS') =~ m/-Wl,-Bsymbolic-functions/,
+   'LDFLAGS contains -Bsymbolic-functions');
+
+# Test the optimization flag override only for ppc64el.
+$ENV{DEB_HOST_ARCH} = 'ppc64el';
+$bf = Dpkg::BuildFlags->new();
+
+test_optflag($bf, '-O3');
+
+1;
-- 
2.20.0.rc2.403.gdbc3b29805

Reply via email to