[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#542747: [checks/changes-file] check mismatch between Distribution and Changes in *.changes



On Tue, 11 Mar 2014 at 13:11:33 -0700, Russ Allbery wrote:
> Simon McVittie <smcv@debian.org> writes:
> > On the Lintian side of things, I attached a patch to
> > <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=542747> in 2010 and
> > am still waiting for comments.
> 
> > My patch just looks for Distribution=unstable, Changes=experimental
> > because that's the most annoying case ("I uploaded a version that
> > wasn't ready and now I have to use an epoch to roll it back").

Since I was looking at Lintian today for dbus security stuff, I have revised
the patch so it now emits one of these:

# sbuild -d unstable, debian/changelog says experimental
# (the case you typically have to fix with epochs)
E: mypackage changes: distribution-and-experimental-mismatch unstable

# sbuild -d unstable, debian/changelog says UNRELEASED
E: mypackage changes: unreleased-changes

# sbuild -d stable, debian/changelog says unstable
# (or other distributions known to Lintian)
W: mypackage changes: distribution-and-changes-mismatch stable unstable

It also now has regression tests, and the full test suite passes.

Also available from
ssh://git.debian.org/git/users/smcv/lintian.git changes-vs-changes

> > I don't know what those valid use-cases are. BinNMUs in testing
> > for a package uploaded to experimental, possibly? Anything else?
> 
> The use case that I was thinking of is not really a Debian use case.  It's
> relatively common for people with separate repositories to build once and
> upload multiple times to different distributions if the same package can
> work on multiple distributions and their archive software requires that
> (as debarchiver, for example, did, or at least it was the easiest way to
> make the right thing happen).

I have made the check not trigger on unknown distributions, so if
these people use a suite named jessie-myapp or for-jessie or something,
they won't get the tag.

I can't distinguish between a changes file that says "jessie" and is
intended to be uploaded to packages.example.com and a changes file
that says "jessie" and is intended to be uploaded to ftp.debian.org.

Perhaps more importantly, dak can't distinguish between those either,
which means that if your key is in the (Debian|Ubuntu|...) keyring, you
should never sign a .changes file whose suite is valid for the
(Debian|Ubuntu|...) archive if you do not actually intend for someone
to do that upload on your behalf - so I think a suite prefix or suffix
is, or should be, best-practice.

I suspect that in practice, if these people do their multi-uploading
in this workflow:

* build for stable [1]
* upload to their stable repo
* edit .changes to remove signature (if present) and s/stable/testing/
* upload to their testing repo
* edit .changes again, s/testing/unstable/
* upload to their unstable repo

(or the equivalent for successive Ubuntu suites) then they are probably
only going to run Lintian after [1], not after every time they hack up
the .changes? So this check doesn't interfere anyway?

> That said, one, this is an outside-of-Debian use case, [...]
> Also, now that reprepro is more widespread and
> doesn't require this sort of workaround for not having simple distribution
> migration, it's not clear that use case is particularly important any
> more.

These seem like further reasons not to worry about third parties being
hit by this check; so I have made it relatively high severity and
certainty, because when this happens in Debian and e.g. triggers
an unintended transition, the result can be extra work for quite a
lot of people.

    S
>From 0e8e8e174102155e58b9e8ed0f1c3670247cf742 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Wed, 28 Jan 2015 18:56:11 +0000
Subject: [PATCH] Bug #540294: add checks for mismatched .changes and Changes:
 distribution

The tag is only emitted for known suites, in an attempt to avoid
interfering with non-Debian workflows.
---
 checks/changes-file.desc                           | 33 ++++++++++++++++++++++
 checks/changes-file.pm                             | 27 ++++++++++++++++++
 t/changes/changes-distribution-mismatch.changes.in | 16 +++++++++++
 t/changes/changes-distribution-mismatch.desc       |  6 ++++
 t/changes/changes-distribution-mismatch.tags       |  1 +
 t/changes/changes-experimental-mismatch.changes.in | 16 +++++++++++
 t/changes/changes-experimental-mismatch.desc       |  6 ++++
 t/changes/changes-experimental-mismatch.tags       |  1 +
 t/changes/changes-unreleased.changes.in            | 16 +++++++++++
 t/changes/changes-unreleased.desc                  |  6 ++++
 t/changes/changes-unreleased.tags                  |  1 +
 11 files changed, 129 insertions(+)
 create mode 100644 t/changes/changes-distribution-mismatch.changes.in
 create mode 100644 t/changes/changes-distribution-mismatch.desc
 create mode 100644 t/changes/changes-distribution-mismatch.tags
 create mode 100644 t/changes/changes-experimental-mismatch.changes.in
 create mode 100644 t/changes/changes-experimental-mismatch.desc
 create mode 100644 t/changes/changes-experimental-mismatch.tags
 create mode 100644 t/changes/changes-unreleased.changes.in
 create mode 100644 t/changes/changes-unreleased.desc
 create mode 100644 t/changes/changes-unreleased.tags

diff --git a/checks/changes-file.desc b/checks/changes-file.desc
index e8bfdea..56ea819 100644
--- a/checks/changes-file.desc
+++ b/checks/changes-file.desc
@@ -146,3 +146,36 @@ Info: The version number doesn't comply with the standard backport version
  rules. It should end in ~bpoX+N, where X is the release version number of
  the target distribution.
 Ref: http://backports.debian.org/Contribute/
+
+Tag: distribution-and-changes-mismatch
+Severity: normal
+Certainty: possible
+Info: The <tt>Distribution</tt> in the <tt>.changes</tt> file indicates
+ that packages should be installed into one distribution (suite), but the
+ distribution in the <tt>Changes</tt> field copied from
+ <tt>debian/changelog</tt> indicates that a different distribution
+ was intended.
+ .
+ This is an easy mistake to make when invoking "sbuild ... foo.dsc".
+ Double-check the <tt>-d</tt> option if using sbuild in this way.
+Ref: #542747, #529281
+
+Tag: distribution-and-experimental-mismatch
+Severity: serious
+Certainty: certain
+Info: The <tt>Distribution</tt> in the <tt>.changes</tt> file indicates
+ that packages should be installed into a non-experimental distribution
+ (suite), but the distribution in the <tt>Changes</tt> field copied from
+ <tt>debian/changelog</tt> indicates that experimental was intended.
+ .
+ This is an easy mistake to make when invoking "sbuild ... foo.dsc".
+ Double-check the <tt>-d</tt> option if using sbuild in this way.
+Ref: #542747, #529281
+
+Tag: unreleased-changes
+Severity: important
+Certainty: certain
+Info: The distribution in the <tt>Changes</tt> field copied from
+ <tt>debian/changelog</tt> indicates that this package was not intended
+ to be released yet.
+Ref: #542747
diff --git a/checks/changes-file.pm b/checks/changes-file.pm
index 4761d09..d86bff2 100644
--- a/checks/changes-file.pm
+++ b/checks/changes-file.pm
@@ -120,6 +120,32 @@ sub run {
                     # bad distribution entry
                     tag 'bad-distribution-in-changes-file', $distribution;
                 }
+
+                my $changes = $info->field('changes');
+                if (defined $changes) {
+                    # take the first non-empty line
+                    $changes =~ s/^\s+//s;
+                    $changes =~ s/\n.*//s;
+
+                    if ($changes
+                        =~ m/^\s*(?:\w[-+0-9a-z.]*)\s*\([^\(\) \t]+\)\s*([-+0-9A-Za-z.]+)\s*;/
+                      ) {
+                        my $changesdist = $1;
+                        if ($changesdist eq 'UNRELEASED') {
+                            tag 'unreleased-changes';
+                        } elsif ($changesdist ne $distribution
+                            && $changesdist ne $dist) {
+                            if (   $changesdist eq 'experimental'
+                                && $dist ne 'experimental') {
+                                tag 'distribution-and-experimental-mismatch',
+                                  $distribution;
+                            } elsif ($KNOWN_DISTS->known($dist)) {
+                                tag 'distribution-and-changes-mismatch',
+                                  $distribution, $changesdist;
+                            }
+                        }
+                    }
+                }
             }
         }
 
