date:20130329

Bug#612610: lintian: should suggest switching to 3.0 (quilt)

2013-03-29 Thread Lucas Nussbaum

On 29/03/13 at 01:28 +0100, Niels Thykier wrote:
 Hi,
 
 I am not sure we can in general promote the use of 3.0 (quilt) over 1.0
 via Lintian at the moment[1].
 
 Though I noticed that people are writing their own tools to extract
 things like what source format is used or what build systems are
 used.  With #359059 being fixed in 2.5.12, perhaps it is worth for us
 to consider if Lintian could be used for more than mere flaw
 reporting.  Like adding a new kind of tag that is not a flaw but
 simply a property of the package[2].
 
 While it would not directly solve your/Luacs's request for promoting a
 switch to 3.0 (quilt), it would still report which source formats are
 used (and would be importable into UDD).  It is also quite possible that
 some of the metrics on mentors.d.n could be replaced by this new
 property tag[3].
 
 ~Niels
 
 [1] Basically it is the same reasons as mentioned in #702671.
 
 [2] Originally I considered using informational tag here, but I
 figured it would be confused with I tags.
 
 [3] I doubt the current ones will be replaced, but one can hope that
 future metrics would be written as such a tag.

Hi,

Actually, I have two motivations for that:

1) be able to easily track the number of affected packages. But actually, I
solve that using another solution (custom script + snapshot.d.o, see
http://www.lucas-nussbaum.net/blog/?p=751).  [ it just occurred to me that
using lintian to do that analysis would have been possible (esp. with
Property tags) and quite nice. ]

2) push for archive renovation/standardisation on good practices.
As I wrote in https://lists.debian.org/debian-vote/2013/03/msg00193.html:
 Discouraging the use of some development practices is part of that.
 There are good reasons for not using any of dh or cdbs, not using 3.0
 (quilt), so I don't think that we should force that in policy, and make
 that RC bugs.
 But I think that we should discuss adding lintian warning or errors for:
 - packages using 1.0 format and having files modified directly
   = should move to 3.0 (quilt)
 - packages using 1.0 format and simple-patchsys, quilt, or dpatch
   = should move to 3.0 (quilt)
 - packages using debhelper directly (not dh or cdbs)
   = should move to dh
 [ there are good reasons in some cases for doing some of the above.
   Adding lintian override in those cases would be totally OK, and also
   a good way to identify current limitations in 3.0 (quilt) or dh. ]
 
 I would hope that the increasing visibility brought by lintian
 warnings/errors, and as well as the advertised project consensus that
 such practices are discouraged, would help us get rid of such practices.

Now, as was suggested in #702671, there should be prior discussion on -devel@
about that.  I'll raise the topic after the DPL election (it might sound like a
political move if I did it now) -- unless someone beats me with it.

Lucas


-- 
To UNSUBSCRIBE, email to debian-lint-maint-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20130329071729.ga15...@xanadu.blop.info

Bug#704197: Please review: systemd checks

2013-03-29 Thread Michael Stapelberg

Package: lintian
Version: 2.5.10.4
Severity: wishlist

Attached you can find my first stab at systemd-related checks for
lintian. While some details in parsing the service files are not
implemented (see the TODOs in the code), I’d like you to have a look at
the checks in general. Is there anything that needs to be improved
before these can be shipped with lintian?

Thanks!
Check-Script: systemd
Author: Michael Stapelberg stapelb...@debian.org
Abbrev: systemd
Type: binary
Info: Checks various systemd policy things
Needs-Info: scripts, index, unpacked, file-info

Tag: systemd-service-file-outside-lib
Severity: serious
Certainty: certain
Info: The package ships a systemd service file outside
 tt/lib/systemd/system//tt
 .
 System administrators should have the possibility to overwrite a service file
 (or parts of it, in newer systemd versions) by placing a file in
 tt/etc/systemd/system/tt, so the canonical location we use for service
 files is tt/lib/systemd/system//tt.

Tag: systemd-tmpfiles.d-outside-usr-lib
Severity: serious
Certainty: certain
Info: The package ships a systemd tmpfiles.d(5) conf file outside
 tt/usr/lib/tmpfiles.d//tt

Tag: systemd-service-file-refers-to-obsolete-target
Severity: normal
Certainty: certain
Info: The systemd service file refers to an obsolete target.
 .
 Some targets are obsolete by now, e.g. syslog.target or dbus.target. For
 example, declaring ttAfter=syslog.target/tt is unnecessary by now because
 syslog is socket-activated and will therefore be started when needed.

