diff options
author | rillig <rillig@pkgsrc.org> | 2020-03-18 08:24:49 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2020-03-18 08:24:49 +0000 |
commit | a5a897a90400fbe9ad797dd40900c4fb3a3ac0e5 (patch) | |
tree | 60388aad2b2afaa44d9c3650c877c435f435c4ee /pkgtools | |
parent | 07a7300bcab8817c424df6af6b8010f8d3783e2f (diff) | |
download | pkgsrc-a5a897a90400fbe9ad797dd40900c4fb3a3ac0e5.tar.gz |
pkgtools/pkglint: update to 19.4.12
Changes since 19.4.11:
Redundant additions to BUILDLINK_API_DEPENDS and BUILDLINK_ABI_DEPENDS
get warnings since they may have been needed in the past but the
dependent package has increased its required version numbers over time.
Diffstat (limited to 'pkgtools')
24 files changed, 719 insertions, 182 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index a35746cd92f..df38c0bb4e4 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.634 2020/03/15 11:31:24 rillig Exp $ +# $NetBSD: Makefile,v 1.635 2020/03/18 08:24:49 rillig Exp $ -PKGNAME= pkglint-19.4.11 +PKGNAME= pkglint-19.4.12 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} diff --git a/pkgtools/pkglint/files/buildlink3.go b/pkgtools/pkglint/files/buildlink3.go index 910313da1c0..9b63c6eb7f6 100644 --- a/pkgtools/pkglint/files/buildlink3.go +++ b/pkgtools/pkglint/files/buildlink3.go @@ -13,8 +13,8 @@ type Buildlink3Checker struct { abi, api *DependencyPattern } -func NewBuildlink3Checker(mklines *MkLines) *Buildlink3Checker { - return &Buildlink3Checker{mklines: mklines} +func CheckLinesBuildlink3Mk(mklines *MkLines) { + (&Buildlink3Checker{mklines: mklines}).Check() } func (ck *Buildlink3Checker) Check() { @@ -90,6 +90,9 @@ func (ck *Buildlink3Checker) checkFirstParagraph(mlex *MkLinesLexer) bool { mlex.SkipEmptyOrNote() ck.pkgbase = pkgbase + if pkg := ck.mklines.pkg; pkg != nil { + pkg.buildlinkID = ck.pkgbase + } ck.pkgbaseLine = pkgbaseLine return true } @@ -309,3 +312,65 @@ func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbaseLine *MkLine) { "after the specific version has been decided.") } } + +type Buildlink3Data struct { + id Buildlink3ID + pkgsrcdir PackagePath + apiDepends *DependencyPattern + apiDependsLine *MkLine + abiDepends *DependencyPattern + abiDependsLine *MkLine +} + +// Buildlink3ID is the identifier that is used in the BUILDLINK_TREE +// for referring to a dependent package. +// +// It almost uniquely identifies a package. +// Packages that are alternatives to each other may use the same identifier. +type Buildlink3ID string + +func LoadBuildlink3Data(mklines *MkLines) *Buildlink3Data { + assert(mklines.lines.BaseName == "buildlink3.mk") + + var data Buildlink3Data + mklines.ForEach(func(mkline *MkLine) { + if mkline.IsVarassign() { + varname := mkline.Varname() + varbase := varnameBase(varname) + varid := Buildlink3ID(varnameParam(varname)) + + if varname == "BUILDLINK_TREE" { + value := mkline.Value() + if !hasPrefix(value, "-") { + data.id = Buildlink3ID(mkline.Value()) + } + } + + if varbase == "BUILDLINK_API_DEPENDS" && varid == data.id { + p := NewMkParser(nil, mkline.Value()) + dep := p.DependencyPattern() + if dep != nil && p.EOF() { + data.apiDepends = dep + data.apiDependsLine = mkline + } + } + + if varbase == "BUILDLINK_ABI_DEPENDS" && varid == data.id { + p := NewMkParser(nil, mkline.Value()) + dep := p.DependencyPattern() + if dep != nil && p.EOF() { + data.abiDepends = dep + data.abiDependsLine = mkline + } + } + + if varbase == "BUILDLINK_PKGSRCDIR" && varid == data.id { + data.pkgsrcdir = NewPackagePathString(mkline.Value()) + } + } + }) + if data.id != "" && !data.pkgsrcdir.IsEmpty() && data.apiDepends != nil && data.abiDepends != nil { + return &data + } + return nil +} diff --git a/pkgtools/pkglint/files/buildlink3_test.go b/pkgtools/pkglint/files/buildlink3_test.go index 9f2599b73e2..aaf306236e1 100644 --- a/pkgtools/pkglint/files/buildlink3_test.go +++ b/pkgtools/pkglint/files/buildlink3_test.go @@ -2,10 +2,6 @@ package pkglint import "gopkg.in/check.v1" -func CheckLinesBuildlink3Mk(mklines *MkLines) { - NewBuildlink3Checker(mklines).Check() -} - // This test ensures that CheckLinesBuildlink3Mk really checks for // buildlink3.mk files that are included by the buildlink3.mk file // but not by the package. @@ -1066,3 +1062,31 @@ func (s *Suite) Test_Buildlink3Checker_checkVaruseInPkgbase__PKGBASE_with_unknow "WARN: buildlink3.mk:3: Please replace \"${LICENSE}\" with a simple string "+ "(also in other variables in this file).") } + +func (s *Suite) Test_LoadBuildlink3Data(c *check.C) { + t := s.Init(c) + + t.CreateFileBuildlink3("category/package/buildlink3.mk", + "BUILDLINK_ABI_DEPENDS.package+=\tpackage>=0.1") + t.Chdir("category/package") + mklines := LoadMk("buildlink3.mk", nil, MustSucceed) + + data := LoadBuildlink3Data(mklines) + + t.CheckDeepEquals(data, &Buildlink3Data{ + id: "package", + pkgsrcdir: "../../category/package", + apiDepends: &DependencyPattern{ + Pkgbase: "package", + LowerOp: ">=", + Lower: "0", + }, + apiDependsLine: mklines.mklines[7], + abiDepends: &DependencyPattern{ + Pkgbase: "package", + LowerOp: ">=", + Lower: "0.1", + }, + abiDependsLine: mklines.mklines[11], + }) +} diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 6d66e754a51..66e78929c48 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -243,7 +243,7 @@ func (t *Tester) SetUpVartypes() { func (t *Tester) SetUpMasterSite(varname string, urls ...string) { if !G.Pkgsrc.vartypes.IsDefinedExact(varname) { - G.Pkgsrc.vartypes.DefineParse(varname, BtFetchURL, + t.SetUpType(varname, BtFetchURL, List|SystemProvided, "buildlink3.mk: none", "*: use") @@ -613,6 +613,7 @@ func (t *Tester) CreateFileBuildlink3Id(filename RelPath, id string, customLines sprintf("%s_BUILDLINK3_MK:=", upperID), "", aligned("BUILDLINK_API_DEPENDS.%s+=", id)+sprintf("%s>=0", id), + // TODO: Add ABI_DEPENDS; see Test_LoadBuildlink3Data aligned("BUILDLINK_PKGSRCDIR.%s?=", id)+sprintf("../../%s", dir), aligned("BUILDLINK_DEPMETHOD.%s?=", id)+"build", "") diff --git a/pkgtools/pkglint/files/mkassignchecker_test.go b/pkgtools/pkglint/files/mkassignchecker_test.go index d234188d6f6..cff62ac1175 100644 --- a/pkgtools/pkglint/files/mkassignchecker_test.go +++ b/pkgtools/pkglint/files/mkassignchecker_test.go @@ -427,9 +427,9 @@ func (s *Suite) Test_MkAssignChecker_checkVarassignLeftPermissions(c *check.C) { t.SetUpVartypes() t.SetUpTool("awk", "AWK", AtRunTime) - G.Pkgsrc.vartypes.DefineParse("SET_ONLY", BtUnknown, NoVartypeOptions, + t.SetUpType("SET_ONLY", BtUnknown, NoVartypeOptions, "options.mk: set") - G.Pkgsrc.vartypes.DefineParse("SET_ONLY_DEFAULT_ELSEWHERE", BtUnknown, NoVartypeOptions, + t.SetUpType("SET_ONLY_DEFAULT_ELSEWHERE", BtUnknown, NoVartypeOptions, "options.mk: set", "*.mk: default, set") mklines := t.NewMkLines("options.mk", diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index 80e7679f950..3a77ac31733 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -248,13 +248,7 @@ func (mklines *MkLines) collectDocumentedVariables() { } func (mklines *MkLines) collectVariables() { - if trace.Tracing { - defer trace.Call0()() - } - - mklines.ForEach(func(mkline *MkLine) { - mklines.collectVariable(mkline) - }) + mklines.ForEach(mklines.collectVariable) } func (mklines *MkLines) collectVariable(mkline *MkLine) { diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go index 46f3458d294..67b50f050d6 100644 --- a/pkgtools/pkglint/files/mkvarusechecker_test.go +++ b/pkgtools/pkglint/files/mkvarusechecker_test.go @@ -455,9 +455,9 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_in_condition(c t := s.Init(c) t.SetUpPkgsrc() - G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathPattern, List, + t.SetUpType("LOAD_TIME", BtPathPattern, List, "special:filename.mk: use-loadtime") - G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathPattern, List, + t.SetUpType("RUN_TIME", BtPathPattern, List, "special:filename.mk: use") t.Chdir(".") t.FinishSetUp() @@ -479,9 +479,9 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_in_for_loop(c * t := s.Init(c) t.SetUpPkgsrc() - G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtPathPattern, List, + t.SetUpType("LOAD_TIME", BtPathPattern, List, "special:filename.mk: use-loadtime") - G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtPathPattern, List, + t.SetUpType("RUN_TIME", BtPathPattern, List, "special:filename.mk: use") t.Chdir(".") t.FinishSetUp() @@ -532,16 +532,16 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_run_time(c *che t := s.Init(c) t.SetUpPkgsrc() - G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtUnknown, NoVartypeOptions, + t.SetUpType("LOAD_TIME", BtUnknown, NoVartypeOptions, "*.mk: use, use-loadtime") - G.Pkgsrc.vartypes.DefineParse("RUN_TIME", BtUnknown, NoVartypeOptions, + t.SetUpType("RUN_TIME", BtUnknown, NoVartypeOptions, "*.mk: use") - G.Pkgsrc.vartypes.DefineParse("WRITE_ONLY", BtUnknown, NoVartypeOptions, + t.SetUpType("WRITE_ONLY", BtUnknown, NoVartypeOptions, "*.mk: set") - G.Pkgsrc.vartypes.DefineParse("LOAD_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions, + t.SetUpType("LOAD_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions, "Makefile: use-loadtime", "*.mk: set") - G.Pkgsrc.vartypes.DefineParse("RUN_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions, + t.SetUpType("RUN_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions, "Makefile: use", "*.mk: set") t.Chdir(".") @@ -637,7 +637,7 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__write_only_usable_in_othe func (s *Suite) Test_MkVarUseChecker_checkPermissions__usable_only_at_loadtime_in_other_file(c *check.C) { t := s.Init(c) - G.Pkgsrc.vartypes.DefineParse("VAR", BtFilename, NoVartypeOptions, + t.SetUpType("VAR", BtFilename, NoVartypeOptions, "*: set, use-loadtime") mklines := t.NewMkLines("Makefile", MkCvsID, @@ -656,9 +656,8 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__assigned_to_infrastructur // This combination of BtUnknown and all permissions is typical for // otherwise unknown variables from the pkgsrc infrastructure. - G.Pkgsrc.vartypes.Define("INFRA", BtUnknown, NoVartypeOptions, - NewACLEntry("*", aclpAll)) - G.Pkgsrc.vartypes.DefineParse("VAR", BtUnknown, NoVartypeOptions, + t.SetUpType("INFRA", BtUnknown, NoVartypeOptions) + t.SetUpType("VAR", BtUnknown, NoVartypeOptions, "buildlink3.mk: none", "*: use") mklines := t.NewMkLines("buildlink3.mk", @@ -692,10 +691,10 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__assigned_to_load_time(c * // to use its value in LOAD_TIME, as the latter might be evaluated later // at load time, and at that point VAR would be evaluated as well. - G.Pkgsrc.vartypes.DefineParse("LOAD_TIME", BtMessage, NoVartypeOptions, + t.SetUpType("LOAD_TIME", BtMessage, NoVartypeOptions, "buildlink3.mk: set", "*.mk: use-loadtime") - G.Pkgsrc.vartypes.DefineParse("VAR", BtUnknown, NoVartypeOptions, + t.SetUpType("VAR", BtUnknown, NoVartypeOptions, "buildlink3.mk: none", "*.mk: use") mklines := t.NewMkLines("buildlink3.mk", @@ -963,7 +962,8 @@ func (s *Suite) Test_MkVarUseChecker_checkAssignable(c *check.C) { t.SetUpVartypes() mklines := t.NewMkLines("filename.mk", "BUILTIN_FIND_FILES_VAR:=\tBIN_FILE", - "BUILTIN_FIND_FILES.BIN_FILE=\t${TOOLS_PLATFORM.file} /bin/file /usr/bin/file") + "BUILTIN_FIND_FILES.BIN_FILE=\t${TOOLS_PLATFORM.file} /bin/file /usr/bin/file", + "PKG_SHELL.user=\t${TOOLS_PLATFORM.false:Q}") mklines.ForEach(func(mkline *MkLine) { ck := NewMkAssignChecker(mkline, mklines) @@ -971,8 +971,11 @@ func (s *Suite) Test_MkVarUseChecker_checkAssignable(c *check.C) { }) t.CheckOutputLines( - "WARN: filename.mk:2: Incompatible types: " + - "TOOLS_PLATFORM.file (type \"ShellCommand\") " + + "WARN: filename.mk:2: Incompatible types: "+ + "TOOLS_PLATFORM.file (type \"ShellCommand\") "+ + "cannot be assigned to type \"Pathname\".", + "WARN: filename.mk:3: Incompatible types: "+ + "TOOLS_PLATFORM.false (type \"ShellCommand\") "+ "cannot be assigned to type \"Pathname\".") } @@ -993,7 +996,8 @@ func (s *Suite) Test_MkVarUseChecker_checkAssignable__shell_command_to_pathname( mklines := t.NewMkLines("filename.mk", "PKG_SHELL.user=\t${TOOLS_PLATFORM.sh}", "PKG_SHELL.user=\t${SH}", - "PKG_SHELL.user=\t${BASH}") + "PKG_SHELL.user=\t${BASH}", + "PKG_SHELL.user=\t/bin/sh") mklines.ForEach(func(mkline *MkLine) { ck := NewMkAssignChecker(mkline, mklines) @@ -1232,11 +1236,17 @@ func (s *Suite) Test_MkVarUseChecker_checkToolsPlatform(c *check.C) { t.SetUpTool("cond1", "", AfterPrefsMk) t.SetUpTool("cond2", "", AfterPrefsMk) t.SetUpTool("undefined", "", AfterPrefsMk) + t.SetUpTool("non-const", "", AfterPrefsMk) t.CreateFileLines("mk/tools/tools.NetBSD.mk", + "OTHER_VAR?=\tother value", // Just for code coverage "TOOLS_PLATFORM.available?=\t/bin/available", "TOOLS_PLATFORM.cond1?=\t/usr/cond1", "TOOLS_PLATFORM.cond2?=\t/usr/cond2", - "TOOLS_PLATFORM.undefined?=\t/usr/undefined") + "TOOLS_PLATFORM.undefined?=\t/usr/undefined", + "", + "TOOLS_PLATFORM.non-const?=\t/non-const-initial", + "READ=\t${TOOLS_PLATFORM.non-const}", // Make the variable non-const + "TOOLS_PLATFORM.non-const?=\t/non-const-final") t.CreateFileLines("mk/tools/tools.SunOS.mk", "TOOLS_PLATFORM.available?=\t/bin/available", "", diff --git a/pkgtools/pkglint/files/options.go b/pkgtools/pkglint/files/options.go index e9a84c8d978..a3ca5cf367e 100755 --- a/pkgtools/pkglint/files/options.go +++ b/pkgtools/pkglint/files/options.go @@ -53,7 +53,7 @@ func (ck *OptionsLinesChecker) collect() { seenPkgOptionsVar = true buildlinkID := ck.buildlinkID optionsID := varnameParam(mkline.Value()) - if buildlinkID != "" && optionsID != "" && buildlinkID != optionsID { + if buildlinkID != "" && buildlinkID != optionsID { mkline.Warnf("The buildlink3 identifier %q should be the same as the options identifier %q.", buildlinkID, optionsID) mkline.Explain( diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index fec7a7340d4..b9d8bded8f8 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -43,6 +43,9 @@ type Package struct { // These included files should match. bl3 map[PackagePath]*MkLine + // The default dependencies from the buildlink3.mk files. + 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". @@ -103,6 +106,7 @@ func NewPackage(dir CurrPath) *Package { Plist: NewPlistContent(), vars: NewScope(), bl3: make(map[PackagePath]*MkLine), + bl3Data: make(map[Buildlink3ID]*Buildlink3Data), included: Once{}, conditionalIncludes: make(map[PackagePath]*MkLine), unconditionalIncludes: make(map[PackagePath]*MkLine), @@ -167,11 +171,7 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) { if isRelevantMk(filename, basename) { fragmentMklines := LoadMk(filename, pkg, MustSucceed) pkg.collectConditionalIncludes(fragmentMklines) - fragmentMklines.ForEach(func(mkline *MkLine) { - if mkline.IsVarassign() && mkline.Varname() == "pkgbase" { - pkg.seenPkgbase.FirstTime(mkline.Value()) - } - }) + pkg.loadBuildlink3Pkgbase(filename, fragmentMklines) } if hasPrefix(basename, "PLIST") { pkg.loadPlistDirs(filename) @@ -182,6 +182,17 @@ func (pkg *Package) load() ([]CurrPath, *MkLines, *MkLines) { return files, mklines, allLines } +func (pkg *Package) loadBuildlink3Pkgbase(filename CurrPath, fragmentMklines *MkLines) { + if !filename.HasBase("buildlink3.mk") { + return + } + fragmentMklines.ForEach(func(mkline *MkLine) { + if mkline.IsVarassign() && mkline.Varname() == "pkgbase" { + pkg.seenPkgbase.FirstTime(mkline.Value()) + } + }) +} + func (pkg *Package) loadPackageMakefile() (*MkLines, *MkLines) { filename := pkg.File("Makefile") if trace.Tracing { @@ -271,13 +282,18 @@ func (pkg *Package) parse(mklines *MkLines, allLines *MkLines, includingFileForU // For every included buildlink3.mk, include the corresponding builtin.mk // automatically since the pkgsrc infrastructure does the same. filename := mklines.lines.Filename - if filename.Base() == "buildlink3.mk" { + 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() { builtinMkLines := LoadMk(builtin, pkg, MustSucceed|LogErrors) pkg.parse(builtinMkLines, allLines, "", false) } + + data := LoadBuildlink3Data(mklines) + if data != nil { + pkg.bl3Data[data.id] = data + } } return result diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 0bc06a815d3..473f7da1a89 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -516,6 +516,33 @@ func (s *Suite) Test_Package_load__extra_files(c *check.C) { "WARN: patches/readme.mk: Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.") } +func (s *Suite) Test_Package_loadBuildlink3Pkgbase(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + ".include \"../../category/lib/buildlink3.mk\"") + t.CreateFileBuildlink3("category/package/buildlink3.mk", + "pkgbase := package", + ".include \"../../mk/pkg-build-options.mk\"", + ".include \"../../category/lib/buildlink3.mk\"") + t.SetUpPackage("category/lib") + t.CreateFileBuildlink3("category/lib/buildlink3.mk", + "pkgbase := lib", + ".include \"../../mk/pkg-build-options.mk\"") + t.CreateFileLines("mk/pkg-build-options.mk") + t.Chdir("category/package") + t.FinishSetUp() + pkg := NewPackage(".") + + pkg.Check() + + t.CheckOutputEmpty() + seenPkgbase := pkg.seenPkgbase + t.Check(seenPkgbase.seen, check.HasLen, 2) + t.CheckEquals(seenPkgbase.Seen("lib"), true) + t.CheckEquals(seenPkgbase.Seen("package"), true) +} + func (s *Suite) Test_Package_loadPackageMakefile__dump(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 858d1a9bb07..f40bbdd9823 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -571,11 +571,7 @@ func (p *Pkglint) checkReg(filename CurrPath, basename string, depth int, pkg *P case basename == "buildlink3.mk": if mklines := LoadMk(filename, pkg, NotEmpty|LogErrors); mklines != nil { - ck := NewBuildlink3Checker(mklines) - ck.Check() - if pkg != nil { - pkg.buildlinkID = ck.pkgbase - } + CheckLinesBuildlink3Mk(mklines) } case hasPrefix(basename, "DESCR"): diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 71fa539b482..57d2102ca60 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -740,6 +740,23 @@ func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) { t.CheckEquals(resolved, "gst-plugins0.10-x11/distinfo") } +func (s *Suite) Test_resolveVariableRefs__indeterminate(c *check.C) { + t := s.Init(c) + + pkg := NewPackage(G.Pkgsrc.File("category/package")) + pkg.vars.Define("PKGVAR", t.NewMkLine("filename.mk", 123, "PKGVAR!=\tcommand")) + mklines := t.NewMkLinesPkg("filename.mk", pkg, + "VAR!=\tcommand") + mklines.collectVariables() + + resolved := resolveVariableRefs("${VAR} ${PKGVAR}", mklines, nil) + + // VAR and PKGVAR are defined, but since they contain the result of + // a shell command, their value is indeterminate. + // Therefore they are not replaced. + t.CheckEquals(resolved, "${VAR} ${PKGVAR}") +} + // Just for code coverage. func (s *Suite) Test_CheckFileOther__no_tracing(c *check.C) { t := s.Init(c) @@ -1106,6 +1123,24 @@ func (s *Suite) Test_Pkglint_checkReg__spec(c *check.C) { "WARN: ~/category/package/spec: Only packages in regress/ may have spec files.") } +func (s *Suite) Test_Pkglint_checkReg__options_mk(c *check.C) { + t := s.Init(c) + + t.SetUpPkgsrc() + t.CreateFileLines("mk/bsd.options.mk") + t.CreateFileLines("category/package/options.mk", + MkCvsID, + "", + "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package", + "", + ".include \"../../mk/bsd.options.mk\"") + t.FinishSetUp() + + G.Check(t.File("category/package/options.mk")) + + t.CheckOutputEmpty() +} + func (s *Suite) Test_Pkglint_checkRegCvsSubst(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go index e176fe7f190..5bc0fb725ac 100644 --- a/pkgtools/pkglint/files/pkgsrc.go +++ b/pkgtools/pkglint/files/pkgsrc.go @@ -14,8 +14,8 @@ import ( // It just doesn't make sense to check multiple pkgsrc installations at once. // // This type only contains data that is loaded once and then stays constant. -// Everything else (distfile hashes, package names) is recorded in the Pkglint -// type instead. +// Everything else (distfile hashes, package names) is recorded in the +// InterPackage type instead. type Pkgsrc struct { // The top directory (PKGSRCDIR). topdir CurrPath @@ -530,11 +530,8 @@ func (src *Pkgsrc) loadToolsPlatform() { conditional[opsys] = true } for opsys, status := range statusByNameAndOpsys[name] { - switch status { - case 1: - delete(undefined, opsys) - case 2: - delete(undefined, opsys) + delete(undefined, opsys) + if status == 2 { delete(conditional, opsys) } } @@ -1090,6 +1087,7 @@ func (src *Pkgsrc) ReadDir(dirName PkgsrcPath) []os.FileInfo { var relevantFiles []os.FileInfo for _, dirent := range files { name := dirent.Name() + // TODO: defer isEmptyDir, for performance reasons; run ktrace or strace to see why. if !dirent.IsDir() || !isIgnoredFilename(name) && !isEmptyDir(dir.JoinNoClean(NewRelPathString(name))) { relevantFiles = append(relevantFiles, dirent) } diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go index 9773186b872..a666e44d07d 100644 --- a/pkgtools/pkglint/files/pkgsrc_test.go +++ b/pkgtools/pkglint/files/pkgsrc_test.go @@ -782,6 +782,19 @@ func (s *Suite) Test_Pkgsrc_loadTools__no_tools_found(c *check.C) { "FATAL: ~/mk/tools/bsd.tools.mk: Too few tool files.") } +// Just for code coverage, for the IsRelevant callback. +func (s *Suite) Test_Pkgsrc_loadToolsPlatform__redundant(c *check.C) { + t := s.Init(c) + + t.SetUpPkgsrc() + t.SetUpTool("tool", "", AfterPrefsMk) + t.CreateFileLines("mk/tools/tools.NetBSD.mk", + "TOOLS_PLATFORM.tool?=\t/bin/available", + "TOOLS_PLATFORM.tool?=\t/bin/available") + t.Chdir(".") + t.FinishSetUp() +} + func (s *Suite) Test_Pkgsrc_initDeprecatedVars(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index bad22f7f4f3..6d5a1f5d9fa 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -528,7 +528,9 @@ type PlistLine struct { text string // Line.Text without any conditions of the form ${PLIST.cond} } -func (pline *PlistLine) Path() RelPath { return NewRelPathString(pline.text) } +func (pline *PlistLine) HasPath() bool { + return pline.text != "" && plistLineStart.Contains(pline.text[0]) +} func (pline *PlistLine) HasPlainPath() bool { text := pline.text @@ -537,6 +539,11 @@ func (pline *PlistLine) HasPlainPath() bool { !containsVarUse(text) } +func (pline *PlistLine) Path() RelPath { + assert(pline.HasPath()) + return NewRelPathString(pline.text) +} + func (pline *PlistLine) CheckTrailingWhitespace() { if hasSuffix(pline.text, " ") || hasSuffix(pline.text, "\t") { pline.Errorf("Pkgsrc does not support filenames ending in whitespace.") @@ -687,3 +694,53 @@ func (s *plistLineSorter) Sort() { s.autofixed = SaveAutofixChanges(NewLines(lines[0].Filename(), lines)) } + +type PlistRank uint8 + +const ( + Plain PlistRank = iota + Common + CommonEnd + Opsys + Arch + OpsysArch + EmulOpsysArch +) + +// The ranks among the files are: +// PLIST +// -> PLIST.common +// -> PLIST.common_end +// -> { PLIST.OPSYS, PLIST.ARCH } +// -> { PLIST.OPSYS.ARCH, PLIST.EMUL_PLATFORM } +// Files are a later level must not mention files that are already +// mentioned at an earlier level. +func (r PlistRank) Dominates(other PlistRank) bool { + return r <= other && + !(r == Opsys && other == Arch) && + !(r == OpsysArch && other == EmulOpsysArch) +} + +type PlistLines struct { + all map[RelPath][]*plistLineData +} + +func NewPlistLines() *PlistLines { + return &PlistLines{make(map[RelPath][]*plistLineData)} +} + +type plistLineData struct { + line *PlistLine + rank PlistRank +} + +func (pl *PlistLines) Add(line *PlistLine, rank PlistRank) { + path := line.Path() + for _, existing := range pl.all[path] { + if existing.rank.Dominates(rank) { + line.Errorf("Path %s is already listed in %s.", + path, line.RelLine(existing.line.Line)) + } + } + pl.all[path] = append(pl.all[path], &plistLineData{line, rank}) +} diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 756c3cad403..210d8a5db34 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -731,6 +731,56 @@ func (s *Suite) Test_PlistChecker_checkDuplicate(c *check.C) { "already appeared in line 2.") } +func (s *Suite) Test_PlistChecker_checkDuplicate__OPSYS(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package") + t.Chdir("category/package") + t.CreateFileLines("PLIST", + PlistCvsID, + "bin/common", + "bin/common_end", + "${PLIST.cond}bin/conditional", + "bin/plist") + t.CreateFileLines("PLIST.Linux", + PlistCvsID, + "bin/common", + "bin/common_end", + "${PLIST.cond}bin/conditional", + "bin/os-specific", + "bin/plist") + t.CreateFileLines("PLIST.NetBSD", + PlistCvsID, + "bin/common", + "bin/common_end", + "${PLIST.cond}bin/conditional", + "bin/os-specific", + "bin/plist") + t.CreateFileLines("PLIST.common", + PlistCvsID, + "bin/common", + "${PLIST.cond}bin/conditional") + t.CreateFileLines("PLIST.common_end", + PlistCvsID, + "bin/common_end", + "${PLIST.cond}bin/conditional") + t.FinishSetUp() + + // TODO: Use the same order as in PLIST_SRC_DFLT, see mk/plist/plist.mk. + // PLIST.common + // PLIST.${OPSYS} + // PLIST.${MACHINE_ARCH:C/i[3-6]86/i386/g} + // PLIST.${OPSYS}-${MACHINE_ARCH:C/i[3-6]86/i386/g} + // ${defined(EMUL_PLATFORM):?PLIST.${EMUL_PLATFORM}:} + // PLIST + // PLIST.common_end + // + G.Check(".") + + // TODO: Warn that bin/program is duplicate, but not bin/os-specific. + t.CheckOutputEmpty() +} + func (s *Suite) Test_PlistChecker_checkPathBin(c *check.C) { t := s.Init(c) @@ -1158,15 +1208,25 @@ func (s *Suite) Test_PlistChecker_checkOmf__ok(c *check.C) { nil...) } -func (s *Suite) Test_PlistLine_Path(c *check.C) { +func (s *Suite) Test_PlistLine_HasPath(c *check.C) { t := s.Init(c) - t.CheckEquals( - (&PlistLine{text: "relative"}).Path(), - NewRelPathString("relative")) + test := func(text string, hasPath bool) { + t.CheckEquals((&PlistLine{text: text}).HasPath(), hasPath) + } - t.ExpectAssert( - func() { (&PlistLine{text: "/absolute"}).Path() }) + test("abc", true) + test("9plan", true) + test("bin/program", true) + + test("", false) + test("@", false) + test(":", false) + test("/absolute", false) + test("-rf", false) + test("\\", false) + test("bin/$<", true) + test("bin/${VAR}", true) } func (s *Suite) Test_PlistLine_HasPlainPath(c *check.C) { @@ -1190,6 +1250,17 @@ func (s *Suite) Test_PlistLine_HasPlainPath(c *check.C) { test("bin/${VAR}", false) } +func (s *Suite) Test_PlistLine_Path(c *check.C) { + t := s.Init(c) + + t.CheckEquals( + (&PlistLine{text: "relative"}).Path(), + NewRelPathString("relative")) + + t.ExpectAssert( + func() { (&PlistLine{text: "/absolute"}).Path() }) +} + func (s *Suite) Test_PlistLine_CheckTrailingWhitespace(c *check.C) { t := s.Init(c) @@ -1328,3 +1399,56 @@ func (s *Suite) Test_plistLineSorter_Sort(c *check.C) { "sbin/program", "@exec echo \"after lib/after.la\"") // The footer starts here } + +func (s *Suite) Test_PlistRank_Dominates(c *check.C) { + var rel relation + rel.add(Plain, Common) + rel.add(Common, CommonEnd) + rel.add(CommonEnd, Opsys) + rel.add(CommonEnd, Arch) + rel.add(Opsys, OpsysArch) + rel.add(Opsys, EmulOpsysArch) + rel.add(Arch, OpsysArch) + rel.add(Arch, EmulOpsysArch) + rel.reflexive = true + rel.transitive = true + rel.antisymmetric = true + + rel.check(func(a, b int) bool { return PlistRank(a).Dominates(PlistRank(b)) }) +} + +func (s *Suite) Test_NewPlistLines(c *check.C) { + lines := NewPlistLines() + + c.Check(lines.all, check.NotNil) +} + +func (s *Suite) Test_PlistLines_Add(c *check.C) { + t := s.Init(c) + + t.SetUpFileLines("PLIST", + PlistCvsID, + "bin/program") + t.SetUpFileLines("PLIST.common", + PlistCvsID, + "bin/program") + plistLines := NewPlistChecker(nil).Load(Load(t.File("PLIST"), MustSucceed)) + plistCommonLines := NewPlistChecker(nil).Load(Load(t.File("PLIST.common"), MustSucceed)) + lines := NewPlistLines() + + for _, line := range plistLines { + if line.HasPath() { + lines.Add(line, Plain) + } + } + for _, line := range plistCommonLines { + if line.HasPath() { + lines.Add(line, Common) + } + } + + t.CheckOutputLines( + // TODO: Wrong order. The diagnostics should be in the same order + // as in mk/plist/plist.mk. + "ERROR: ~/PLIST.common:2: Path bin/program is already listed in PLIST:2.") +} diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index 99c9f4449a0..0fbe14c344f 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -57,8 +57,6 @@ func (scc *SimpleCommandChecker) checkCommandStart() { break case hasPrefix(shellword, "./"): // All commands from the current directory are fine. break - case matches(shellword, `\$\{(PKGSRCDIR|PREFIX)(:Q)?\}`): - break default: if G.WarnExtra && !scc.mklines.indentation.DependsOn("OPSYS") { scc.mkline.Warnf("Unknown shell command %q.", shellword) diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index 1664ed71b9c..fc79c381d09 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -145,6 +145,7 @@ func (s *Suite) Test_SimpleCommandChecker_handleForbiddenCommand(c *check.C) { func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable(c *check.C) { t := s.Init(c) + t.SetUpVartypes() t.SetUpTool("runtime", "RUNTIME", AtRunTime) t.SetUpTool("nowhere", "NOWHERE", Nowhere) mklines := t.NewMkLines("Makefile", @@ -157,7 +158,8 @@ func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable(c *check.C) { "", "pre-configure:", "\t: ${RUNTIME_Q_CMD} ${NOWHERE_Q_CMD}", - "\t: ${RUNTIME_CMD} ${NOWHERE_CMD}") + "\t: ${RUNTIME_CMD} ${NOWHERE_CMD}", + "\t${PKGNAME}") // This doesn't make sense; it's just for code coverage mklines.Check() @@ -168,7 +170,8 @@ func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable(c *check.C) { // TODO: Add a warning that in lines 3 and 4, the :Q is wrong. t.CheckOutputLines( "WARN: Makefile:4: The \"${NOWHERE:Q}\" tool is used but not added to USE_TOOLS.", - "WARN: Makefile:6: The \"${NOWHERE}\" tool is used but not added to USE_TOOLS.") + "WARN: Makefile:6: The \"${NOWHERE}\" tool is used but not added to USE_TOOLS.", + "WARN: Makefile:11: Unknown shell command \"${PKGNAME}\".") } func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable__parameterized(c *check.C) { diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index e12ad7964e9..14a45565145 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -555,7 +555,8 @@ type Once struct { } func (o *Once) FirstTime(what string) bool { - firstTime := o.check(o.keyString(what)) + key := o.keyString(what) + firstTime := o.check(key) if firstTime && o.Trace { G.Logger.out.WriteLine(sprintf("FirstTime: %s", what)) } @@ -563,7 +564,8 @@ func (o *Once) FirstTime(what string) bool { } func (o *Once) FirstTimeSlice(whats ...string) bool { - firstTime := o.check(o.keyStrings(whats)) + key := o.keyStrings(whats) + firstTime := o.check(key) if firstTime && o.Trace { G.Logger.out.WriteLine(sprintf("FirstTime: %s", strings.Join(whats, ", "))) } @@ -1260,6 +1262,8 @@ func joinOxford(conn string, elements ...string) string { return strings.Join(nonempty, ", ") } +var pathMatchers = make(map[string]*pathMatcher) + type pathMatcher struct { matchType pathMatchType pattern string @@ -1267,6 +1271,15 @@ type pathMatcher struct { } func newPathMatcher(pattern string) *pathMatcher { + matcher := pathMatchers[pattern] + if matcher == nil { + matcher = newPathMatcherUncached(pattern) + pathMatchers[pattern] = matcher + } + return matcher +} + +func newPathMatcherUncached(pattern string) *pathMatcher { assert(strings.IndexByte(pattern, '[') == -1) assert(strings.IndexByte(pattern, '?') == -1) diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index 5070d495b60..201ba90a446 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -1267,3 +1267,93 @@ func (s *Suite) Test_interval(c *check.C) { t.CheckEquals(i.min, -5) t.CheckEquals(i.max, 7) } + +type relation struct { + pairs []struct{ a, b int } + reflexive bool + transitive bool + antisymmetric bool +} + +func (r *relation) add(a interface{}, b interface{}) { + ia := int(reflect.ValueOf(a).Uint()) + ib := int(reflect.ValueOf(b).Uint()) + r.pairs = append(r.pairs, struct{ a, b int }{ia, ib}) +} + +func (r *relation) size() int { + max := 0 + for _, pair := range r.pairs { + if pair.a > max { + max = pair.a + } + if pair.b > max { + max = pair.b + } + } + return max + 1 +} + +func (r *relation) check(actual func(int, int) bool) { + n := r.size() + rel := make([][]bool, n) + for i := 0; i < n; i++ { + rel[i] = make([]bool, n) + } + + if r.reflexive { + for i := 0; i < n; i++ { + rel[i][i] = true + } + } + + for _, pair := range r.pairs { + rel[pair.a][pair.b] = true + } + + if r.transitive { + for { + changed := false + for i := 0; i < n; i++ { + for j := 0; j < n; j++ { + for k := 0; k < n; k++ { + if rel[i][j] && rel[j][k] && !rel[i][k] { + rel[i][k] = true + changed = true + } + } + } + } + if !changed { + break + } + } + } + + if r.antisymmetric { + for i := 0; i < n; i++ { + for j := 0; j < n; j++ { + if i != j && rel[i][j] && rel[j][i] { + panic(sprintf( + "the antisymmetric relation must not contain "+ + "both (%[1]d, %[2]d) and (%[2]d, %[1]d)", + i, j)) + } + } + } + } + + actualRel := make([][]bool, n) + for i := 0; i < n; i++ { + actualRel[i] = make([]bool, n) + for j := 0; j < n; j++ { + actualRel[i][j] = actual(i, j) + } + } + + if sprintf("%#v", rel) != sprintf("%#v", actualRel) { + // The line breaks at these positions make the two relations + // visually comparable in the output. + panic(sprintf("the relation must be\n%#v, not \n%#v", rel, actualRel)) + } +} diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 445fde5039f..1e9781d6618 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -29,10 +29,11 @@ import ( type VarTypeRegistry struct { types map[string]*Vartype // varcanon => type + cache map[string][]ACLEntry } func NewVarTypeRegistry() VarTypeRegistry { - return VarTypeRegistry{make(map[string]*Vartype)} + return VarTypeRegistry{make(map[string]*Vartype), make(map[string][]ACLEntry)} } func (reg *VarTypeRegistry) Canon(varname string) *Vartype { @@ -55,10 +56,15 @@ func (reg *VarTypeRegistry) DefineType(varcanon string, vartype *Vartype) { reg.types[varcanon] = vartype } -func (reg *VarTypeRegistry) Define(varname string, basicType *BasicType, options vartypeOptions, aclEntries ...ACLEntry) { +func (reg *VarTypeRegistry) Define(varname string, basicType *BasicType, options vartypeOptions, aclEntries []ACLEntry) { m, varbase, varparam := match2(varname, `^([A-Z_.][A-Z0-9_]*|@|\.newline)(|\*|\.\*)$`) assert(m) // invalid variable name + // If this assertion fails, it usually means that + // the test calls SetUpVartypes redundantly. + // For example, it is called by SetUpPkgsrc or SetUpPackage as well. + assertf(!reg.IsDefinedExact(varname), "Variable %q must only be defined once.", varname) + vartype := NewVartype(basicType, options, aclEntries...) if varparam == "" || varparam == "*" { @@ -69,7 +75,14 @@ func (reg *VarTypeRegistry) Define(varname string, basicType *BasicType, options } } -// DefineParse defines a variable with the given type and permissions. +func (reg *VarTypeRegistry) DefineName(varname string, basicType *BasicType, options vartypeOptions, aclName string) { + aclEntries := reg.cache[aclName] + assertNotNil(aclEntries) + reg.Define(varname, basicType, options, aclEntries) +} + +// acl defines the permissions of a variable by listing the permissions +// individually. // // A permission entry looks like this: // "Makefile, Makefile.*, *.mk: default, set, append, use, use-loadtime" @@ -77,30 +90,18 @@ func (reg *VarTypeRegistry) Define(varname string, basicType *BasicType, options // to prevent typos. To use arbitrary filenames, prefix them with // "special:". // -// TODO: When prefixed with "infra:", the entry should only -// apply to files within the pkgsrc infrastructure. Without this prefix, -// the pattern should only apply to files outside the pkgsrc infrastructure. -func (reg *VarTypeRegistry) DefineParse(varname string, basicType *BasicType, options vartypeOptions, aclEntries ...string) { - parsedEntries := reg.parseACLEntries(varname, aclEntries...) - reg.Define(varname, basicType, options, parsedEntries...) -} - -// acl defines the permissions of a variable by listing the permissions -// individually. -// // Each variable that uses this function directly must document: // - which of the predefined permission sets is the closest // - how this individual permission set differs // - why the predefined permission set is not good enough // - which packages need this custom permission set. +// +// TODO: When prefixed with "infra:", the entry should only +// apply to files within the pkgsrc infrastructure. Without this prefix, +// the pattern should only apply to files outside the pkgsrc infrastructure. func (reg *VarTypeRegistry) acl(varname string, basicType *BasicType, options vartypeOptions, aclEntries ...string) { - - // If this assertion fails, it usually means that - // the test calls SetUpVartypes redundantly. - // For example, it is called by SetUpPkgsrc or SetUpPackage as well. - assertf(!reg.IsDefinedExact(varname), "Variable %q must only be defined once.", varname) - - reg.DefineParse(varname, basicType, options, aclEntries...) + parsedEntries := reg.parseACLEntries(varname, aclEntries...) + reg.Define(varname, basicType, options, parsedEntries) } // acllist defines the permissions of a list variable by listing @@ -117,27 +118,17 @@ func (reg *VarTypeRegistry) acllist(varname string, basicType *BasicType, option // A package-settable variable may be set in all Makefiles except buildlink3.mk and builtin.mk. func (reg *VarTypeRegistry) pkg(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, use") + reg.DefineName(varname, basicType, PackageSettable, "pkg") } // Like pkg, but always needs a rationale. func (reg *VarTypeRegistry) pkgrat(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable|NeedsRationale, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, use") + reg.DefineName(varname, basicType, PackageSettable|NeedsRationale, "pkg") } // pkgload is the same as pkg, except that the variable may be accessed at load time. func (reg *VarTypeRegistry) pkgload(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable, - "buildlink3.mk: none", - "builtin.mk: use, use-loadtime", - "Makefile, Makefile.*, *.mk: default, set, use, use-loadtime") + reg.DefineName(varname, basicType, PackageSettable, "pkgload") } // A package-defined list may be defined and appended to in all Makefiles @@ -146,27 +137,18 @@ func (reg *VarTypeRegistry) pkgload(varname string, basicType *BasicType) { // assignment overriding a previous value, the redundancy check will // catch it. func (reg *VarTypeRegistry) pkglist(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List|PackageSettable, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, List|PackageSettable, "pkglist") } // Like pkglist, but always needs a rationale. func (reg *VarTypeRegistry) pkglistrat(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List|PackageSettable|NeedsRationale, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, List|PackageSettable|NeedsRationale, "pkglist") } // Like pkglist, but only one value per line should be given. // Typical example: PKG_FAIL_REASON. func (reg *VarTypeRegistry) pkglistone(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List|PackageSettable|OnePerLine, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, List|PackageSettable|OnePerLine, "pkglist") } // A package-defined load-time list may be used or defined or appended to in @@ -174,10 +156,7 @@ func (reg *VarTypeRegistry) pkglistone(varname string, basicType *BasicType) { // (instead of appending) is also allowed. If this leads to an unconditional // assignment overriding a previous value, the redundancy check will catch it. func (reg *VarTypeRegistry) pkgloadlist(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List|PackageSettable, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, append, use, use-loadtime") + reg.DefineName(varname, basicType, List|PackageSettable, "pkgloadlist") } // pkgappend declares a variable that may use the += operator, @@ -192,40 +171,29 @@ func (reg *VarTypeRegistry) pkgloadlist(varname string, basicType *BasicType) { // that is sometimes composed of a common prefix and a package-specific // suffix. func (reg *VarTypeRegistry) pkgappend(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable, - "buildlink3.mk, builtin.mk: none", - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, PackageSettable, "pkgappend") } func (reg *VarTypeRegistry) pkgappendbl3(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable, - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, PackageSettable, "pkgappendbl3") } // Some package-defined variables may be modified in buildlink3.mk files. // These variables are typically related to compiling and linking files // from C and related languages. func (reg *VarTypeRegistry) pkgbl3(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable, - "Makefile, Makefile.*, *.mk: default, set, use") + reg.DefineName(varname, basicType, PackageSettable, "pkgbl3") } // Some package-defined lists may also be modified in buildlink3.mk files, // for example platform-specific CFLAGS and LDFLAGS. func (reg *VarTypeRegistry) pkglistbl3(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - List|PackageSettable, - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, List|PackageSettable, "pkglistbl3") } // Like pkglistbl3, but always needs a rationale. func (reg *VarTypeRegistry) pkglistbl3rat(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - List|PackageSettable|NeedsRationale, - "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.DefineName(varname, basicType, List|PackageSettable|NeedsRationale, "pkglistbl3") } // sys declares a user-defined or system-defined variable that must not @@ -238,41 +206,25 @@ func (reg *VarTypeRegistry) pkglistbl3rat(varname string, basicType *BasicType) // TODO: These timing issues should be handled separately from the permissions. // They can be made more precise. func (reg *VarTypeRegistry) sys(varname string, basicType *BasicType, options ...vartypeOptions) { - reg.acl(varname, basicType, - reg.options(SystemProvided, options), - "buildlink3.mk: none", - "*: use") + reg.DefineName(varname, basicType, reg.options(SystemProvided, options), "sys") } func (reg *VarTypeRegistry) sysbl3(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - SystemProvided, - "*: use") + reg.DefineName(varname, basicType, SystemProvided, "sysbl3") } func (reg *VarTypeRegistry) syslist(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List|SystemProvided, - "buildlink3.mk: none", - "*: use") + reg.DefineName(varname, basicType, List|SystemProvided, "syslist") } // usr declares a user-defined variable that must not be modified by packages. func (reg *VarTypeRegistry) usr(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - // TODO: why is builtin.mk missing here? - UserSettable, - "buildlink3.mk: none", - "*: use, use-loadtime") + reg.DefineName(varname, basicType, UserSettable, "usr") } // usr declares a user-defined list variable that must not be modified by packages. func (reg *VarTypeRegistry) usrlist(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - // TODO: why is builtin.mk missing here? - List|UserSettable, - "buildlink3.mk: none", - "*: use, use-loadtime") + reg.DefineName(varname, basicType, List|UserSettable, "usrlist") } // A few variables from mk/defaults/mk.conf may be overridden by packages. @@ -286,52 +238,37 @@ func (reg *VarTypeRegistry) usrlist(varname string, basicType *BasicType) { // // TODO: parse all the below information directly from mk/defaults/mk.conf. func (reg *VarTypeRegistry) usrpkg(varname string, basicType *BasicType) { - reg.acl(varname, basicType, - PackageSettable|UserSettable, - "Makefile: default, set, use, use-loadtime", - "buildlink3.mk, builtin.mk: none", - "Makefile.*, *.mk: default, set, use, use-loadtime", - "*: use, use-loadtime") + reg.DefineName(varname, basicType, PackageSettable|UserSettable, "usrpkg") } // sysload declares a system-provided variable that may already be used at load time. // // TODO: For some of these variables, bsd.prefs.mk has to be included first. func (reg *VarTypeRegistry) sysload(varname string, basicType *BasicType, options ...vartypeOptions) { - reg.acl(varname, basicType, - reg.options(SystemProvided, options), - "*: use, use-loadtime") + reg.DefineName(varname, basicType, reg.options(SystemProvided, options), "sysload") } func (reg *VarTypeRegistry) sysloadlist(varname string, basicType *BasicType, options ...vartypeOptions) { - reg.acl(varname, basicType, - reg.options(List|SystemProvided, options), - "*: use, use-loadtime") + reg.DefineName(varname, basicType, reg.options(List|SystemProvided, options), "sysload") } // bl3list declares a list variable that is defined by buildlink3.mk and // builtin.mk and can later be used by the package. func (reg *VarTypeRegistry) bl3list(varname string, basicType *BasicType) { - reg.acl(varname, basicType, + reg.DefineName(varname, basicType, List, // not PackageSettable since the package uses it more than setting it. - "buildlink3.mk, builtin.mk: append", - "*: use") + "bl3list") } // cmdline declares a variable that is defined on the command line. There // are only few variables of this type, such as PKG_DEBUG_LEVEL. func (reg *VarTypeRegistry) cmdline(varname string, basicType *BasicType, options ...vartypeOptions) { - reg.acl(varname, basicType, - reg.options(CommandLineProvided, options), - "buildlink3.mk, builtin.mk: none", - "*: use, use-loadtime") + reg.DefineName(varname, basicType, reg.options(CommandLineProvided, options), "cmdline") } // Only for infrastructure files; see mk/misc/show.mk func (reg *VarTypeRegistry) infralist(varname string, basicType *BasicType) { - reg.acllist(varname, basicType, - List, - "*: set, append, use") + reg.DefineName(varname, basicType, List, "infralist") } // compilerLanguages reads the available languages that are typically @@ -490,10 +427,67 @@ func (reg *VarTypeRegistry) options(base vartypeOptions, additional []vartypeOpt return opts } +func (reg *VarTypeRegistry) compile(name string, aclEntries ...string) { + reg.cache[name] = reg.parseACLEntries(name, aclEntries...) +} + // Init initializes the long list of predefined pkgsrc variables. // After this is done, PKGNAME, MAKE_ENV and all the other variables // can be used in Makefiles without triggering warnings about typos. func (reg *VarTypeRegistry) Init(src *Pkgsrc) { + reg.compile("pkg", + "buildlink3.mk, builtin.mk: none", + "Makefile, Makefile.*, *.mk: default, set, use") + reg.compile("pkgload", + "buildlink3.mk: none", + "builtin.mk: use, use-loadtime", + "Makefile, Makefile.*, *.mk: default, set, use, use-loadtime") + reg.compile("pkglist", + "buildlink3.mk, builtin.mk: none", + "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.compile("pkgloadlist", + "buildlink3.mk, builtin.mk: none", + "Makefile, Makefile.*, *.mk: default, set, append, use, use-loadtime") + reg.compile("pkgappend", + "buildlink3.mk, builtin.mk: none", + "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.compile("pkgappendbl3", + "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.compile("pkgbl3", + "Makefile, Makefile.*, *.mk: default, set, use") + reg.compile("pkglistbl3", + "Makefile, Makefile.*, *.mk: default, set, append, use") + reg.compile("sys", + "buildlink3.mk: none", + "*: use") + reg.compile("sysbl3", + "*: use") + reg.compile("syslist", + "buildlink3.mk: none", + "*: use") + reg.compile("usr", + // TODO: why is builtin.mk missing here? + "buildlink3.mk: none", + "*: use, use-loadtime") + reg.compile("usrlist", + // TODO: why is builtin.mk missing here? + "buildlink3.mk: none", + "*: use, use-loadtime") + reg.compile("usrpkg", + "Makefile: default, set, use, use-loadtime", + "buildlink3.mk, builtin.mk: none", + "Makefile.*, *.mk: default, set, use, use-loadtime", + "*: use, use-loadtime") + reg.compile("sysload", + "*: use, use-loadtime") + reg.compile("bl3list", + "buildlink3.mk, builtin.mk: append", + "*: use") + reg.compile("cmdline", + "buildlink3.mk, builtin.mk: none", + "*: use, use-loadtime") + reg.compile("infralist", + "*: set, append, use") compilers := reg.enumFrom(src, "mk/compiler.mk", diff --git a/pkgtools/pkglint/files/vardefs_test.go b/pkgtools/pkglint/files/vardefs_test.go index e78116b0449..640fb9bfb85 100644 --- a/pkgtools/pkglint/files/vardefs_test.go +++ b/pkgtools/pkglint/files/vardefs_test.go @@ -6,6 +6,7 @@ func (s *Suite) Test_VarTypeRegistry_acl__assertion(c *check.C) { t := s.Init(c) reg := NewVarTypeRegistry() + reg.compile("pkg", "*.mk: use") reg.pkg("VARNAME", BtUnknown) t.ExpectPanic( diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index 50ae70f455d..699c55a2f32 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -1,6 +1,7 @@ package pkglint import ( + "netbsd.org/pkglint/pkgver" "netbsd.org/pkglint/regex" "netbsd.org/pkglint/textproc" "path" @@ -371,6 +372,59 @@ func (cv *VartypeCheck) DependencyPattern() { "To make the \"2.0\" above part of the package basename, the hyphen", "must be omitted, so the full package name becomes \"foo2.0-2.1.x\".") } + + checkBuildlinkApiDepends := func() { + if deppat.LowerOp == "" { + return + } + pkg := cv.MkLines.pkg + if pkg == nil { + return + } + if !hasPrefix(cv.Varname, "BUILDLINK_API_DEPENDS.") { + return + } + bl3id := Buildlink3ID(varnameParam(cv.Varname)) + data := pkg.bl3Data[bl3id] + if data == nil { + return + } + if data.apiDepends.LowerOp != deppat.LowerOp { + return + } + if pkgver.Compare(deppat.Lower, data.apiDepends.Lower) < 0 { + cv.Warnf("Version %s is smaller than the default version %s from %s.", + deppat.Lower, data.apiDepends.Lower, cv.MkLine.RelMkLine(data.apiDependsLine)) + } + } + + checkBuildlinkAbiDepends := func() { + if deppat.LowerOp == "" { + return + } + pkg := cv.MkLines.pkg + if pkg == nil { + return + } + if !hasPrefix(cv.Varname, "BUILDLINK_ABI_DEPENDS.") { + return + } + bl3id := Buildlink3ID(varnameParam(cv.Varname)) + data := pkg.bl3Data[bl3id] + if data == nil { + return + } + if data.abiDepends.LowerOp != deppat.LowerOp { + return + } + if pkgver.Compare(deppat.Lower, data.abiDepends.Lower) < 0 { + cv.Warnf("Version %s is smaller than the default version %s from %s.", + deppat.Lower, data.abiDepends.Lower, cv.MkLine.RelMkLine(data.abiDependsLine)) + } + } + + checkBuildlinkApiDepends() + checkBuildlinkAbiDepends() } func (cv *VartypeCheck) DependencyWithPath() { diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index 34daf8cc839..3eb1ccc6c31 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -484,6 +484,29 @@ func (s *Suite) Test_VartypeCheck_DependencyPattern(c *check.C) { "WARN: filename.mk:67: Invalid dependency pattern \"${RUBY_PKGPREFIX}-theme-[a-z0-9]*\".") } +func (s *Suite) Test_VartypeCheck_DependencyPattern__smaller_version(c *check.C) { + t := s.Init(c) + + t.SetUpPackage("category/package", + ".include \"../../category/lib/buildlink3.mk\"", + "BUILDLINK_API_DEPENDS.lib+=\tlib>=1.0pkg", + "BUILDLINK_ABI_DEPENDS.lib+=\tlib>=1.1pkg") + t.SetUpPackage("category/lib") + t.CreateFileBuildlink3("category/lib/buildlink3.mk", + "BUILDLINK_API_DEPENDS.lib+=\tlib>=1.3api", + "BUILDLINK_ABI_DEPENDS.lib+=\tlib>=1.4abi") + t.Chdir("category/package") + t.FinishSetUp() + + G.checkdirPackage(".") + + t.CheckOutputLines( + "WARN: Makefile:21: Version 1.0pkg is smaller than the default "+ + "version 1.3api from ../../category/lib/buildlink3.mk:12.", + "WARN: Makefile:22: Version 1.1pkg is smaller than the default "+ + "version 1.4abi from ../../category/lib/buildlink3.mk:13.") +} + func (s *Suite) Test_VartypeCheck_DependencyWithPath(c *check.C) { t := s.Init(c) @@ -610,7 +633,7 @@ func (s *Suite) Test_VartypeCheck_EmulPlatform(c *check.C) { func (s *Suite) Test_VartypeCheck_Enum(c *check.C) { basicType := enum("jdk1 jdk2 jdk4") - G.Pkgsrc.vartypes.Define("JDK", basicType, UserSettable) + G.Pkgsrc.vartypes.Define("JDK", basicType, UserSettable, []ACLEntry{}) vt := NewVartypeCheckTester(s.Init(c), basicType) vt.Varname("JDK") @@ -1334,11 +1357,12 @@ func (s *Suite) Test_VartypeCheck_Pathname(c *check.C) { } func (s *Suite) Test_VartypeCheck_PathnameSpace(c *check.C) { + t := s.Init(c) // Invent a variable name since this data type is only used as part // of CONF_FILES. - G.Pkgsrc.vartypes.DefineParse("CONFIG_FILE", BtPathnameSpace, + t.SetUpType("CONFIG_FILE", BtPathnameSpace, NoVartypeOptions, "*.mk: set, use") - vt := NewVartypeCheckTester(s.Init(c), BtPathnameSpace) + vt := NewVartypeCheckTester(t, BtPathnameSpace) vt.Varname("CONFIG_FILE") vt.Values( |