Bug#229357: New Build-Options field and build-arch option, please review

2008-07-11 Thread Raphael Hertzog
On Thu, 10 Jul 2008, Felipe Sateler wrote:
 El 10/07/08 18:02 Raphael Hertzog escribió:
  Hello,
 
  in order to fix #229357 I decided to add a new Build-Options field.
  I modified Dpkg::BuildOptions to parse this field and DEB_BUILD_OPTIONS.
  And I added support for a build-arch option, that if present, will let
  dpkg-buildpackage call debian/rules build-arch and build-indep.
 
  It's not obvious that this was the right choice when you think of the
  currently existing build options but once you start thinking of possible
  additions (as requested in #489771), it becomes more evident that it makes
  sense. Even if some build options should really only be used in
  the field while others should only be used in the environment variable,
  the possibility to override the former with the latter is nice.
 
 I'm not really sure this is right. There are two things that we want to do 
 here: declare that a package supports something, and asking the package to do 
 something. This difference is blurred now, and I think it is confusing.

Even if there's only two things, the fact is that the package maintainer
wants not only to decide what is supported but he might also want to
enable some features... if you check the case that I listed above, we also
want to use Build-Options to _enable_ specific hardening measures. Because
the maintainer knows best which hardening measures should be enabled. But
we also want the builder to be able to override them for example to test
if the package now supports a previously disabled hardening measure.

The meaning of each build options is specific to each, there's no global
rule that works for all cases. That's why we have documentation of each
option in dpkg-buildpackage.

 I fear it will give rise to abuses such as setting parallel=n in the control 
 file.

There are dozens of ways to abuse any interface if you choose to use
it in a way that contradicts the documentation. But that's not a reason
to limit the flexibility offered by an interface.

Cheers,
-- 
Raphaël Hertzog

Le best-seller français mis à jour pour Debian Etch :
http://www.ouaza.com/livre/admin-debian/




--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]



Bug#229357: New Build-Options field and build-arch option, please review

2008-07-10 Thread Raphael Hertzog
Hello,

in order to fix #229357 I decided to add a new Build-Options field.
I modified Dpkg::BuildOptions to parse this field and DEB_BUILD_OPTIONS.
And I added support for a build-arch option, that if present, will let
dpkg-buildpackage call debian/rules build-arch and build-indep.

