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

Bug#738591: lintian: Add checker for timestamped gzip files



On 2014-02-11 10:53, Tomasz Buchert wrote:
> On 11/02/14 10:09, Jérémy Bobbio wrote:
>> > Tomasz Buchert:
>>>>>> > > > >> +Severity: normal
>>>>> > > > > 
>>>>> > > > > It think it should be at most "wishlist", perhaps even "pedantic".
>>>>> > > > > 
>>> > > 
>>> > > Let's make it "pedantic", but hopefully one day
>>> > > it will be "normal".
>> > 
>> > Could we go for “wishlist” instead?
> Hi,
> I reworked the patch so that it reuses the machinery
> in files.pm. I also made it "wishlist" this time. I attach the patch.
> 


Hi,

Thanks for working on it and producing a patch for it as well. :)

> Currently, it will emit "package-contains-timestamped-gzip"
> on any file ending with ".gz", being a gzip file and containing
> a timestamp. It means that currently the tag "gzip-file-is-not-multi-arch-same-safe" 
> will imply "package-contains-timestamped-gzip".

Ok - not sure if anyone has any feeling for or against that. I am a
/little/ concerned with it creating "too much output" (for new users),
but other than that I don't care too much.


> Moreover, the new tag breaks multiple
> tests (files-gzip, manpages-general, etc.) because they use timestamped gzips. I will
> fix it, but first I'd like to know that implementation of the tag is ok.
> 
> Cheers,
> Tomasz
> 

Noted, I have written my comments below:

> [...]
> 
> 0001-new-tag-package-contains-timestamped-gzip-test.patch
> 
> 
>>From 4dcc45c75df792820c356beca0fa84b067cf0268 Mon Sep 17 00:00:00 2001
> From: Tomasz Buchert <tomasz.buchert@inria.fr>
> Date: Tue, 11 Feb 2014 10:11:20 +0100
> Subject: [PATCH] new tag: package-contains-timestamped-gzip (+ test)
> 
> ---
>  checks/files.desc                                      |   8 ++++++++
>  checks/files.pm                                        |  14 +++++++++-----
>  t/tests/reproducibility/debian/debian/control.in       |  17 +++++++++++++++++

Test should be renamed to "files-reproducibility" (or something else
starting with "files-").  Otherwise, it won't be run with "debian/rules
runtests onlyrun=files" as it should.

>  .../debian/debian/unreproducible-pkg.install           |   1 +
>  t/tests/reproducibility/debian/file                    |   1 +
>  t/tests/reproducibility/debian/file-with-timestamp.gz  | Bin 0 -> 39 bytes
>  .../reproducibility/debian/file-without-timestamp.gz   | Bin 0 -> 34 bytes

The gz files seem like a mistake?

>  t/tests/reproducibility/debian/prepare                 |   4 ++++
>  t/tests/reproducibility/desc                           |   6 ++++++
>  t/tests/reproducibility/tags                           |   1 +
>  10 files changed, 47 insertions(+), 5 deletions(-)
>  create mode 100644 t/tests/reproducibility/debian/debian/control.in
>  create mode 100644 t/tests/reproducibility/debian/debian/unreproducible-pkg.install
>  create mode 100644 t/tests/reproducibility/debian/file
>  create mode 100644 t/tests/reproducibility/debian/file-with-timestamp.gz
>  create mode 100644 t/tests/reproducibility/debian/file-without-timestamp.gz
>  create mode 100755 t/tests/reproducibility/debian/prepare
>  create mode 100644 t/tests/reproducibility/desc
>  create mode 100644 t/tests/reproducibility/tags
> 
> diff --git a/checks/files.desc b/checks/files.desc
> index 760f86a..e8237f0 100644
> --- a/checks/files.desc
> +++ b/checks/files.desc
> @@ -1448,3 +1448,11 @@ Info: The given file is in PATH but consists of non-ASCII characters.
>   .
>   Note that Lintian may be unable to display the filename accurately.
>   Unprintable characters may have been replaced.
> +
> +Tag: package-contains-timestamped-gzip
> +Severity: wishlist
> +Certainty: certain
> +Info: The package contains a gzip'ed file that has timestamps.
> + Such files make the produces packages unreproducible.

