pkgsrc-Changes-HG archive

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

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 22.2.4



details:   https://anonhg.NetBSD.org/pkgsrc/rev/7f5f019b1ad4
branches:  trunk
changeset: 382435:7f5f019b1ad4
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Thu Jul 28 06:37:04 2022 +0000

description:
pkgtools/pkglint: update to 22.2.4

Changes since 22.2.3:

Manual pages must not be listed in the ALTERNATIVES file. Instead, they
are handled automatically based on the program in bin/ or sbin/.

Detect packages that use the tool pkg-config even though they don't need
it.

diffstat:

 pkgtools/pkglint/Makefile                   |   4 +-
 pkgtools/pkglint/files/alternatives.go      |   6 +--
 pkgtools/pkglint/files/alternatives_test.go |  29 +++++++----
 pkgtools/pkglint/files/mkcondsimplifier.go  |  26 ++++++++++-
 pkgtools/pkglint/files/package.go           |  70 ++++++++++++++++++++++++++--
 pkgtools/pkglint/files/package_test.go      |  52 +++++++++++++++++++++
 pkgtools/pkglint/files/plist.go             |   4 +-
 7 files changed, 162 insertions(+), 29 deletions(-)

diffs (truncated from 355 to 300 lines):

diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/Makefile Thu Jul 28 06:37:04 2022 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.723 2022/07/24 20:07:20 rillig Exp $
+# $NetBSD: Makefile,v 1.724 2022/07/28 06:37:04 rillig Exp $
 
-PKGNAME=       pkglint-22.2.3
+PKGNAME=       pkglint-22.2.4
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/files/alternatives.go
--- a/pkgtools/pkglint/files/alternatives.go    Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/files/alternatives.go    Thu Jul 28 06:37:04 2022 +0000
@@ -47,10 +47,9 @@
                        line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper)
                }
                if !wrapper.HasPrefixText("bin/") &&
-                       !wrapper.HasPrefixText("@PKGMANDIR@/") &&
                        !wrapper.HasPrefixText("sbin/") {
                        line.Errorf("Alternative wrapper %q must be in "+
-                               "\"bin\", \"@PKGMANDIR@\" or \"sbin\".", wrapper)
+                               "\"bin\" or \"sbin\".", wrapper)
                }
        }
 
@@ -95,9 +94,6 @@
        if plistFiles[rel] != nil || pkg.vars.IsDefined("ALTERNATIVES_SRC") {
                return
        }
-       if plistFiles[rel.Replace("${PKGMANDIR}", "man")] != nil {
-               return
-       }
 
        if plistName == alternative {
                line.Errorf("Alternative implementation %q must appear in the PLIST.",
diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/files/alternatives_test.go
--- a/pkgtools/pkglint/files/alternatives_test.go       Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/files/alternatives_test.go       Thu Jul 28 06:37:04 2022 +0000
@@ -72,7 +72,7 @@
                "ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must appear in the PLIST.",
                "ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must be an absolute path.",
                "ERROR: ALTERNATIVES:7: Alternative wrapper \"highscores\" "+
-                       "must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+                       "must be in \"bin\" or \"sbin\".",
                "ERROR: ALTERNATIVES:7: Alternative implementation \"@VARBASE@/game/scores\" "+
                        "must appear in the PLIST as \"${VARBASE}/game/scores\".")
 
@@ -120,7 +120,7 @@
 
        t.CheckOutputLines(
                "ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"relative\" "+
-                       "must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+                       "must be in \"bin\" or \"sbin\".",
                "ERROR: ~/ALTERNATIVES:2: Alternative wrapper \"/absolute\" "+
                        "must be relative to PREFIX.")
 }
@@ -160,11 +160,11 @@
 
        t.CheckOutputLines(
                "ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"in/typo\" "+
-                       "must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+                       "must be in \"bin\" or \"sbin\".",
                "ERROR: ~/ALTERNATIVES:3: Alternative wrapper \"typo/program\" "+
-                       "must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+                       "must be in \"bin\" or \"sbin\".",
                "ERROR: ~/ALTERNATIVES:4: Alternative wrapper \"man/man1/program.1\" "+
-                       "must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".")
+                       "must be in \"bin\" or \"sbin\".")
 }
 
 func (s *Suite) Test_AlternativesChecker_checkAlternativeAbs(c *check.C) {
@@ -211,12 +211,9 @@
                        "must appear in the PLIST as \"bin/not-found\".")
 }
 
-// When a man page is mentioned in the ALTERNATIVES file, it must use the
-// PKGMANDIR variable. In the PLIST files though, there is some magic
-// in the pkgsrc infrastructure that maps man/ to ${PKGMANDIR}, which
-// leads to a bit less typing.
-//
-// Seen in graphics/py-blockdiag.
+// Manual pages must not be listed in the ALTERNATIVES file.
+// Instead, they are handled automatically based on the program in bin/ or
+// sbin/.
 func (s *Suite) Test_AlternativesChecker_checkAlternativePlist__PKGMANDIR(c *check.C) {
        t := s.Init(c)
 
@@ -230,5 +227,13 @@
 
        G.Check(t.File("category/package"))
 
-       t.CheckOutputEmpty()
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/ALTERNATIVES:1: "+
+                       "Alternative wrapper \"@PKGMANDIR@/man1/blockdiag\" "+
+                       "must be in \"bin\" or \"sbin\".",
+               "ERROR: ~/category/package/ALTERNATIVES:1: "+
+                       "Alternative implementation "+
+                       "\"@PREFIX@/@PKGMANDIR@/man1/blockdiag-@PYVERSSUFFIX@.1\" "+
+                       "must appear in the PLIST as "+
+                       "\"${PKGMANDIR}/man1/blockdiag-${PYVERSSUFFIX}.1\".")
 }
diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/files/mkcondsimplifier.go
--- a/pkgtools/pkglint/files/mkcondsimplifier.go        Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/files/mkcondsimplifier.go        Thu Jul 28 06:37:04 2022 +0000
@@ -136,7 +136,31 @@
                return
        }
 
-       if !(fromEmpty && positive && !exact && vartype != nil) {
+       if !fromEmpty {
+               return // Already using the simple form.
+       }
+
+       // Only simplify the form ':M' for now, but not ':N'.
+       // The form ':M' is much more popular than ':N'.
+       if !positive {
+               return
+       }
+
+       // For now, only suggest the replacement if the pattern
+       // actually contains wildcards. This mainly affects
+       // PKG_OPTIONS in the second part of options.mk files.
+       // There are many of these, and the pkgsrc developers got
+       // used to using '!empty' for them.
+       if exact {
+               return
+       }
+
+       // TODO: Even if the variable type is not known,
+       //  the RedundantScope may know that the variable is always
+       //  defined at this point, so that the variable expression
+       //  does not need the modifier ':U'.
+       //  Example: doc/guide/Makefile.common, _GUIDE_OUTPUTS.
+       if vartype == nil {
                return
        }
 
diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/files/package.go
--- a/pkgtools/pkglint/files/package.go Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/files/package.go Thu Jul 28 06:37:04 2022 +0000
@@ -47,13 +47,12 @@
        bl3Data map[Buildlink3ID]*Buildlink3Data
 
        // Remembers the Makefile fragments that have already been included.
-       // The key to the map is the filename relative to the package directory.
        // Typical keys are "../../category/package/buildlink3.mk".
        //
        // TODO: Include files with multiple-inclusion guard only once.
        //
        // TODO: Include files without multiple-inclusion guard as often as needed.
-       included Once
+       included IncludedMap
 
        // Does the package have any .includes?
        //
@@ -110,7 +109,7 @@
                vars:                  NewScope(),
                bl3:                   make(map[PackagePath]*MkLine),
                bl3Data:               make(map[Buildlink3ID]*Buildlink3Data),
-               included:              Once{},
+               included:              NewIncludedMap(),
                conditionalIncludes:   make(map[PackagePath]*MkLine),
                unconditionalIncludes: make(map[PackagePath]*MkLine),
        }