It's not obvious that this was the right choice when you think of the
currently existing build options but once you start thinking of possible
additions (as requested in #489771), it becomes more evident that it makes
sense. Even if some build options should really only be used in
the field while others should only be used in the environment variable,
the possibility to override the former with the latter is nice.

The current patchset is available in my public repository but I'll attach
it as well so that you can easily review it. I intend to merge it this
week-end after some tests but feel free to test and comment in the mean
time.

http://git.debian.org/?p=users/hertzog/dpkg.git;a=shortlog;h=refs/heads/pu/bug229357-build-options

The patchset only applies on top of master.

Cheers,
-- 
Raphaël Hertzog

Le best-seller français mis à jour pour Debian Etch :
http://www.ouaza.com/livre/admin-debian/
From 1ebeff797bc36c91e50f02c6d32cba094e827add Mon Sep 17 00:00:00 2001
From: Raphael Hertzog [EMAIL PROTECTED]
Date: Sun, 6 Jul 2008 22:03:27 +0200
Subject: [PATCH] Refactor Dpkg::BuildOptions to handle Build-Options field

* scripts/Dpkg/BuildOptions.pm: complete rewrite of the module
to handle various sources of build options: some options are auto-set
based on the standards version, then the maintainer can define options
with the Build-Options field in debian/control and last the builder
can use DEB_BUILD_OPTIONS to override everything. Some options are
meant to be exported through DEB_BUILD_OPTIONS and some are not.
* scripts/t/300_Dpkg_BuildOptions.t: adjust test suite for the new module
* scripts/dpkg-buildpackage.pl: adjust to use the new Dpkg::BuildOptions
API.
* scripts/Dpkg/Fields.pm, scripts/Dpkg/Source/Package.pm: add the new
Build-Options field as a valid field in the source section of
debian/control (and in .dsc files).
---
 scripts/Dpkg/BuildOptions.pm  |  257 +
 scripts/Dpkg/Fields.pm|2 +-
 scripts/Dpkg/Source/Package.pm|5 +-
 scripts/dpkg-buildpackage.pl  |   10 +-
 scripts/t/300_Dpkg_BuildOptions.t |   61 +
 5 files changed, 273 insertions(+), 62 deletions(-)

diff --git a/scripts/Dpkg/BuildOptions.pm b/scripts/Dpkg/BuildOptions.pm
index 9d6741b..5b2acdd 100644
--- a/scripts/Dpkg/BuildOptions.pm
+++ b/scripts/Dpkg/BuildOptions.pm
@@ -5,51 +5,252 @@ use warnings;
 
 use Dpkg::Gettext;
 use Dpkg::ErrorHandling qw(warning);
+use Dpkg::Control;
+use Dpkg::Version qw(compare_versions);
 
-sub parse {
-my ($env) = @_;
+# Define behavior for known options:
+# export - the option is meant to be exported in DEB_BUILD_OPTIONS
+# valued - the option can have a value
+# check_value_rx - if defined, a regex to check the value, invalid value
+#   will lead to the option being discarded
+# min_standards_version - if the s-v field is = to the version given,
+#  the option is auto-enabled
+our %OPTIONS = (
+noopt = {
+export = 1,
+valued = 0,
+},
+nostrip = {
+export = 1,
+valued = 0,
+},
+nocheck = {
+export = 1,
+valued = 0,
+},
+parallel = {
+export = 1,
+valued = 1,
+check_value_rx = qr/^-?\d+$/,
+},
+);
 
-$env ||= $ENV{DEB_BUILD_OPTIONS};
+=head1 NAME
 
-unless ($env) { return {}; }
+Dpkg::BuildOptions - handle build options from debian/control and environment
 
-my %opts;
+=head1 DESCRIPTION
 
-foreach (split(/\s+/, $env)) {
-	unless (/^([a-z][a-z0-9_-]*)(=(\S*))?$/) {
-warning(_g(invalid flag in DEB_BUILD_OPTIONS: %s), $_);
-next;
+It provides an object to analyze and manipulate build options as defined
+by combining information provided by the debian/control file and by the
+DEB_BUILD_OPTIONS environment variable.
+
+=head1 FUNCTIONS
+
+=over 4
+
+=item $b = Dpkg::BuildOptions($file)
+
+Create a new Dpkg::BuildOptions object. The $file parameter is simply
+forwarded to Dpkg::Control-new($file). If undef, it will simply use
+debian/control by default.
+
+=cut
+sub new {
+my ($this, $ctl_file) = @_;
+my $class = ref($this) || $this;
+my $self = {
+'opts' = {},
+'control' = Dpkg::Control-new($ctl_file),
+};
+bless $self, $class;
+$self-parse_options();
+return $self;
+}
+
+=item $b-reset()
+
+Forget all options already parsed. Start afresh.
+
+=cut
+sub reset {
+my ($self) = @_;
+$self-{'opts'} = {};
+}
+
+=item $b-parse_options()
+
+Do a full parse of options, including the Build-Options field in
+debian/control and the DEB_BUILD_OPTIONS variable.
+
+=cut
+sub parse_options {
+my ($self) = @_;
+

Bug#229357: New Build-Options field and build-arch option, please review

2008-07-10 Thread Felipe Sateler
El 10/07/08 18:02 Raphael Hertzog escribió:
 Hello,

 in order to fix #229357 I decided to add a new Build-Options field.
 I modified Dpkg::BuildOptions to parse this field and DEB_BUILD_OPTIONS.
 And I added support for a build-arch option, that if present, will let
 dpkg-buildpackage call debian/rules build-arch and build-indep.

 It's not obvious that this was the right choice when you think of the
 currently existing build options but once you start thinking of possible
 additions (as requested in #489771), it becomes more evident that it makes
 sense. Even if some build options should really only be used in
 the field while others should only be used in the environment variable,
 the possibility to override the former with the latter is nice.

I'm not really sure this is right. There are two things that we want to do 
here: declare that a package supports something, and asking the package to do 
something. This difference is blurred now, and I think it is confusing.
OTOH, it gives the benefit of being able to ignore the package capabilities 
via the environment (ie, unset a given option).
I fear it will give rise to abuses such as setting parallel=n in the control 
file.


Saludos,
Felipe Sateler


signature.asc
Description: This is a digitally signed message part.