You probably want to define "unreproducible" here a bit more.  (Also,
could use a grammar check "files make the produces packages").

Feel free to add a "Ref: <URL explaining "reproducible builds">

> + .
> + Pass "-n" flag to gzip to avoid it.

[nitpick] Maybe consider something like:

"""
Please consider passing the "-n" flag to gzip to avoid this.
"""

Being polite sometimes goes a long way.

> diff --git a/checks/files.pm b/checks/files.pm
> index 5c5a60d..21a0f0c 100644
> --- a/checks/files.pm
> +++ b/checks/files.pm
> @@ -1400,23 +1400,27 @@ sub run {
>                  [...]
>                      close($fd);
> +                    if ($mtime != 0) {
> +                        if ($isma_same && $file !~ m/\Q$arch\E/o) {
> +                            tag 'gzip-file-is-not-multi-arch-same-safe', $file;
> +                        }
> +                        tag 'package-contains-timestamped-gzip', $file;
> +                    }
>                  }
>              }
>  

Looks good (and also seems to be trivial to disable the "implication"
issue if needed be).

> diff --git a/t/tests/reproducibility/debian/debian/control.in b/t/tests/reproducibility/debian/debian/control.in
> new file mode 100644
> index 0000000..a7e8050
> --- /dev/null
> +++ b/t/tests/reproducibility/debian/debian/control.in
> @@ -0,0 +1,17 @@
> +Source: {$source}
> +Priority: extra
> +Section: devel
> +Maintainer: {$author}
> +Standards-Version: {$standards_version}
> +Build-Depends: debhelper (>= 9)
> +
> +Package: unreproducible-pkg
> +Architecture: all
> +Depends: $\{misc:Depends\}
> +Description: {$description} - gzip files
> + This is a test package designed to exercise some feature or tag of
> + Lintian.  It is part of the Lintian test suite and may do very odd
> + things.  It should not be installed like a regular package.  It may
> + be an empty package.
> + .
> + Contains a few GZIP files.

Is this file actually needed? It does not seem to provide any
customisation (other than the package name)?  If only the package name
is changed, please just kill the file and change the package name in the
(expected) tags file

Note that most of the fields (incl. architecture) can be set in the desc
file and it will "Just Work(tm)".

> [...]
> diff --git a/t/tests/reproducibility/debian/prepare b/t/tests/reproducibility/debian/prepare
> new file mode 100755
> index 0000000..a0feb41
> --- /dev/null
> +++ b/t/tests/reproducibility/debian/prepare
> @@ -0,0 +1,4 @@
> +#!/bin/bash

[nitpick, /bin/sh - nothing here needs bash]

> +
> +gzip file -c > file-with-timestamp.gz
> +gzip file -nc > file-without-timestamp.gz
> diff --git a/t/tests/reproducibility/desc b/t/tests/reproducibility/desc
> new file mode 100644
> index 0000000..f0e18bd
> --- /dev/null
> +++ b/t/tests/reproducibility/desc
> @@ -0,0 +1,6 @@
> +Testname: reproducibility

As mentioned, should be renamed to start with "files-"

> +Sequence: 6000
> +Version: 1.0
> +Description: Test if package is reproducible
> +Test-For:
> + package-contains-timestamped-gzip
> diff --git a/t/tests/reproducibility/tags b/t/tests/reproducibility/tags
> new file mode 100644
> index 0000000..8057dae
> --- /dev/null
> +++ b/t/tests/reproducibility/tags
> @@ -0,0 +1 @@
> +I: unreproducible-pkg: package-contains-timestamped-gzip usr/share/pkg-with-gzips/file-with-timestamp.gz
> -- 1.8.5.3
> 


Otherwise, it looks god at first glance (without having tested it).

~Niels


Reply to: