17061 pkglint -L output could be improved
17072 pkglint print_fmri check is insufficient for debugging
17086 pkglint -c has different behavior with relative and absolute paths
--- a/src/man/pkglint.1.txt Tue Sep 28 14:54:22 2010 -0700
+++ b/src/man/pkglint.1.txt Wed Sep 29 11:55:47 2010 +1300
@@ -1,4 +1,4 @@
-
+
User Commands pkglint(1)
@@ -10,9 +10,9 @@
SYNOPSIS
/usr/bin/pkglint [ -c dir ] [ -r uri ] [ -p regexp ]
- [ -f rcfile ] [ -b build_no ]
+ [ -f rcfile ] [ -b build_no ] [ -v ]
[ -l uri ] | manifest ...
- /usr/bin/pkglint -L
+ /usr/bin/pkglint -L [ -v ]
DESCRIPTION
@@ -60,7 +60,7 @@
-SunOS 5.11 Last change: 20 Aug 2010 1
+SunOS 5.11 Last change: 27 Sep 2010 1
@@ -100,7 +100,11 @@
-L List the known and excluded lint
- checks, then exit.
+ checks, showing the short name of each
+ check, and its description then exit.
+ When combined with the -v flag, the
+ method that implements the check is
+ printed instead of the description.
-f config_file Configure the pkglint session using the
@@ -120,13 +124,9 @@
reference repository.
- -v Run the tool in a verbose mode, over-
- riding any log_level settings in the
- config file. This option also causes
-
-SunOS 5.11 Last change: 20 Aug 2010 2
+SunOS 5.11 Last change: 27 Sep 2010 2
@@ -137,8 +137,9 @@
- progress information on the lint run to
- be printed when using repositories.
+ -v Run the tool in a verbose mode, over-
+ riding any log_level settings in the
+ config file.
--help or -? Displays a usage message.
@@ -191,8 +192,7 @@
-
-SunOS 5.11 Last change: 20 Aug 2010 3
+SunOS 5.11 Last change: 27 Sep 2010 3
@@ -215,7 +215,9 @@
cial keyword argument, "pkglint_id" are invoked during the
course of the lint session. Those methods should have the
same signature as the corresponding check(..) method in the
- super class.
+ super class. Methods should also be assigned a
+ "pkglint_desc" attribute, which is used as the description,
+ printed by pkglint -L.
EXAMPLES
Example 1: Running a pkglint session for the first time.
@@ -254,11 +256,9 @@
pkglint.ext.mycheck = org.timf.mychecks
pkglint.ext.opensolaris = pkg.lint.opensolaris
- pkglint.exclude: pkg.lint.opensolaris.OpenSolarisActionChecker pkg.lint.pkglint.PkgActionChecker.unusual_perms
-
-SunOS 5.11 Last change: 20 Aug 2010 4
+SunOS 5.11 Last change: 27 Sep 2010 4
@@ -269,6 +269,7 @@
+ pkglint.exclude: pkg.lint.opensolaris.OpenSolarisActionChecker pkg.lint.pkglint.PkgActionChecker.unusual_perms
pkg.lint.pkglint.PkgManifestChecker pkg.lint.opensolaris.OpenSolarisManifestChecker
@@ -320,8 +321,7 @@
-
-SunOS 5.11 Last change: 20 Aug 2010 5
+SunOS 5.11 Last change: 27 Sep 2010 5
--- a/src/modules/lint/engine.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/modules/lint/engine.py Wed Sep 29 11:55:47 2010 +1300
@@ -303,7 +303,7 @@
"reference or lint repositories."))
if cache:
- self.basedir = cache
+ self.basedir = os.path.abspath(cache)
try:
self.lint_image = os.path.join(self.basedir,
@@ -635,6 +635,9 @@
self.info("Not checking linted manifest %s" %
manifest.fmri, msgid="pkglint001.1")
return
+
+ self.debug(_("Checking %s") % manifest.fmri, "pkglint001.3")
+
for checker in manifest_checks:
try:
checker.check(manifest, self)
--- a/src/modules/lint/opensolaris.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/modules/lint/opensolaris.py Wed Sep 29 11:55:47 2010 +1300
@@ -71,6 +71,8 @@
"pkg": manifest.fmri},
msgid="%s%s.3" % (self.name, pkglint_id))
+ username_format.pkglint_desc = _("User names should be valid.")
+
class OpenSolarisManifestChecker(base.ManifestChecker):
"""An opensolaris.org-specific class to check manifests."""
@@ -140,15 +142,8 @@
manifest.fmri,
msgid="%s%s.2" % (self.name, pkglint_id))
- def print_fmri(self, manifest, engine, pkglint_id="002"):
- """For now, this is simply a convenient way to output
- the FMRI being checked. We pkglint.exclude this check
- by default."""
- # Using the python logger attached to the engine rather than
- # emitting a lint message - this message is purely informational
- # and isn't a source of lint messages (which would otherwise
- # cause a non-zero exit from pkglint)
- engine.logger.info(" %s" % manifest.fmri)
+ missing_attrs.pkglint_desc = _(
+ "Standard package attributes should be present.")
def info_classification(self, manifest, engine, pkglint_id="003"):
"""Checks that the info.classification attribute is valid."""
@@ -178,6 +173,9 @@
self._check_info_classification_value(engine, value,
manifest.fmri, "%s%s" % (self.name, pkglint_id))
+ info_classification.pkglint_desc = _(
+ "info.classification attribute should be valid.")
+
def _check_info_classification_value(self, engine, value, fmri, msgid):
prefix = "org.opensolaris.category.2008:"
--- a/src/modules/lint/pkglint_action.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/modules/lint/pkglint_action.py Wed Sep 29 11:55:47 2010 +1300
@@ -205,12 +205,17 @@
self.dup_attr_check(["file", "license"], "path", self.ref_paths,
self.processed_paths, action, engine, msgid=pkglint_id)
+ duplicate_paths.pkglint_desc = _(
+ "Paths should be unique.")
+
def duplicate_drivers(self, action, manifest, engine, pkglint_id="002"):
"""Checks for duplicate driver names."""
self.dup_attr_check(["driver"], "name", self.ref_drivers,
self.processed_drivers, action, engine, msgid=pkglint_id)
+ duplicate_drivers.pkglint_desc = _("Driver names should be unique.")
+
def duplicate_usernames(self, action, manifest, engine,
pkglint_id="003"):
"""Checks for duplicate user names."""
@@ -218,12 +223,16 @@
self.dup_attr_check(["user"], "username", self.ref_usernames,
self.processed_usernames, action, engine, msgid=pkglint_id)
+ duplicate_usernames.pkglint_desc = _("User names should be unique.")
+
def duplicate_uids(self, action, manifest, engine, pkglint_id="004"):
"""Checks for duplicate uids."""
self.dup_attr_check(["user"], "uid", self.ref_uids,
self.processed_uids, action, engine, msgid=pkglint_id)
+ duplicate_uids.pkglint_desc = _("UIDs should be unique.")
+
def duplicate_groupnames(self, action, manifest, engine,
pkglint_id="005"):
"""Checks for duplicate group names."""
@@ -231,12 +240,17 @@
self.dup_attr_check(["group"], "groupname", self.ref_groupnames,
self.processed_groupnames, action, engine, msgid=pkglint_id)
+ duplicate_groupnames.pkglint_desc = _(
+ "Group names should be unique.")
+
def duplicate_gids(self, action, manifest, engine, pkglint_id="006"):
"""Checks for duplicate gids."""
self.dup_attr_check(["group"], "name", self.ref_gids,
self.processed_gids, action, engine, msgid=pkglint_id)
+ duplicate_gids.pkglint_desc = _("GIDs should be unique.")
+
def duplicate_refcount_path_attrs(self, action, manifest, engine,
pkglint_id="007"):
"""Checks that for duplicated reference-counted actions,
@@ -302,6 +316,9 @@
msgid="%s%s" % (self.name, pkglint_id))
self.processed_refcount_paths[p] = True
+ duplicate_refcount_path_attrs.pkglint_desc = _(
+ "Duplicated reference counted actions should have the same attrs.")
+
def dup_attr_check(self, action_names, attr_name, ref_dic,
processed_dic, action, engine, msgid=""):
"""This method does generic duplicate action checking where
@@ -402,6 +419,9 @@
msgid="%s%s" % (self.name, pkglint_id))
self.seen_dup_types[p] = True
+ duplicate_path_types.pkglint_desc = _(
+ "Paths should be delivered by one action type only.")
+
def _merge_dict(self, src, target):
"""Merges the given src dictionary into the target
dictionary, giving us the target content as it would appear,
@@ -514,6 +534,9 @@
"fmri": manifest.fmri},
msgid="%s%s" % (self.name, pkglint_id))
+ underscores.pkglint_desc = _(
+ "Underscores are discouraged in action attributes.")
+
def unusual_perms(self, action, manifest, engine, pkglint_id="002"):
"""Checks that the permissions in this action look sane."""
@@ -565,6 +588,9 @@
"pkg": manifest.fmri},
msgid="%s%s.2" % (self.name, pkglint_id))
+ unusual_perms.pkglint_desc = _(
+ "UNIX file modes should be sensible.")
+
def legacy(self, action, manifest, engine, pkglint_id="003"):
"""Cross-check that the 'pkg' attribute points to a package
that depends on the package containing this legacy action.
@@ -628,6 +654,9 @@
"contain a REV= string") % manifest.fmri,
msgid="%s%s.3" % (self.name, pkglint_id))
+ legacy.pkglint_desc = _(
+ "'legacy' actions should have valid attributes.")
+
def unknown(self, action, manifest, engine, pkglint_id="004"):
"""We should never have actions called 'unknown'."""
@@ -636,6 +665,8 @@
manifest.fmri,
msgid="%s%s" % (self.name, pkglint_id))
+ unknown.pkglint_desc = _("'unknown' actions should never occur.")
+
def license(self, action, manifest, engine, pkglint_id="005"):
"""License actions should not have path attributes."""
@@ -647,6 +678,8 @@
"path": action.attrs["path"]},
msgid="%s%s" % (self.name, pkglint_id))
+ license.pkglint_desc = _("'license' actions should not have paths.")
+
def dep_obsolete(self, action, manifest, engine, pkglint_id="005"):
"""We should not have a require dependency on a package that has
been marked as obsolete.
@@ -726,6 +759,9 @@
"pkg": manifest.fmri},
msgid="%s.1" % lint_id)
+ dep_obsolete.pkglint_desc = _(
+ "Packages should not have dependencies on obsolete packages.")
+
def valid_fmri(self, action, manifest, engine, pkglint_id="006"):
"""We should be given a valid FMRI as a dependency, allowing
for a potentially missing component value"""
@@ -747,3 +783,4 @@
"action": action},
msgid="%s%s" % (self.name, pkglint_id))
+ valid_fmri.pkglint_desc = _("pkg(5) FMRIs should be valid.")
--- a/src/modules/lint/pkglint_manifest.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/modules/lint/pkglint_manifest.py Wed Sep 29 11:55:47 2010 +1300
@@ -107,6 +107,9 @@
"set or signature actions") % manifest.fmri,
msgid="%s%s.2" % (self.name, pkglint_id))
+ obsoletion.pkglint_desc = _(
+ "Obsolete packages should have valid contents.")
+
def renames(self, manifest, engine, pkglint_id="002"):
"""Checks for correct package renaming.
* error if renamed packages contain anything other than set,
@@ -126,12 +129,13 @@
manifest.fmri, msgid="%s%s" %
(self.name, pkglint_id))
+ renames.pkglint_desc = _("Renamed packages should have valid contents.")
def variants(self, manifest, engine, pkglint_id="003"):
"""Checks for correct use of variant tags.
* if variant tags present, matching variant descriptions
exist and are correctly specified
- * All manifests that deliver file actions of a given\
+ * All manifests that deliver file actions of a given
architecture declare variant.arch
These checks are only performed on published packages."""
@@ -181,6 +185,8 @@
manifest.fmri,
msgid="%s%s.3" % (self.name, pkglint_id))
+ variants.pkglint_desc = _("Variants used by packages should be valid.")
+
def naming(self, manifest, engine, pkglint_id="004"):
"""Warn when there's a namespace clash where the last component
of the pkg name matches an existing one in the catalog."""
@@ -204,6 +210,9 @@
self.processed_lastnames.append(lastname)
+ naming.pkglint_desc = _(
+ "Packages are encouraged to use unique leaf names.")
+
def duplicate_deps(self, manifest, engine, pkglint_id="005"):
"""Checks for repeated dependencies, including package version
substrings."""
@@ -244,6 +253,9 @@
"actions": " ".join([str(d) for d in duplicates])},
msgid="%s%s.2" % (self.name, pkglint_id))
+ duplicate_deps.pkglint_desc = _(
+ "Packages should not have duplicate 'depend' actions.")
+
def duplicate_sets(self, manifest, engine, pkglint_id="006"):
"""Checks for duplicate set actions."""
seen_sets = {}
@@ -271,6 +283,9 @@
"pkg": manifest.fmri},
msgid="%s%s" % (self.name, pkglint_id))
+ duplicate_sets.pkglint_desc = _(
+ "Packages should not have duplicate 'set' actions.")
+
def _merge_names(self, src, target):
"""Merges the given src list into the target list"""
--- a/src/tests/api/t_pkglint.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/tests/api/t_pkglint.py Wed Sep 29 11:55:47 2010 +1300
@@ -1382,6 +1382,26 @@
"manifests that should contain no errors: %s %s" %
(basename, "\n".join(lint_msgs)))
+ def test_relative_path(self):
+ """The engine can start with a relative path to its cache."""
+ lint_logger = TestLogFormatter()
+ lint_engine = engine.LintEngine(lint_logger,
+ use_tracker=False,
+ config_file=os.path.join(self.test_root,
+ "pkglintrc"))
+
+ lint_engine.setup(cache=self.cache_dir,
+ lint_uris=[self.ref_uri])
+
+ lint_engine.execute()
+ lint_engine.teardown()
+
+ relative = os.path.join("..", os.path.basename(self.cache_dir))
+ cache = os.path.join(self.cache_dir, relative)
+ lint_engine.setup(cache=cache)
+ lint_engine.execute()
+ lint_engine.teardown(clear_cache=True)
+
def read_manifests(names, lint_logger):
"Read a list of filenames, return a list of Manifest objects"
manifests = []
--- a/src/tests/cli/t_pkglint.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/tests/cli/t_pkglint.py Wed Sep 29 11:55:47 2010 +1300
@@ -107,11 +107,23 @@
ret, output, err = self.pkglint(opt, exit=2)
def test_3_list(self):
- """Tests that a given checker method is listed with -L."""
- ret, output, err = self.pkglint("-L")
- self.assert_(
- "pkg.lint.pkglint_action.PkgDupActionChecker.duplicate_paths"
- in output, "List output didn't show expected checker")
+ """Tests that -L prints headers and descriptions or methods."""
+ for flag in ["-L", "-vL"]:
+ ret, output, err = self.pkglint(flag)
+ self.assert_("pkglint.dupaction001" in output,
+ "short name didn't appear in %s output" % flag)
+
+ self.assert_("NAME" in output,
+ "Header not printed in %s output" % flag)
+ if flag == "-vL":
+ self.assert_(
+ "pkg.lint.pkglint_action.PkgDupActionChecker.duplicate_paths"
+ in output,
+ "verbose list output didn't show method")
+ elif flag == "-L":
+ self.assert_(
+ "Paths should be unique." in output,
+ "description didn't appear in -L output: %s" % output)
def test_4_manifests(self):
"""Tests that we exit normally with a correct manifest."""
@@ -146,14 +158,14 @@
self.pkglint("-f %s/rcfile1 %s" % (self.test_root, mpath1),
exit=0)
- ret, output, err = self.pkglint("-f %s/rcfile -L" %
+ ret, output, err = self.pkglint("-f %s/rcfile -vL" %
self.test_root, testrc=False, exit=0)
self.assert_(
"pkg.lint.pkglint_manifest.PkgManifestChecker.duplicate_sets"
in output, "List output missing excluded checker")
self.assert_("Excluded checks:" in output)
- ret, output, err = self.pkglint("-f %s/rcfile1 -L" %
+ ret, output, err = self.pkglint("-f %s/rcfile1 -vL" %
self.test_root, testrc=False, exit=0)
self.assert_(
"pkg.lint.pkglint_manifest.PkgManifestChecker.duplicate_sets"
--- a/src/util/pkglintrc Tue Sep 28 14:54:22 2010 -0700
+++ b/src/util/pkglintrc Wed Sep 29 11:55:47 2010 +1300
@@ -20,7 +20,7 @@
# List modules or methods which should be excluded from
# execution during each lint run.
-pkglint.exclude = pkg.lint.opensolaris.OpenSolarisManifestChecker.print_fmri
+pkglint.exclude =
pkglint.ext.opensolaris = pkg.lint.opensolaris
# The version pattern we use when searching for manifests
--- a/src/util/publish/pkglint.py Tue Sep 28 14:54:22 2010 -0700
+++ b/src/util/publish/pkglint.py Wed Sep 29 11:55:47 2010 +1300
@@ -130,7 +130,7 @@
if opts.list_checks:
list_checks(lint_engine.checkers,
- lint_engine.excluded_checkers)
+ lint_engine.excluded_checkers, opts.verbose)
sys.exit(0)
if (opts.lint_uris or opts.ref_uris) and not opts.cache:
@@ -164,16 +164,19 @@
else:
return 0
-def list_checks(checkers, exclude):
+def list_checks(checkers, exclude, verbose=False):
"""Prints a human-readable version of configured checks."""
# used for justifying output
width = 28
- def get_method_name(method):
- return "%s.%s.%s" % (method.im_class.__module__,
- method.im_class.__name__,
- method.im_func.func_name)
+ def get_method_desc(method, verbose):
+ if "pkglint_desc" in method.func_dict and not verbose:
+ return method.pkglint_desc
+ else:
+ return "%s.%s.%s" % (method.im_class.__module__,
+ method.im_class.__name__,
+ method.im_func.func_name)
def get_pkglint_id(method):
"""Inspects a given checker method to find the 'pkglint_id'
@@ -188,33 +191,48 @@
i = arg_spec.args.index("pkglint_id")
except ValueError:
return "%s.?" % name
- return "%s.%s" % (name, arg_spec.defaults[c - i])
+ return "%s%s" % (name, arg_spec.defaults[c - i])
+
+ def emit(name, value):
+ msg("%s %s" % (name.ljust(width), value))
- has_excluded_methods = False
+ def print_list(items):
+ k = items.keys()
+ k.sort()
+ for lint_id in k:
+ emit(lint_id, items[lint_id])
+
+ include_items = {}
+ exclude_items = {}
+
for checker in checkers:
for m in checker.included_checks:
- msg("%s %s" % (get_pkglint_id(m).ljust(width),
- get_method_name(m)))
- if checker.excluded_checks:
- has_excluded_methods = True
+ lint_id = get_pkglint_id(m)
+ include_items[lint_id] = get_method_desc(m, verbose)
- if not (exclude or has_excluded_methods):
- return
-
- msg(_("\nExcluded checks:"))
for checker in exclude:
for m in checker.excluded_checks:
- msg("%s %s" % (get_pkglint_id(m).ljust(width),
- get_method_name(m)))
+ lint_id = get_pkglint_id(m)
+ exclude_items[lint_id] = get_method_desc(m, verbose)
for m in checker.included_checks:
- msg("%s %s" % (get_pkglint_id(m).ljust(width),
- get_method_name(m)))
+ lint_id = get_pkglint_id(m)
+ exclude_items[lint_id] = get_method_desc(m, verbose)
+
for checker in checkers:
for m in checker.excluded_checks:
- msg("%s %s" % (get_pkglint_id(m).ljust(width),
- get_method_name(m)))
+ lint_id = get_pkglint_id(m)
+ exclude_items[lint_id] = get_method_desc(m, verbose)
+ if include_items or exclude_items:
+ if verbose:
+ emit(_("NAME"), _("METHOD"))
+ else:
+ emit(_("NAME"), _("DESCRIPTION"))
+ print_list(include_items)
+ if exclude_items:
+ msg(_("\nExcluded checks:"))
+ print_list(exclude_items)
def read_manifests(names, lint_logger):
"""Read a list of filenames, return a list of Manifest objects."""