Tag: systemd-no-service-for-init-script
Severity: serious
Certainty: certain
Info: The listed init.d script has no systemd equivalent.
 .
 Systemd has a SysV init.d script compatibility mode. It provides access to
 each SysV init.d script as long as there is no native service file with the
 same name (e.g. tt/lib/systemd/system/rsyslog.service/tt corresponds to
 tt/etc/init.d/rsyslog/tt).
 .
 Your package ships a service file, but for the listed init.d script, there is
 no corresponding systemd service file.

Tag: init.d-script-does-not-source-init-functions
Severity: normal
Certainty: certain
Info: The tt/etc/init.d/tt script does not source
 tt/lib/lsb/init-functions/tt. The ttsystemd/tt package provides
 tt/lib/lsb/init-functions.d/40-systemd/tt to redirect
 tt/etc/init.d/$script/tt calls to systemctl.
 .
 Please add a line like this to your tt/etc/init.d/tt script:
 .
  . /lib/lsb/init-functions

Tag: maintainer-script-calls-systemctl
Severity: normal
Certainty: certain
Info: The maintainer script calls systemctl directly. Actions such as enabling
 or starting a service have to be done via ttupdate-rc.d/tt or
 ttinvoke-rc.d/tt, respectively, which both do the right thing when systemd
 is installed/running.
# systemd -- lintian check script -*- perl -*-
#
# Copyright © 2013 Michael Stapelberg
#
# based on the apache2 checks file by:
# Copyright © 2012 Arno Töll
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, you can find it on the World Wide
# Web at http://www.gnu.org/copyleft/gpl.html, or write to the Free
# Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
# MA 02110-1301, USA.

package Lintian::systemd;

use strict;
use warnings;

use File::Basename;
use Lintian::Collect::Binary ();
use Lintian::Tags qw(tag);
use Lintian::Relation qw(:constants);
use Lintian::Util qw(fail);
use Data::Dumper;