@@ -127,6 +153,7 @@ sub run {
             tag 'multiple-distributions-in-changes-file',
               $info->field('distribution');
         }
+
     }
 
     # Urgency is only recommended by Policy.
diff --git a/t/changes/changes-distribution-mismatch.changes.in b/t/changes/changes-distribution-mismatch.changes.in
new file mode 100644
index 0000000..22dab07
--- /dev/null
+++ b/t/changes/changes-distribution-mismatch.changes.in
@@ -0,0 +1,16 @@
+Format: 1.8
+Date: {$date}
+Source: {$source}
+Binary: {$source}
+Architecture: source all
+Version: {$version}
+Distribution: stable
+Urgency: low
+Maintainer: {$author}
+Changed-By: {$author}
+Description:
+ {$source} - {$description}
+Changes:
+ {$source} ({$version}) unstable; urgency=low
+ .
+   * I used the wrong argument to `sbuild -d`.
diff --git a/t/changes/changes-distribution-mismatch.desc b/t/changes/changes-distribution-mismatch.desc
new file mode 100644
index 0000000..5771b28
--- /dev/null
+++ b/t/changes/changes-distribution-mismatch.desc
@@ -0,0 +1,6 @@
+Testname: changes-distribution-mismatch
+Sequence: 6000
+Version: 1.0
+Description: Test for unstable package to be installed in stable
+Test-For:
+ distribution-and-changes-mismatch
diff --git a/t/changes/changes-distribution-mismatch.tags b/t/changes/changes-distribution-mismatch.tags
new file mode 100644
index 0000000..b558b63
--- /dev/null
+++ b/t/changes/changes-distribution-mismatch.tags
@@ -0,0 +1 @@
+W: changes-distribution-mismatch changes: distribution-and-changes-mismatch stable unstable
diff --git a/t/changes/changes-experimental-mismatch.changes.in b/t/changes/changes-experimental-mismatch.changes.in
new file mode 100644
index 0000000..fa344d6
--- /dev/null
+++ b/t/changes/changes-experimental-mismatch.changes.in
@@ -0,0 +1,16 @@
+Format: 1.8
+Date: {$date}
+Source: {$source}
+Binary: {$source}
+Architecture: source all
+Version: {$version}
+Distribution: unstable
+Urgency: low
+Maintainer: {$author}
+Changed-By: {$author}
+Description:
+ {$source} - {$description}
+Changes:
+ {$source} ({$version}) experimental; urgency=low
+ .
+   * I used the wrong argument to `sbuild -d`.
diff --git a/t/changes/changes-experimental-mismatch.desc b/t/changes/changes-experimental-mismatch.desc
new file mode 100644
index 0000000..cbc7210
--- /dev/null
+++ b/t/changes/changes-experimental-mismatch.desc
@@ -0,0 +1,6 @@
+Testname: changes-experimental-mismatch
+Sequence: 6000
+Version: 1.0
+Description: Test for experimental package to be installed in unstable
+Test-For:
+ distribution-and-experimental-mismatch
diff --git a/t/changes/changes-experimental-mismatch.tags b/t/changes/changes-experimental-mismatch.tags
new file mode 100644
index 0000000..8302562
--- /dev/null
+++ b/t/changes/changes-experimental-mismatch.tags
@@ -0,0 +1 @@
+E: changes-experimental-mismatch changes: distribution-and-experimental-mismatch unstable
diff --git a/t/changes/changes-unreleased.changes.in b/t/changes/changes-unreleased.changes.in
new file mode 100644
index 0000000..d6fbdd8
--- /dev/null
+++ b/t/changes/changes-unreleased.changes.in
@@ -0,0 +1,16 @@
+Format: 1.8
+Date: {$date}
+Source: {$source}
+Binary: {$source}
+Architecture: source all
+Version: {$version}
+Distribution: unstable
+Urgency: low
+Maintainer: {$author}
+Changed-By: {$author}
+Description:
+ {$source} - {$description}
+Changes:
+ {$source} ({$version}) UNRELEASED; urgency=low
+ .
+   * I'm still working on this package, do not upload.
diff --git a/t/changes/changes-unreleased.desc b/t/changes/changes-unreleased.desc
new file mode 100644
index 0000000..29b0a8d
--- /dev/null
+++ b/t/changes/changes-unreleased.desc
@@ -0,0 +1,6 @@
+Testname: changes-unreleased
+Sequence: 6000
+Version: 1.0
+Description: Test for UNRELEASED package uploaded to unstable
+Test-For:
+ unreleased-changes
diff --git a/t/changes/changes-unreleased.tags b/t/changes/changes-unreleased.tags
new file mode 100644
index 0000000..dc61e25
--- /dev/null
+++ b/t/changes/changes-unreleased.tags
@@ -0,0 +1 @@
+E: changes-unreleased changes: unreleased-changes
-- 
2.1.4


Reply to: