Updated pkglint to 5.4.1.

Changes since 5.4.0:

* PKG_SKIP_REASON is no longer marked as deprecated, since it still
  has its value
* When PKG_SKIP_REASON is defined depending on OPSYS, suggest to
  use NOT_FOR_PLATFORM instead.
* Check for ROOT_USER/ROOT_GROUP being used in special file
  permissions; using REAL_ROOT_USER/REAL_ROOT_GROUP is better.

9 files changed, 119 insertions, 19 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index c0bfb270d0f..d645157ebae 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.485 2016/06/05 11:24:32 rillig Exp $
+# $NetBSD: Makefile,v 1.486 2016/06/10 19:42:41 rillig Exp $
 
-PKGNAME=	pkglint-5.4.0
+PKGNAME=	pkglint-5.4.1
 DISTFILES=	# none
 CATEGORIES=	pkgtools
 
diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go
index 753cbb36ec4..fc78473ac1d 100644
--- a/pkgtools/pkglint/files/globaldata.go
+++ b/pkgtools/pkglint/files/globaldata.go
@@ -478,7 +478,6 @@ func (gd *GlobalData) loadDeprecatedVars() {
 
 		// November 2006
 		"SKIP_PORTABILITY_CHECK": "Use CHECK_PORTABILITY_SKIP (a list of patterns) instead.",
-		"PKG_SKIP_REASON":        "Use PKG_FAIL_REASON instead.",
 
 		// January 2007
 		"BUILDLINK_TRANSFORM.*": "Use BUILDLINK_FNAME_TRANSFORM.* instead.",
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 0953797f768..ebe45b91baf 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -216,8 +216,9 @@ func (mkline *MkLine) checkInclude() {
 	}
 }
 
