diff options
-rw-r--r-- | pkgtools/pkglint/Makefile | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/line.go | 1 | ||||
-rw-r--r-- | pkgtools/pkglint/files/plist.go | 87 | ||||
-rw-r--r-- | pkgtools/pkglint/files/plist_test.go | 51 |
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"+ |