sub run {
my ($pkg, $type, $info) = @_;

if ($type eq 'binary') {
# Figure out whether the maintainer of this package did any effort to
# make the package work with systemd. If not, we will not warn in case
# of an init script that has no systemd equivalent, for example.
my $ships_systemd_file = (scalar ( grep { m,/systemd/, } 
$info-sorted_index )  0);

# An array of names which are provided by the service files.
# This includes Alias= directives, so after parsing
# NetworkManager.service, it will contain NetworkManager and
# network-manager.
my @systemd_targets;

for my $file ($info-sorted_index) {
if ($file =~ m,^etc/tmpfiles\.d/.*\.conf$,) {
tag('systemd-tmpfiles.d-outside-usr-lib', $file);
}
if ($file =~ m,^etc/systemd/system/.*\.service$,) {
tag('systemd-service-file-outside-lib', $file);
}
if ($file =~ m,/systemd/system/.*\.service$,) {

Bug#704197: Please review: systemd checks

2013-03-29 Thread Niels Thykier

On 2013-03-29 11:11, Michael Stapelberg wrote:
 Package: lintian
 Version: 2.5.10.4
 Severity: wishlist
 
 Attached you can find my first stab at systemd-related checks for
 lintian. While some details in parsing the service files are not
 implemented (see the TODOs in the code), I’d like you to have a look at
 the checks in general. Is there anything that needs to be improved
 before these can be shipped with lintian?
 
 Thanks!
 

Hi,

Thanks for working on Lintian checks, it is much appreciated it.  :)

I have annotated some comments with [style], which are minor stylistic
guidelines.  I know Lintian's code style is a mess in general, so it
describes the style I hope we will eventually reach[1].  :)
  Note that I will try to (remember to) not repeat style hints, even if
the same mistake is made multiple times.


~Niels

[1] When in doubt, I tend to use:
  http://www.eyrie.org/~eagle/notes/style/perl.html

 
 systemd.desc
 
 
 Check-Script: systemd
 Author: Michael Stapelberg stapelb...@debian.org
 Abbrev: systemd

Abrrev is optional and is only useful if the name is shorter than the
name in Check-Script (it is an abbrevation of the name, like manpages
is aliased man).

 Type: binary
 Info: Checks various systemd policy things
 Needs-Info: scripts, index, unpacked, file-info
 
 [... some tags ...]

I noticed that there appear to be no use of references (Ref: URL, #bug,
policy X.Y ...).  I would recommend finding such so people can quickly
find more information.  Links to systemd documentation, specification or
even just a Debian wiki page.

 
 systemd
 
 [...]
 
 use File::Basename;

[style] I like to group Lintian modules separately (and after) external
modules.  That basically gives 3-4 groups; pragmas[, constants],
external modules and then Lintian modules.

 use Lintian::Collect::Binary ();

This import is not needed (in general, Perl do not require you to load
modules just because you use instances of classes from that module).

 use Lintian::Relation qw(:constants);

Does not appear to be used?

 use Data::Dumper;

Left-over from debugging?

 
 sub run {
 my ($pkg, $type, $info) = @_;
 
 if ($type eq 'binary') {

This condition will always be true - the Type:-field determines what
values $type can have.  In this particular case, it is set to binary.

If you do not need $pkg or $type, then you can replace them with undef. E.g.
my (undef, undef, $info) = @_;

That has the advantage that we know that argument is unused.  (As far as
I can tell, $pkg is passed around to various subs but never actually used).

 [...]
 
 for my $file ($info-sorted_index) {
 if ($file =~ m,^etc/tmpfiles\.d/.*\.conf$,) {
 tag('systemd-tmpfiles.d-outside-usr-lib', $file);

[style] The tag sub is a special case in regards to style; it tends to
be treated like a perl built-in or die-like sub (i.e. no parentheses
unless needed for understanding or semantic reasons).  So:

tag 'systemd-tmpfiles.d-outside-usr-lib', $file;

Note if you must use parentheses around tag, a built-in or a die-like
sub, please use the same style as for regular subs (see next comment).

 [...]
 if ($file =~ m,/systemd/system/.*\.service$,) {
 check_systemd_service_file($pkg, $info, $file);

[style] For non-built sub, we tend to have space between the sub name
and the argument parentheses.  For subs that takes no arguments, the
parentheses are omitted where possible.  So:

check_systemd_service_file ($pkg, $info, $file);

  [...]
 my @init_scripts = grep(m,^etc/init\.d/.+,, $info-sorted_index);
 

Erh, I think , might have been a poor choice of regex delimiter here
(as it is also the argument delimiter).  For this you could use :
(among others) or alternatively call grep with a block:

 my @init_scripts = grep {m,^etc/init\.d/.+,} $info-sorted_index;


  [...]
 if ($ships_systemd_file) {
 for my $init_script (@init_scripts) {
 if (grep(/\Q$init_script\E\.service/, @systemd_targets) == 0) 
 {
 tag('systemd-no-service-for-init-script', $init_script);
 }

We sometimes use the reversed form of if/unless to reduce scope level.
 E.g. something like:

tag 'systemd-no-service-for-init-script', $init_script
unless grep /\Q$init_script\E\.service/, @systemd_targets;

There is no hard rule on went; just an FYI that you are free to use it.

  [...]
 
 sub check_init_script {
 my ($pkg, $info, $file) = @_;

$pkg argument does not appear to be used?

 
 my $lsb_source_seen;
 open(my $fh, '', $info-unpacked($file));

No error handling if open fails!  Something as simple as:

   or fail open $file: $!;

is fine.

Secondly, there is no check for file type.  If someone (deliberately)
creates $file as a fifo-pipe or a symlink it will DoS or (possibly) read
host system files (respectively).  Usually, a

$info-index ($file)-is_regular_file

should do (if symlinks/hardlinks can

Bug#704197: Please review: systemd checks

2013-03-29 Thread Michael Stapelberg

Hi Niels,

Thanks for the super-fast review. New version is attached, I have fixed
everything you mentioned, and for the other things I commented inline:

Niels Thykier ni...@thykier.net writes:
 guidelines.  I know Lintian's code style is a mess in general, so it
 describes the style I hope we will eventually reach[1].  :)
Have you tried using perltidy for Lintian? I loathe manual source code
formatting after working with gofmt and subsequently perltidy.

 I noticed that there appear to be no use of references (Ref: URL, #bug,
 policy X.Y ...).  I would recommend finding such so people can quickly
 find more information.  Links to systemd documentation, specification or
 even just a Debian wiki page.
Will do once we’ve put up some wiki pages on that.

 If you do not need $pkg or $type, then you can replace them with undef. E.g.
 my (undef, undef, $info) = @_;
I prefer to have the variables around, just in case the code needs to be
changed to use those.

 That has the advantage that we know that argument is unused.
I don’t understand what the advantage of knowing that is :-).

 Secondly, there is no check for file type.  If someone (deliberately)
 creates $file as a fifo-pipe or a symlink it will DoS or (possibly) read
 host system files (respectively).  Usually, a

 $info-index ($file)-is_regular_file

 should do (if symlinks/hardlinks can be ignored).  Alternatively, (for
 symlinks) please check that the symlink can be safely resolved before
 opening the file (e.g. via the link_resolved method).  For more
 information, please see the Lintian::Path module's API.
I came up with this:

sub check_init_script {
my ($pkg, $info, $file) = @_;

my $lsb_source_seen;
my $path = $info-index ($file);
fail $file is neither a regular file nor a resolvable symlink
unless ($path-is_regular_file || defined($path-link_resolved));
open(my $fh, '', $info-unpacked($file))
or fail cannot open $file: $!;

# …
}

Does that seem alright to you?

 sub split_quoted {
 [...]
 }
 

 Is this something that could be done by Text::ParseWords?
I’m not entirely sure about it. The code I’m using is a 1:1 port of the
corresponding systemd C code. This obviously has the benefit that there
are no subtle differences between what we do and what systemd does.

-- 
Best regards,
Michael


systemd
Description: Binary data


systemd.desc
Description: Binary data

[SCM] Debian package checker branch, master, updated. 2.5.11-196-gf399c98

2013-03-29 Thread Niels Thykier

The following commit has been merged in the master branch:
commit f399c98ee24cfc18afd72e38af032fb10b977a8b
Author: Niels Thykier ni...@thykier.net
Date:   Fri Mar 29 13:14:12 2013 +0100

L::Util: Introduce {,l,r}strip ...

... to replace uses of s/^\s++//; or/and s/\s++$//;.  In void context
they modify their input inplace and in other contexts they copy the
input.  Thus they can replace the chomp like:

  while ( my $line = ) { while () {
 strip ($line);   strip;
 # do something ...   # do something...
  }}

For cases where the original input might be useful, they can also
return a copy:

  while ( my $orig = ) {
 my $stripped = strip ($orig);
  }

Signed-off-by: Niels Thykier ni...@thykier.net

diff --git a/checks/binaries b/checks/binaries
index e2a2bc4..717eec6 100644
--- a/checks/binaries
+++ b/checks/binaries
@@ -28,7 +28,7 @@ use Lintian::Data;
 use Lintian::Relation qw(:constants);
 use Lintian::Tags qw(tag);
 use Lintian::Output qw(debug_msg);
-use Lintian::Util qw(fail slurp_entire_file);
+use Lintian::Util qw(fail slurp_entire_file strip);
 
 use File::Spec;
 
@@ -45,8 +45,7 @@ sub _embedded_libs {
 $regex = $opts;
 $opts = '';
 } else {
-$opts=~ s/^\s++//o;
-$opts=~ s/^\s++//o;
+strip ($opts);
 foreach my $optstr (split m/\s++/, $opts) {
 my ($opt, $val) = split m/=/, $optstr, 2;
 if ($opt eq 'source' or $opt eq 'libname') {
diff --git a/checks/debhelper b/checks/debhelper
index 18451f2..9cd47da 100644
--- a/checks/debhelper
+++ b/checks/debhelper
@@ -25,7 +25,7 @@ use warnings;
 use Lintian::Data;
 use Lintian::Relation;
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail slurp_entire_file);
+use Lintian::Util qw(fail slurp_entire_file strip);
 
 # If compat is less than or equal to this, then a missing version
 # for this level is only a pedantic issue.
@@ -523,16 +523,13 @@ sub _shebang_cmd {
 if (read $fd, $magic, 2) {
 if ($magic eq '#!') {
 $cmd = $fd;
-chomp $cmd;
 
 # It is beyond me why anyone would place a lincity data
 # file here...  but if they do, we will handle it
 # correctly.
 $cmd = '' if $cmd =~ m/^#!/o;
 
-# Strip whitespace if any
-$cmd =~ s/^\s++//o;
-$cmd =~ s/\s++$//o;
+strip ($cmd);
 }
 }
 close $fd;
diff --git a/checks/filename-length b/checks/filename-length
index 28581e3..3d58574 100644
--- a/checks/filename-length
+++ b/checks/filename-length
@@ -23,6 +23,7 @@ use strict;
 use warnings;
 
 use Lintian::Tags qw(tag);
+use Lintian::Util qw(strip);
 
 use constant FILENAME_LENGTH_LIMIT = 80;
 
@@ -72,7 +73,7 @@ $len = 0;
 foreach my $entry (split m/\n/o, $info-field ('files', '')){
 my $filename;
 my $flen;
-$entry =~ s/^\s++//o;
+strip ($entry);
 next unless $entry;
 (undef, undef, $filename) = split m/\s++/o, $entry;
 next unless $filename;
diff --git a/checks/rules b/checks/rules
index 4aad759..91192aa 100644
--- a/checks/rules
+++ b/checks/rules
@@ -19,7 +19,7 @@ use warnings;
 
 use Lintian::Data;
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(fail rstrip);
 
 our $PYTHON_DEPEND = 'python | python-dev | python-all | python-all-dev';
 our $PYTHON3_DEPEND = 'python3 | python3-dev | python3-all | python3-all-dev';
@@ -247,8 +247,7 @@ while (RULES) {
 # we think we know what it will expand to - note
 # we ought to delay it was a = variable rather
 # than := or +=.
-$val =~ s/\s++$//o;
-for (split m/\s++/o, $val) {
+for (split m/\s++/o, rstrip ($val)) {
 $seen{$_}++ if $required{$_};
 $seen{$_}++ if $recommended{$_};
 }
@@ -288,9 +287,8 @@ while (RULES) {
 # we think we know what it will expand to - note
 # we ought to delay it was a = variable rather
 # than := or +=.
-$val =~ s/\s++$//o;
 local $_;
-for (split m/\s++/o, $val) {
+for (split m/\s++/o, rstrip ($val)) {
 $seen{$_}++ if $required{$_};
 $seen{$_}++ if $recommended{$_};
 }
diff --git a/collection/java-info b/collection/java-info
index 14b78d9..cfe4d3a 100755
--- a/collection/java-info
+++ b/collection/java-info
@@ -30,7 +30,7 @@ use FileHandle;
 use lib $ENV{'LINTIAN_ROOT'}/lib/;
 use Lintian::Collect;
 use Lintian::Command qw(spawn reap);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(fail rstrip);
 
 sub collect {
 my ($pkg, $type, $dir) = @_;
@@ -61,7

Bug#704197: Please review: systemd checks

2013-03-29 Thread Russ Allbery

Michael Stapelberg stapelb...@debian.org writes:
 Niels Thykier ni...@thykier.net writes:

 guidelines.  I know Lintian's code style is a mess in general, so it
 describes the style I hope we will eventually reach[1].  :)

 Have you tried using perltidy for Lintian? I loathe manual source code
 formatting after working with gofmt and subsequently perltidy.

I desperately need to update the page that Niels is looking at to
incorporate the results of a very comprehensive style review we did at
Stanford with reference to Perl Best Practices, as it's changed a lot.  We
have a new house style that's similar but incorporates a ton of advice
from Perl Best Practices, and I have both perlcritic and perltidy
configurations that correspond to it (as closely as one can, at least --
perltidy is sadly not very flexible).

Some of those changes are, alas, annoyingly intrusive compared to all of
my previous Perl code (which is a chunk of the code in Lintian); for
example, I was convinced to drop the space before the parens on function
calls because it seems to really confuse people coming from just about any
other language except GNU-style C.

I definitely recommend adopting perlcritic and perltidy for this sort of
thing, though, and now use them as part of an automated test suite for my
Perl code.

For a copy of the standard tests that I'm now using, as well as the
perlcriticrc and perltidyrc files (until I can get everything else
updated), clone:

git://git.eyrie.org/devel/rra-c-util.git

and look at tests/data/perl{critic,tidy}rc and the tests in perl/t.
tests/tap/perl/Test/RRA.pm and tests/tap/perl/test/Config.pm may also be
of interest (particularly the former; the latter is used by my tests to
load configuration so that I can reuse the same test scripts across
multiple packages with different requirements).

I'll send the raw Markdown of our new internal style to the Lintian list.

-- 
Russ Allbery (r...@debian.org)   http://www.eyrie.org/~eagle/


-- 
To UNSUBSCRIBE, email to debian-lint-maint-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/87obe2dsy2@windlord.stanford.edu

Stanford IDG internal Perl style

2013-03-29 Thread Russ Allbery

This is the evolution of my original style document, much modified by
contact with Perl Best Practices.  There are some things here that I'd
personally do differently (I prefer omitting the parens around arguments
to Perl built-ins, for example), but this was the compromise reached
across the group among people with very different styles and different
first languages.

[[!meta title=Perl Coding Style]]

In general, follow the rules in the perlstyle man page and Perl Best
Practices, except we use cuddled elses (else on the same line after }).
And, of course, follow rules below rather than perlstyle if they
conflict.

Note that these guidelines are for internal projects.  If we release
something as open source that needs to be compatible with Perl 5.8
rather than 5.10 (which the document guidelines assump), there are
the following exceptions:

* Do not use autodie.
* 'use base' instead of 'use parent'
* Do not include Stanford::Infrared::*.
* Other things to be added.


# General Guidelines

* Always use strict.

* Always use warnings.

* Always check the results of operations. For many functions this can
be done with 'use autodie' at the start of a script or module.  For
anything not covered by autodie, you should add 'or die some error:
$!\n' to the end of function calls that mail fail.  For print and say,
death checking is is provided by the Stanford::Infrared::Wrappers
module with the functions print_fh, say_fh, print_stdout, and
say_stdout.

* Global variables should be set at the top of the script and have
names in all caps. They must either be declared with my (recommended)
or with Readonly. All other variables should use my to restrict their
scope as appropriate.

* All Perl code should pass perl -wc cleanly.

* Don't use use English; it confuses experienced Perl programmers.

* Place a semicolon after every statement.

* Code in paragraphs, with statements serving one function grouped
together in a block and a comment explaining that block before.

* Factor out long expressions in the middle of statements.  It's better
to read three clear statements than to have them all combined into a
more complicated all-in-one longer line.

* Use perltidy and perlcritic.  Default [perltidyrc](perltidyrc) and
[perlcriticrc](perlcriticrc) files are available.

* On list and hash assignments more than one line, always have a comma
after the last item, and have the closing paren on a new line equal to
the starting line.  This helps if you need to add more items to the
list or hash in the future, or need to re-order the items.

my %server_to_admin = (
paradox  = 'jonrober',
frankoz1 = 'adamhl',
windlord = 'rra',
mysql05  = 'sfeng',
);

* Align items vertically for readability.

my $uname= 'jonrober';
my $fullname = 'Jon Robertson';
my $uid  = 1034;

* Dereference variables with arrows for readability.  (ie:
$record-{name}.

* When you must dereference with a prefix, use braces to make it more
obvious how the variable is being interpreted.  (ie: @{$list_ref}
instead of @$list_ref.

* Only use  or other interpolating string delimiters for strings that
need them.  If you are printing out plain text with no special
characters, having  can make the reader pause to look for
non-existant variables in the string.

* For clarity in some fonts, try to make single-character strings more
obvious as to their intent:

''  q{}
' ' q{ }
',' q{,}

* Use 'local' when you have to modify a package variable, or special
variable such as %SIG, %ENV, or $_.  This will localize any
changes to avoid unpleasant side-effects, and ensure that other things
are not also affecting the value you are using.


# Naming

* Use underscores to separate words in multiword identifiers.

* Modules should have mixed case, normally with each word starting
uppercase and otherwise lowercase.  Constants should be all uppercase,
and other variables should be all lowercase.

* When you have to abbreviate for a sensible variable name, do so by
cutting off the end rather than middle, to make a sensible prefix.
Make certain that the end result is actually unambiguous.  Avoid words
that are inherently ambiguous or form homophones, like 'record'.

* Name modules using Noun::Adjective::Adjective.
(Disk::DVD::Rewritable)

* Name scalar variables as [adjective_]*noun.  ($next_client, $total,
$final_total, $final_office_total).

* Name booleans after their associated test.  ($done_loading,
$found_bad_record, sub is_valid, sub invalid_record)

* Name hashes as %noun_to_noun, describing the mapping of objects
together, when it fits.  (%server_to_admin, %title_to_author)

* Name arrays in the plural.

* Name a scalar reference to hash or array with an ending _ref.

* Name functions meant to be only internal to a module and never used
outside it with an underscore.  This makes it obvious which functions
are meant to be exported or used by an

[SCM] Debian package checker branch, master, updated. 2.5.11-197-g69e0ac5

2013-03-29 Thread Niels Thykier

The following commit has been merged in the master branch:
commit 69e0ac591bbc9d4a380de37d9ea013bf404cf97e
Author: Niels Thykier ni...@thykier.net
Date:   Fri Mar 29 19:04:47 2013 +0100

*: Replace ad-hoc regexes with {,l,r}strip

Signed-off-by: Niels Thykier ni...@thykier.net

diff --git a/checks/control-file b/checks/control-file
index 3081d98..cd00a6f 100644
--- a/checks/control-file
+++ b/checks/control-file
@@ -27,7 +27,8 @@ use List::Util qw(first);
 use Lintian::Data ();
 use Lintian::Relation ();
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail file_is_encoded_in_non_utf8 read_dpkg_control);
+use Lintian::Util qw(fail file_is_encoded_in_non_utf8 read_dpkg_control
+ rstrip strip);
 
 # The list of libc packages, used for checking for a hard-coded dependency
 # rather than using ${shlibs:Depends}.
@@ -272,8 +273,7 @@ for my $i (0 .. $#descriptions) {
 # version.
 sub check_dev_depends {
 my ($info, $package, $depends, @packages) = @_;
-$depends =~ s/^\s+//;
-$depends =~ s/\s+$//;
+strip ($depends);
 for my $target (@packages) {
 next unless ($target =~ /^lib[\w.+-]+\d/
  and $target !~ /-(?:dev|docs?|common)$/);
@@ -343,7 +343,7 @@ sub check_relation {
)/x) {
 my ($prev, $next) = ($1, $2);
 for ($prev, $next) {
-s/\s+$//;
+rstrip;
 }
 tag 'missing-separator-between-items', 'in', $pkg,
 $field field between '$prev' and '$next';
diff --git a/checks/debhelper b/checks/debhelper
index 9cd47da..ea54703 100644
--- a/checks/debhelper
+++ b/checks/debhelper
@@ -25,7 +25,7 @@ use warnings;
 use Lintian::Data;
 use Lintian::Relation;
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail slurp_entire_file strip);
+use Lintian::Util qw(fail slurp_entire_file strip strip);
 
 # If compat is less than or equal to this, then a missing version
 # for this level is only a pedantic issue.
@@ -240,10 +240,8 @@ my $compatnan = 0;
 if (-f $droot/compat) {
 my $compat_file = slurp_entire_file($droot/compat);
 ($compat) = split(/\n/, $compat_file);
-$compat =~ s/^\s+$//;
-if ($compat) {
-$compat =~ s/^\s+//;
-$compat =~ s/\s+$//;
+strip ($compat);
+if ($compat ne '') {
 if ($compat !~ m/^\d+$/) {
 tag 'debhelper-compat-not-a-number', $compat;
 $compat =~ s/[^\d]//g;
diff --git a/checks/menus b/checks/menus
index 6e4c6ef..0006d50 100644
--- a/checks/menus
+++ b/checks/menus
@@ -27,7 +27,7 @@ use warnings;
 use Lintian::Check qw(check_spelling check_spelling_picky $known_shells_regex);
 use Lintian::Data;
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail file_is_encoded_in_non_utf8);
+use Lintian::Util qw(fail file_is_encoded_in_non_utf8 strip);
 
 # Supported documentation formats for doc-base files.
 our %known_doc_base_formats = map { $_ = 1 }
@@ -371,8 +371,7 @@ sub check_doc_base_field {
 # Format field.
 } elsif ($field eq 'format') {
 my $format = join (' ', @$vals);
-$format =~ s/^\s+//o;
-$format =~ s/\s+$//o;
+strip ($format);
 $format = lc $format;
 tag 'doc-base-file-unknown-format', $dbfile:$line, $format
 unless $known_doc_base_formats{$format};
diff --git a/checks/patch-systems b/checks/patch-systems
index 0b740fc..01f462a 100644
--- a/checks/patch-systems
+++ b/checks/patch-systems
@@ -24,7 +24,7 @@ use strict;
 use warnings;
 
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(fail strip);
 use Cwd qw(realpath);
 
 sub run {
@@ -124,7 +124,7 @@ sub run {
 my @patches;
 my @badopts;
 while(IN) {
-chomp; s/^\s+//; s/\s+$//; # Strip leading/trailing spaces
+strip; # Strip leading/trailing spaces
 s/(^|\s+)#.*$//; # Strip comment
 next unless $_;
 if (/^(\S+)\s+(\S.*)$/) {
diff --git a/checks/scripts b/checks/scripts
index 1559a45..da8d5fb 100644
--- a/checks/scripts
+++ b/checks/scripts
@@ -30,7 +30,7 @@ use Lintian::Check qw($known_shells_regex);
 use Lintian::Data;
 use Lintian::Relation;
 use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(fail strip);
 
 # This is a map of all known interpreters.  The key is the interpreter
 # name (the binary invoked on the #! line).  The value is an anonymous
@@ -859,8 +859,7 @@ while (SCRIPTS) {
 $divert =~ s,^/,,;
 
 # remove any remaining leading or trailing whitespace.
-$divert =~ s/^\s+//;
-$divert =~ s/\s+$//;
+strip ($divert);
 
 $divert = quotemeta($divert);
 
diff --git a/collection/scripts b/collection/scripts
index 2136991..859b752 100755
--- a/collection/scripts
+++ b/collection/scripts
@@ -26,7 +26,7 @@ use warnings;
 
 use lib $ENV{'LINTIAN_ROOT'}/lib/;
 use Lintian::Collect;

Re: Stanford IDG internal Perl style

2013-03-29 Thread Niels Thykier

On 2013-03-29 18:24, Russ Allbery wrote:
 This is the evolution of my original style document, much modified by
 contact with Perl Best Practices.  There are some things here that I'd
 personally do differently (I prefer omitting the parens around arguments
 to Perl built-ins, for example), but this was the compromise reached
 across the group among people with very different styles and different
 first languages.
 

Great :)  I was just considering whether to codify a Lintian style guide.

 [[!meta title=Perl Coding Style]]
 
 In general, follow the rules in the perlstyle man page and Perl Best
 Practices, except we use cuddled elses (else on the same line after }).
 And, of course, follow rules below rather than perlstyle if they
 conflict.
 

I don't think I have read the Perl Best Practises (assuming its the
O'Reilly one[1]), so I have to come back on that.

[1] http://refcards.com/docs/vromansj/perl-best-practices/refguide.pdf

 Note that these guidelines are for internal projects.  If we release
 something as open source that needs to be compatible with Perl 5.8
 rather than 5.10 (which the document guidelines assump), there are
 the following exceptions:
 

s/assump/assume/ ?

 * Do not use autodie.
 * 'use base' instead of 'use parent'
 * Do not include Stanford::Infrared::*.
 * Other things to be added.
 
 
 # General Guidelines
 
 [...]
 
 * Always check the results of operations. For many functions this can
 be done with 'use autodie' at the start of a script or module.  For
 anything not covered by autodie, you should add 'or die some error:
 $!\n' to the end of function calls that mail fail.  For print and say,
 death checking is is provided by the Stanford::Infrared::Wrappers
 module with the functions print_fh, say_fh, print_stdout, and
 say_stdout.
 

s/is is/is/;

I have been considering to use autodie before (possible exception being
system/exec where it is always not compatible).  I tend to prefer
exception based I/O instead of checking return values (which gets old
really soon), so I could be convinced to use autodie by default.  Too
bad it does not cover all cases (print et al).

 [...]
 
 * Factor out long expressions in the middle of statements.  It's better
 to read three clear statements than to have them all combined into a
 more complicated all-in-one longer line.
 

Is this:

   my $result = part1
  + part2
  + part3;

 vs:

  my $result = part1 + part2 + part3;

(where partX is a complex computation)  Or is it about splitting it
into temporary variables?

 * Use perltidy and perlcritic.  Default [perltidyrc](perltidyrc) and
 [perlcriticrc](perlcriticrc) files are available.
 

If we can codify our style in a fashion these can check for that would
definitely help in maintaining the style.  Also, we have a perlcritic
check already but it is skipped by default (and I never remember to set
the proper variables to enable it... .)

 [...]
 
 * Dereference variables with arrows for readability.  (ie:
 $record-{name}.
 

Also in chaining?
  $record-{$last_name}-{$first_name}
  $matrix-[$i]-[$j]

vs

  $record-{$last_name}{$first_name}
  $matrix-[$i][$j]


 [...]
 * For clarity in some fonts, try to make single-character strings more
 obvious as to their intent:
 
 ''  q{}
 ' ' q{ }
 ',' q{,}
 

I do not think I understand this one.  Is it to use the second column
variants instead of the first?  I don't see how any of those expresses
their intent more than the other (but I might be a missing a reference
to the Perl Best Practises here?).

 [...]
 
 # Naming
 
 [...]
 
 * Name modules using Noun::Adjective::Adjective.
 (Disk::DVD::Rewritable)
 

Mmm... Is that strictly one Noun followed by adjective(s).  Also the
example appear to be Noun::Noun::Adjective (or is a DVD an
adjective?).  Anyway I assume namespacing is allowed (e.g.
Lintian::$module)

 [...]
 
 # Formatting and Indentation
 
 Don't use tabs in perl scripts, since they can expand differently in
 different environments. In particular, please try not to use the mix of
 tabs and spaces that is the default in Emacs.
 
 Please follow these guidelines for spacing and formatting:
 
 * Each block is indented four spaces. Continuations of commands should
 be indented two more spaces unless parenthesized, or indented to line
 up one space after the opening parentheses if parenthesized.
 

So if parentheses are used it should be lined up as... ?

  really_long_name(argument1,
argument2,
argument3)

  if (really_long_condition1
really_long_condition2
really_long_condition3) {

Or does this rule not apply to sub arguments?

 * Lines should be 79 characters or less. Continue long lines on the
 next line. Continue long strings by breaking the string before a space
 and using string concatenation (.) to combine shorter strings.
 
 * Use a space between keywords (if, elsif, while) and functions and
 parenthesized arguments. Do not put a space

Bug#612610: lintian: should suggest switching to 3.0 (quilt)

Bug#704197: Please review: systemd checks

Bug#704197: Please review: systemd checks

Bug#704197: Please review: systemd checks

[SCM] Debian package checker branch, master, updated. 2.5.11-196-gf399c98

Bug#704197: Please review: systemd checks

Stanford IDG internal Perl style

[SCM] Debian package checker branch, master, updated. 2.5.11-197-g69e0ac5

Re: Stanford IDG internal Perl style

9 matches

Site Navigation

Mail list logo

Footer information