On Sun, 05 Jun 2011, Bill Allombert wrote: > Please find a new patch that use the name Build-Features and the module name > Dpkg::BuildFeatures.
Thanks, here's a short review with a few details to clean. 1/ You're not consistent with the coding style (see doc/coding-style.txt). 2/ Why are you not calling "build-indep" for dpkg-buildpackage -A? AFAIU it should be exactly like the binary target: With flag | Without flag -b => build | build -B => build-arch | build -A => build-indep | build -F => build | build 3/ You should document the new field in po/deb-src-control.5 > --- a/man/dpkg-buildpackage.1 > +++ b/man/dpkg-buildpackage.1 The doc needs to be updated as per 2/ above. > --- /dev/null > +++ b/scripts/Dpkg/BuildFeatures.pm > +package Dpkg::BuildFeatures; > + > +use strict; > +use warnings; > + > +our $VERSION = "1.00"; Please use version 0.01. I don't want this module to be part of the stable API yet. > +The Dpkg::BuildFeatures object can be used to query the options > +stored in the Build-Features control field Missing dot at the end. > +=head1 FUNCTIONS > + > +=over 4 > + > +=item my $bo = Dpkg::BuildFeatures->new($controlfile) s/bo/bf/ > + if (defined($src_fields->{'Build-Features'})) > + { > + my @buildfeats= split(/\s*,\s*/m, $src_fields->{'Build-Features'}); missing space before "=" > + %buildfeats = map { $_ => 1 } @buildfeats; > + } else > + { > + %buildfeats=(); > + } bad indentation and curly braces on the wrong lines > + my $self = { options => \%buildfeats }; > + bless $self, $class; > + return $self; > +} > +=item $bo->has($option) Missing empty line before the POD doc. s/bo/bp/ > diff --git a/scripts/Makefile.am b/scripts/Makefile.am > index 9b4d394..be376c9 100644 > --- a/scripts/Makefile.am > +++ b/scripts/Makefile.am > @@ -56,6 +56,7 @@ nobase_dist_perllib_DATA = \ > Dpkg/Arch.pm \ > Dpkg/BuildFlags.pm \ > Dpkg/BuildOptions.pm \ > + Dpkg/BuildFeatures.pm \ > Dpkg/Changelog.pm \ > Dpkg/Changelog/Debian.pm \ > Dpkg/Changelog/Entry.pm \ You also want to add it to scripts/po/POTFILES.in in case we add translatable strings in the future. > diff --git a/scripts/dpkg-buildpackage.pl b/scripts/dpkg-buildpackage.pl > index aaea544..17294cb 100755 > --- a/scripts/dpkg-buildpackage.pl > +++ b/scripts/dpkg-buildpackage.pl > @@ -27,6 +27,7 @@ use Dpkg::Gettext; > use Dpkg::ErrorHandling; > use Dpkg::BuildFlags; > use Dpkg::BuildOptions; > +use Dpkg::BuildFeatures; > use Dpkg::Compression; > use Dpkg::Version; > use Dpkg::Changelog::Parse; > @@ -100,6 +101,8 @@ Options: > --version show the version. > "), $progname; > } > +my $controlfile = "debian/control"; > +my $buildfeats = Dpkg::BuildFeatures->new($controlfile); Do not use a useless intermediary variable. In fact, do not give a value at all and fix the object's new method to assume "debian/control" if no explicit value has been given. > my @debian_rules = ("debian/rules"); > my @rootcommand = (); > @@ -119,6 +122,7 @@ my $checkbuilddep = 1; > my $signsource = 1; > my $signchanges = 1; > my $binarytarget = 'binary'; > +my $buildtarget = 'build'; Don't align the value when nothing else is aligned. > my $targetarch = my $targetgnusystem = ''; > my $call_target = ''; > my $call_target_as_root = 0; > @@ -247,6 +251,9 @@ if ($noclean) { > $include = BUILD_BINARY if ($include & BUILD_DEFAULT); > } > > +$buildtarget='build-arch' if ( $binarytarget eq 'binary-arch' && > + $buildfeats->has('build-arch')); Define buildtarget to build-indep/build-arch while parsing the options but reset it to "build" here if the flag is missing: $buildtarget = "build" unless $buildfeats->has('build-arch'); Thank you for your work on this patch! Cheers, -- Raphaël Hertzog ◈ Debian Developer Follow my Debian News ▶ http://RaphaelHertzog.com (English) ▶ http://RaphaelHertzog.fr (Français) -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org