Hey Guillem,

Guillem Jover [2016-05-09  3:34 +0200]:
> > But I'm not sure at which point the Xs- prefix disappears, nor when the new
> > field would become official -- is it necessary to check for it here? Or just
> > for 'Testsuite-Triggers'?
> 
> This depends on whether the field has had usage before it was known to
> dpkg-dev (in the form XS-Field), otherwise if it has just been added
> now then there's no need to check for the export markers.

This has never been used, so I dropped the Xs-* thing.

> You can use deps_iterate() from Dpkg::Deps instead.

Done.

> You could use Dpkg::Index here instead which can parse the control
> file and store a Dpkg::Control per paragraph. Eventually we might want
> to add something like Dpkg::Control::Tests(uite) or something like that.

I did this, but kept it as a separate patch. It does not make the
logic significantly easier, and requires gems like

  foreach my $rec (values %{$index->{items}}) {

(Dpkg::Index does not have any documented way of iterating over all
fields). It also needs a non-trivial get_key_func() -- while I don't
actually use the keys, not having a key_func causes nasty perl
warnings.

If you prefer this, rebase -i 'f'ixup this into the first patch.

> 
> > +        $control->{'Depends'} =~ 
> > s/(^|,)\s*(@|\@builddeps@)([[:space:],]|$)/$1$3/g;
> 
> I didn't like this part which seems a bit fragile, and has to be kept
> in sync with dependency parsing if something changes in that sense. So
> I've added a tests_dep option to deps_parse() for 1.18.7, that will
> allow package names with @ in them, then you can filter them out with
> the rest.

Very nice! Did that now.

> > +    $fields->{'Testsuite-Triggers'} = join(', ', sort keys %testdeps) if 
> > %testdeps;
> 
> No need for the conditional, the dumper will ignore undef fields.

It'll actually be an empty string, but indeed the dumper ignores that
too, so I dropped it.

Updated patches attached, rebased against master from today.  This
looks a lot more succinct and elegant now, thanks for these hints!

As before, I tested various d/t/control scenarios, with/without manual
T-T: field in d/control, package build, etc.

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From c0fb9e65ddcd3f8189b2127653722f99b282e645 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Sun, 1 May 2016 09:43:39 -0500
Subject: [PATCH 1/2] dpkg-source: Generate Testsuite-Triggers field from test
 dependencies

Sometimes autopkgtests regress due to change in a package which is only a test
dependency (Depends: in debian/tests/control), not a build or binary one. It is
useful to trigger a test if such a test dependency changes.

Record the union of all test dependency packages in a new Testsuite-Triggers:
field in the .dsc, so that they will be recorded in the Sources package index.
Ignore versions and flatten OR dependencies as they are not interesting for
determining reverse test dependencies and should not be (ab)used for replacing
debian/tests/control parsing.

Closes: #779559
LP: #1491145
---
 debian/changelog                   |  5 +++++
 scripts/Dpkg/Control/FieldsCore.pm |  4 ++++
 scripts/dpkg-source.pl             | 39 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index f963c99..225dad4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -9,6 +9,11 @@ dpkg (1.18.8) UNRELEASED; urgency=medium
   [ Updated manpages translations ]
   * German (Helge Kreutzmann).
 
+  [ Martin Pitt ]
+  * dpkg-source: Record the union of all autopkgtest dependency packages in a
+    new Testsuite-Triggers: field in the .dsc, so that they will be recorded
+    in the Sources package index. Closes: #779559, LP: #1491145
+
  -- Guillem Jover <guil...@debian.org>  Mon, 09 May 2016 05:10:36 +0200
 
 dpkg (1.18.7) unstable; urgency=medium
diff --git a/scripts/Dpkg/Control/FieldsCore.pm b/scripts/Dpkg/Control/FieldsCore.pm
index 3af6fa7..36e591d 100644
--- a/scripts/Dpkg/Control/FieldsCore.pm
+++ b/scripts/Dpkg/Control/FieldsCore.pm
@@ -332,6 +332,10 @@ our %FIELDS = (
         allowed => ALL_SRC,
         separator => FIELD_SEP_COMMA,
     },
+    'Testsuite-Triggers' => {
+        allowed => ALL_SRC,
+        separator => FIELD_SEP_COMMA,
+    },
     'Triggers-Awaited' => {
         allowed => CTRL_FILE_STATUS,
         separator => FIELD_SEP_SPACE,
diff --git a/scripts/dpkg-source.pl b/scripts/dpkg-source.pl
index 89281c1..b2334c9 100755
--- a/scripts/dpkg-source.pl
+++ b/scripts/dpkg-source.pl
@@ -359,6 +359,7 @@ if ($options{opmode} =~ /^(build|print-format|(before|after)-build|commit)$/) {
 
     # Check if we have a testsuite, and handle manual and automatic values.
     set_testsuite_field($fields);
+    set_testsuite_triggers_field($fields, @binarypackages);
 
     # Scan fields of dpkg-parsechangelog
     foreach (keys %{$changelog}) {
@@ -513,6 +514,44 @@ sub set_testsuite_field
     $fields->{'Testsuite'} = join ', ', sort keys %testsuite;
 }
 
+sub set_testsuite_triggers_field
+{
+    my $fields = shift;
+    my @binarypackages = shift;
+    my $tc_path = "$dir/debian/tests/control";
+    my %testdeps;
+
+    # never overwrite a manually defined field
+    return if $fields->{'Testsuite-Triggers'};
+
+    # autopkgtest is the only test we can parse
+    return unless -e $tc_path;
+
+    # parse Tests: from debian/tests/control
+    my $control = Dpkg::Control::HashCore->new(
+        drop_empty => 1,
+        allow_duplicate => 1,
+        name => 'autopkgtest control');
+    open(my $tc_fh, '<', $tc_path)
+        or syserr(g_('cannot read %s'), $tc_path);
+
+    my $cb_dep = sub {
+        # ignore autopkgtest meta-depends like @ or @builddeps@
+        $testdeps{$_[0]->{package}} = 1 unless $_[0]->{package} =~ /^@/;
+    };
+
+    while($control->parse($tc_fh, $tc_path)) {
+        my $deps = deps_parse($control->{'Depends'}, use_arch => 0, tests_dep => 1);
+        deps_iterate($deps, $cb_dep);
+    }
+
+    # remove our own binaries
+    foreach my $p (@binarypackages) {
+        delete $testdeps{$p};
+    }
+    $fields->{'Testsuite-Triggers'} = join(', ', sort keys %testdeps);
+}
+
 sub setopmode {
     my $opmode = shift;
 
-- 
2.7.4

From 39d57273571abaca5de407d96ac8264f0dff2456 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Fri, 13 May 2016 13:54:02 +0200
Subject: [PATCH 2/2] dpkg-source.pl: Use Dpkg::Index to parse
 debian/tests/control

... instead of Dpkg::Control::HashCore.

Merge this into the previous commit if you like this.
---
 scripts/dpkg-source.pl | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/scripts/dpkg-source.pl b/scripts/dpkg-source.pl
index b2334c9..c9a8acd 100755
--- a/scripts/dpkg-source.pl
+++ b/scripts/dpkg-source.pl
@@ -42,6 +42,7 @@ use Dpkg::Compression;
 use Dpkg::Conf;
 use Dpkg::Control::Info;
 use Dpkg::Control::Fields;
+use Dpkg::Index;
 use Dpkg::Substvars;
 use Dpkg::Version;
 use Dpkg::Vars;
@@ -528,21 +529,14 @@ sub set_testsuite_triggers_field
     return unless -e $tc_path;
 
     # parse Tests: from debian/tests/control
-    my $control = Dpkg::Control::HashCore->new(
-        drop_empty => 1,
-        allow_duplicate => 1,
-        name => 'autopkgtest control');
-    open(my $tc_fh, '<', $tc_path)
-        or syserr(g_('cannot read %s'), $tc_path);
-
-    my $cb_dep = sub {
-        # ignore autopkgtest meta-depends like @ or @builddeps@
-        $testdeps{$_[0]->{package}} = 1 unless $_[0]->{package} =~ /^@/;
-    };
+    my $index = Dpkg::Index->new(get_key_func =>
+        sub { return $_[0]->{'Tests'} || $_[0]->{'Test-Command'} });
+    $index->load($tc_path);
+    foreach my $rec (values %{$index->{items}}) {
+        my $deps = deps_parse($rec->{'Depends'} || '', use_arch => 0, tests_dep => 1);
 
-    while($control->parse($tc_fh, $tc_path)) {
-        my $deps = deps_parse($control->{'Depends'}, use_arch => 0, tests_dep => 1);
-        deps_iterate($deps, $cb_dep);
+        # ignore autopkgtest meta-depends like @ or @builddeps@
+        deps_iterate($deps, sub { $testdeps{$_[0]->{package}} = 1 unless $_[0]->{package} =~ /^@/ });
     }
 
     # remove our own binaries
-- 
2.7.4

Attachment: signature.asc
Description: PGP signature

Reply via email to