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 <rkito...@gmail.com> > > 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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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 <debian-lint-ma...@lists.debian.org> > > +Source: http://git.debian.org/?p=lintian/lintian.git > > + > > +Files: * > > +Copyright: 2015 Lintian Maintainers <debian-lint-ma...@lists.debian.org> > > +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.