@@ -295,8 +294,8 @@
        filename := mklines.lines.Filename
        if mklines.lines.BaseName == "buildlink3.mk" {
                builtin := filename.Dir().JoinNoClean("builtin.mk").CleanPath()
-               builtinRel := G.Pkgsrc.Relpath(pkg.dir, builtin)
-               if pkg.included.FirstTime(builtinRel.String()) && builtin.IsFile() {
+               builtinRel := NewPackagePath(G.Pkgsrc.Relpath(pkg.dir, builtin))
+               if pkg.included.FirstTime(builtinRel) && builtin.IsFile() {
                        builtinMkLines := LoadMk(builtin, pkg, MustSucceed|LogErrors)
                        pkg.parse(builtinMkLines, allLines, "", false)
                }
@@ -377,13 +376,13 @@
        dirname, _ := includingFile.Split()
        dirname = dirname.CleanPath()
        fullIncluded := dirname.JoinNoClean(includedFile)
-       relIncludedFile := G.Pkgsrc.Relpath(pkg.dir, fullIncluded)
 
        if !pkg.shouldDiveInto(includingFile, includedFile) {
                return nil, true
        }
 
-       if !pkg.included.FirstTime(relIncludedFile.String()) {
+       relIncludedFile := NewPackagePath(G.Pkgsrc.Relpath(pkg.dir, fullIncluded))
+       if !pkg.included.FirstTime(relIncludedFile) {
                return nil, true
        }
 
@@ -604,6 +603,7 @@
        }
 
        pkg.checkDistfilesInDistinfo(allLines)
+       pkg.checkPkgConfig(allLines)
 }
 
 func (pkg *Package) checkDescr(filenames []CurrPath, mklines *MkLines) {
@@ -655,6 +655,34 @@
        }
 }
 
+func (pkg *Package) checkPkgConfig(allLines *MkLines) {
+       pkgConfig := allLines.Tools.ByName("pkg-config")
+       if pkgConfig == nil || !pkgConfig.UsableAtRunTime() {
+               return
+       }
+
+       for included := range pkg.included.m {
+               included := included.String()
+               if hasSuffix(included, "buildlink3.mk") ||
+                       hasSuffix(included, "/mk/apache.mk") ||
+                       hasSuffix(included, "/mk/apache.module.mk") {
+                       return
+               }
+       }
+
+       mkline := allLines.mklines[0]
+       mkline.Warnf("The package uses the tool \"pkg-config\" " +
+               "but doesn't include any buildlink3 file.")
+       mkline.Explain(
+               "The pkgsrc tool wrappers replace the \"pkg-config\" command",
+               "with a pkg-config implementation that looks in the buildlink3",
+               "directory.",
+               "This directory is populated by including the dependencies via",
+               "the buildlink3.mk files.",
+               "Since this package does not include any such files, the buildlink3",
+               "directory will be empty and pkg-config will not find anything.")
+}
+
 func (pkg *Package) checkfilePackageMakefile(filename CurrPath, mklines *MkLines, allLines *MkLines) {
        if trace.Tracing {
                defer trace.Call(filename)()
@@ -1802,6 +1830,34 @@
                make(map[string]bool)}
 }
 
+// IncludedMap remembers which files the package Makefile has included,
+// including indirect files.
+// See Once.
+type IncludedMap struct {
+       m     map[PackagePath]struct{}
+       Trace bool
+}
+
+func NewIncludedMap() IncludedMap {
+       return IncludedMap{make(map[PackagePath]struct{}), false}
+}
+
+func (im *IncludedMap) FirstTime(p PackagePath) bool {
+       _, found := im.m[p]
+       if !found {
+               im.m[p] = struct{}{}
+               if im.Trace {
+                       G.Logger.out.WriteLine("FirstTime: " + p.String())
+               }
+       }
+       return !found
+}
+
+func (im *IncludedMap) Seen(p PackagePath) bool {
+       _, seen := im.m[p]
+       return seen
+}
+
 // matchPkgname tests whether the string has the form of a package name that
 // does not contain any variable expressions.
 func matchPkgname(s string) (m bool, base string, version string) {
diff -r 273b75c8050e -r 7f5f019b1ad4 pkgtools/pkglint/files/package_test.go
--- a/pkgtools/pkglint/files/package_test.go    Thu Jul 28 05:25:22 2022 +0000
+++ b/pkgtools/pkglint/files/package_test.go    Thu Jul 28 06:37:04 2022 +0000
@@ -1581,6 +1581,58 @@
                "WARN: distinfo: This file should not exist.")
 }
 
+func (s *Suite) Test_Package_checkPkgConfig__no_buildlink3(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpTool("pkg-config", "", Nowhere)
+       t.SetUpPackage("category/package",
+               "USE_TOOLS+=\tpkg-config")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       pkg := NewPackage(".")
+       pkg.Check()
+
+       t.CheckOutputLines(
+               "WARN: Makefile:1: The package uses the tool \"pkg-config\" " +
+                       "but doesn't include any buildlink3 file.")
+}
+
+func (s *Suite) Test_Package_checkPkgConfig__plain_buildlink3(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpTool("pkg-config", "", Nowhere)



Home | Main Index | Thread Index | Old Index