Le dimanche 16 août 2015, 14:20:33 Niels Thykier a écrit : > Control: tags -1 - moreinfo > > On 2015-08-16 00:13, Rafael Kitover wrote: > > Package: lintian > > Version: 2.5.35 > > Severity: normal > > > > This patch fixes lack-of-common-license-reference false positives in > > new-style copyright files when a license refers to one of these common > > licenses. > > > > It includes a test for this against all common licenses. > > > > All testsuite tests pass. > > > > The patch is against the debcheckout of lintian. > > > > [...] > > > > Hi, > > Thanks for looking in to this. > > I do have some concerns with the implementation, see below interleaved. > > > 0001-fix-common-lic.-false-pos.-in-new-style-copyright.patch > > > > > >>From 441f44c5be0fe70d9a86b3ee0cec49430b0c2a9d Mon Sep 17 00:00:00 2001 > > From: Rafael Kitover <[email protected]> > > Date: Sat, 15 Aug 2015 17:50:34 -0400 > > Subject: [PATCH] fix common lic. false pos. in new-style copyright > > > > Fix false positives for lack of common license references in new-style > > copyright files when a license refers to another license, by trying to > > parse the file and then checking both the names of the licenses and the > > texts. > > > > Add new test for references to common licenses as well. > > > > The test suite passes with these changes. > > --- > > [...] > > > > diff --git a/checks/copyright-file.pm b/checks/copyright-file.pm > > index c6e35ef..09b664e 100644 > > --- a/checks/copyright-file.pm > > +++ b/checks/copyright-file.pm > > [...] > > @@ -373,6 +426,45 @@ sub check_cross_link { > > return; > > } > > > > +# Checks the name and text of every license in the file against given name > > and > > +# text check coderefs, if the file is in the new format, if the file is in > > the > > +# old format only runs the text coderef against the whole file. > > +sub check_names_texts { > > This is called multiple times. Each time it opens and parses the > copyright file at least once each. > > I would like to see this done smarter and less wastefully. Please note > that we have some very large copyright files or/and source packages > producing many binaries with a copyright file in each of them. > > Have you looked at the performance of this check on some packages? > Lintian has a few built-in options to help you here:
I would not try to parse anything except copyright 1.0 format.
The other format is slowly dying...
>
> $ lintian --help=extended | grep perf
> --perf-debug turn on performance debugging
> --perf-output X send performance logging to file (...)
>
>
> > + my ($name_check, $text_check, $file) = @_;
> > +
> > + local $@;
> > + eval {
> > + foreach my $paragraph (read_dpkg_control($file)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^
If you use copyright 1.0 we have parsed the format and we have hash
> (First read of the file.)
>
> > + next
> > + unless (keys %$paragraph == 1)
> > + && ((keys %$paragraph)[0] =~ /^license$/i);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Please use "exists($paragraph->{'license'})" instead.
Yes
>
> > +
> > + my ($license_name, $license_text)
> > + = (values %$paragraph)[0] =~ /^([^\r\n]+)\r?\n(.*)\z/s;
> > +
> > + my $matches = do {
> > + local $_ = $license_name || '';
> > + $name_check->($_);
>
> $name_check is always a single regex. Consider passing it as a qr// and
> do a normal regex check here instead of calling a separate sub. This
> would also avoid the "local $_", which is somewhat expensive.
>
> > + }
> > + && do {
> > + local $_ = $license_text || '';
> > + $text_check->($_);
> > + };
>
> If we could avoid the subroutine + local $_; here too that would be nice
> as well.
>
> > +
> > + die 'MATCH' if $matches;
> > + }
> > + };
> > + if ($@)
> > + { # match or parse error: copyright not in new format, just check text
> > + return 1 if $@ =~ /^MATCH/;
> > +
> > + local $_ = slurp_entire_file($file);
> ^^^^^^^^^^^^^^^^^^^^^^^^
> (Second read of the file.)
>
> > + return $text_check->($_);
> > + }
> > +
> > + return; # did not match anything
> > +}
> > +
> > 1;
> >
> > # Local Variables:
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/control.in
> > b/t/tests/copyright-file-non-common-license/debian/debian/control.in
> > new file mode 100644
> > index 0000000..e80822d
> > --- /dev/null
> > +++ b/t/tests/copyright-file-non-common-license/debian/debian/control.in
> > @@ -0,0 +1,94 @@
> > +Source: {$source}
> > +Priority: extra
> > +Section: {$section}
> > +Maintainer: {$author}
> > +Standards-Version: {$standards_version}
> > +Build-Depends: debhelper (>= 9)
> > +
> > +Package: copyright-mentions-apache
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for Apache
> > + Tests against common license false positive for Apache.
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-apache2
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for Apache (2)
> > + Tests against common license false positive for Apache (2).
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-apache3
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for Apache (3)
> > + Tests against common license false positive for Apache (3).
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-gfdl
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for gfdl
> > + Tests against common license false positive for gfdl.
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-gpl
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for gpl
> > + Tests against common license false positive for gpl.
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-lgpl
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for lgpl
> > + Tests against common license false positive for lgpl.
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-lgpl2
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for lgpl (2)
> > + Tests against common license false positive for lgpl (2).
> > + .
> > + 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.
> > +
> > +Package: copyright-mentions-perl
> > +Architecture: all
> > +Depends: $\{shlibs:Depends\}, $\{misc:Depends\}
> > +Description: checks against common license false positive for perl
> > + Tests against common license false positive for perl.
> > + .
> > + 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.
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache.copyright
> > new file mode 100644
> > index 0000000..b4fe499
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum Apache License , Version 2.0 lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache2.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache2.copyright
> > new file mode 100644
> > index 0000000..035ee22
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache2.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum Apache License Version 2.0 lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache3.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache3.copyright
> > new file mode 100644
> > index 0000000..dab0d47
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-apache3.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum Apache-2 License lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gfdl.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gfdl.copyright
> > new file mode 100644
> > index 0000000..5a8f46b
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gfdl.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum GNU Free Documentation License (GFDL) lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gpl.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gpl.copyright
> > new file mode 100644
> > index 0000000..248debf
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-gpl.copyright
> > @@ -0,0 +1,14 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum GNU General Public License (GPL) applies to the changes,
> > + .
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl.copyright
> > new file mode 100644
> > index 0000000..89c5e79
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum GNU Lesser General Public License (LGPL) lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl2.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl2.copyright
> > new file mode 100644
> > index 0000000..291c0e6
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-lgpl2.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum GNU Library General Public License (LGPL) lorem ipsum
> > + lorem ipsum
> > diff --git
> > a/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-perl.copyright
> >
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-perl.copyright
> > new file mode 100644
> > index 0000000..b2c896a
> > --- /dev/null
> > +++
> > b/t/tests/copyright-file-non-common-license/debian/debian/copyright-mentions-perl.copyright
> > @@ -0,0 +1,13 @@
> > +Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> > +Upstream-Name: lintian
> > +Upstream-Contact: Lintian Maintainers <[email protected]>
> > +Source: http://git.debian.org/?p=lintian/lintian.git
> > +
> > +Files: *
> > +Copyright: 2015 Lintian Maintainers <[email protected]>
> > +License: Mentions-Other-License
> > +
> > +License: Mentions-Other-License
> > + lorem ipsum
> > + lorem ipsum under the same terms as Perl itself lorem ipsum
> > + lorem ipsum
> > diff --git a/t/tests/copyright-file-non-common-license/desc
> > b/t/tests/copyright-file-non-common-license/desc
> > new file mode 100644
> > index 0000000..c652192
> > --- /dev/null
> > +++ b/t/tests/copyright-file-non-common-license/desc
> > @@ -0,0 +1,12 @@
> > +Testname: copyright-file-non-common-license
> > +Sequence: 6000
> > +Version: 1.0
> > +Description: Test for false positive for a common license
> > +Skeleton: pedantic
> > +Options: -IE --pedantic
> > +Test-Against:
> > + copyright-should-refer-to-common-license-file-for-gpl
> > + copyright-should-refer-to-common-license-file-for-gfdl
> > + copyright-should-refer-to-common-license-file-for-lgpl
> > + copyright-should-refer-to-common-license-file-for-apache-2
> > + copyright-file-lacks-pointer-to-perl-license
> > diff --git a/t/tests/copyright-file-non-common-license/tags
> > b/t/tests/copyright-file-non-common-license/tags
> > new file mode 100644
> > index 0000000..e69de29
> > -- 2.1.4
> >
>
signature.asc
Description: This is a digitally signed message part.
