pkgtools/pkglint: update to 5.5.3

Changes since 5.5.2:

* Fixed lots of bugs regarding autofixing variable assignments in
  continuation lines.
* Fixed checking of MESSAGE files, which also get fixed now.
* In variable assignments, commented assignments are aligned too.
* Fixed a crash when checking an empty patch file.
* The :Q modifier is only checked on predefined variables, to prevent
  the --autofix mode from removing :Q from user-defined variables.
* Fixed lots of bugs in PLIST autofixing: relevant lines had been
  removed, and the sorting was not correct.

21 files changed, 652 insertions, 147 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index bbbbab8af83..ce4dfe4847d 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.527 2018/01/28 13:40:22 rillig Exp $
+# $NetBSD: Makefile,v 1.528 2018/01/28 23:21:16 rillig Exp $
 
-PKGNAME=	pkglint-5.5.2
+PKGNAME=	pkglint-5.5.3
 DISTFILES=	# none
 CATEGORIES=	pkgtools
 
diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go
index 0063d5d2eb0..3604ed1ae9e 100644
--- a/pkgtools/pkglint/files/autofix.go
+++ b/pkgtools/pkglint/files/autofix.go
@@ -62,25 +62,42 @@ func (fix *Autofix) ReplaceAfter(prefix, from string, to string) {
 	}
 }
 
