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

Bug#722084: lintian: [checks] Add Ruby pkg team checks to lintian



On 2013-09-07 19:00, Jonas Genannt wrote:
> Package: lintian
> Version: 2.5.17
> Severity: wishlist
> Tags: patch
> 
> Dear Lintian-Maintainer,
> 

Hi Jonas,

Thanks for your interest in supplying Lintian checks for Ruby packages.

> we the Debian Ruby Team would like to add some lintian checks for the Ruby packages.
> 
> Please see attched patch.
> 
> Acknowledgment of Ruby pkg team:
> 	https://lists.debian.org/debian-ruby/2013/09/msg00011.html
> 
> Would you please include that checks into the next release of Lintian?
> 

Indeed, if they are generally useful to all ruby packages (i.e. even for
packages maintained by people not in the pkg-ruby team).  Though, for
now I am trying to keep "team specific" checks separate.  It looks like
the checks you provided are of the "former" (despite $SUBJECT), so they
should be mostly okay.  Mind you, I am not sure that they proposed
severities can be retained.


I got a couple remarks about the check (interleaved in the patch below).
 Also, a little bonus trivia.  Did you know you can have non-standard
checks loaded via "--include-dir"?  E.g. if you change the layout of the
"ruby-lintian" git repository, so the checks are in a directory called
"checks".

  $RUBY_LINTIAN_CHECKOUT/
    checks/local
        my-check.desc
        my-check.pm
    profiles/local/main.profile

Then:

  lintian --include-dir $RUBY_LINTIAN_CHECKOUT --profile local <pkg>

will DTRT.  I find that very useful for development and testing
non-standard checks. :)

> Thanks,
> 	Jonas
> 
> 
> 
> 0001-added-Ruby-pkg-team-checks.patch
> 
> 
>>From d7a2bb97de6be5f125cbbd7d72ada75e95fa53be Mon Sep 17 00:00:00 2001
> From: Jonas Genannt <jonas@brachium-system.net>
> Date: Sat, 7 Sep 2013 18:52:44 +0200
> Subject: [PATCH] added Ruby pkg team checks
> 
> ---
>  checks/ruby.desc             |   36 ++++++++++++++++++++++++++++++++++++
>  checks/ruby.pm               |   39 +++++++++++++++++++++++++++++++++++++++
>  profiles/debian/main.profile |    2 +-
>  3 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100644 checks/ruby.desc
>  create mode 100644 checks/ruby.pm
> 
> diff --git a/checks/ruby.desc b/checks/ruby.desc
> new file mode 100644
> index 0000000..f635090
> --- /dev/null
> +++ b/checks/ruby.desc
> @@ -0,0 +1,36 @@
> +Check-Script: ruby
> +Author: Jonas Genannt <jonas.genannt@capi2name.de>
> +Type: binary
> +Info: Checks various Ruby PKG Team stuff

>From what I can tell, these checks are not specific to pkg-ruby, so the
description could use a minor fixup.

> +Needs-Info: bin-pkg-control, index, scripts, unpacked

I think these are all pretty much unused and can be dropped.  At least,
I cannot find anything requiring those at first glance.

E.g. $info->field and $info->relation have no requirements. :)

> +
> +Tag: ruby-depends-on-ruby1.8
> +Severity: serious
> +Certainty: certain
> +Info: This package depends on ruby1.8, but ruby 1.8 is at its end of life.
> + Please check your package against newer Ruby versions and update your
> + dependencies to "ruby | ruby-interpreter", or in the worst case, to a newer
> + Ruby version.
> + .
> + https://wiki.debian.org/Teams/Ruby/Packaging
> +

This particular tag looks like a case of depends-on-obsolete-package and
its sister tag, build-depends-on-obsolete-package.  Would you be okay
with us just adding ruby1.8 to data/fields/obsolete-packages?

> +Tag: ruby-depends-on-specific-ruby-version
> +Severity: normal
> +Certainty: certain
> +Info: This package depends on a specific ruby version.
> + Packages should not depend on a specific ruby version if possible, because
> + this makes it harder to drop support for obsolete Ruby versions. Ideally
> + packages should declare a dependency on Ruby using a generic
> + "ruby | ruby-interpreter".
> + .
> + https://wiki.debian.org/Teams/Ruby/Packaging
> +
> +Tag: ruby-library-old-package-name-schema
> +Severity: serious

I am not quite sure we can retain this severity; dropping it to normal
(W) might be more reasonable.

> +Certainty: certain
> +Info: Your ruby library package name does not fit the Debian standards for Ruby.
> + The package name does not fit the current naming scheme for Ruby packages.
> + Ruby libraries should named like `ruby-foo`. Packages named libfoo-ruby* are
> + deprecated.
> + .
> + https://wiki.debian.org/Teams/Ruby/Packaging
> diff --git a/checks/ruby.pm b/checks/ruby.pm
> new file mode 100644
> index 0000000..89f1987
> --- /dev/null
> +++ b/checks/ruby.pm
> @@ -0,0 +1,39 @@
> +package Lintian::ruby;
> +

Please insert a license header (like the one we have in all checks).

> +use strict;
> +use warnings;
> +
> +use Lintian::Collect::Binary ();

No need for loading L::C::Binary.

> +use Lintian::Tags qw(tag);
> +
> +sub run {
> +    my $pkg = shift;
> +    my $type = shift;
> +    my $info = shift;

Replace with:
       my ($pkg, undef, $info) = @_;

(the $type argument is not used in this check, see below).

> +
> +    if ($type eq 'binary') {

Always true (due to the Type field from the .desc file).  This kind of
check only makes sense, if the "Type"-field contains multiple types.  E.g.

Type: binary, udeb

> +
> +        my $pkg_description = $info->field('description');
> +        if ($pkg_description) {

Why do you check if the description is present?  While the description
may be missing, I am pretty sure that "is_pkg_class" will correctly
handle the absence of the description.

> +          if ($pkg =~ /^lib.*-ruby.*/ and !$info->is_pkg_class('transitional') ) {
                                     ^^
The marked ".*" is redundant (as the regex is not anchored at the end),
so it can be simplified to /^lib.*-ruby/.  Alternatively, it can be
rewritten as /^lib.*-ruby.*$/, but they will (still) match the same thing.

> +            tag 'ruby-library-old-package-name-schema';
> +          }
> +        }
> +
> +        my $str_deps = $info->relation('strong');

NB: $info->relation always returns a defined object, so the "boolean"
check of $str_deps will always be true.  If the underlying fields are
missing, it will return an "empty-relation" object.

> +        if ($str_deps and $str_deps->implies("ruby1.8")) {
> +            tag 'ruby-depends-on-ruby1.8';
> +        }
> +
> +        if ($str_deps and $str_deps->matches(qr/^ruby[0-9.]+$/o) and !$str_deps->implies("ruby1.8")) {

TBH, I would probably just emit this tag even if "ruby1.8" is the
trigger (since depending on ruby1.8 is technically a "specific" ruby
version).  In particular, if ruby-depends-on-ruby1.8 is replaced by the
existing obsolete-package tags, then it makes even more sense (due to
#686553).

> +            tag 'ruby-depends-on-specific-ruby-version';
> +        }
> +    }
> +}
> +
> +1;
> +# Local Variables:
> +# indent-tabs-mode: nil
> +# cperl-indent-level: 4
> +# End:
> +# vim: syntax=perl sw=4 sts=4 sr et
> [...]
>  
> -- 1.7.10.4
> 

With these minor things dealt with, I think we just need tests for the tags.

~Niels


Reply to: