diff options
author | rillig <rillig> | 2015-12-05 11:27:23 +0000 |
---|---|---|
committer | rillig <rillig> | 2015-12-05 11:27:23 +0000 |
commit | 337a831572538c5384e22d205b060671fedf4dbd (patch) | |
tree | ae88832f87eb9480e76c313bb3eeccc4f203519d /pkgtools/pkglint | |
parent | a6601258d22cdad9cff4e3ed767f7c2aa660829d (diff) | |
download | pkgsrc-337a831572538c5384e22d205b060671fedf4dbd.tar.gz |
Split PlistLine.checkPathname into smaller functions
Diffstat (limited to 'pkgtools/pkglint')
-rw-r--r-- | pkgtools/pkglint/files/expecter.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/globaldata.go | 10 | ||||
-rw-r--r-- | pkgtools/pkglint/files/line.go | 6 | ||||
-rw-r--r-- | pkgtools/pkglint/files/makefiles.go | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkcontext.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/patches.go | 1 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkgcontext.go | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/plist.go | 184 | ||||
-rw-r--r-- | pkgtools/pkglint/files/shell.go | 2 | ||||
-rw-r--r-- | pkgtools/pkglint/files/substcontext.go | 4 | ||||
-rw-r--r-- | pkgtools/pkglint/files/vartype.go | 4 |
12 files changed, 123 insertions, 102 deletions
diff --git a/pkgtools/pkglint/files/expecter.go b/pkgtools/pkglint/files/expecter.go index 81fa1fcd19c..d84273d333b 100644 --- a/pkgtools/pkglint/files/expecter.go +++ b/pkgtools/pkglint/files/expecter.go @@ -1,6 +1,6 @@ package main -// High-level iterating through lines and checking them. +// Expecter records the state when checking a list of lines from top to bottom. type Expecter struct { lines []*Line index int diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go index bc6299c50b6..38c8f2bbffc 100644 --- a/pkgtools/pkglint/files/globaldata.go +++ b/pkgtools/pkglint/files/globaldata.go @@ -6,7 +6,7 @@ import ( "sort" ) -// Constant data that is loaded once. +// GlobalData contains data describing pkgsrc as a whole. type GlobalData struct { pkgsrcdir string // Relative to the current working directory. masterSiteUrls map[string]string // "https://github.com/" => "MASTER_SITE_GITHUB" @@ -22,12 +22,12 @@ type GlobalData struct { suggestedUpdates []SuggestedUpdate // suggestedWipUpdates []SuggestedUpdate // lastChange map[string]*Change // - userDefinedVars map[string]*Line // + userDefinedVars map[string]*Line // varname => line (after calling parselineMk on it) deprecated map[string]string // vartypes map[string]*Vartype // varcanon => type } -// A change entry from doc/CHANGES-* +// Change is a change entry from the `doc/CHANGES-*` files. type Change struct { line *Line action string @@ -37,7 +37,7 @@ type Change struct { date string } -// From the doc/TODO file. +// SuggestedUpdate is from the `doc/TODO` file. type SuggestedUpdate struct { line *Line pkgname string @@ -357,7 +357,7 @@ func (gd *GlobalData) loadUserDefinedVars() { gd.userDefinedVars = make(map[string]*Line) for _, line := range lines { parselineMk(line) - if m, varname, _, _, _ := matchVarassign(line.text); m { + if varname, ok := line.extra["varname"].(string); ok { gd.userDefinedVars[varname] = line } } diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index 66eb1cbef45..d5d53398698 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -137,10 +137,10 @@ func (ln *Line) replaceRegex(from, to string) { } } -func (line *Line) noteAutofix(format string, args ...interface{}) { - line.changed = true +func (ln *Line) noteAutofix(format string, args ...interface{}) { + ln.changed = true if G.opts.Autofix || G.opts.PrintAutofix { - line.notef(format, args...) + ln.notef(format, args...) } G.autofixAvailable = true } diff --git a/pkgtools/pkglint/files/makefiles.go b/pkgtools/pkglint/files/makefiles.go index 681712e7105..faf95c41b71 100644 --- a/pkgtools/pkglint/files/makefiles.go +++ b/pkgtools/pkglint/files/makefiles.go @@ -394,11 +394,11 @@ func ChecklinesMk(lines []*Line) { line.notef("For efficiency reasons, please include bsd.fast.prefs.mk instead of bsd.prefs.mk.") } if G.pkgContext != nil { - G.pkgContext.seen_bsd_prefs_mk = true + G.pkgContext.seenBsdPrefsMk = true } } else if includefile == "../../mk/bsd.fast.prefs.mk" { if G.pkgContext != nil { - G.pkgContext.seen_bsd_prefs_mk = true + G.pkgContext.seenBsdPrefsMk = true } } diff --git a/pkgtools/pkglint/files/mkcontext.go b/pkgtools/pkglint/files/mkcontext.go index 6c8ac7a9337..5385e7f6151 100644 --- a/pkgtools/pkglint/files/mkcontext.go +++ b/pkgtools/pkglint/files/mkcontext.go @@ -1,6 +1,6 @@ package main -// Context of the Makefile that is currently checked. +// MkContext contains data for the Makefile (or *.mk) that is currently checked. type MkContext struct { forVars map[string]bool // The variables currently used in .for loops indentation []int // Indentation depth of preprocessing directives diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index e72005ec76d..b30ec6ff56d 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -282,7 +282,7 @@ func checklineMkVarassign(line *Line, varname, op, value, comment string) { checklineMkVardef(line, varname, op) - if G.opts.WarnExtra && op == "?=" && G.pkgContext != nil && !G.pkgContext.seen_bsd_prefs_mk { + if G.opts.WarnExtra && op == "?=" && G.pkgContext != nil && !G.pkgContext.seenBsdPrefsMk { switch varbase { case "BUILDLINK_PKGSRCDIR", "BUILDLINK_DEPMETHOD", "BUILDLINK_ABI_DEPENDS": // FIXME: What about these ones? They occur quite often. diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index b3c82439abd..b6c620e9b67 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -132,7 +132,6 @@ const ( rePatchUniLineNoNewline = `^\\ No newline at end of file$` ) -// See doc/statemachine.patch.dia type PatchState string const ( diff --git a/pkgtools/pkglint/files/pkgcontext.go b/pkgtools/pkglint/files/pkgcontext.go index 23374666b9c..43b1d8b36e1 100644 --- a/pkgtools/pkglint/files/pkgcontext.go +++ b/pkgtools/pkglint/files/pkgcontext.go @@ -1,6 +1,6 @@ package main -// Context of the package that is currently checked. +// PkgContext contains data for the package that is currently checked. type PkgContext struct { pkgpath string // e.g. "category/pkgdir" pkgdir string // PKGDIR from the package Makefile @@ -11,7 +11,7 @@ type PkgContext struct { effectivePkgbase string // The effective PKGNAME without the version effectivePkgversion string // The version part of the effective PKGNAME effectivePkgnameLine *Line // The origin of the three effective_* values - seen_bsd_prefs_mk bool // Has bsd.prefs.mk already been included? + seenBsdPrefsMk bool // Has bsd.prefs.mk already been included? vardef map[string]*Line // varname => line varuse map[string]*Line // varname => line diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 40d5fec0897..0d5ea3b86d6 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -165,19 +165,7 @@ func (pline *PlistLine) checkPathname(pctx *PlistContext, fullname string) { sdirname, basename := path.Split(fullname) dirname := strings.TrimSuffix(sdirname, "/") - if G.opts.WarnPlistSort && hasAlnumPrefix(text) && !containsVarRef(text) { - if pctx.lastFname != "" { - if pctx.lastFname > text { - line.warnf("%q should be sorted before %q.", text, pctx.lastFname) - line.explain( - "For aesthetic reasons, the files in the PLIST should be sorted", - "alphabetically.") - } else if pctx.lastFname == text { - line.errorf("Duplicate filename.") - } - } - pctx.lastFname = text - } + pline.checkSorted(pctx) if contains(basename, "${IMAKE_MANNEWSUFFIX}") { pline.warnAboutPlistImakeMannewsuffix() @@ -188,20 +176,7 @@ func (pline *PlistLine) checkPathname(pctx *PlistContext, fullname string) { line.warnf("The bin/ directory should not have subdirectories.") case dirname == "bin": - switch { - case pctx.allFiles["man/man1/"+basename+".1"] != nil: - case pctx.allFiles["man/man6/"+basename+".6"] != nil: - case pctx.allFiles["${IMAKE_MAN_DIR}/"+basename+".${IMAKE_MANNEWSUFFIX}"] != nil: - default: - if G.opts.WarnExtra { - line.warnf("Manual page missing for bin/%s.", basename) - line.explain( - "All programs that can be run directly by the user should have a manual", - "page for quick reference. The programs in the bin/ directory should have", - "corresponding manual pages in section 1 (filename program.1), not in", - "section 8.") - } - } + pline.checkpathBin(pctx, basename) case hasPrefix(text, "doc/"): line.errorf("Documentation must be installed under share/doc, not doc.") @@ -232,61 +207,13 @@ func (pline *PlistLine) checkPathname(pctx *PlistContext, fullname string) { line.errorf("\"lib/locale\" must not be listed. Use ${PKGLOCALEDIR}/locale and set USE_PKGLOCALEDIR instead.") case hasPrefix(text, "lib/"): - if m, dir, lib, ext := match3(text, `^(lib/(?:.*/)*)([^/]+)\.(so|a|la)$`); m { - if dir == "lib/" && !hasPrefix(lib, "lib") { - _ = G.opts.WarnExtra && line.warnf("Library filename does not start with \"lib\".") - } - if ext == "la" { - if G.pkgContext != nil && G.pkgContext.vardef["USE_LIBTOOL"] == nil { - line.warnf("Packages that install libtool libraries should define USE_LIBTOOL.") - } - } - } + pline.checkpathLib(pctx, basename) case hasPrefix(text, "man/"): - if m, catOrMan, section, manpage, ext, gz := match5(text, `^man/(cat|man)(\w+)/(.*?)\.(\w+)(\.gz)?$`); m { - - if !matches(section, `^[\dln]$`) { - line.warnf("Unknown section %q for manual page.", section) - } - - if catOrMan == "cat" && pctx.allFiles["man/man"+section+"/"+manpage+"."+section] == nil { - line.warnf("Preformatted manual page without unformatted one.") - } - - if catOrMan == "cat" { - if ext != "0" { - line.warnf("Preformatted manual pages should end in \".0\".") - } - } else { - if section != ext { - line.warnf("Mismatch between the section (%s) and extension (%s) of the manual page.", section, ext) - } - } - - if gz != "" { - line.notef("The .gz extension is unnecessary for manual pages.") - line.explain( - "Whether the manual pages are installed in compressed form or not is", - "configured by the pkgsrc user. Compression and decompression takes place", - "automatically, no matter if the .gz extension is mentioned in the PLIST", - "or not.") - } - } else { - // maybe: line.warnf("Invalid filename %q for manual page.", text) - } + pline.checkpathMan(pctx) case hasPrefix(text, "sbin/"): - binname := text[5:] - - if pctx.allFiles["man/man8/"+binname+".8"] == nil && G.opts.WarnExtra { - line.warnf("Manual page missing for sbin/%s.", binname) - line.explain( - "All programs that can be run directly by the user should have a manual", - "page for quick reference. The programs in the sbin/ directory should have", - "corresponding manual pages in section 8 (filename program.8), not in", - "section 1.") - } + pline.checkpathSbin(pctx) case hasPrefix(text, "share/applications/") && hasSuffix(text, ".desktop"): f := "../../sysutils/desktop-file-utils/desktopdb.mk" @@ -359,16 +286,111 @@ func (pline *PlistLine) checkPathname(pctx *PlistContext, fullname string) { "This file is handled automatically by the INSTALL/DEINSTALL scripts,", "since its contents changes frequently.") } +} + +func (pline *PlistLine) checkSorted(pctx *PlistContext) { + if text := pline.line.text; G.opts.WarnPlistSort && hasAlnumPrefix(text) && !containsVarRef(text) { + if pctx.lastFname != "" { + if pctx.lastFname > text { + pline.line.warnf("%q should be sorted before %q.", text, pctx.lastFname) + pline.line.explain( + "The files in the PLIST should be sorted alphabetically.") + } else if pctx.lastFname == text { + pline.line.errorf("Duplicate filename.") + } + } + pctx.lastFname = text + } +} + +func (pline *PlistLine) checkpathBin(pctx *PlistContext, basename string) { + switch { + case pctx.allFiles["man/man1/"+basename+".1"] != nil: + case pctx.allFiles["man/man6/"+basename+".6"] != nil: + case pctx.allFiles["${IMAKE_MAN_DIR}/"+basename+".${IMAKE_MANNEWSUFFIX}"] != nil: + default: + if G.opts.WarnExtra { + pline.line.warnf("Manual page missing for bin/%s.", basename) + pline.line.explain( + "All programs that can be run directly by the user should have a manual", + "page for quick reference. The programs in the bin/ directory should have", + "corresponding manual pages in section 1 (filename program.1), not in", + "section 8.") + } + } +} + +func (pline *PlistLine) checkpathLib(pctx *PlistContext, basename string) { + if m, dir, lib, ext := match3(pline.line.text, `^(lib/(?:.*/)*)([^/]+)\.(so|a|la)$`); m { + if dir == "lib/" && !hasPrefix(lib, "lib") { + _ = G.opts.WarnExtra && pline.line.warnf("Library filename does not start with \"lib\".") + } + if ext == "la" { + if G.pkgContext != nil && G.pkgContext.vardef["USE_LIBTOOL"] == nil { + pline.line.warnf("Packages that install libtool libraries should define USE_LIBTOOL.") + } + } + } if contains(basename, ".a") || contains(basename, ".so") { - if m, basename := match1(text, `^(.*)(?:\.a|\.so[0-9.]*)$`); m { - if laLine := pctx.allFiles[basename+".la"]; laLine != nil { - line.warnf("Redundant library found. The libtool library is in line %s.", laLine) + if m, noext := match1(pline.line.text, `^(.*)(?:\.a|\.so[0-9.]*)$`); m { + if laLine := pctx.allFiles[noext+".la"]; laLine != nil { + pline.line.warnf("Redundant library found. The libtool library is in line %s.", laLine) } } } } +func (pline *PlistLine) checkpathMan(pctx *PlistContext) { + line := pline.line + + m, catOrMan, section, manpage, ext, gz := match5(pline.line.text, `^man/(cat|man)(\w+)/(.*?)\.(\w+)(\.gz)?$`) + if !m { + // maybe: line.warnf("Invalid filename %q for manual page.", text) + return + } + + if !matches(section, `^[\dln]$`) { + line.warnf("Unknown section %q for manual page.", section) + } + + if catOrMan == "cat" && pctx.allFiles["man/man"+section+"/"+manpage+"."+section] == nil { + line.warnf("Preformatted manual page without unformatted one.") + } + + if catOrMan == "cat" { + if ext != "0" { + line.warnf("Preformatted manual pages should end in \".0\".") + } + } else { + if section != ext { + line.warnf("Mismatch between the section (%s) and extension (%s) of the manual page.", section, ext) + } + } + + if gz != "" { + line.notef("The .gz extension is unnecessary for manual pages.") + line.explain( + "Whether the manual pages are installed in compressed form or not is", + "configured by the pkgsrc user. Compression and decompression takes place", + "automatically, no matter if the .gz extension is mentioned in the PLIST", + "or not.") + } +} + +func (pline *PlistLine) checkpathSbin(pctx *PlistContext) { + binname := pline.line.text[5:] + + if pctx.allFiles["man/man8/"+binname+".8"] == nil && G.opts.WarnExtra { + pline.line.warnf("Manual page missing for sbin/%s.", binname) + pline.line.explain( + "All programs that can be run directly by the user should have a manual", + "page for quick reference. The programs in the sbin/ directory should have", + "corresponding manual pages in section 8 (filename program.8), not in", + "section 1.") + } +} + func (pline *PlistLine) warnAboutPlistImakeMannewsuffix() { line := pline.line diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index 190bb7614d8..e2157b034f7 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -55,7 +55,7 @@ const ( reShVarassign = `^([A-Z_a-z]\w*)=` ) -// Shell command state; see doc/statemachine.shellcmd.dia. +// ShellCommandState type scState string const ( diff --git a/pkgtools/pkglint/files/substcontext.go b/pkgtools/pkglint/files/substcontext.go index d442ec5ca82..654eae85adc 100644 --- a/pkgtools/pkglint/files/substcontext.go +++ b/pkgtools/pkglint/files/substcontext.go @@ -1,7 +1,7 @@ package main -// Records the state of a block of variable assignments that make up a SUBST -// class (see mk/subst.mk). +// SubstContext records the state of a block of variable assignments +// that make up a SUBST class (see `mk/subst.mk`). type SubstContext struct { id string stage string diff --git a/pkgtools/pkglint/files/vartype.go b/pkgtools/pkglint/files/vartype.go index 865268c944a..de1b723d39e 100644 --- a/pkgtools/pkglint/files/vartype.go +++ b/pkgtools/pkglint/files/vartype.go @@ -27,8 +27,8 @@ type AclEntry struct { permissions string // Some of: "a"ppend, "d"efault, "s"et; "p"reprocessing, "u"se } -// Whether the type definition is guessed (based on the variable name) -// or explicitly defined (see vardefs.go). +// Guessed says whether the type definition is guessed (based on the +// variable name) or explicitly defined (see vardefs.go). type Guessed bool const ( |