-func (mkline *MkLine) checkCond(indentation *Indentation, forVars map[string]bool) {
+func (mkline *MkLine) checkCond(forVars map[string]bool) {
 	indent, directive, args := mkline.Indent(), mkline.Directive(), mkline.Args()
+	indentation := &G.Mk.indentation
 
 	switch directive {
 	case "endif", "endfor", "elif", "else":
@@ -943,6 +944,10 @@ func (mkline *MkLine) checkVarassignSpecific() {
 	if hasPrefix(varname, "SITES_") {
 		mkline.Warn0("SITES_* is deprecated. Please use SITES.* instead.")
 	}
+
+	if varname == "PKG_SKIP_REASON" && G.Mk.indentation.DependsOn("OPSYS") {
+		mkline.Note0("Consider defining NOT_FOR_PLATFORM instead of setting PKG_SKIP_REASON depending on ${OPSYS}.")
+	}
 }
 
 func (mkline *MkLine) checkVarassignBsdPrefs() {
@@ -1224,6 +1229,34 @@ func (mkline *MkLine) CheckCond() {
 			mkline.CheckVartype(varname, opUseMatch, value, "")
 		}
 	})
+
+	mkline.rememberUsedVariables(cond)
+}
+
+func (mkline *MkLine) rememberUsedVariables(cond *Tree) {
+	if G.Mk == nil {
+		return
+	}
+
+	indentation := &G.Mk.indentation
+	arg0varname := func(node *Tree) {
+		varname := node.args[0].(string)
+		indentation.AddVar(varname)
+	}
+	arg0varuse := func(node *Tree) {
+		varuse := node.args[0].(MkVarUse)
+		indentation.AddVar(varuse.varname)
+	}
+	arg2varuse := func(node *Tree) {
+		varuse := node.args[2].(MkVarUse)
+		indentation.AddVar(varuse.varname)
+	}
+	cond.Visit("defined", arg0varname)
+	cond.Visit("empty", arg0varuse)
+	cond.Visit("compareVarNum", arg0varuse)
+	cond.Visit("compareVarStr", arg0varuse)
+	cond.Visit("compareVarVar", arg0varuse)
+	cond.Visit("compareVarVar", arg2varuse)
 }
 
 func (mkline *MkLine) CheckValidCharactersInValue(reValid string) {
@@ -1679,10 +1712,42 @@ func (vuc *VarUseContext) String() string {
 }
 
 type Indentation struct {
-	data []int
+	depth         []int             // Number of space characters; always a multiple of 2
+	conditionVars []map[string]bool // Variables on which the current path depends
+}
+
+func (ind *Indentation) Len() int {
+	return len(ind.depth)
 }
 
-func (ind *Indentation) Len() int        { return len(ind.data) }
-func (ind *Indentation) Depth() int      { return ind.data[len(ind.data)-1] }
-func (ind *Indentation) Pop()            { ind.data = ind.data[:len(ind.data)-1] }
-func (ind *Indentation) Push(indent int) { ind.data = append(ind.data, indent) }
+func (ind *Indentation) Depth() int {
+	return ind.depth[len(ind.depth)-1]
+}
+
+func (ind *Indentation) Pop() {
+	newlen := ind.Len() - 1
+	ind.depth = ind.depth[:newlen]
+	ind.conditionVars = ind.conditionVars[:newlen]
+}
+
+func (ind *Indentation) Push(indent int) {
+	ind.depth = append(ind.depth, indent)
+	ind.conditionVars = append(ind.conditionVars, nil)
+}
+
+func (ind *Indentation) AddVar(varname string) {
+	level := ind.Len() - 1
+	if ind.conditionVars[level] == nil {
+		ind.conditionVars[level] = make(map[string]bool)
+	}
+	ind.conditionVars[level][varname] = true
+}
+
+func (ind *Indentation) DependsOn(varname string) bool {
+	for _, vars := range ind.conditionVars {
+		if vars[varname] {
+			return true
+		}
+	}
+	return false
+}
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index 0ea785ea4b9..9735a2a93a7 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -17,6 +17,7 @@ type MkLines struct {
 	plistVars      map[string]bool    // Variables that are registered in PLIST_VARS, to ensure that all user-defined variables are added to it.
 	tools          map[string]bool    // Set of tools that are declared to be used.
 	SeenBsdPrefsMk bool
+	indentation    Indentation // Indentation depth of preprocessing directives
 }
 
 func NewMkLines(lines []*Line) *MkLines {
@@ -41,7 +42,8 @@ func NewMkLines(lines []*Line) *MkLines {
 		make(map[string]bool),
 		make(map[string]bool),
 		tools,
-		false}
+		false,
+		Indentation{}}
 }
 
 func (mklines *MkLines) DefineVar(mkline *MkLine, varname string) {
@@ -99,7 +101,8 @@ func (mklines *MkLines) Check() {
 
 	var substcontext SubstContext
 	var varalign VaralignBlock
-	indentation := Indentation{[]int{0}} // Indentation depth of preprocessing directives
+	indentation := &mklines.indentation
+	indentation.Push(0)
 	for _, mkline := range mklines.mklines {
 		mkline.Check()
 		varalign.Check(mkline)
@@ -120,7 +123,7 @@ func (mklines *MkLines) Check() {
 			}
 
 		case mkline.IsCond():
-			mkline.checkCond(&indentation, mklines.forVars)
+			mkline.checkCond(mklines.forVars)
 
 		case mkline.IsDependency():
 			mkline.checkDependencyRule(allowedTargets)
diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go
index 750a45de08e..8de3aafd8dd 100644
--- a/pkgtools/pkglint/files/mklines_test.go
+++ b/pkgtools/pkglint/files/mklines_test.go
@@ -244,3 +244,17 @@ func (s *Suite) Test_MkLines_LoopModifier(c *check.C) {
 		"WARN: chat/xchat/Makefile:4: Unknown shell command \"${GCONF_SCHEMAS:@.s.@"+
 		"${INSTALL_DATA} ${WRKSRC}/src/common/dbus/${.s.} ${DESTDIR}${GCONF_SCHEMAS_DIR}/@}\".\n")
 }
+
+func (s *Suite) Test_MkLines_Indentation_DependsOn(c *check.C) {
+	G.globalData.InitVartypes()
+	mklines := s.NewMkLines("Makefile",
+		"# $"+"NetBSD$",
+		"PKG_SKIP_REASON+=\t\"Fails everywhere\"",
+		".if ${OPSYS} == \"Cygwin\"",
+		"PKG_SKIP_REASON+=\t\"Fails on Cygwin\"",
+		".endif")
+
+	mklines.Check()
+
+	c.Check(s.Output(), equals, "NOTE: Makefile:4: Consider defining NOT_FOR_PLATFORM instead of setting PKG_SKIP_REASON depending on ${OPSYS}.\n")
+}
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 9027a707521..a65343e620d 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -206,7 +206,7 @@ func (gd *GlobalData) InitVartypes() {
 	pkglist("CONFLICTS", lkSpace, CheckvarDependency)
 	pkglist("CONF_FILES", lkShell, CheckvarShellWord)
 	pkg("CONF_FILES_MODE", lkNone, enum("0644 0640 0600 0400"))
-	pkglist("CONF_FILES_PERMS", lkShell, CheckvarShellWord)
+	pkglist("CONF_FILES_PERMS", lkShell, CheckvarPerms)
 	sys("COPY", lkNone, enum("-c")) // The flag that tells ${INSTALL} to copy a file
 	sys("CPP", lkNone, CheckvarShellCommand)
 	pkglist("CPPFLAGS*", lkShell, CheckvarCFlag)
@@ -400,7 +400,7 @@ func (gd *GlobalData) InitVartypes() {
 	pkglist("MAKEFLAGS", lkShell, CheckvarShellWord)
 	acl("MAKEVARS", lkShell, CheckvarVarname, "builtin.mk: append; buildlink3.mk: append; hacks.mk: append")
 	pkglist("MAKE_DIRS", lkShell, CheckvarPathname)
-	pkglist("MAKE_DIRS_PERMS", lkShell, CheckvarShellWord)
+	pkglist("MAKE_DIRS_PERMS", lkShell, CheckvarPerms)
 	acl("MAKE_ENV", lkShell, CheckvarShellWord, "Makefile: append, set, use; Makefile.common: append, set, use; buildlink3.mk: append; builtin.mk: append; *.mk: append, use")
 	pkg("MAKE_FILE", lkNone, CheckvarPathname)
 	pkglist("MAKE_FLAGS", lkShell, CheckvarShellWord)
@@ -477,7 +477,7 @@ func (gd *GlobalData) InitVartypes() {
 	pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", lkNone, CheckvarYes)
 	acl("OWNER", lkNone, CheckvarMailAddress, "Makefile: set; Makefile.common: default")
 	pkglist("OWN_DIRS", lkShell, CheckvarPathname)
-	pkglist("OWN_DIRS_PERMS", lkShell, CheckvarShellWord)
+	pkglist("OWN_DIRS_PERMS", lkShell, CheckvarPerms)
 	sys("PAMBASE", lkNone, CheckvarPathname)
 	usr("PAM_DEFAULT", lkNone, enum("linux-pam openpam solaris-pam"))
 	acl("PATCHDIR", lkNone, CheckvarRelativePkgPath, "Makefile: set; Makefile.common: default, set")
@@ -564,7 +564,7 @@ func (gd *GlobalData) InitVartypes() {
 	acl("PKG_SUGGESTED_OPTIONS", lkShell, CheckvarOption, "options.mk: set, append; Makefile: set, append; Makefile.common: set")
 	acl("PKG_SUPPORTED_OPTIONS", lkShell, CheckvarOption, "options.mk: set, append, use; Makefile: set, append; Makefile.common: set")
 	pkg("PKG_SYSCONFDIR*", lkNone, CheckvarPathname)
-	pkglist("PKG_SYSCONFDIR_PERMS", lkShell, CheckvarShellWord)
+	pkglist("PKG_SYSCONFDIR_PERMS", lkShell, CheckvarPerms)
 	sys("PKG_SYSCONFBASEDIR", lkNone, CheckvarPathname)
 	pkg("PKG_SYSCONFSUBDIR", lkNone, CheckvarPathname)
 	acl("PKG_SYSCONFVAR", lkNone, CheckvarIdentifier, "") // FIXME: name/type mismatch.
@@ -614,10 +614,10 @@ func (gd *GlobalData) InitVartypes() {
 	pkglist("REPLACE_PYTHON", lkShell, CheckvarPathmask)
 	pkglist("REPLACE_SH", lkShell, CheckvarPathmask)
 	pkglist("REQD_DIRS", lkShell, CheckvarPathname)
-	pkglist("REQD_DIRS_PERMS", lkShell, CheckvarShellWord)
+	pkglist("REQD_DIRS_PERMS", lkShell, CheckvarPerms)
 	pkglist("REQD_FILES", lkShell, CheckvarPathname)
 	pkg("REQD_FILES_MODE", lkNone, enum("0644 0640 0600 0400"))
-	pkglist("REQD_FILES_PERMS", lkShell, CheckvarShellWord)
+	pkglist("REQD_FILES_PERMS", lkShell, CheckvarPerms)
 	pkg("RESTRICTED", lkNone, CheckvarMessage)
 	usr("ROOT_USER", lkNone, CheckvarUserGroupName)
 	usr("ROOT_GROUP", lkNone, CheckvarUserGroupName)
@@ -634,7 +634,7 @@ func (gd *GlobalData) InitVartypes() {
 	acl("SHLIBTOOL", lkNone, CheckvarShellCommand, "Makefile: use")
 	acl("SHLIBTOOL_OVERRIDE", lkShell, CheckvarPathmask, "Makefile: set, append; Makefile.common: append")
 	acl("SITES.*", lkShell, CheckvarFetchURL, "Makefile, Makefile.common, options.mk: set, append, use")
-	pkglist("SPECIAL_PERMS", lkShell, CheckvarShellWord)
+	pkglist("SPECIAL_PERMS", lkShell, CheckvarPerms)
 	sys("STEP_MSG", lkNone, CheckvarShellCommand)
 	acl("SUBDIR", lkShell, CheckvarFilename, "Makefile: append; *:")
 	acl("SUBST_CLASSES", lkShell, CheckvarIdentifier, "Makefile: set, append; *: append")
@@ -672,6 +672,7 @@ func (gd *GlobalData) InitVartypes() {
 	acl("USE_BUILTIN.*", lkNone, CheckvarYesNoIndirectly, "builtin.mk: set")
 	pkg("USE_CMAKE", lkNone, CheckvarYes)
 	acl("USE_CROSSBASE", lkNone, CheckvarYes, "Makefile: set")
+	usr("USE_DESTDIR", lkNone, CheckvarYes)
 	pkg("USE_FEATURES", lkShell, CheckvarIdentifier)
 	pkg("USE_GCC_RUNTIME", lkNone, CheckvarYesNo)
 	pkg("USE_GNU_CONFIGURE_HOST", lkNone, CheckvarYesNo)
diff --git a/pkgtools/pkglint/files/vartype.go b/pkgtools/pkglint/files/vartype.go
index c385712ac67..bafba977eec 100644
--- a/pkgtools/pkglint/files/vartype.go
+++ b/pkgtools/pkglint/files/vartype.go
@@ -236,6 +236,7 @@ var (
 	CheckvarPathmask               = &VarChecker{"Pathmask", (*VartypeCheck).Pathmask}
 	CheckvarPathname               = &VarChecker{"Pathname", (*VartypeCheck).Pathname}
 	CheckvarPerl5Packlist          = &VarChecker{"Perl5Packlist", (*VartypeCheck).Perl5Packlist}
+	CheckvarPerms                  = &VarChecker{"Perms", (*VartypeCheck).Perms}
 	CheckvarPkgName                = &VarChecker{"PkgName", (*VartypeCheck).PkgName}
 	CheckvarPkgPath                = &VarChecker{"PkgPath", (*VartypeCheck).PkgPath}
 	CheckvarPkgOptionsVar          = &VarChecker{"PkgOptionsVar", (*VartypeCheck).PkgOptionsVar}
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index b60e05abf9f..76d1d741311 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -662,6 +662,13 @@ func (cv *VartypeCheck) Perl5Packlist() {
 	}
 }
 
+func (cv *VartypeCheck) Perms() {
+	if cv.value == "${ROOT_USER}" || cv.value == "${ROOT_GROUP}" {
+		valuename := cv.value[2 : len(cv.value)-1]
+		cv.line.Error1("%s must not be used in permission definitions. Use REAL_%[1]s instead.", valuename)
+	}
+}
+
 func (cv *VartypeCheck) PkgName() {
 	if cv.op != opUseMatch && cv.value == cv.valueNovar && !matches(cv.value, rePkgname) {
 		cv.line.Warn1("%q is not a valid package name. A valid package name has the form packagename-version, where version consists only of digits, letters and dots.", cv.value)
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index d5a5be6cafd..e15ebaf2f30 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -277,6 +277,16 @@ func (s *Suite) TestVartypeCheck_Pathlist(c *check.C) {
 	c.Check(s.Output(), equals, "WARN: fname:1: All components of PATH (in this case \".\") should be absolute paths.\n")
 }
 
+func (s *Suite) Test_VartypeCheck_Perms(c *check.C) {
+	runVartypeChecks("CONF_FILES_PERMS", opAssignAppend, (*VartypeCheck).Perms,
+		"root",
+		"${ROOT_USER}",
+		"ROOT_USER",
+		"${REAL_ROOT_USER}")
+
+	c.Check(s.Output(), equals, "ERROR: fname:2: ROOT_USER must not be used in permission definitions. Use REAL_ROOT_USER instead.\n")
+}
+
 func (s *Suite) TestVartypeCheck_PkgOptionsVar(c *check.C) {
 	runVartypeChecks("PKG_OPTIONS_VAR.screen", opAssign, (*VartypeCheck).PkgOptionsVar,
 		"PKG_OPTIONS.${PKGBASE}")