Dpkg::BuildOptions: provide an object oriented interface

Also update the test-suite and dpkg-buildpackage to match the new API.
Increase $VERSION to 1.00 since it's supposed to be a stable API now.

debian/changelog

@@ -15,6 +15,7 @@ dpkg (1.15.6) UNRELEASED; urgency=low
       there management of default compression
     - introduce Dpkg::Interface::Storable and update many modules
       to make use of it
+    - update Dpkg::BuildOptions to provide an object-oriented interface
   * Drop debian-maintainers from Suggests since it's obsolete, the
     corresponding keyring is in debian-keyring.
   * Merge support of symbol patterns in dpkg-gensymbols. Thanks to

scripts/Dpkg/BuildOptions.pm

@@ -1,3 +1,6 @@
+# Copyright © 2007 Frank Lichtenheld <djpig@debian.org>
+# Copyright © 2010 Raphaël Hertzog <hertzog@debian.org>
+#
 # 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
@@ -16,56 +19,172 @@ package Dpkg::BuildOptions;
 use strict;
 use warnings;
 
-our $VERSION = "0.01";
+our $VERSION = "1.00";
 
 use Dpkg::Gettext;
 use Dpkg::ErrorHandling;
 
-sub parse {
-    my ($env) = @_;
+=head1 NAME
+
+Dpkg::BuildOptions - parse and update build options
+
+=head1 DESCRIPTION
+
+The Dpkg::BuildOptions object can be used to manipulate options stored
+in the DEB_BUILD_OPTIONS environment variable.
+
+=head1 FUNCTIONS
+
+=over 4
+
+=item my $bo = Dpkg::BuildOptions->new()
+
+Create a new Dpkg::BuildOptions object. It will be initialized based
+on the value of the DEB_BUILD_OPTIONS environment variable.
+
+=cut
+
+sub new {
+    my ($this, %opts) = @_;
+    my $class = ref($this) || $this;
+
+    my $self = {
+        options => {},
+	source => {},
+    };
+    bless $self, $class;
+    $self->merge($ENV{DEB_BUILD_OPTIONS}, "DEB_BUILD_OPTIONS");
+    return $self;
+}
 
-    $env ||= $ENV{DEB_BUILD_OPTIONS};
+=item $bo->reset()
 
-    unless ($env) { return {}; }
+Reset the object to not have any option (it's empty).
 
-    my %opts;
+=cut
 
-    foreach (split(/\s+/, $env)) {
-	unless (/^([a-z][a-z0-9_-]*)(=(\S*))?$/) {
-            warning(_g("invalid flag in DEB_BUILD_OPTIONS: %s"), $_);
+sub reset {
+    my ($self) = @_;
+    $self->{'options'} = {};
+    $self->{'source'} = {};
+}
+
+=item $bo->merge($content, $source)
+
+Merge the options set in $content and record that they come from the
+source $source. $source is mainly used in warning messages currently
+to indicate where invalid options have been detected.
+
+$content is a space separated list of options with optional assigned
+values like "nocheck parallel=2".
+
+=cut
+
+sub merge {
+    my ($self, $content, $source) = @_;
+    my $count = 0;
+    foreach (split(/\s+/, $content)) {
+	unless (/^([a-z][a-z0-9_-]*)(?:=(\S*))?$/) {
+            warning(_g("invalid flag in %s: %s"), $source, $_);
             next;
         }
+	$count += $self->set($1, $2, $source);
+    }
+    return $count;
+}
+
+=item $bo->set($option, $value, [$source])
 
-	my ($k, $v) = ($1, $3);
+Store the given option in the objet with the given value. It's legitimate
+for a value to be undefined if the option is a simple boolean (its
+presence means true, its absence means false). The $source is optional
+and indicates where the option comes from.
 
-	# Sanity checks
-	if ($k =~ /^(noopt|nostrip|nocheck)$/ && defined($v)) {
-	    $v = undef;
-	} elsif ($k eq 'parallel')  {
-	    $v = "" if not defined($v);
-	    next if $v !~ /^\d*$/;
-	}
+The known options have their values checked for sanity. Options without
+values have their value removed and options with invalid values are
+discarded.
 
-	$opts{$k} = $v;
+=cut
+
+sub set {
+    my ($self, $key, $value, $source) = @_;
+
+    # Sanity checks
+    if ($key =~ /^(noopt|nostrip|nocheck)$/ && defined($value)) {
+	$value = undef;
+    } elsif ($key eq 'parallel')  {
+	$value = "" if not defined($value);
+	return 0 if $value !~ /^\d*$/;
     }
 
-    return \%opts;
+    $self->{'options'}{$key} = $value;
+    $self->{'source'}{$key} = $source;
+
+    return 1;
 }
 
-sub set {
-    my ($opts, $overwrite) = @_;
-    $overwrite = 1 if not defined($overwrite);
+=item $bo->get($option)
 
-    my $new = {};
-    $new = parse() unless $overwrite;
-    while (my ($k, $v) = each %$opts) {
-        $new->{$k} = $v;
-    }
+Return the value associated to the option. It might be undef even if the
+option exists. You might want to check with $bo->has($option) to verify if
+the option is stored in the object.
+
+=cut
+
+sub get {
+    my ($self, $key) = @_;
+    return $self->{'options'}{$key};
+}
+
+=item $bo->has($option)
+
+Returns a boolean indicating whether the option is stored in the object.
+
+=cut
+
+sub has {
+    my ($self, $key) = @_;
+    return exists $self->{'options'}{$key};
+}
 
-    my $env = join(" ", map { defined($new->{$_}) ? $_ . "=" . $new->{$_} : $_ } sort keys %$new);
+=item $string = $bo->output($fh)
 
-    $ENV{DEB_BUILD_OPTIONS} = $env;
-    return $env;
+Return a string representation of the build options suitable to be
+assigned to an environment variable. Can optionnaly output that string to
+the given filehandle.
+
+=cut
+
+sub output {
+    my ($self, $fh) = @_;
+    my $o = $self->{'options'};
+    my $res = join(" ", map { defined($o->{$_}) ? $_ . "=" . $o->{$_} : $_ } sort keys %$o);
+    print $fh $res if defined $fh;
+    return $res;
 }
 
+=item $bo->export([$var])
+
+Export the build options to the given environment variable. If omitted,
+DEB_BUILD_OPTIONS is assumed. The value set to the variable is also
+returned.
+
+=cut
+
+sub export {
+    my ($self, $var) = @_;
+    $var = "DEB_BUILD_OPTIONS" unless defined $var;
+    my $content = $self->output();
+    $ENV{$var} = $content;
+    return $content;
+}
+
+=back
+
+=head1 AUTHOR
+
+Raphaël Hertzog <hertzog@debian.org>
+
+=cut
+
 1;

scripts/dpkg-buildpackage.pl

@@ -259,16 +259,16 @@ if ($signcommand) {
     }
 }
 
-my $build_opts = Dpkg::BuildOptions::parse();
+my $build_opts = Dpkg::BuildOptions->new();
 if (defined $parallel) {
-    $parallel = $build_opts->{parallel} if exists $build_opts->{parallel};
+    $parallel = $build_opts->get("parallel") if $build_opts->has("parallel");
     $ENV{MAKEFLAGS} ||= '';
     $ENV{MAKEFLAGS} .= " -j$parallel";
-    $build_opts->{parallel} = $parallel;
-    Dpkg::BuildOptions::set($build_opts);
+    $build_opts->set("parallel", $parallel);
+    $build_opts->export();
 }
 
-my $default_flags = exists $build_opts->{noopt} ? "-g -O0" : "-g -O2";
+my $default_flags = $build_opts->has("noopt") ? "-g -O0" : "-g -O2";
 my %flags = ( CPPFLAGS => '',
 	      CFLAGS   => $default_flags,
 	      CXXFLAGS => $default_flags,

scripts/t/300_Dpkg_BuildOptions.t

@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-use Test::More tests => 6;
+use Test::More tests => 22;
 use Dpkg::ErrorHandling;
 
 use strict;
@@ -30,37 +30,39 @@ use_ok('Dpkg::BuildOptions');
 
 $ENV{DEB_BUILD_OPTIONS} = 'noopt foonostripbar parallel=3 bazNOCHECK';
 
-my $dbo = Dpkg::BuildOptions::parse();
+my $dbo = Dpkg::BuildOptions->new();
+ok($dbo->has("noopt"), "has noopt");
+is($dbo->get("noopt"), undef, "noopt value");
+ok($dbo->has("foonostripbar"), "has foonostripbar");
+is($dbo->get("foonostripbar"), undef, "foonostripbar value");
+ok($dbo->has("parallel"), "has parallel");
+is($dbo->get("parallel"), 3, "parallel value");
+ok(!$dbo->has("bazNOCHECK"), "not has bazNOCHECK");
 
-my %dbo = (
-	   noopt => undef,
-	   foonostripbar => undef,
-	   parallel => 3,
-	   );
-my %dbo2 = (
-	    no => undef,
-	    opt => undef,
-	    'no-strip' => undef,
-	    nocheck => undef,
-	    parallel => '',
-	   );
+$dbo->reset();
+$dbo->merge('no opt no-strip parallel = 5 nocheck', 'test');
+ok($dbo->has('no'), "has no");
+is($dbo->get('no'), undef, "no value");
+ok($dbo->has('opt'), "has opt");
+is($dbo->get('opt'), undef, "opt value");
+ok($dbo->has('no-strip'), "has no-strip");
+is($dbo->get('no-strip'), undef, "no-strip value");
+ok($dbo->has('parallel'), "has parallel");
+is($dbo->get('parallel'), '', "parallel value");
+ok($dbo->has('nocheck'), "has nocheck");
+is($dbo->get('nocheck'), undef, "nocheck value");
 
+$dbo->reset();
+$dbo->set('parallel', 5);
+$dbo->set('noopt', undef);
 
-is_deeply($dbo, \%dbo, 'parse');
-
-$dbo = Dpkg::BuildOptions::parse('no opt no-strip parallel = 5 nocheck');
-
-is_deeply($dbo, \%dbo2, 'parse (param)');
-
-$dbo->{parallel} = 5;
-$dbo->{noopt} = undef;
-
-my $env = Dpkg::BuildOptions::set($dbo, 1);
-
-is($ENV{DEB_BUILD_OPTIONS}, $env, 'set (return value)');
-is_deeply(Dpkg::BuildOptions::parse(), $dbo, 'set (env)');
+my $env = $dbo->export();
+is($env, "noopt parallel=5", "value of export");
+is($ENV{DEB_BUILD_OPTIONS}, $env, 'env match return value of export');
+$env = $dbo->export("OTHER_VARIABLE");
+is($ENV{OTHER_VARIABLE}, $env, 'export to other variable');
 
 $ENV{DEB_BUILD_OPTIONS} = 'foobar';
-$dbo = { noopt => undef };
-$env = Dpkg::BuildOptions::set($dbo, 0);
-is($env, "foobar noopt", 'set (append)');
+$dbo = Dpkg::BuildOptions->new();
+$dbo->set("noopt", 1);
+is($dbo->output(), "foobar noopt", "output");