diff options
| -rw-r--r-- | pkgtools/pkglint/Makefile | 4 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/homepage_test.go | 3 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mkassignchecker_test.go | 20 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mklinechecker.go | 2 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mklinechecker_test.go | 2 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mktypes.go | 10 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mktypes_test.go | 34 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/mkvarusechecker_test.go | 4 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/package.go | 11 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/package_test.go | 45 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/patches.go | 87 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/patches_test.go | 162 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/pkglint.go | 6 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/pkgver/vercmp.go | 23 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/plist.go | 7 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/regex/regex.go | 14 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/util.go | 10 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/vardefs.go | 12 | ||||
| -rw-r--r-- | pkgtools/pkglint/files/vartypecheck.go | 4 |
19 files changed, 366 insertions, 94 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index b80898a1cca..3bea1707805 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.641 2020/04/13 19:46:44 rillig Exp $ +# $NetBSD: Makefile,v 1.642 2020/04/30 21:15:03 rillig Exp $ -PKGNAME= pkglint-20.1.2 +PKGNAME= pkglint-20.1.3 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} diff --git a/pkgtools/pkglint/files/homepage_test.go b/pkgtools/pkglint/files/homepage_test.go index 49a6d90e491..ca7ae4f9efc 100644 --- a/pkgtools/pkglint/files/homepage_test.go +++ b/pkgtools/pkglint/files/homepage_test.go @@ -393,7 +393,8 @@ func (s *Suite) Test_HomepageChecker_checkReachable(c *check.C) { "https://no-such-name.example.org/", // The "unknown network error" is for compatibility with Go < 1.13. `^WARN: filename\.mk:1: Homepage "https://no-such-name.example.org/" `+ - `cannot be checked: (name not found|timeout|unknown network error:.*)$`) + `cannot be checked: `+ + `(name not found|timeout|connection refused|unknown network error:.*)$`) // Syntactically invalid URLs are silently skipped since VartypeCheck.URL // already warns about them. diff --git a/pkgtools/pkglint/files/mkassignchecker_test.go b/pkgtools/pkglint/files/mkassignchecker_test.go index cff62ac1175..7164f4ae625 100644 --- a/pkgtools/pkglint/files/mkassignchecker_test.go +++ b/pkgtools/pkglint/files/mkassignchecker_test.go @@ -995,23 +995,25 @@ func (s *Suite) Test_MkAssignChecker_checkVarassignDecreasingVersions(c *check.C mklines.Check() - // Half of these warnings are from VartypeCheck.Version, the - // other half are from checkVarassignDecreasingVersions. - // Strictly speaking some of them are redundant, but that would - // mean to reject only variable references in checkVarassignDecreasingVersions. - // This is probably ok. - // TODO: Fix this. + // Half of these warnings are from VartypeCheck.Enum, + // the other half are from checkVarassignDecreasingVersions. + // Strictly speaking some of them are redundant, but that's ok. + // They all need to be fixed in the end. t.CheckOutputLines( - "WARN: Makefile:2: Invalid version number \"__future__\".", + "WARN: Makefile:2: \"__future__\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+ + "Use one of { 27 36 37 38 } instead.", "ERROR: Makefile:2: Value \"__future__\" for "+ "PYTHON_VERSIONS_ACCEPTED must be a positive integer.", - "WARN: Makefile:3: Invalid version number \"-13\".", + "WARN: Makefile:3: \"-13\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+ + "Use one of { 27 36 37 38 } instead.", "ERROR: Makefile:3: Value \"-13\" for "+ "PYTHON_VERSIONS_ACCEPTED must be a positive integer.", "ERROR: Makefile:4: Value \"${PKGVERSION_NOREV}\" for "+ "PYTHON_VERSIONS_ACCEPTED must be a positive integer.", "WARN: Makefile:5: The values for PYTHON_VERSIONS_ACCEPTED "+ - "should be in decreasing order (37 before 36).") + "should be in decreasing order (37 before 36).", + "WARN: Makefile:6: \"25\" is not valid for PYTHON_VERSIONS_ACCEPTED. "+ + "Use one of { 27 36 37 38 } instead.") } func (s *Suite) Test_MkAssignChecker_checkVarassignMiscRedundantInstallationDirs__AUTO_MKDIRS_yes(c *check.C) { diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 7e4818c0f17..5f74fcb5064 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -98,7 +98,7 @@ func (ck MkLineChecker) checkTextWrksrcDotDot(text string) { "", "Example:", "", - "\tWRKSRC=\t${WRKDIR}", + "\tWRKSRC=\t\t${WRKDIR}", "\tCONFIGURE_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src", "\tBUILD_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src ${WRKSRC}/cmd", "", diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index c397163629b..2c6126c1908 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -161,7 +161,7 @@ func (s *Suite) Test_MkLineChecker_checkText__WRKSRC(c *check.C) { "", "\tExample:", "", - "\t\tWRKSRC=\t${WRKDIR}", + "\t\tWRKSRC=\t\t${WRKDIR}", "\t\tCONFIGURE_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src", "\t\tBUILD_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src ${WRKSRC}/cmd", "", diff --git a/pkgtools/pkglint/files/mktypes.go b/pkgtools/pkglint/files/mktypes.go index b862346c075..9ad08e0ae9d 100644 --- a/pkgtools/pkglint/files/mktypes.go +++ b/pkgtools/pkglint/files/mktypes.go @@ -40,7 +40,7 @@ func NewMkVarUse(varname string, modifiers ...MkVarUseModifier) *MkVarUse { func (vu *MkVarUse) String() string { return sprintf("${%s%s}", vu.varname, vu.Mod()) } type MkVarUseModifier struct { - Text string + Text string // The text of the modifier, without the initial colon. } func (m MkVarUseModifier) IsQ() bool { return m.Text == "Q" } @@ -59,13 +59,13 @@ func (m MkVarUseModifier) MatchSubst() (ok bool, regex bool, from string, to str // // Example: // MkVarUseModifier{"S,name,file,g"}.Subst("distname-1.0") => "distfile-1.0" -func (m MkVarUseModifier) Subst(str string) (string, bool) { +func (m MkVarUseModifier) Subst(str string) (bool, string) { // XXX: The call to MatchSubst is usually redundant because MatchSubst // is typically called directly before calling Subst. // This comes from a time when there was no boolean return value. ok, isRegex, from, to, options := m.MatchSubst() if !ok { - return "", false + return false, "" } leftAnchor := hasPrefix(from, "^") @@ -86,14 +86,14 @@ func (m MkVarUseModifier) Subst(str string) (string, bool) { if isRegex { // XXX: Maybe implement regular expression substitutions later. - return "", false + return false, "" } ok, result := m.EvalSubst(str, leftAnchor, from, rightAnchor, to, options) if trace.Tracing && ok && result != str { trace.Stepf("Subst: %q %q => %q", str, m.Text, result) } - return result, ok + return ok, result } // mkopSubst evaluates make(1)'s :S substitution operator. diff --git a/pkgtools/pkglint/files/mktypes_test.go b/pkgtools/pkglint/files/mktypes_test.go index d9911462585..b927bd91e5d 100644 --- a/pkgtools/pkglint/files/mktypes_test.go +++ b/pkgtools/pkglint/files/mktypes_test.go @@ -90,60 +90,60 @@ func (s *Suite) Test_MkVarUseModifier_MatchSubst__backslash_as_separator(c *chec func (s *Suite) Test_MkVarUseModifier_Subst(c *check.C) { t := s.Init(c) - test := func(mod, str, result string, ok bool) { + test := func(mod, str string, ok bool, result string) { m := MkVarUseModifier{mod} - actualResult, actualOk := m.Subst(str) + actualOk, actualResult := m.Subst(str) t.CheckDeepEquals( - []interface{}{actualResult, actualOk}, - []interface{}{result, ok}) + []interface{}{actualOk, actualResult}, + []interface{}{ok, result}) } - test("???", "anything", "", false) + test("???", "anything", false, "") - test("S,from,to,", "from", "to", true) + test("S,from,to,", "from", true, "to") - test("C,from,to,", "from", "to", true) + test("C,from,to,", "from", true, "to") - test("C,syntax error", "anything", "", false) + test("C,syntax error", "anything", false, "") // The substitution modifier does not match, therefore // the value is returned unmodified, but successful. - test("C,no_match,replacement,", "value", "value", true) + test("C,no_match,replacement,", "value", true, "value") // As of December 2019, pkglint doesn't know how to handle // complicated :C modifiers. - test("C,.*,,", "anything", "", false) + test("C,.*,,", "anything", false, "") // When given a modifier that is not actually a :S or :C, Subst // doesn't do anything. - test("Mpattern", "anything", "", false) + test("Mpattern", "anything", false, "") - test("S,from,to,", "from a to b", "to a to b", true) + test("S,from,to,", "from a to b", true, "to a to b") // Since the replacement text is not a simple string, the :C modifier // cannot be treated like the :S modifier. The variable could contain // one of the special characters that would need to be escaped in the // replacement text. - test("C,from,${VAR},", "from a to b", "", false) + test("C,from,${VAR},", "from a to b", false, "") // As of December 2019, nothing is substituted. If pkglint should ever // handle variables in the modifier, this test would need to provide a // context in which to resolve the variables. If that happens, the // .TARGET variable needs to be set to "target". - test("S/$@/replaced/", "The target", "The target", true) - test("S,${PREFIX},/prefix,", "${PREFIX}/dir", "", false) + test("S/$@/replaced/", "The target", true, "The target") + test("S,${PREFIX},/prefix,", "${PREFIX}/dir", false, "") // Just for code coverage. t.DisableTracing() - test("S,long,long long,g", "A long story", "A long long story", true) + test("S,long,long long,g", "A long story", true, "A long long story") t.EnableTracing() // And now again with full tracing, to investigate cases where // pkglint produces results that are not easily understandable. t.EnableTracingToLog() - test("S,long,long long,g", "A long story", "A long long story", true) + test("S,long,long long,g", "A long story", true, "A long long story") t.EnableTracing() t.CheckOutputLines( "TRACE: Subst: \"A long story\" " + diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go index 67b50f050d6..ac063a982d9 100644 --- a/pkgtools/pkglint/files/mkvarusechecker_test.go +++ b/pkgtools/pkglint/files/mkvarusechecker_test.go @@ -592,13 +592,13 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__indirectly(c *check.C) { t.SetUpVartypes() mklines := t.NewMkLines("file.mk", MkCvsID, - "IGNORE_PKG.package=\t${ONLY_FOR_UNPRIVILEGED}") + "IGNORE_PKG.package=\t${NOT_FOR_UNPRIVILEGED}") mklines.Check() t.CheckOutputLines( "WARN: file.mk:2: IGNORE_PKG.package should be set to YES or yes.", - "WARN: file.mk:2: ONLY_FOR_UNPRIVILEGED should not be used indirectly at load time (via IGNORE_PKG.package).") + "WARN: file.mk:2: NOT_FOR_UNPRIVILEGED should not be used indirectly at load time (via IGNORE_PKG.package).") } // This test is only here for branch coverage. diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 9b0e881c922..cbd18d941b9 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -275,7 +275,6 @@ func (pkg *Package) loadPackageMakefile() (*MkLines, *MkLines) { return mainLines, allLines } -// TODO: What is allLines used for, is it still necessary? Would it be better as a field in Package? func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForUsedCheck CurrPath, main bool) bool { if trace.Tracing { defer trace.Call(mklines.lines.Filename)() @@ -891,7 +890,6 @@ func (pkg *Package) CheckVarorder(mklines *MkLines) { {"NOT_FOR_COMPILER", many}, {"ONLY_FOR_COMPILER", many}, {"NOT_FOR_UNPRIVILEGED", optional}, - {"ONLY_FOR_UNPRIVILEGED", optional}, emptyLine, {"BUILD_DEPENDS", many}, {"TOOL_DEPENDS", many}, @@ -1147,7 +1145,7 @@ func (pkg *Package) determineEffectivePkgVars() { effname := pkgname if distname != "" && effname != "" { - merged, ok := pkg.pkgnameFromDistname(effname, distname) + merged, ok := pkg.pkgnameFromDistname(effname, distname, pkgnameLine) if ok { effname = merged } @@ -1209,7 +1207,7 @@ func (pkg *Package) nbPart() string { return "" } -func (pkg *Package) pkgnameFromDistname(pkgname, distname string) (string, bool) { +func (pkg *Package) pkgnameFromDistname(pkgname, distname string, diag Diagnoser) (string, bool) { tokens, rest := NewMkLexer(pkgname, nil).MkTokens() if rest != "" { return "", false @@ -1228,7 +1226,10 @@ func (pkg *Package) pkgnameFromDistname(pkgname, distname string) (string, bool) for _, mod := range token.Varuse.modifiers { if mod.IsToLower() { newDistname = strings.ToLower(newDistname) - } else if subst, ok := mod.Subst(newDistname); ok { + } else if ok, subst := mod.Subst(newDistname); ok { + if subst == newDistname && !containsVarUse(subst) { + diag.Notef("The modifier :%s does not have an effect.", mod.Text) + } newDistname = subst } else { return "", false diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index f9bdf9df121..b772c1049d8 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -2609,6 +2609,43 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__ineffective_C_modifier(c pkg.check(files, mklines, allLines) t.CheckEquals(pkg.EffectivePkgname, "distname-1.0") + t.CheckOutputLines( + "NOTE: ~/category/package/Makefile:4: " + + "The modifier :C:does_not_match:replacement: does not have an effect.") +} + +func (s *Suite) Test_Package_determineEffectivePkgVars__ineffective_S_modifier_with_variable(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + "VERSION=\t1.008", + "DISTNAME=\tdistname-v${VERSION}", + "PKGNAME=\t${DISTNAME:S/v1/1/}") + t.FinishSetUp() + pkg := NewPackage(t.File("category/package")) + files, mklines, allLines := pkg.load() + + pkg.check(files, mklines, allLines) + + // TODO: Expand ${VERSION}, that's pretty simple. + t.CheckEquals(pkg.EffectivePkgname, "") // Because of the unexpanded VERSION. + t.CheckOutputEmpty() +} + +func (s *Suite) Test_Package_determineEffectivePkgVars__effective_S_modifier_with_variable(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + "MINOR_VERSION=\t1.008", + "DISTNAME=\tdistname-v1.${MINOR_VERSION}", + "PKGNAME=\t${DISTNAME:S/v1/1/}") + t.FinishSetUp() + pkg := NewPackage(t.File("category/package")) + files, mklines, allLines := pkg.load() + + pkg.check(files, mklines, allLines) + + t.CheckEquals(pkg.EffectivePkgname, "") // because of MINOR_VERSION t.CheckOutputEmpty() } @@ -2724,9 +2761,11 @@ func (s *Suite) Test_Package_pkgnameFromDistname(c *check.C) { // the package version. Therefore it is discarded completely. test("${DISTNAME:S|^lib||}", "libncurses", "") - // The substitution succeeds, but the substituted value is missing - // the package version. Therefore it is discarded completely. - test("${DISTNAME:S|^lib||}", "mylib", "") + // The substitution does not have an effect. + // The substituted value is missing the package version. + // Therefore it is discarded completely. + test("${DISTNAME:S|^lib||}", "mylib", "", + "NOTE: ~/category/package/Makefile:4: The modifier :S|^lib|| does not have an effect.") test("${DISTNAME:tl:S/-/./g:S/he/-/1}", "SaxonHE9-5-0-1J", "saxon-9.5.0.1j") diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index 31c71a3d57a..34147872aaa 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -224,15 +224,90 @@ func (ck *PatchChecker) checkConfigure(addedText string, isConfigure bool) { } func (ck *PatchChecker) checkAddedLine(addedText string) { - if !matches(addedText, `/usr/pkg\b`) { - return + dirs := regcomp(`(?:^|[^.@)}])(/usr/pkg|/var|/etc)([^\w-]|$)`) + for _, m := range dirs.FindAllStringSubmatchIndex(addedText, -1) { + before := addedText[:m[2]] + dir := NewPath(addedText[m[2]:m[3]]) + ck.checkAddedAbsPath(before, dir, addedText[m[4]:]) } +} +func (ck *PatchChecker) checkAddedAbsPath(before string, dir Path, after string) { line := ck.llex.PreviousLine() - line.Errorf("Patches must not hard-code the pkgsrc PREFIX.") - line.Explain( - "Instead of hard-coding /usr/pkg, packages should use the PREFIX variable.", - "The usual way of doing this is to use the SUBST framework in mk/subst.mk.") + + // Remove the #define from C and C++ macros. + before = replaceAll(before, `^[ \t]*#[ \t]*define[ \t]*\w+[ \t]*(.+)[ \t]*$`, "$1") + + // Remove the "set(VAR" from CMakeLists.txt. + before = replaceAll(before, `^[ \t]*set\(\w+[ \t]*`, "") + + // Ignore comments in shell programs. + if m, first := match1(before, `^[ \t]*#[ \t]*(\w*)`); m && first != "define" { + return + } + + // Ignore paths inside C-style comments. + if contains(before, "/*") && contains(after, "*/") { + return + } + + // Ignore composed C string literals such as PREFIX "/etc". + if matches(before, `\w+[ \t]*"$`) { + return + } + + // Ignore shell literals such as $PREFIX/etc. + // But keep compiler options like -I/usr/pkg even though they look + // like a relative pathname. + if matches(before, `\w$`) && !matches(before, `(^|[ \t])-(I|L|R|rpath|Wl,-R)$`) { + return + } + + switch dir { + case "/usr/pkg": + + line.Errorf("Patches must not hard-code the pkgsrc PREFIX.") + line.Explain( + "Not every pkgsrc installation uses /usr/pkg as its PREFIX.", + "To keep the PREFIX configurable, the patch files should contain", + "the placeholder @PREFIX@ instead.", + "", + "In the pre-configure stage, this placeholder should then be", + "replaced with the actual configuration directory", + "using a SUBST block containing SUBST_VARS.dirs=PREFIX.", + "See mk/subst.mk for details.") + + case "/var": + afterPath := NewPath(after) + if afterPath.HasPrefixPath("/tmp") || afterPath.HasPrefixPath("/shm") { + break + } + + line.Errorf("Patches must not hard-code the pkgsrc VARBASE.") + line.Explain( + "Not every pkgsrc installation uses /var as its directory", + "for writable files.", + "To keep the VARBASE configurable, the patch files should", + "contain the placeholder @VARBASE@ instead.", + "", + "In the pre-configure stage, this placeholder should then be", + "replaced with the actual configuration directory", + "using a SUBST block containing SUBST_VARS.dirs=VARBASE.", + "See mk/subst.mk for details.") + + default: + line.Errorf("Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + line.Explain( + "Not every pkgsrc installation uses /etc as its directory", + "for configuration files.", + "To keep the PKG_SYSCONFDIR configurable, the patch files should", + "contain the placeholder @PKG_SYSCONFDIR@ instead.", + "", + "In the pre-configure stage, this placeholder should then be", + "replaced with the actual configuration directory", + "using a SUBST block containing SUBST_VARS.dirs=PKG_SYSCONFDIR.", + "See mk/subst.mk for details.") + } } func (ck *PatchChecker) checktextUniHunkCr() { diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go index 23977c9e97f..542338e73a4 100644 --- a/pkgtools/pkglint/files/patches_test.go +++ b/pkgtools/pkglint/files/patches_test.go @@ -754,6 +754,168 @@ func (s *Suite) Test_PatchChecker_checkConfigure__configure_ac(c *check.C) { "ERROR: ~/patch-aa:9: This code must not be included in patches.") } +func (s *Suite) Test_PatchChecker_checkAddedAbsPath(c *check.C) { + t := s.Init(c) + + test := func(addedLine string, diagnostics ...string) { + lines := t.NewLines("patch-file", + CvsID, + "", + "Demonstrates absolute paths.", + "", + "--- before", + "+++ after", + "@@ -1,0 +1,1 @@", + "+"+addedLine) + + CheckLinesPatch(lines, nil) + + t.CheckOutput(diagnostics) + } + + test( + "/usr/pkg", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + test( + "/usr/pkgsrc", + nil...) + + test( + "/usr/pkg/bin", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + test( + "/usr/local:/usr/pkg:/opt", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + test( + "/var", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.") + + test( + "/var/db", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.") + + test( + "/var/run", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.") + + // A well-known path that is not specific to pkgsrc. + test( + "/var/shm", + nil...) + + // A well-known path that is not specific to pkgsrc. + test( + "/var/tmp", + nil...) + + test( + "/etc", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + + // BSD-style Makefile + test( + "${PREFIX}/etc", + nil...) + + // GNU automake-style Makefile + test( + "$(prefix)/etc", + nil...) + + // C source code. + // Instead of PREFIX/etc, this should rather be PKG_SYSCONFDIR. + // This is a relative path because of the PREFIX. + test( + "const char *conf_dir = PREFIX \"/etc\"", + nil...) + + // CMakeLists.txt + test( + "set(ETC_DIR \"/etc\")", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + + test( + "/etc/mk.conf", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + + test( + "/etc/rc.d/daemon", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + + test( + "/usr/pkg and /var and /etc", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR.") + + // From the --help text of a GNU configure script. + test( + "[PREFIX/etc]", + nil...) + + // Shell program, default value for a variable. + test( + "DIR=${DIR-/var/bytebench}", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.") + + // Shell program or Makefile. + // The placeholder will make this a relative path. + test( + "dir=@prefix@/etc", + nil...) + + // Makefile with flags for the C compiler. + test( + "CFLAGS+= -I/usr/pkg/include", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + // Makefile with flags for the linker. + test( + "LDFLAGS+= -L/usr/pkg/lib", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + // Makefile with flags for the linker. + // There should be an additional warning for using COMPILER_RPATH_FLAG. + test( + "LDFLAGS+= -rpath/usr/pkg/lib", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + // Makefile with flags for the linker. + // There should be an additional warning for using COMPILER_RPATH_FLAG. + test( + "LDFLAGS+= -Wl,-R/usr/pkg/lib", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc PREFIX.") + + // The dot before the "/etc" makes it a relative pathname. + test( + "cp ./etc/hostname /tmp") + + // +> +# from /etc/inittab (SYSV systems) + // +ERROR: devel/tet3/patches/patch-ac:51: Patches must not hard-code the pkgsrc PKG_SYSCONFDIR. + + test( + "# SysV /etc/install, /usr/sbin/install") + + // C or C++ program, macro definition. + // This is an absolute path since the PID_FILE is the macro name, + // and not part of the macro body containing the path. + test( + "#define PID_FILE \"/var/run/daemon.pid\"", + "ERROR: patch-file:8: Patches must not hard-code the pkgsrc VARBASE.") + + // This is a relative path because of the PREFIX before it. + test( + "#define PID_FILE PREFIX \"/etc/conf\"", + nil...) + + test( + "#define L 150 /* Length of a line in /etc/passwd */", + nil...) +} + func (s *Suite) Test_PatchChecker_checktextCvsID(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 7732369a113..0d38983a4cd 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -823,6 +823,12 @@ func (ip *InterPackage) Enable() { make(map[string]*Hash), make(map[string]struct{}), make(map[string]Location)} + + // This is the only license that is added by an infrastructure file, + // mk/djbware.mk. The correct way to handle this situation would be + // to scan Package.check.allLines for LICENSE lines, but that would + // be too much just to cover this special case. + ip.UseLicense("djb-unlicense") } func (ip *InterPackage) Enabled() bool { return ip.hashes != nil } diff --git a/pkgtools/pkglint/files/pkgver/vercmp.go b/pkgtools/pkglint/files/pkgver/vercmp.go index ae5843b5309..e9f3f237875 100644 --- a/pkgtools/pkglint/files/pkgver/vercmp.go +++ b/pkgtools/pkglint/files/pkgver/vercmp.go @@ -31,7 +31,7 @@ func Compare(left, right string) int { m := imax(len(lv.v), len(rv.v)) for i := 0; i < m; i++ { - if c := icmp(lv.Field(i), rv.Field(i)); c != 0 { + if c := icmp(lv.field(i), rv.field(i)); c != 0 { return c } } @@ -52,24 +52,24 @@ func newVersion(vstr string) *version { case lex.TestByteSet(textproc.Digit): num := lex.NextBytesSet(textproc.Digit) n, _ := strconv.Atoi(num) - v.Add(n) + v.add(n) case lex.SkipByte('_') || lex.SkipByte('.'): - v.Add(0) + v.add(0) case lex.SkipString("alpha"): - v.Add(-3) + v.add(-3) case lex.SkipString("beta"): - v.Add(-2) + v.add(-2) case lex.SkipString("pre"): - v.Add(-1) + v.add(-1) case lex.SkipString("rc"): - v.Add(-1) + v.add(-1) case lex.SkipString("pl"): - v.Add(0) + v.add(0) case lex.SkipString("nb"): num := lex.NextBytesSet(textproc.Digit) v.nb, _ = strconv.Atoi(num) case lex.TestByteSet(textproc.Lower): - v.Add(int(lex.Rest()[0] - 'a' + 1)) + v.add(int(lex.Rest()[0] - 'a' + 1)) lex.Skip(1) default: lex.Skip(1) @@ -78,11 +78,12 @@ func newVersion(vstr string) *version { return v } -func (v *version) Add(i int) { +//go:noinline +func (v *version) add(i int) { v.v = append(v.v, i) } -func (v *version) Field(i int) int { +func (v *version) field(i int) int { if i < len(v.v) { return v.v[i] } diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 6a097ebb905..a31507806bb 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -370,7 +370,12 @@ func (ck *PlistChecker) checkPathLib(pline *PlistLine, rel RelPath) { } func (ck *PlistChecker) checkPathMan(pline *PlistLine) { - m, catOrMan, section, manpage, ext, gz := match5(pline.text, `^man/(cat|man)(\w+)/(.*?)\.(\w+)(\.gz)?$`) + m, catOrMan, section, base := match3(pline.text, `^man/(cat|man)(\w+)/(.*)$`) + if !m { + // maybe: line.Warnf("Invalid filename %q for manual page.", text) + return + } + m, manpage, ext, gz := match3(base, `^(.*?)\.(\w+)(\.gz)?$`) if !m { // maybe: line.Warnf("Invalid filename %q for manual page.", text) return diff --git a/pkgtools/pkglint/files/regex/regex.go b/pkgtools/pkglint/files/regex/regex.go index 97f40acf6ac..77cf6246e64 100644 --- a/pkgtools/pkglint/files/regex/regex.go +++ b/pkgtools/pkglint/files/regex/regex.go @@ -103,20 +103,6 @@ func (r *Registry) Match3(s string, re Pattern) (matched bool, m1, m2, m3 string return } -func (r *Registry) Match4(s string, re Pattern) (matched bool, m1, m2, m3, m4 string) { - if m := r.matchn(s, re, 4); m != nil { - return true, m[1], m[2], m[3], m[4] - } - return -} - -func (r *Registry) Match5(s string, re Pattern) (matched bool, m1, m2, m3, m4, m5 string) { - if m := r.matchn(s, re, 5); m != nil { - return true, m[1], m[2], m[3], m[4], m[5] - } - return -} - func (r *Registry) ReplaceFirst(s string, re Pattern, replacement string) ([]string, string) { if m := r.Compile(re).FindStringSubmatchIndex(s); m != nil { replaced := s[:m[0]] + replacement + s[m[1]:] diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index a10f8328c8e..8c96eef38fb 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -58,12 +58,6 @@ func match2(s string, re regex.Pattern) (matched bool, m1, m2 string) { func match3(s string, re regex.Pattern) (matched bool, m1, m2, m3 string) { return G.res.Match3(s, re) } -func match4(s string, re regex.Pattern) (matched bool, m1, m2, m3, m4 string) { - return G.res.Match4(s, re) -} -func match5(s string, re regex.Pattern) (matched bool, m1, m2, m3, m4, m5 string) { - return G.res.Match5(s, re) -} func replaceAll(s string, re regex.Pattern, repl string) string { return G.res.Compile(re).ReplaceAllString(s, repl) } @@ -558,7 +552,7 @@ func (o *Once) FirstTime(what string) bool { key := o.keyString(what) firstTime := o.check(key) if firstTime && o.Trace { - G.Logger.out.WriteLine(sprintf("FirstTime: %s", what)) + G.Logger.out.WriteLine("FirstTime: " + what) } return firstTime } @@ -567,7 +561,7 @@ func (o *Once) FirstTimeSlice(whats ...string) bool { key := o.keyStrings(whats) firstTime := o.check(key) if firstTime && o.Trace { - G.Logger.out.WriteLine(sprintf("FirstTime: %s", strings.Join(whats, ", "))) + G.Logger.out.WriteLine("FirstTime: " + strings.Join(whats, ", ")) } return firstTime } diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 0415a9ae8b2..2423e7939a9 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -1102,7 +1102,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { reg.pkg("DJB_RESTRICTED", BtYesNo) reg.pkg("DJB_SLASHPACKAGE", BtYesNo) reg.pkg("DLOPEN_REQUIRE_PTHREADS", BtYesNo) - reg.pkg("DL_AUTO_VARS", BtYes) + reg.pkg("DL_AUTO_VARS", BtYesNo) reg.acllist("DL_LIBS", BtLdFlag, PackageSettable, "*: append, use") @@ -1357,7 +1357,6 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { reg.sysload("OBJECT_FMT", enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out")) reg.pkglistrat("ONLY_FOR_COMPILER", compilers) reg.pkglistrat("ONLY_FOR_PLATFORM", BtMachinePlatformPattern) - reg.pkgrat("ONLY_FOR_UNPRIVILEGED", BtYesNo) reg.sysload("OPSYS", operatingSystems, DefinedIfInScope|NonemptyIfDefined) reg.pkglistbl3("OPSYSVARS", BtVariableName) reg.pkg("OSVERSION_SPECIFIC", BtYes) @@ -1569,11 +1568,12 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { "special:pyversion.mk: set", "*: use, use-loadtime") // See lang/python/pyversion.mk + py := reg.enumFromDirs(src, "lang", `^python(\d+)$`, "$1", "27 36 37 38") reg.pkg("PYTHON_FOR_BUILD_ONLY", enum("yes no test tool YES")) - reg.pkglistrat("PYTHON_VERSIONS_ACCEPTED", BtVersion) - reg.pkglistrat("PYTHON_VERSIONS_INCOMPATIBLE", BtVersion) - reg.usr("PYTHON_VERSION_DEFAULT", BtVersion) - reg.usr("PYTHON_VERSION_REQD", BtVersion) + reg.pkglistrat("PYTHON_VERSIONS_ACCEPTED", py) + reg.pkglistrat("PYTHON_VERSIONS_INCOMPATIBLE", py) + reg.usr("PYTHON_VERSION_DEFAULT", py) + reg.sys("PYTHON_VERSION_REQD", py) reg.pkglist("PYTHON_VERSIONED_DEPENDENCIES", BtPythonDependency) reg.sys("RANLIB", BtShellCommand) reg.pkglist("RCD_SCRIPTS", BtFilename) diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index 528f9e89b35..d0009d11a17 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -1387,7 +1387,7 @@ func (cv *VartypeCheck) URL() { } else if containsVarUse(value) { // No further checks - } else if m, _, host, _, _ := match4(value, `^(https?|ftp|gopher)://([-0-9A-Za-z.]+)(?::(\d+))?/([-%&+,./0-9:;=?@A-Z_a-z~]|#)*$`); m { + } else if m, host := match1(value, `^(?:https?|ftp|gopher)://([-0-9A-Za-z.]+)(?::\d+)?/[-#%&+,./0-9:;=?@A-Z_a-z~]*$`); m { if matches(host, `(?i)\.NetBSD\.org$`) && !matches(host, `\.NetBSD\.org$`) { prefix := host[:len(host)-len(".NetBSD.org")] fix := cv.Autofix() @@ -1566,7 +1566,7 @@ func (cv *VartypeCheck) WrksrcSubdirectory() { "", "Example:", "", - "\tWRKSRC=\t${WRKDIR}", + "\tWRKSRC=\t\t${WRKDIR}", "\tCONFIGURE_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src", "\tBUILD_DIRS=\t${WRKSRC}/lib ${WRKSRC}/src ${WRKSRC}/cmd", "", |