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

Bug#813904: lintian: Check if upstream/metadata files are well formed



Hi Petter!

IANA Lintian expert, but here's my review:

* Petter Reinholdtsen <pere@hungry.com>, 2016-02-06, 15:59:
+Needs-Info: unpacked, diffstat, file-info, md5sums

AFAICS, you only need "unpacked" for this check.

+Info: This script checks the <tt>upstream/metadata</tt> file for problems.

<tt>...</tt> is not strictly necessary. The only consumer of Info from the first stanza is private/generate-lintian-pod, which already knows that strings containing slashes are filenames, and highlights them the same way as <tt>-ed strings.

+Tag: upstream-metadata-yaml-invalid
+Severity: normal
+Certainty: certain
+Info: The DEP 12 metadata file is not well formed.  The formatting
+ need to be adjusted to match the YAML specification.

I'd add "Ref"s to DEP-12 and to the YAML specification here.

+use YAML;

Linian already uses YAML::XS in lib/Lintian/Util.pm, so I wonder if we should use this instead. (See bug #784639 for rationale for using this particular module.)

+    my $droot = $info->index_resolved_path('debian/');
+    return if not $droot;
+    my $dudir = $droot->resolve_path('upstream');
+    return if not $dudir;
+    my $yamlfile = $dudir->resolve_path('metadata') if $dudir;

This makes perlcritic[0] upset:

#   checks/upstream-metadata.pm:38:5:Variable declared in conditional statement

You could probably replace the whole 5 lines with:

my $yamlfile = $info->index_resolved_path('debian/upstream/metadata');
return if not $yamlfile;

+    my $path = $yamlfile->fs_path();

fs_path() can fail in certain circumstances (e.g. if the object is a broken symlink), so you should call is_open_ok() first...

+    if ( -f $path ) {

...and then this check won't be needed.

+        eval '$yaml = YAML::LoadFile($path);';

String eval is evil. Please make it:

eval { $yaml = YAML::LoadFile($path); };

+        if (! $yaml) {

There shouldn't be space after "!", according to perltidy.


[0] Try running:
prove -l t/scripts/01-critic/checks.t

--
Jakub Wilk


Reply to: