aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZac Medico <zmedico@gentoo.org>2021-03-27 16:15:16 -0700
committerZac Medico <zmedico@gentoo.org>2021-03-28 21:46:41 -0700
commit15cbe87076b512b318fac1729ec94e6d6674a95a (patch)
treeee885cdcbcdc874480b22448687aaca1b2c628fc
parentSimpleRepomanTestCase: compare QATracker results to expected values (diff)
downloadportage-15cbe87076b512b318fac1729ec94e6d6674a95a.tar.gz
portage-15cbe87076b512b318fac1729ec94e6d6674a95a.tar.bz2
portage-15cbe87076b512b318fac1729ec94e6d6674a95a.zip
repoman: add variable.phase check like pkgcheck VariableScopeCheck (bug 608664)
The variable.phase check is inspired by pkgcheck's VariableScopeCheck, and uses essentially the same PMS data to drive the check. References: - https://projects.gentoo.org/pms/7/pms.html#x1-10900011.1 - https://pkgcore.github.io/pkgcheck/_modules/pkgcheck/checks/codingstyle.html#VariableScopeCheck - https://bugs.gentoo.org/775191 Bug: https://bugs.gentoo.org/608664 Signed-off-by: Zac Medico <zmedico@gentoo.org>
-rw-r--r--repoman/cnf/qa_data/qa_data.yaml1
-rw-r--r--repoman/cnf/repository/qa_data.yaml1
-rw-r--r--repoman/cnf/repository/repository.yaml1
-rw-r--r--repoman/lib/repoman/modules/linechecks/phases/__init__.py6
-rw-r--r--repoman/lib/repoman/modules/linechecks/phases/phase.py132
-rw-r--r--repoman/lib/repoman/tests/simple/test_simple.py28
-rw-r--r--repoman/man/repoman.15
7 files changed, 162 insertions, 12 deletions
diff --git a/repoman/cnf/qa_data/qa_data.yaml b/repoman/cnf/qa_data/qa_data.yaml
index 29a3d6e9f..530c8c806 100644
--- a/repoman/cnf/qa_data/qa_data.yaml
+++ b/repoman/cnf/qa_data/qa_data.yaml
@@ -129,6 +129,7 @@ qahelp:
obsolete: "The ebuild makes use of an obsolete construct"
variable:
invalidchar: "A variable contains an invalid character that is not part of the ASCII character set"
+ phase: "Variable referenced found within scope of incorrect ebuild phase as specified by PMS"
readonly: "Assigning a readonly variable"
usedwithhelpers: "Ebuild uses D, ROOT, BROOT, ED, EROOT or EPREFIX with helpers"
virtual:
diff --git a/repoman/cnf/repository/qa_data.yaml b/repoman/cnf/repository/qa_data.yaml
index 3fe6b53d5..2249000c3 100644
--- a/repoman/cnf/repository/qa_data.yaml
+++ b/repoman/cnf/repository/qa_data.yaml
@@ -80,6 +80,7 @@ qawarnings:
- usage.obsolete
- upstream.workaround
- uri.https
+ - variable.phase
- virtual.suspect
- wxwidgets.eclassnotused
diff --git a/repoman/cnf/repository/repository.yaml b/repoman/cnf/repository/repository.yaml
index ad00d18c1..dbc1decaa 100644
--- a/repoman/cnf/repository/repository.yaml
+++ b/repoman/cnf/repository/repository.yaml
@@ -61,6 +61,7 @@ linechecks_modules:
emakeparallel
srccompileeconf
srcunpackpatches
+ pmsvariablerefphasescope
portageinternal
portageinternalvariableassignment
quote
diff --git a/repoman/lib/repoman/modules/linechecks/phases/__init__.py b/repoman/lib/repoman/modules/linechecks/phases/__init__.py
index 686c675d2..e166b31a3 100644
--- a/repoman/lib/repoman/modules/linechecks/phases/__init__.py
+++ b/repoman/lib/repoman/modules/linechecks/phases/__init__.py
@@ -29,6 +29,12 @@ module_spec = {
'class': "SrcUnpackPatches",
'description': doc,
},
+ 'pmsvariablerefphasescope-check': {
+ 'name': "pmsvariablerefphasescope",
+ 'sourcefile': "phase",
+ 'class': "PMSVariableReference",
+ 'description': doc,
+ },
},
'version': 1,
}
diff --git a/repoman/lib/repoman/modules/linechecks/phases/phase.py b/repoman/lib/repoman/modules/linechecks/phases/phase.py
index 74cf4608f..433e93601 100644
--- a/repoman/lib/repoman/modules/linechecks/phases/phase.py
+++ b/repoman/lib/repoman/modules/linechecks/phases/phase.py
@@ -1,7 +1,19 @@
+import fnmatch
import re
-
-from portage.eapi import eapi_has_src_prepare_and_src_configure
+import types
+
+from portage.eapi import (
+ eapi_has_broot,
+ eapi_has_sysroot,
+ eapi_has_src_prepare_and_src_configure,
+ eapi_exports_AA,
+ eapi_exports_replace_vars,
+ eapi_exports_ECLASSDIR,
+ eapi_exports_PORTDIR,
+ eapi_supports_prefix,
+ eapi_exports_merge_type,
+)
from repoman.modules.linechecks.base import LineCheck
@@ -9,11 +21,22 @@ class PhaseCheck(LineCheck):
""" basic class for function detection """
func_end_re = re.compile(r'^\}$')
- phases_re = re.compile('(%s)' % '|'.join((
- 'pkg_pretend', 'pkg_setup', 'src_unpack', 'src_prepare',
- 'src_configure', 'src_compile', 'src_test', 'src_install',
- 'pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm',
- 'pkg_config')))
+ phase_funcs = (
+ 'pkg_pretend',
+ 'pkg_setup',
+ 'src_unpack',
+ 'src_prepare',
+ 'src_configure',
+ 'src_compile',
+ 'src_test',
+ 'src_install',
+ 'pkg_preinst',
+ 'pkg_postinst',
+ 'pkg_prerm',
+ 'pkg_postrm',
+ 'pkg_config',
+ )
+ phases_re = re.compile('(%s)' % '|'.join(phase_funcs))
in_phase = ''
def check(self, num, line):
@@ -69,3 +92,98 @@ class SrcUnpackPatches(PhaseCheck):
if m is not None:
return ("'%s'" % m.group(1)) + \
" call should be moved to src_prepare"
+
+# Refererences
+# - https://projects.gentoo.org/pms/7/pms.html#x1-10900011.1
+# - https://pkgcore.github.io/pkgcheck/_modules/pkgcheck/checks/codingstyle.html#VariableScopeCheck
+_pms_vars = (
+ ("A", None, ("src_*", "pkg_nofetch")),
+ ("AA", eapi_exports_AA, ("src_*", "pkg_nofetch")),
+ ("FILESDIR", None, ("src_*",)),
+ ("DISTDIR", None, ("src_*",)),
+ ("WORKDIR", None, ("src_*",)),
+ ("S", None, ("src_*",)),
+ ("PORTDIR", eapi_exports_PORTDIR, ("src_*",)),
+ ("ECLASSDIR", eapi_exports_ECLASSDIR, ("src_*",)),
+ ("ROOT", None, ("pkg_*",)),
+ ("EROOT", eapi_supports_prefix, ("pkg_*",)),
+ ("SYSROOT", eapi_has_sysroot, ("src_*", "pkg_setup")),
+ ("ESYSROOT", eapi_has_sysroot, ("src_*", "pkg_setup")),
+ ("BROOT", eapi_has_broot, ("src_*", "pkg_setup")),
+ ("D", None, ("src_install", "pkg_preinst", "pkg_postint")),
+ ("ED", eapi_supports_prefix, ("src_install", "pkg_preinst", "pkg_postint")),
+ ("DESTTREE", None, ("src_install",)),
+ ("INSDESTTREE", None, ("src_install",)),
+ ("MERGE_TYPE", eapi_exports_merge_type, ("pkg_*",)),
+ ("REPLACING_VERSIONS", eapi_exports_replace_vars, ("pkg_*",)),
+ ("REPLACED_BY_VERSION", eapi_exports_replace_vars, ("pkg_prerm", "pkg_postrm")),
+)
+
+
+def _compile_phases():
+ phase_vars = {}
+ for phase_func in PhaseCheck.phase_funcs:
+ for variable, eapi_filter, allowed_scopes in _pms_vars:
+ allowed = False
+ for scope in allowed_scopes:
+ if fnmatch.fnmatch(phase_func, scope):
+ allowed = True
+ break
+
+ if not allowed:
+ phase_vars.setdefault(phase_func, []).append((variable, eapi_filter))
+
+ phase_info = {}
+ for phase_func, prohibited_vars in phase_vars.items():
+ phase_func_vars = []
+ for variable, eapi_filter in prohibited_vars:
+ phase_func_vars.append(variable)
+ phase_obj = phase_info[phase_func] = types.SimpleNamespace()
+ phase_obj.prohibited_vars = dict(prohibited_vars)
+ phase_obj.var_names = "(%s)" % "|".join(
+ variable for variable, eapi_filter in prohibited_vars
+ )
+ phase_obj.var_reference = re.compile(
+ r"\$(\{|)%s(\}|\W)" % (phase_obj.var_names,)
+ )
+
+ return phase_info
+
+
+class PMSVariableReference(PhaseCheck):
+ """Check phase scope for references to variables specified by PMS"""
+
+ repoman_check_name = "variable.phase"
+ phase_info = _compile_phases()
+
+ def new(self, pkg):
+ self._eapi = pkg.eapi
+
+ def end(self):
+ self._eapi = None
+
+ def phase_check(self, num, line):
+ try:
+ phase_info = self.phase_info[self.in_phase]
+ except KeyError:
+ return
+
+ eapi = self._eapi
+ issues = []
+ for m in phase_info.var_reference.finditer(line):
+ open_brace = m.group(1)
+ var_name = m.group(2)
+ close_brace = m.group(3)
+ # discard \W if matched by (\}|\W)
+ close_brace = close_brace if close_brace == "}" else ""
+ if bool(open_brace) != bool(close_brace):
+ continue
+ var_name = m.group(2)
+ eapi_filter = phase_info.prohibited_vars[var_name]
+ if eapi_filter is not None and not eapi_filter(eapi):
+ continue
+ issues.append(
+ "phase %s: EAPI %s: variable %s: Forbidden reference to variable specified by PMS"
+ % (self.in_phase, eapi, var_name)
+ )
+ return issues
diff --git a/repoman/lib/repoman/tests/simple/test_simple.py b/repoman/lib/repoman/tests/simple/test_simple.py
index e64ba4f07..3a699a708 100644
--- a/repoman/lib/repoman/tests/simple/test_simple.py
+++ b/repoman/lib/repoman/tests/simple/test_simple.py
@@ -3,6 +3,7 @@
import collections
import subprocess
+import sys
import time
import types
@@ -141,6 +142,12 @@ class SimpleRepomanTestCase(TestCase):
""" % time.gmtime().tm_year
+ pkg_preinst_references_forbidden_var = """
+pkg_preinst() {
+ echo "This ${A} reference is not allowed. Neither is this $BROOT reference."
+}
+"""
+
repo_configs = {
"test_repo": {
"layout.conf":
@@ -194,13 +201,14 @@ class SimpleRepomanTestCase(TestCase):
"dev-libs/C-0": {
"COPYRIGHT_HEADER" : copyright_header,
"DESCRIPTION" : "Desc goes here",
- "EAPI" : "4",
+ "EAPI" : "7",
"HOMEPAGE" : "https://example.com",
"IUSE" : "flag",
# must be unstable, since dev-libs/A[flag] is stable masked
"KEYWORDS": "~x86",
"LICENSE": "GPL-2",
"RDEPEND": "flag? ( dev-libs/A[flag] )",
+ "MISC_CONTENT": pkg_preinst_references_forbidden_var,
},
}
licenses = ["GPL-2"]
@@ -292,7 +300,15 @@ class SimpleRepomanTestCase(TestCase):
committer_name = "Gentoo Dev"
committer_email = "gentoo-dev@gentoo.org"
- expected_warnings = {"returncode": 0}
+ expected_warnings = {
+ "returncode": 0,
+ "warns": {
+ "variable.phase": [
+ "dev-libs/C/C-0.ebuild: line 15: phase pkg_preinst: EAPI 7: variable A: Forbidden reference to variable specified by PMS",
+ "dev-libs/C/C-0.ebuild: line 15: phase pkg_preinst: EAPI 7: variable BROOT: Forbidden reference to variable specified by PMS",
+ ]
+ },
+ }
git_test = (
("", RepomanRun(args=["manifest"])),
@@ -380,7 +396,7 @@ class SimpleRepomanTestCase(TestCase):
# triggered by python -Wd will be visible.
stdout = subprocess.PIPE
- for cwd in ("", "dev-libs", "dev-libs/A", "dev-libs/B"):
+ for cwd in ("", "dev-libs", "dev-libs/A", "dev-libs/B", "dev-libs/C"):
abs_cwd = os.path.join(test_repo_symlink, cwd)
proc = await asyncio.create_subprocess_exec(
@@ -413,7 +429,11 @@ class SimpleRepomanTestCase(TestCase):
await cmd.run()
if cmd.result["result"] != cmd.expected and cmd.result.get("stdio"):
portage.writemsg(cmd.result["stdio"])
- self.assertEqual(cmd.result["result"], cmd.expected)
+ try:
+ self.assertEqual(cmd.result["result"], cmd.expected)
+ except Exception:
+ print(cmd.result["result"], file=sys.stderr, flush=True)
+ raise
continue
proc = await asyncio.create_subprocess_exec(
diff --git a/repoman/man/repoman.1 b/repoman/man/repoman.1
index 0926e806c..5dbc41560 100644
--- a/repoman/man/repoman.1
+++ b/repoman/man/repoman.1
@@ -1,4 +1,4 @@
-.TH "REPOMAN" "1" "Aug 2020" "Repoman VERSION" "Repoman"
+.TH "REPOMAN" "1" "March 2021" "Repoman VERSION" "Repoman"
.SH NAME
repoman \- Gentoo's program to enforce a minimal level of quality assurance in
packages added to the ebuild repository
@@ -445,6 +445,9 @@ The ebuild makes use of an obsolete construct
A variable contains an invalid character that is not part of the ASCII
character set.
.TP
+.B variable.phase
+Variable referenced found within scope of incorrect ebuild phase as specified by PMS.
+.TP
.B variable.readonly
Assigning a readonly variable
.TP