Updated pkglint to 5.4.23.

Changes since 5.4.22:
Only autofix PLIST sorting in simple cases. Before this version, pkglint
sorted the PLIST even when it contained unresolved variable references like
${IMAKE_MAN_DIR}, which resulted in strange and unintuitive orders. These
complicated files are left for human inspection.

4 files changed, 91 insertions, 52 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 07745bfa03a..84fb73bbf58 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.519 2018/01/01 18:04:15 rillig Exp $
+# $NetBSD: Makefile,v 1.520 2018/01/01 21:55:36 rillig Exp $
 
-PKGNAME=	pkglint-5.4.22
+PKGNAME=	pkglint-5.4.23
 DISTFILES=	# none
 CATEGORIES=	pkgtools
 
diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go
index 7f8c6012265..37896088251 100644
--- a/pkgtools/pkglint/files/line.go
+++ b/pkgtools/pkglint/files/line.go
@@ -65,6 +65,7 @@ func NewLineWhole(fname string) Line {
 	return NewLine(fname, 0, "", nil)
 }
 
+// modifiedLines returns the text after the fixes, including line breaks and newly inserted lines
 func (line *LineImpl) modifiedLines() []string {
 	var result []string
 	result = append(result, line.before...)
diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go
index f90210eb78e..6c7dc9f3a3b 100644
--- a/pkgtools/pkglint/files/plist.go
+++ b/pkgtools/pkglint/files/plist.go
@@ -83,7 +83,7 @@ func (ck *PlistChecker) NewLines(lines []Line) []*PlistLine {
 	for i, line := range lines {
 		conditional, text := "", line.Text
 		if hasPrefix(text, "${PLIST.") {
-			if m, cond, rest := match2(text, `^\$\{(PLIST\.[\w-.]+)\}(.*)`); m {
+			if m, cond, rest := match2(text, `^(?:\$\{(PLIST\.[\w-.]+)\})+(.*)`); m {
 				conditional, text = cond, rest
 			}
 		}
@@ -127,6 +127,10 @@ func (ck *PlistChecker) checkline(pline *PlistLine) {
 		pline.CheckDirective(cmd, arg)
 	} else if hasPrefix(text, "$") {
 		ck.checkpath(pline)
+	} else if text == "" {
+		if !pline.line.AutofixDelete() {
+			pline.line.Warnf("PLISTs should not contain empty lines.")
+		}
 	} else {
 		pline.line.Warnf("Unknown line type.")
 	}
@@ -143,7 +147,9 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) {
 		pline.warnImakeMannewsuffix()
 	}
 	if hasPrefix(text, "${PKGMANDIR}/") {
-		if !line.AutofixReplace("${PKGMANDIR}/", "man/") {
+		if line.AutofixReplace("${PKGMANDIR}/", "man/") {
+			pline.text = strings.Replace(pline.text, "${PKGMANDIR}/", "man/", 1)
+		} else {
 			line.Notef("PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".")
 			Explain(
 				"The pkgsrc infrastructure takes care of replacing the correct value",
@@ -479,55 +485,78 @@ func (pline *PlistLine) warnImakeMannewsuffix() {
 }
 
 type plistLineSorter struct {
-	first     *PlistLine
-	plines    []*PlistLine
-	lines     []Line
-	after     map[*PlistLine][]Line
-	swapped   bool
-	autofixed bool
+	header     []*PlistLine // Does not take part in sorting
+	middle     []*PlistLine // Only this part is sorted
+	footer     []*PlistLine // Does not take part in sorting, typically contains @exec or @pkgdir
+	unsortable Line         // Some lines so difficult to sort that only humans can do that
+	changed    bool         // Whether the sorting actually changed something
+	autofixed  bool         // Whether the newly sorted file has been written to disk
 }
 
 func NewPlistLineSorter(plines []*PlistLine) *plistLineSorter {
-	s := &plistLineSorter{first: plines[0], after: make(map[*PlistLine][]Line)}
-	prev := plines[0]
-	for _, pline := range plines[1:] {
-		if hasPrefix(pline.text, "@") || contains(pline.text, "$") {
-			s.after[prev] = append(s.after[prev], pline.line)
-		} else {
-			s.plines = append(s.plines, pline)
-			s.lines = append(s.lines, pline.line)
-			prev = pline
+	headerEnd := 0
+	for headerEnd < len(plines) && hasPrefix(plines[headerEnd].text, "@comment") {
+		headerEnd++
+	}
+
+	footerStart := len(plines)
+	for footerStart > headerEnd && hasPrefix(plines[footerStart-1].text, "@") {
+		footerStart--
+	}
+
+	header := plines[0:headerEnd]
+	middle := plines[headerEnd:footerStart]
+	footer := plines[footerStart:]
+	var unsortable Line
+
+	for _, pline := range middle {
+		if unsortable == nil && (hasPrefix(pline.text, "@") || contains(pline.text, "$")) {
+			unsortable = pline.line
 		}
 	}
-	return s
+	return &plistLineSorter{header, middle, footer, unsortable, false, false}
 }
 
 func (s *plistLineSorter) Len() int {
-	return len(s.plines)
+	return len(s.middle)
 }
 func (s *plistLineSorter) Less(i, j int) bool {
-	return s.plines[i].text < s.plines[j].text
+	return s.middle[i].text < s.middle[j].text
 }
 func (s *plistLineSorter) Swap(i, j int) {
-	s.swapped = true
-	s.lines[i], s.lines[j] = s.lines[j], s.lines[i]
-	s.plines[i], s.plines[j] = s.plines[j], s.plines[i]
+	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 {
+			line.Notef("This line prevents pkglint from sorting the PLIST automatically.")
+		}
+		return
+	}
+
+	if len(s.middle) == 0 {
+		return
+	}
+	firstLine := s.middle[0].line
 	sort.Stable(s)
 
-	if !s.swapped {
+	if !s.changed {
 		return
 	}
 
-	firstLine := s.first.line
 	firstLine.AutofixMark("Sorting the whole file.")
-	lines := []Line{firstLine}
-	lines = append(lines, s.after[s.first]...)
-	for _, pline := range s.plines {
+	var lines []Line
+	for _, pline := range s.header {
+		lines = append(lines, pline.line)
+	}
+	for _, pline := range s.middle {
+		lines = append(lines, pline.line)
+	}
+	for _, pline := range s.footer {
 		lines = append(lines, pline.line)
-		lines = append(lines, s.after[pline]...)
 	}
+	lines[0].Changed = true // Without this, autofix doesn't know that anything changed
 	s.autofixed = SaveAutofixChanges(lines)
 }
diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go
index 088b5bf990a..c4c674f1381 100644
--- a/pkgtools/pkglint/files/plist_test.go
+++ b/pkgtools/pkglint/files/plist_test.go
@@ -118,39 +118,48 @@ func (s *Suite) Test_PlistLineSorter_Sort(c *check.C) {
 		"lib/${UNKNOWN}.la",
 		"C",
 		"ddd",
-		"@exec echo \"after ddd\"",
+		"@exec echo \"after ddd\"", // Makes the PLIST unsortable
 		"sbin/program",
 		"${PLIST.one}bin/program",
-		"${PKGMANDIR}/man1/program.1",
+		"man/man1/program.1",
 		"${PLIST.two}bin/program2",
 		"lib/before.la",
+		"${PLIST.linux}${PLIST.x86_64}lib/lib-linux-x86_64.so", // Double conditional, see graphics/graphviz
 		"lib/after.la",
 		"@exec echo \"after lib/after.la\"")
 	ck := &PlistChecker{nil, nil, "", false}
 	plines := ck.NewLines(lines)
 
-	NewPlistLineSorter(plines).Sort()
+	sorter1 := NewPlistLineSorter(plines)
+	c.Check(sorter1.unsortable, equals, lines[5])
+
+	cleanedLines := append(append(lines[0:5], lines[6:8]...), lines[9:]...) // Remove ${UNKNOWN} and @exec
+
+	sorter2 := NewPlistLineSorter((&PlistChecker{nil, nil, "", false}).NewLines(cleanedLines))
+
+	c.Check(sorter2.unsortable, check.IsNil)
+
+	sorter2.Sort()
 
 	s.CheckOutputLines(
-		"AUTOFIX: ~/PLIST:1: Sorting the whole file.",
+		"AUTOFIX: ~/PLIST:3: Sorting the whole file.",
 		"AUTOFIX: ~/PLIST: Has been auto-fixed. Please re-run pkglint.")
 	c.Check(s.LoadTmpFile("PLIST"), equals, ""+
 		"@comment $"+"NetBSD$\n"+
-		"@comment Do not remove\n"+
+		"@comment Do not remove\n"+ // The header ends here
 		"A\n"+
 		"C\n"+
 		"CCC\n"+
-		"lib/${UNKNOWN}.la\n"+ // Stays below the previous line
 		"b\n"+
 		"${PLIST.one}bin/program\n"+ // Conditionals are ignored while sorting
-		"${PKGMANDIR}/man1/program.1\n"+ // Stays below the previous line
 		"${PLIST.two}bin/program2\n"+
 		"ddd\n"+
-		"@exec echo \"after ddd\"\n"+ // Stays below the previous line
 		"lib/after.la\n"+
-		"@exec echo \"after lib/after.la\"\n"+
 		"lib/before.la\n"+
-		"sbin/program\n")
+		"${PLIST.linux}${PLIST.x86_64}lib/lib-linux-x86_64.so\n"+
+		"man/man1/program.1\n"+
+		"sbin/program\n"+
+		"@exec echo \"after lib/after.la\"\n") // The footer starts here
 }
 
 func (s *Suite) Test_PlistChecker_checkpathShare_Desktop(c *check.C) {
@@ -217,11 +226,11 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
 		"lib/libvirt/lock-driver/lockd.la",
 		"${PKGMANDIR}/man1/sh.1",
 		"share/augeas/lenses/virtlockd.aug",
-		"share/doc/${PKGNAME}/html/32favicon.png",
-		"share/doc/${PKGNAME}/html/404.html",
-		"share/doc/${PKGNAME}/html/acl.html",
-		"share/doc/${PKGNAME}/html/aclpolkit.html",
-		"share/doc/${PKGNAME}/html/windows.html",
+		"share/doc/pkgname-1.0/html/32favicon.png",
+		"share/doc/pkgname-1.0/html/404.html",
+		"share/doc/pkgname-1.0/html/acl.html",
+		"share/doc/pkgname-1.0/html/aclpolkit.html",
+		"share/doc/pkgname-1.0/html/windows.html",
 		"share/examples/libvirt/libvirt.conf",
 		"share/locale/zh_CN/LC_MESSAGES/libvirt.mo",
 		"share/locale/zh_TW/LC_MESSAGES/libvirt.mo",
@@ -245,7 +254,7 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
 
 	s.CheckOutputLines(
 		"AUTOFIX: ~/PLIST:6: Replacing \"${PKGMANDIR}/\" with \"man/\".",
-		"AUTOFIX: ~/PLIST:1: Sorting the whole file.",
+		"AUTOFIX: ~/PLIST:2: Sorting the whole file.",
 		"AUTOFIX: ~/PLIST: Has been auto-fixed. Please re-run pkglint.")
 	c.Check(len(lines), equals, len(fixedLines))
 	c.Check(s.LoadTmpFile("PLIST"), equals, ""+
@@ -256,11 +265,11 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
 		"lib/libvirt/lock-driver/lockd.la\n"+
 		"man/man1/sh.1\n"+
 		"share/augeas/lenses/virtlockd.aug\n"+
-		"share/doc/${PKGNAME}/html/32favicon.png\n"+
-		"share/doc/${PKGNAME}/html/404.html\n"+
-		"share/doc/${PKGNAME}/html/acl.html\n"+
-		"share/doc/${PKGNAME}/html/aclpolkit.html\n"+
-		"share/doc/${PKGNAME}/html/windows.html\n"+
+		"share/doc/pkgname-1.0/html/32favicon.png\n"+
+		"share/doc/pkgname-1.0/html/404.html\n"+
+		"share/doc/pkgname-1.0/html/acl.html\n"+
+		"share/doc/pkgname-1.0/html/aclpolkit.html\n"+
+		"share/doc/pkgname-1.0/html/windows.html\n"+
 		"share/examples/libvirt/libvirt.conf\n"+
 		"share/locale/zh_CN/LC_MESSAGES/libvirt.mo\n"+
 		"share/locale/zh_TW/LC_MESSAGES/libvirt.mo\n"+