-func (fix *Autofix) ReplaceRegex(from regex.Pattern, to string) {
+// ReplaceRegex replaces the first or all occurrences of the `from` pattern
+// with the fixed string `toText`. Placeholders like `$1` are _not_ expanded.
+// (If you know how to do the expansion correctly, feel free to implement it.)
+func (fix *Autofix) ReplaceRegex(from regex.Pattern, toText string, howOften int) {
 	if fix.skip() {
 		return
 	}
 
+	done := 0
 	for _, rawLine := range fix.lines {
 		if rawLine.Lineno != 0 {
-			if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl {
+			var froms []string // The strings that have actually changed
+
+			replace := func(fromText string) string {
+				if howOften >= 0 && done >= howOften {
+					return fromText
+				}
+				froms = append(froms, fromText)
+				done++
+				return toText
+			}
+
+			if replaced := regex.Compile(from).ReplaceAllStringFunc(rawLine.textnl, replace); replaced != rawLine.textnl {
 				if G.opts.PrintAutofix || G.opts.Autofix {
 					rawLine.textnl = replaced
 				}
-				fix.Describef(rawLine.Lineno, "Replacing regular expression %q with %q.", from, to)
+				for _, fromText := range froms {
+					fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
+				}
 			}
 		}
 	}
 }
 
 func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
-	if fix.skip() || !mkline.IsMultiline() || !mkline.IsVarassign() {
+	if fix.skip() || !mkline.IsMultiline() || !(mkline.IsVarassign() || mkline.IsCommentedVarassign()) {
 		return
 	}
 
@@ -90,19 +107,19 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
 	{
 		// Interpreting the continuation marker as variable value
 		// is cheating, but works well.
-		m, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
+		m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
 		if m && value != "\\" {
 			oldWidth = tabWidth(valueAlign)
 		}
 	}
 
 	for _, rawLine := range fix.lines[1:] {
-		_, space := regex.Match1(rawLine.textnl, `^(\s*)`)
-		width := tabWidth(space)
-		if oldWidth == 0 || width < oldWidth {
+		_, comment, space := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
+		width := tabWidth(comment + space)
+		if (oldWidth == 0 || width < oldWidth) && width >= 8 && rawLine.textnl != "\n" {
 			oldWidth = width
 		}
-		if !regex.Matches(space, `^\t*\s{0,7}`) {
+		if !regex.Matches(space, `^\t* {0,7}`) {
 			normalized = false
 		}
 	}
@@ -119,10 +136,11 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
 	}
 
 	for _, rawLine := range fix.lines[1:] {
-		_, oldSpace := regex.Match1(rawLine.textnl, `^(\s*)`)
+		_, comment, oldSpace := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
 		newWidth := tabWidth(oldSpace) - oldWidth + newWidth
 		newSpace := strings.Repeat("\t", newWidth/8) + strings.Repeat(" ", newWidth%8)
-		if replaced := strings.Replace(rawLine.textnl, oldSpace, newSpace, 1); replaced != rawLine.textnl {
+		replaced := strings.Replace(rawLine.textnl, comment+oldSpace, comment+newSpace, 1)
+		if replaced != rawLine.textnl {
 			if G.opts.PrintAutofix || G.opts.Autofix {
 				rawLine.textnl = replaced
 			}
@@ -131,6 +149,8 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
 	}
 }
 
+// InsertBefore prepends a line before the current line.
+// The newline is added internally.
 func (fix *Autofix) InsertBefore(text string) {
 	if fix.skip() {
 		return
@@ -140,6 +160,8 @@ func (fix *Autofix) InsertBefore(text string) {
 	fix.Describef(fix.lines[0].Lineno, "Inserting a line %q before this line.", text)
 }
 
+// InsertBefore appends a line after the current line.
+// The newline is added internally.
 func (fix *Autofix) InsertAfter(text string) {
 	if fix.skip() {
 		return
diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go
index bf9c4d08bc9..e4e20628ab4 100644
--- a/pkgtools/pkglint/files/autofix_test.go
+++ b/pkgtools/pkglint/files/autofix_test.go
@@ -13,7 +13,7 @@ func (s *Suite) Test_Autofix_ReplaceRegex(c *check.C) {
 
 	fix := lines[1].Autofix()
 	fix.Warnf("Something's wrong here.")
-	fix.ReplaceRegex(`.`, "X")
+	fix.ReplaceRegex(`.`, "X", -1)
 	fix.Apply()
 	SaveAutofixChanges(lines)
 
@@ -24,7 +24,11 @@ func (s *Suite) Test_Autofix_ReplaceRegex(c *check.C) {
 		"line3")
 	t.CheckOutputLines(
 		"WARN: ~/Makefile:2: Something's wrong here.",
-		"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
+		"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".")
 }
 
 func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) {
@@ -38,27 +42,32 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) {
 
 	fix := lines[1].Autofix()
 	fix.Warnf("Something's wrong here.")
-	fix.ReplaceRegex(`.`, "X")
+	fix.ReplaceRegex(`.`, "X", 3)
 	fix.Apply()
 
+	t.CheckOutputLines(
+		"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+		"-\tline2",
+		"+\tXXXe2")
+
 	fix.Warnf("Use Y instead of X.")
 	fix.Replace("X", "Y")
 	fix.Apply()
 
+	t.CheckOutputLines(
+		"",
+		"AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
+		"-\tline2",
+		"+\tYXXe2")
+
 	SaveAutofixChanges(lines)
 
 	t.CheckFileLines("Makefile",
 		"line1",
-		"YXXXX",
+		"YXXe2",
 		"line3")
-	t.CheckOutputLines(
-		"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
-		"-\tline2",
-		"+\tXXXXX",
-		"",
-		"AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
-		"-\tline2",
-		"+\tYXXXX")
 }
 
 func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
@@ -72,7 +81,7 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
 
 	fix := lines[1].Autofix()
 	fix.Warnf("Something's wrong here.")
-	fix.ReplaceRegex(`.`, "X")
+	fix.ReplaceRegex(`.`, "X", -1)
 	fix.Apply()
 
 	fix.Warnf("Use Y instead of X.")
@@ -83,7 +92,11 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
 
 	t.CheckOutputLines(
 		"WARN: ~/Makefile:2: Something's wrong here.",
-		"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".",
 		"-\tline2",
 		"+\tXXXXX",
 		"",
@@ -108,17 +121,27 @@ func (s *Suite) Test_autofix_MkLines(c *check.C) {
 
 	fix := mklines.mklines[1].Autofix()
 	fix.Warnf("Something's wrong here.")
-	fix.ReplaceRegex(`.`, "X")
+	fix.ReplaceRegex(`...`, "XXX", -1)
+	fix.Apply()
+
+	fix = mklines.mklines[2].Autofix()
+	fix.Warnf("Something's wrong here.")
+	fix.ReplaceRegex(`...`, "XXX", 1)
 	fix.Apply()
 
 	SaveAutofixChanges(mklines.lines)
 
+	t.CheckOutputLines(
+		"AUTOFIX: ~/Makefile:2: Replacing \"lin\" with \"XXX\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"e2 \" with \"XXX\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \":= \" with \"XXX\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"val\" with \"XXX\".",
+		"AUTOFIX: ~/Makefile:2: Replacing \"ue2\" with \"XXX\".",
+		"AUTOFIX: ~/Makefile:3: Replacing \"lin\" with \"XXX\".")
 	t.CheckFileLines("Makefile",
 		"line1 := value1",
 		"XXXXXXXXXXXXXXX",
-		"line3 := value3")
-	t.CheckOutputLines(
-		"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
+		"XXXe3 := value3")
 }
 
 func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) {
@@ -134,14 +157,14 @@ func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) {
 	{
 		fix := line.Autofix()
 		fix.Warnf("Silent-Magic-Diagnostic")
-		fix.ReplaceRegex(`(.)(.*)(.)`, "$3$2$1")
+		fix.ReplaceRegex(`(.)(.*)(.)`, "lriginao", 1) // XXX: the replacement should be "$3$2$1"
 		fix.Apply()
 	}
 
 	c.Check(line.autofix, check.NotNil)
 	c.Check(line.raw, check.DeepEquals, t.NewRawLines(1, "original\n", "lriginao\n"))
 	t.CheckOutputLines(
-		"AUTOFIX: fname:1: Replacing regular expression \"(.)(.*)(.)\" with \"$3$2$1\".")
+		"AUTOFIX: fname:1: Replacing \"original\" with \"lriginao\".")
 
 	{
 		fix := line.Autofix()
@@ -304,7 +327,7 @@ func (s *Suite) Test_Autofix_suppress_unfixable_warnings(c *check.C) {
 
 	fix := lines[1].Autofix()
 	fix.Warnf("Something's wrong here.")
-	fix.ReplaceRegex(`.`, "X")
+	fix.ReplaceRegex(`.`, "X", -1)
 	fix.Apply()
 
 	fix.Warnf("The XXX marks are usually not fixed, use TODO instead.")
@@ -315,7 +338,11 @@ func (s *Suite) Test_Autofix_suppress_unfixable_warnings(c *check.C) {
 
 	t.CheckOutputLines(
 		"WARN: Makefile:2: Something's wrong here.",
-		"AUTOFIX: Makefile:2: Replacing regular expression \".\" with \"X\".",
+		"AUTOFIX: Makefile:2: Replacing \"l\" with \"X\".",
+		"AUTOFIX: Makefile:2: Replacing \"i\" with \"X\".",
+		"AUTOFIX: Makefile:2: Replacing \"n\" with \"X\".",
+		"AUTOFIX: Makefile:2: Replacing \"e\" with \"X\".",
+		"AUTOFIX: Makefile:2: Replacing \"2\" with \"X\".",
 		"-\tline2",
 		"+\tXXXXX",
 		"",
@@ -333,7 +360,7 @@ func (s *Suite) Test_Autofix_failed_replace(c *check.C) {
 
 	fix := line.Autofix()
 	fix.Warnf("All-uppercase words should not be used at all.")
-	fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---")
+	fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---", -1)
 	fix.Apply()
 
 	// No output since there was no all-uppercase word in the text.
diff --git a/pkgtools/pkglint/files/category.go b/pkgtools/pkglint/files/category.go
index c5f7ecced45..05a27c1da65 100644
--- a/pkgtools/pkglint/files/category.go
+++ b/pkgtools/pkglint/files/category.go
@@ -41,7 +41,7 @@ func CheckdirCategory() {
 	// collect the SUBDIRs in the Makefile and in the file system.
 
 	fSubdirs := getSubdirs(G.CurrentDir)
-	sort.Sort(sort.StringSlice(fSubdirs))
+	sort.Strings(fSubdirs)
 	var mSubdirs []subdir
 
 	prevSubdir := ""
diff --git a/pkgtools/pkglint/files/files.go b/pkgtools/pkglint/files/files.go
index 46d08c83ffe..7997a97b803 100644
--- a/pkgtools/pkglint/files/files.go
+++ b/pkgtools/pkglint/files/files.go
@@ -26,7 +26,7 @@ func LoadExistingLines(fname string, joinBackslashLines bool) []Line {
 	return lines
 }
 
-func getLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
+func nextLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
 	{ // Handle the common case efficiently
 		index := *pindex
 		rawLine := rawLines[index]
@@ -68,26 +68,30 @@ func getLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
 }
 
 func splitRawLine(textnl string) (leadingWhitespace, text, trailingWhitespace, cont string) {
-	i, m := 0, len(textnl)
+	end := len(textnl)
 
-	if m > i && textnl[m-1] == '\n' {
-		m--
+	if end-1 >= 0 && textnl[end-1] == '\n' {
+		end--
 	}
 
-	if m > i && textnl[m-1] == '\\' {
-		m--
-		cont = textnl[m : m+1]
+	backslashes := 0
+	for end-1 >= 0 && textnl[end-1] == '\\' {
+		end--
+		backslashes++
 	}
+	cont = textnl[end : end+backslashes%2]
+	end += backslashes / 2
 
-	trailingEnd := m
-	for m > i && (textnl[m-1] == ' ' || textnl[m-1] == '\t') {
-		m--
+	trailingEnd := end
+	for end-1 >= 0 && (textnl[end-1] == ' ' || textnl[end-1] == '\t') {
+		end--
 	}
-	trailingStart := m
+	trailingStart := end
 	trailingWhitespace = textnl[trailingStart:trailingEnd]
 
+	i := 0
 	leadingStart := i
-	for i < m && (textnl[i] == ' ' || textnl[i] == '\t') {
+	for i < end && (textnl[i] == ' ' || textnl[i] == '\t') {
 		i++
 	}
 	leadingEnd := i
@@ -117,7 +121,7 @@ func convertToLogicalLines(fname string, rawText string, joinBackslashLines bool
 	var loglines []Line
 	if joinBackslashLines {
 		for lineno := 0; lineno < len(rawLines); {
-			loglines = append(loglines, getLogicalLine(fname, rawLines, &lineno))
+			loglines = append(loglines, nextLogicalLine(fname, rawLines, &lineno))
 		}
 	} else {
 		for _, rawLine := range rawLines {
diff --git a/pkgtools/pkglint/files/files_test.go b/pkgtools/pkglint/files/files_test.go
index 6bc857c65f8..255ab2ff33a 100644
--- a/pkgtools/pkglint/files/files_test.go
+++ b/pkgtools/pkglint/files/files_test.go
@@ -1,7 +1,7 @@
 package main
 
 import (
-	check "gopkg.in/check.v1"
+	"gopkg.in/check.v1"
 )
 
 func (s *Suite) Test_convertToLogicalLines_no_continuation(c *check.C) {
@@ -29,6 +29,92 @@ func (s *Suite) Test_convertToLogicalLines_continuation(c *check.C) {
 	c.Check(lines[1].String(), equals, "fname_cont:3: third")
 }
 
+// In Makefiles, comment lines can also have continuations.
+// See devel/bmake/files/unit-tests/comment.mk
+func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) {
+	t := s.Init(c)
+
+	lines := t.SetupFileLinesContinuation("comment.mk",
+		"# This is a comment",
+		"",
+		"#\\",
+		"\tMultiline comment",
+		"# Another escaped comment \\",
+		"that \\",
+		"goes \\",
+		"on",
+		"# This is NOT an escaped comment due to the double backslashes \\\\",
+		"VAR=\tThis is not a comment",
+		"",
+		"#\\",
+		"This is a comment",
+		"#\\\\",
+		"This is no comment",
+		"#\\\\\\",
+		"This is a comment",
+		"#\\\\\\\\",
+		"This is no comment",
+		"#\\\\\\\\\\",
+		"This is a comment",
+		"#\\\\\\\\\\\\",
+		"This is no comment")
+
+	var texts []string
+	for _, line := range lines {
+		texts = append(texts, line.Text)
+	}
+
+	c.Check(texts, deepEquals, []string{
+		"# This is a comment",
+		"",
+		"# Multiline comment",
+		"# Another escaped comment that goes on",
+		"# This is NOT an escaped comment due to the double backslashes \\",
+		"VAR=\tThis is not a comment",
+		"",
+		"# This is a comment",
+		"#\\",
+		"This is no comment",
+		"#\\ This is a comment",
+		"#\\\\",
+		"This is no comment",
+		"#\\\\ This is a comment",
+		"#\\\\\\",
+		"This is no comment"})
+
+	var rawTexts []string
+	for _, line := range lines {
+		for _, rawLine := range line.raw {
+			rawTexts = append(rawTexts, rawLine.textnl)
+		}
+	}
+
+	c.Check(rawTexts, deepEquals, []string{
+		"# This is a comment\n",
+		"\n",
+		"#\\\n",
+		"\tMultiline comment\n",
+		"# Another escaped comment \\\n",
+		"that \\\n",
+		"goes \\\n",
+		"on\n",
+		"# This is NOT an escaped comment due to the double backslashes \\\\\n",
+		"VAR=\tThis is not a comment\n",
+		"\n",
+		"#\\\n",
+		"This is a comment\n",
+		"#\\\\\n",
+		"This is no comment\n",
+		"#\\\\\\\n",
+		"This is a comment\n",
+		"#\\\\\\\\\n",
+		"This is no comment\n",
+		"#\\\\\\\\\\\n",
+		"This is a comment\n",
+		"#\\\\\\\\\\\\\n",
+		"This is no comment\n"})
+}
+
 func (s *Suite) Test_convertToLogicalLines_continuationInLastLine(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go
index 9af8b61a1b5..4646d3700ef 100644
--- a/pkgtools/pkglint/files/globaldata.go
+++ b/pkgtools/pkglint/files/globaldata.go
@@ -108,8 +108,8 @@ func (gd *GlobalData) loadDistSites() {
 	name2url := make(map[string]string)
 	url2name := make(map[string]string)
 	for _, line := range lines {
-		if m, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
-			if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
+		if m, commented, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
+			if !commented && hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
 				for _, url := range splitOnSpace(urls) {
 					if matches(url, `^(?:http://|https://|ftp://)`) {
 						if name2url[varname] == "" {
@@ -187,8 +187,11 @@ func (gd *GlobalData) loadTools() {
 		lines := LoadExistingLines(fname, true)
 		for _, line := range lines {
 			text := line.Text
+			if hasPrefix(text, "#") {
+				continue
+			}
 
-			if m, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
+			if m, _, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
 				if varname == "USE_TOOLS" {
 					if trace.Tracing {
 						trace.Stepf("[condDepth=%d] %s", condDepth, value)
@@ -618,7 +621,10 @@ func (tr *ToolRegistry) Trace() {
 }
 
 func (tr *ToolRegistry) ParseToolLine(line Line) {
-	if m, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
+	if m, commented, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
+		if commented {
+			return
+		}
 		if varname == "TOOLS_CREATE" && (value == "[" || matches(value, `^?[-\w.]+$`)) {
 			tr.Register(value)
 
diff --git a/pkgtools/pkglint/files/histogram/histogram.go b/pkgtools/pkglint/files/histogram/histogram.go
index a915c382daf..ad802d949a9 100644
--- a/pkgtools/pkglint/files/histogram/histogram.go
+++ b/pkgtools/pkglint/files/histogram/histogram.go
@@ -19,6 +19,11 @@ func (h *Histogram) Add(s string, n int) {
 }
 
 func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
+	type entry struct {
+		s     string
+		count int
+	}
+
 	entries := make([]entry, len(h.histo))
 
 	i := 0
@@ -27,7 +32,11 @@ func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
 		i++
 	}
 
-	sort.Sort(byCountDesc(entries))
+	sort.SliceStable(entries, func(i, j int) bool {
+		ei := entries[i]
+		ej := entries[j]
+		return ej.count < ei.count || (ei.count == ej.count && ei.s < ej.s)
+	})
 
 	for i, entry := range entries {
 		fmt.Fprintf(out, "%s %6d %s\n", caption, entry.count, entry.s)
@@ -36,20 +45,3 @@ func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
 		}
 	}
 }
-
-type entry struct {
-	s     string
-	count int
-}
-
-type byCountDesc []entry
-
-func (a byCountDesc) Len() int {
-	return len(a)
-}
-func (a byCountDesc) Swap(i, j int) {
-	a[i], a[j] = a[j], a[i]
-}
-func (a byCountDesc) Less(i, j int) bool {
-	return a[j].count < a[i].count || (a[i].count == a[j].count && a[i].s < a[j].s)
-}
diff --git a/pkgtools/pkglint/files/linechecker.go b/pkgtools/pkglint/files/linechecker.go
index 37aaa989e4f..d6c53840772 100644
--- a/pkgtools/pkglint/files/linechecker.go
+++ b/pkgtools/pkglint/files/linechecker.go
@@ -54,7 +54,7 @@ func CheckLineTrailingWhitespace(line Line) {
 		fix.Explain(
 			"When a line ends with some white-space, that space is in most cases",
 			"irrelevant and can be removed.")
-		fix.ReplaceRegex(`\s+\n$`, "\n")
+		fix.ReplaceRegex(`[ \t\r]+\n$`, "\n", 1)
 		fix.Apply()
 	}
 }
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index 61ed55c58b5..7d7a1a619a9 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -16,6 +16,7 @@ type MkLineImpl struct {
 	data interface{} // One of the following mkLine* types
 }
 type mkLineAssign struct {
+	commented  bool       // Whether the whole variable assignment is commented out
 	varname    string     // e.g. "HOMEPAGE", "SUBST_SED.perl"
 	varcanon   string     // e.g. "HOMEPAGE", "SUBST_SED.*"
 	varparam   string     // e.g. "", "perl"
@@ -59,7 +60,7 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
 			"white-space.")
 	}
 
-	if m, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
+	if m, commented, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
 		if G.opts.WarnSpace && spaceAfterVarname != "" {
 			switch {
 			case hasSuffix(varname, "+") && op == "=":
@@ -85,6 +86,7 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
 		varparam := varnameParam(varname)
 
 		mkline.data = mkLineAssign{
+			commented,
 			varname,
 			varnameCanon(varname),
 			varparam,
@@ -148,13 +150,46 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
 func (mkline *MkLineImpl) String() string {
 	return fmt.Sprintf("%s:%s", mkline.Filename, mkline.Linenos())
 }
-func (mkline *MkLineImpl) IsVarassign() bool { _, ok := mkline.data.(mkLineAssign); return ok }
-func (mkline *MkLineImpl) IsShellcmd() bool  { _, ok := mkline.data.(mkLineShell); return ok }
-func (mkline *MkLineImpl) IsComment() bool   { _, ok := mkline.data.(mkLineComment); return ok }
-func (mkline *MkLineImpl) IsEmpty() bool     { _, ok := mkline.data.(mkLineEmpty); return ok }
+
+func (mkline *MkLineImpl) IsVarassign() bool {
+	data, ok := mkline.data.(mkLineAssign)
+	return ok && !data.commented
+}
+
+// IsCommentedVarassign returns true for commented-out variable assignments.
+// In most cases these are treated as ordinary comments, but in some others
+// they are treated like variable assignments, just inactive ones.
+func (mkline *MkLineImpl) IsCommentedVarassign() bool {
+	data, ok := mkline.data.(mkLineAssign)
+	return ok && data.commented
+}
+
+// IsShellcmd returns true for tab-indented lines that are assigned to a Make
+// target. Example:
+//
+//  pre-configure:    # IsDependency
+//          ${ECHO}   # IsShellcmd
+func (mkline *MkLineImpl) IsShellcmd() bool {
+	_, ok := mkline.data.(mkLineShell)
+	return ok
+}
+
+func (mkline *MkLineImpl) IsComment() bool {
+	_, ok := mkline.data.(mkLineComment)
+	return ok || mkline.IsCommentedVarassign()
+}
+
+func (mkline *MkLineImpl) IsEmpty() bool {
+	_, ok := mkline.data.(mkLineEmpty)
+	return ok
+}
 
 // IsCond checks whether the line is a conditional (.if/.ifelse/.else/.if) or a loop (.for/.endfor).
-func (mkline *MkLineImpl) IsCond() bool { _, ok := mkline.data.(mkLineConditional); return ok }
+func (mkline *MkLineImpl) IsCond() bool {
+	_, ok := mkline.data.(mkLineConditional)
+	return ok
+}
+
 func (mkline *MkLineImpl) IsInclude() bool {
 	incl, ok := mkline.data.(mkLineInclude)
 	return ok && !incl.sys
@@ -163,11 +198,15 @@ func (mkline *MkLineImpl) IsSysinclude() bool {
 	incl, ok := mkline.data.(mkLineInclude)
 	return ok && incl.sys
 }
-func (mkline *MkLineImpl) IsDependency() bool { _, ok := mkline.data.(mkLineDependency); return ok }
-func (mkline *MkLineImpl) Varname() string    { return mkline.data.(mkLineAssign).varname }
-func (mkline *MkLineImpl) Varcanon() string   { return mkline.data.(mkLineAssign).varcanon }
-func (mkline *MkLineImpl) Varparam() string   { return mkline.data.(mkLineAssign).varparam }
-func (mkline *MkLineImpl) Op() MkOperator     { return mkline.data.(mkLineAssign).op }
+func (mkline *MkLineImpl) IsDependency() bool {
+	_, ok := mkline.data.(mkLineDependency)
+	return ok
+}
+
+func (mkline *MkLineImpl) Varname() string  { return mkline.data.(mkLineAssign).varname }
+func (mkline *MkLineImpl) Varcanon() string { return mkline.data.(mkLineAssign).varcanon }
+func (mkline *MkLineImpl) Varparam() string { return mkline.data.(mkLineAssign).varparam }
+func (mkline *MkLineImpl) Op() MkOperator   { return mkline.data.(mkLineAssign).op }
 
 // For a variable assignment, the text up to and including the assignment operator, e.g. VARNAME+=\t
 func (mkline *MkLineImpl) ValueAlign() string       { return mkline.data.(mkLineAssign).valueAlign }
@@ -356,6 +395,9 @@ func (mkline *MkLineImpl) VariableNeedsQuoting(varname string, vartype *Vartype,
 
 	if vartype.basicType.IsEnum() || vartype.IsBasicSafe() {
 		if vartype.kindOfList == lkNone {
+			if vartype.guessed {
+				return nqDontKnow
+			}
 			return nqDoesntMatter
 		}
 		if vartype.kindOfList == lkShell && !vuc.IsWordPart {
@@ -740,11 +782,16 @@ func (ind *Indentation) Varnames() string {
 	return varnames
 }
 
-func MatchVarassign(text string) (m bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
+func MatchVarassign(text string) (m, commented bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
 	i, n := 0, len(text)
 
-	for i < n && text[i] == ' ' {
+	if i < n && text[i] == '#' {
+		commented = true
 		i++
+	} else {
+		for i < n && text[i] == ' ' {
+			i++
+		}
 	}
 
 	varnameStart := i
diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go
index 02d228d941d..1f70c197e11 100644
--- a/pkgtools/pkglint/files/mkline_test.go
+++ b/pkgtools/pkglint/files/mkline_test.go
@@ -644,6 +644,36 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__tool_in_CONFIGURE_ENV(c *check
 		"NOTE: Makefile:3: The :Q operator isn't necessary for ${TOOLS_TAR} here.")
 }
 
+// For some well-known directory variables like WRKDIR, PREFIX, LOCALBASE,
+// the :Q modifier can be safely removed since pkgsrc will never support
+// having special characters in these directory names.
+// For guessed variable types be cautious and don't autofix them.
+func (s *Suite) Test_MkLine_variableNeedsQuoting__only_remove_known(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall", "--autofix")
+	G.globalData.InitVartypes()
+
+	lines := t.SetupFileLinesContinuation("Makefile",
+		MkRcsId,
+		"",
+		"demo: .PHONY",
+		"\t${ECHO} ${WRKSRC:Q}",
+		"\t${ECHO} ${FOODIR:Q}")
+	mklines := NewMkLines(lines)
+
+	mklines.Check()
+
+	t.CheckOutputLines(
+		"AUTOFIX: ~/Makefile:4: Replacing \"${WRKSRC:Q}\" with \"${WRKSRC}\".")
+	t.CheckFileLines("Makefile",
+		MkRcsId,
+		"",
+		"demo: .PHONY",
+		"\t${ECHO} ${WRKSRC}",
+		"\t${ECHO} ${FOODIR:Q}")
+}
+
 func (s *Suite) Test_MkLine_Pkgmandir(c *check.C) {
 	t := s.Init(c)
 
@@ -750,38 +780,46 @@ func (s *Suite) Test_MkLine_ConditionVars(c *check.C) {
 func (s *Suite) Test_MatchVarassign(c *check.C) {
 	s.Init(c)
 
-	checkVarassign := func(text string, ck check.Checker, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
+	checkVarassign := func(text string, commented bool, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
 		type VarAssign struct {
+			commented                                                              bool
 			varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string
 		}
-		expected := VarAssign{varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
-		am, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
+		expected := VarAssign{commented, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
+		am, acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
 		if !am {
 			c.Errorf("Text %q doesn't match variable assignment", text)
 			return
 		}
-		actual := VarAssign{avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
-		c.Check(actual, ck, expected)
+		actual := VarAssign{acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
+		c.Check(actual, equals, expected)
 	}
 	checkNotVarassign := func(text string) {
-		m, _, _, _, _, _, _, _ := MatchVarassign(text)
+		m, _, _, _, _, _, _, _, _ := MatchVarassign(text)
 		if m {
 			c.Errorf("Text %q matches variable assignment, but shouldn't.", text)
 		}
 	}
 
-	checkVarassign("C++=c11", equals, "C+", "", "+=", "C++=", "c11", "", "")
-	checkVarassign("V=v", equals, "V", "", "=", "V=", "v", "", "")
-	checkVarassign("VAR=#comment", equals, "VAR", "", "=", "VAR=", "", "", "#comment")
-	checkVarassign("VAR=\\#comment", equals, "VAR", "", "=", "VAR=", "#comment", "", "")
-	checkVarassign("VAR=\\\\\\##comment", equals, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
-	checkVarassign("VAR=\\", equals, "VAR", "", "=", "VAR=", "\\", "", "")
-	checkVarassign("VAR += value", equals, "VAR", " ", "+=", "VAR += ", "value", "", "")
-	checkVarassign(" VAR=value", equals, "VAR", "", "=", " VAR=", "value", "", "")
-	checkVarassign("VAR=value #comment", equals, "VAR", "", "=", "VAR=", "value", " ", "#comment")
+	checkVarassign("C++=c11", false, "C+", "", "+=", "C++=", "c11", "", "")
+	checkVarassign("V=v", false, "V", "", "=", "V=", "v", "", "")
+	checkVarassign("VAR=#comment", false, "VAR", "", "=", "VAR=", "", "", "#comment")
+	checkVarassign("VAR=\\#comment", false, "VAR", "", "=", "VAR=", "#comment", "", "")
+	checkVarassign("VAR=\\\\\\##comment", false, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
+	checkVarassign("VAR=\\", false, "VAR", "", "=", "VAR=", "\\", "", "")
+	checkVarassign("VAR += value", false, "VAR", " ", "+=", "VAR += ", "value", "", "")
+	checkVarassign(" VAR=value", false, "VAR", "", "=", " VAR=", "value", "", "")
+	checkVarassign("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
+	checkVarassign("#VAR=value", true, "VAR", "", "=", "#VAR=", "value", "", "")
+
 	checkNotVarassign("\tVAR=value")
 	checkNotVarassign("?=value")
 	checkNotVarassign("<=value")
+	checkNotVarassign("#")
+
+	// A single space is typically used for writing documentation,
+	// not for commenting out code.
+	checkNotVarassign("# VAR=value")
 }
 
 func (s *Suite) Test_Indentation(c *check.C) {
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index 66ff5f884c3..8ce532f81e4 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -191,7 +191,7 @@ func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) {
 	if expected := strings.Repeat(" ", expectedDepth); indent != expected {
 		fix := mkline.Line.Autofix()
 		fix.Notef("This directive should be indented by %d spaces.", expectedDepth)
-		fix.Replace("."+indent, "."+expected)
+		fix.ReplaceRegex(regex.Pattern(`^\.`+indent), "."+expected, 1)
 		fix.Apply()
 	}
 }
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index f7134677e65..2807eb41655 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -322,3 +322,34 @@ func (s *Suite) Test_MkLineChecker_CheckVartype_CFLAGS(c *check.C) {
 		"WARN: Makefile:2: Unknown compiler flag \"-bs\".",
 		"WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.")
 }
+
+// Up to 2018-01-28, pkglint applied the autofix also to the continuation
+// lines, which is incorrect. It replaced the dot in "4.*" with spaces.
+func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation_autofix(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall", "--autofix")
+	G.globalData.InitVartypes()
+	lines := t.SetupFileLinesContinuation("options.mk",
+		MkRcsId,
+		".if ${PKGNAME} == pkgname",
+		".if \\",
+		"   ${PLATFORM:MNetBSD-4.*}",
+		".endif",
+		".endif")
+	mklines := NewMkLines(lines)
+
+	mklines.Check()
+
+	t.CheckOutputLines(
+		"AUTOFIX: ~/options.mk:3: Replacing \".\" with \".  \".",
+		"AUTOFIX: ~/options.mk:5: Replacing \".\" with \".  \".")
+
+	t.CheckFileLines("options.mk",
+		MkRcsId,
+		".if ${PKGNAME} == pkgname",
+		".  if \\",
+		"   ${PLATFORM:MNetBSD-4.*}",
+		".  endif",
+		".endif")
+}
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index 262d713ee41..a23f9c366b2 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -260,28 +260,42 @@ func (va *VaralignBlock) Check(mkline MkLine) {
 	case !G.opts.WarnSpace:
 		return
 
-	case mkline.IsComment():
+	case mkline.IsEmpty():
+		va.Finish()
 		return
 
-	case mkline.IsCond():
+	case mkline.IsCommentedVarassign():
+		break
+
+	case mkline.IsComment():
 		return
 
-	case mkline.IsEmpty():
-		va.Finish()
+	case mkline.IsCond():
 		return
 
 	case !mkline.IsVarassign():
 		trace.Stepf("Skipping")
 		va.skip = true
 		return
+	}
 
+	switch {
 	case mkline.Op() == opAssignEval && matches(mkline.Varname(), `^[a-z]`):
 		// Arguments to procedures do not take part in block alignment.
+		//
+		// Example:
+		// pkgpath := ${PKGPATH}
 		return
 
 	case mkline.Value() == "" && mkline.VarassignComment() == "":
 		// Multiple-inclusion guards usually appear in a block of
 		// their own and therefore do not need alignment.
+		//
+		// Example:
+		// .if !defined(INCLUSION_GUARD_MK)
+		// INCLUSION_GUARD_MK:=
+		// # ...
+		// .endif
 		return
 	}
 
@@ -289,7 +303,7 @@ func (va *VaralignBlock) Check(mkline MkLine) {
 	if mkline.IsMultiline() {
 		// Interpreting the continuation marker as variable value
 		// is cheating, but works well.
-		m, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
+		m, _, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
 		continuation = m && value == "\\"
 	}
 
@@ -330,8 +344,8 @@ func (va *VaralignBlock) Finish() {
 // variable from forcing the rest of the paragraph to be indented too
 // far to the right.
 func (va *VaralignBlock) optimalWidth(infos []*varalignBlockInfo) int {
-	longest := 0
-	secondLongest := 0
+	longest := 0       // The longest seen varnameOpWidth
+	secondLongest := 0 // The second-longest seen varnameOpWidth
 	for _, info := range infos {
 		if info.continuation {
 			continue
diff --git a/pkgtools/pkglint/files/mklines_varalign_test.go b/pkgtools/pkglint/files/mklines_varalign_test.go
index 2fffca84214..894b48be7cf 100755
--- a/pkgtools/pkglint/files/mklines_varalign_test.go
+++ b/pkgtools/pkglint/files/mklines_varalign_test.go
@@ -850,8 +850,8 @@ func (s *Suite) Test_Varalign__indented_continuation_line_in_paragraph(c *check.
 }
 
 // Up to 2018-01-27, it could happen that some source code was logged
-// without a corresponding diagnostic. This was unintented and confusing.
-func (s *Suite) Test_Varalign__bla(c *check.C) {
+// without a corresponding diagnostic. This was unintended and confusing.
+func (s *Suite) Test_Varalign__fix_without_diagnostic(c *check.C) {
 	vt := NewVaralignTester(s, c)
 	vt.Input(
 		"MESSAGE_SUBST+=\t\tRUBY_DISTNAME=${RUBY_DISTNAME}",
@@ -870,3 +870,114 @@ func (s *Suite) Test_Varalign__bla(c *check.C) {
 	vt.source = true
 	vt.Run()
 }
+
+// The two variables look like they were in two separate paragraphs, but
+// they aren't. This is because the continuation line from the DISTFILES
+// eats up the empty line that would otherwise separate the paragraphs.
+func (s *Suite) Test_Varalign__continuation_line_last_empty(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"DISTFILES= \\",
+		"\ta \\",
+		"\tb \\",
+		"\tc \\",
+		"",
+		"NEXT_VAR=\tmust not be indented")
+	vt.Diagnostics(
+		"NOTE: ~/Makefile:1--5: This variable value should be aligned with tabs, not spaces, to column 17.")
+	vt.Autofixes(
+		"AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".")
+	vt.Fixed(
+		"DISTFILES=      \\",
+		"        a \\",
+		"        b \\",
+		"        c \\",
+		"",
+		"NEXT_VAR=       must not be indented")
+	vt.Run()
+}
+
+// Commented-out variables take part in the realignment.
+// The TZ=UTC below is part of the two-line comment since make(1)
+// interprets it in the same way.
+func (s *Suite) Test_Varalign__realign_commented_single_lines(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"SHORT=\tvalue",
+		"#DISTFILES=\tdistfile-1.0.0.tar.gz",
+		"#CONTINUATION= \\",
+		"#\t\tcontinued",
+		"#CONFIGURE_ENV+= \\",
+		"#TZ=UTC",
+		"SHORT=\tvalue")
+	vt.Diagnostics(
+		"NOTE: ~/Makefile:1: This variable value should be aligned to column 17.",
+		"NOTE: ~/Makefile:3--4: This variable value should be aligned with tabs, not spaces, to column 17.",
+		"NOTE: ~/Makefile:5--6: This line should be aligned with \"\\t\\t\".",
+		"NOTE: ~/Makefile:7: This variable value should be aligned to column 17.")
+	vt.Autofixes(
+		"AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
+		"AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".",
+		"AUTOFIX: ~/Makefile:6: Replacing indentation \"\" with \"\\t\\t\".",
+		"AUTOFIX: ~/Makefile:7: Replacing \"\\t\" with \"\\t\\t\".")
+	vt.Fixed(
+		"SHORT=          value",
+		"#DISTFILES=     distfile-1.0.0.tar.gz",
+		"#CONTINUATION=  \\",
+		"#               continued",
+		"#CONFIGURE_ENV+= \\",
+		"#               TZ=UTC",
+		"SHORT=          value")
+	vt.Run()
+}
+
+func (s *Suite) Test_Varalign__realign_commented_continuation_line(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"BEFORE=\tvalue",
+		"#COMMENTED= \\",
+		"#\tvalue1 \\",
+		"#\tvalue2 \\",
+		"#\tvalue3 \\",
+		"AFTER=\tafter") // This line continues the comment.
+	vt.Diagnostics()
+	vt.Autofixes()
+	vt.Fixed(
+		"BEFORE= value",
+		"#COMMENTED= \\",
+		"#       value1 \\",
+		"#       value2 \\",
+		"#       value3 \\",
+		"AFTER=  after")
+	vt.Run()
+}
+
+// The HOMEPAGE is completely ignored. Since its value is empty it doesn't
+// need any alignment. Whether it is commented out doesn't matter.
+func (s *Suite) Test_Varalign__realign_variable_without_value(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"COMMENT=\t\tShort description of the package",
+		"#HOMEPAGE=")
+	vt.Diagnostics()
+	vt.Autofixes()
+	vt.Fixed(
+		"COMMENT=                Short description of the package",
+		"#HOMEPAGE=")
+	vt.Run()
+}
+
+// This commented multiline variable is already perfectly aligned.
+// Nothing needs to be fixed.
+func (s *Suite) Test_Varalign__realign_commented_multiline(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"#CONF_FILES+=\t\tfile1 \\",
+		"#\t\t\tfile2")
+	vt.Diagnostics()
+	vt.Autofixes()
+	vt.Fixed(
+		"#CONF_FILES+=           file1 \\",
+		"#                       file2")
+	vt.Run()
+}
diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go
index e1b6805076c..19099827ace 100644
--- a/pkgtools/pkglint/files/patches.go
+++ b/pkgtools/pkglint/files/patches.go
@@ -37,6 +37,11 @@ func (ck *PatchChecker) Check() {
 	if CheckLineRcsid(ck.lines[0], ``, "") {
 		ck.exp.Advance()
 	}
+	if ck.exp.EOF() {
+		ck.lines[0].Errorf("Patch files must not be empty.")
+		return
+	}
+
 	ck.previousLineEmpty = ck.exp.ExpectEmptyLine(G.opts.WarnSpace)
 
 	patchedFiles := 0
diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go
index 419cf843fb6..4da652b66bc 100644
--- a/pkgtools/pkglint/files/patches_test.go
+++ b/pkgtools/pkglint/files/patches_test.go
@@ -408,3 +408,30 @@ func (s *Suite) Test_ChecklinesPatch__context_lines_with_tab_instead_of_space(c
 
 	t.CheckOutputEmpty()
 }
+
+// Must not panic.
+func (s *Suite) Test_ChecklinesPatch__autofix_empty_patch(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall", "--autofix")
+	lines := t.NewLines("patch-aa",
+		RcsId)
+
+	ChecklinesPatch(lines)
+
+	t.CheckOutputEmpty()
+}
+
+// Must not panic.
+func (s *Suite) Test_ChecklinesPatch__autofix_long_empty_patch(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall", "--autofix")
+	lines := t.NewLines("patch-aa",
+		RcsId,
+		"")
+
+	ChecklinesPatch(lines)
+
+	t.CheckOutputEmpty()
+}
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 03b9c1e749a..a20b46a11da 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -308,37 +308,45 @@ func ChecklinesMessage(lines []Line) {
 		defer trace.Call1(lines[0].Filename)()
 	}
 
-	explainMessage := func() {
-		Explain(
-			"A MESSAGE file should consist of a header line, having 75 \"=\"",
-			"characters, followed by a line containing only the RCS Id, then an",
-			"empty line, your text and finally the footer line, which is the",
-			"same as the header line.")
-	}
+	explanation := []string{
+		"A MESSAGE file should consist of a header line, having 75 \"=\"",
+		"characters, followed by a line containing only the RCS Id, then an",
+		"empty line, your text and finally the footer line, which is the",
+		"same as the header line."}
 
 	if len(lines) < 3 {
 		lastLine := lines[len(lines)-1]
 		lastLine.Warnf("File too short.")
-		explainMessage()
+		Explain(explanation...)
 		return
 	}
 
 	hline := strings.Repeat("=", 75)
 	if line := lines[0]; line.Text != hline {
-		line.Warnf("Expected a line of exactly 75 \"=\" characters.")
-		explainMessage()
+		fix := line.Autofix()
+		fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
+		fix.Explain(explanation...)
+		fix.InsertBefore(hline)
+		fix.Apply()
+		CheckLineRcsid(lines[0], ``, "")
+	} else if 1 < len(lines) {
+		CheckLineRcsid(lines[1], ``, "")
 	}
-	CheckLineRcsid(lines[1], ``, "")
 	for _, line := range lines {
 		CheckLineLength(line, 80)
 		CheckLineTrailingWhitespace(line)
 		CheckLineValidCharacters(line, `[\t -~]`)
 	}
 	if lastLine := lines[len(lines)-1]; lastLine.Text != hline {
-		lastLine.Warnf("Expected a line of exactly 75 \"=\" characters.")
-		explainMessage()
+		fix := lastLine.Autofix()
+		fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
+		fix.Explain(explanation...)
+		fix.InsertAfter(hline)
+		fix.Apply()
 	}
 	ChecklinesTrailingEmptyLines(lines)
+
+	SaveAutofixChanges(lines)
 }
 
 func CheckfileMk(fname string) {
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index b12fb0bfd01..cf4e9216210 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -186,10 +186,40 @@ func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) {
 
 	t.CheckOutputLines(
 		"WARN: MESSAGE:1: Expected a line of exactly 75 \"=\" characters.",
-		"ERROR: MESSAGE:2: Expected \"$"+"NetBSD$\".",
+		"ERROR: MESSAGE:1: Expected \"$"+"NetBSD$\".",
 		"WARN: MESSAGE:5: Expected a line of exactly 75 \"=\" characters.")
 }
 
+func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall", "--autofix")
+	lines := t.SetupFileLines("MESSAGE",
+		"1",
+		"2",
+		"3",
+		"4",
+		"5")
+
+	ChecklinesMessage(lines)
+
+	t.CheckOutputLines(
+		"AUTOFIX: ~/MESSAGE:1: Inserting a line \"===================================="+
+			"=======================================\" before this line.",
+		"AUTOFIX: ~/MESSAGE:1: Inserting a line \"$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $\" before this line.",
+		"AUTOFIX: ~/MESSAGE:5: Inserting a line \"===================================="+
+			"=======================================\" after this line.")
+	t.CheckFileLines("MESSAGE",
+		"===========================================================================",
+		"$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $",
+		"1",
+		"2",
+		"3",
+		"4",
+		"5",
+		"===========================================================================")
+}
+
 func (s *Suite) Test_GlobalData_Latest(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go
index 2d94fef0be2..b961ab17667 100644
--- a/pkgtools/pkglint/files/plist.go
+++ b/pkgtools/pkglint/files/plist.go
@@ -101,7 +101,7 @@ func (ck *PlistChecker) collectFilesAndDirs(plines []*PlistLine) {
 				first == '$',
 				'A' <= first && first <= 'Z',
 				'0' <= first && first <= '9':
-				if ck.allFiles[text] == nil {
+				if prev := ck.allFiles[text]; prev == nil || pline.conditional < prev.conditional {
 					ck.allFiles[text] = pline
 				}
 				for dir := path.Dir(text); dir != "."; dir = path.Dir(dir) {
@@ -143,6 +143,7 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) {
 	dirname := strings.TrimSuffix(sdirname, "/")
 
 	ck.checkSorted(pline)
+	ck.checkDuplicate(pline)
 
 	if contains(basename, "${IMAKE_MANNEWSUFFIX}") {
 		pline.warnImakeMannewsuffix()
@@ -211,17 +212,29 @@ func (ck *PlistChecker) checkSorted(pline *PlistLine) {
 					"The files in the PLIST should be sorted alphabetically.",
 					"To fix this, run \"pkglint -F PLIST\".")
 			}
-			if prev := ck.allFiles[text]; prev != nil && prev != pline {
-				fix := pline.line.Autofix()
-				fix.Errorf("Duplicate filename %q, already appeared in %s.", text, prev.line.ReferenceFrom(pline.line))
-				fix.Delete()
-				fix.Apply()
-			}
 		}
 		ck.lastFname = text
 	}
 }
 
+func (ck *PlistChecker) checkDuplicate(pline *PlistLine) {
+	text := pline.text
+	if !hasAlnumPrefix(text) || containsVarRef(text) {
+		return
+	}
+
+	prev := ck.allFiles[text]
+	if prev == pline || prev.conditional != "" {
+		return
+	}
+
+	fix := pline.line.Autofix()
+	fix.Errorf("Duplicate filename %q, already appeared in %s.",
+		text, prev.line.ReferenceFrom(pline.line))
+	fix.Delete()
+	fix.Apply()
+}
+
 func (ck *PlistChecker) checkpathBin(pline *PlistLine, dirname, basename string) {
 	if contains(dirname, "/") {
 		pline.line.Warnf("The bin/ directory should not have subdirectories.")
@@ -279,7 +292,8 @@ func (ck *PlistChecker) checkpathLib(pline *PlistLine, dirname, basename string)
 	if contains(basename, ".a") || contains(basename, ".so") {
 		if m, noext := match1(pline.text, `^(.*)(?:\.a|\.so[0-9.]*)$`); m {
 			if laLine := ck.allFiles[noext+".la"]; laLine != nil {
-				pline.line.Warnf("Redundant library found. The libtool library is in %s.", laLine.line.ReferenceFrom(pline.line))
+				pline.line.Warnf("Redundant library found. The libtool library is in %s.",
+					laLine.line.ReferenceFrom(pline.line))
 			}
 		}
 	}
@@ -320,7 +334,7 @@ func (ck *PlistChecker) checkpathMan(pline *PlistLine) {
 			"configured by the pkgsrc user.  Compression and decompression takes",
 			"place automatically, no matter if the .gz extension is mentioned in",
 			"the PLIST or not.")
-		fix.ReplaceRegex(`\.gz\n`, "\n")
+		fix.ReplaceRegex(`\.gz\n`, "\n", 1)
 		fix.Apply()
 	}
 }
@@ -493,17 +507,6 @@ func NewPlistLineSorter(plines []*PlistLine) *plistLineSorter {
 	return &plistLineSorter{header, middle, footer, unsortable, false, false}
 }
 
-func (s *plistLineSorter) Len() int {
-	return len(s.middle)
-}
-func (s *plistLineSorter) Less(i, j int) bool {
-	return s.middle[i].text < s.middle[j].text
-}
-func (s *plistLineSorter) Swap(i, j int) {
-	s.changed = true
-	s.middle[i], s.middle[j] = s.middle[j], s.middle[i]
-}
-
 func (s *plistLineSorter) Sort() {
 	if line := s.unsortable; line != nil {
 		if G.opts.PrintAutofix || G.opts.Autofix {
@@ -516,7 +519,17 @@ func (s *plistLineSorter) Sort() {
 		return
 	}
 	firstLine := s.middle[0].line
-	sort.Stable(s)
+
+	sort.SliceStable(s.middle, func(i, j int) bool {
+		mi := s.middle[i]
+		mj := s.middle[j]
+		less := mi.text < mj.text || (mi.text == mj.text &&
+			mi.conditional < mj.conditional)
+		if (i < j) != less {
+			s.changed = true
+		}
+		return less
+	})
 
 	if !s.changed {
 		return
diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go
index 198ab529904..19f5762b954 100644
--- a/pkgtools/pkglint/files/plist_test.go
+++ b/pkgtools/pkglint/files/plist_test.go
@@ -292,3 +292,47 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
 		"@pkgdir        etc/logrotate.d",
 		"@pkgdir        etc/sasl2")
 }
+
+// When the same entry appears both with and without a conditional,
+// the one with the conditional can be removed.
+// When the same entry appears with several different conditionals,
+// all of them must stay.
+func (s *Suite) Test_PlistChecker__remove_same_entries(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall")
+	lines := t.SetupFileLines("PLIST",
+		PlistRcsId,
+		"${PLIST.option1}bin/true",
+		"bin/true",
+		"${PLIST.option1}bin/true",
+		"bin/true",
+		"${PLIST.option3}bin/false",
+		"${PLIST.option2}bin/false",
+		"bin/true")
+
+	ChecklinesPlist(lines)
+
+	t.CheckOutputLines(
+		"ERROR: ~/PLIST:2: Duplicate filename \"bin/true\", already appeared in line 3.",
+		"ERROR: ~/PLIST:4: Duplicate filename \"bin/true\", already appeared in line 3.",
+		"ERROR: ~/PLIST:5: Duplicate filename \"bin/true\", already appeared in line 3.",
+		"WARN: ~/PLIST:6: \"bin/false\" should be sorted before \"bin/true\".",
+		"ERROR: ~/PLIST:8: Duplicate filename \"bin/true\", already appeared in line 3.")
+
+	t.SetupCommandLine("-Wall", "--autofix")
+
+	ChecklinesPlist(lines)
+
+	t.CheckOutputLines(
+		"AUTOFIX: ~/PLIST:2: Deleting this line.",
+		"AUTOFIX: ~/PLIST:4: Deleting this line.",
+		"AUTOFIX: ~/PLIST:5: Deleting this line.",
+		"AUTOFIX: ~/PLIST:8: Deleting this line.",
+		"AUTOFIX: ~/PLIST:2: Sorting the whole file.")
+	t.CheckFileLines("PLIST",
+		PlistRcsId,
+		"${PLIST.option2}bin/false",
+		"${PLIST.option3}bin/false",
+		"bin/true")
+}