commit: 189883497abaee3708e9348df93800cc4c24fb07
Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
AuthorDate: Wed Oct 5 11:43:57 2022 +0000
Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
CommitDate: Wed Oct 5 11:43:57 2022 +0000
URL:
https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=18988349
EbuildReservedCheck: catch declaration of phase hooks
Phase hooks (`{pre,post}_${phase}`) are used by portage to run user's
hooks defined at `bashrc` file. Defining them in ebuilds might cause
unexpected behavior, so we should warn about it.
Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org>
src/pkgcheck/checks/reserved.py | 36 ++++++++++++++++------
.../EbuildReservedName/expected.json | 12 +++++---
.../EbuildReservedName/fix.patch | 15 +++++++--
.../EbuildReservedName/EbuildReservedName-0.ebuild | 8 +++++
4 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/src/pkgcheck/checks/reserved.py b/src/pkgcheck/checks/reserved.py
index 72b5300f..8448179a 100644
--- a/src/pkgcheck/checks/reserved.py
+++ b/src/pkgcheck/checks/reserved.py
@@ -1,4 +1,7 @@
import re
+
+from pkgcore.ebuild.eapi import EAPI
+
from .. import addons, bash, results, sources
from . import Check
@@ -15,19 +18,18 @@ class _ReservedNameCheck(Check):
variables_usage_whitelist = {"EBUILD_PHASE", "EBUILD_PHASE_FUNC"}
def _check(self, used_type: str, used_names):
- for used_name, node_start in used_names.items():
+ for used_name, (lineno, _) in used_names.items():
if used_name in self.special_whitelist:
continue
test_name = used_name.lower()
- lineno, _ = node_start
for reserved in self.reserved_prefixes:
if test_name.startswith(reserved):
- yield used_name, used_type, reserved, 'prefix', lineno
+ yield used_name, used_type, reserved, 'prefix', lineno+1
for reserved in self.reserved_substrings:
if reserved in test_name:
- yield used_name, used_type, reserved, 'substring', lineno
+ yield used_name, used_type, reserved, 'substring', lineno+1
if self.reserved_ebuild_regex.match(test_name):
- yield used_name, used_type, 'ebuild', 'substring', lineno
+ yield used_name, used_type, 'ebuild', 'substring', lineno+1
def _feed(self, item):
yield from self._check('function', {
@@ -78,16 +80,15 @@ class EclassReservedCheck(_ReservedNameCheck):
class EbuildReservedName(results.LineResult, results.Warning):
"""Ebuild uses reserved variable or function name for package manager."""
- def __init__(self, used_name: str, used_type: str, reserved_word: str,
reserved_type: str, **kwargs):
+ def __init__(self, used_type: str, reserved_word: str, reserved_type: str,
**kwargs):
super().__init__(**kwargs)
- self.used_name = used_name
self.used_type = used_type
self.reserved_word = reserved_word
self.reserved_type = reserved_type
@property
def desc(self):
- return f'line {self.lineno}: {self.used_type} name "{self.used_name}"
is disallowed because "{self.reserved_word}" is a reserved {self.reserved_type}'
+ return f'line {self.lineno}: {self.used_type} name "{self.line}" is
disallowed because "{self.reserved_word}" is a reserved {self.reserved_type}'
class EbuildReservedCheck(_ReservedNameCheck):
@@ -96,6 +97,21 @@ class EbuildReservedCheck(_ReservedNameCheck):
_source = sources.EbuildParseRepoSource
known_results = frozenset([EbuildReservedName])
+ def __init__(self, options, **kwargs):
+ super().__init__(options, **kwargs)
+ self.phases_hooks = {
+ eapi_name: {
+ f'{prefix}_{phase}' for phase in eapi.phases.values() for
prefix in ('pre', 'post')
+ }
+ for eapi_name, eapi in EAPI.known_eapis.items()
+ }
+
def feed(self, pkg):
- for *args, lineno in self._feed(pkg):
- yield EbuildReservedName(*args, lineno=lineno, line='', pkg=pkg)
+ for used_name, *args, lineno in self._feed(pkg):
+ yield EbuildReservedName(*args, lineno=lineno, line=used_name,
pkg=pkg)
+
+ for node, _ in bash.func_query.captures(pkg.tree.root_node):
+ used_name = pkg.node_str(node.child_by_field_name('name'))
+ if used_name in self.phases_hooks[str(pkg.eapi)]:
+ lineno, _ = node.start_point
+ yield EbuildReservedName('function', used_name, 'phase hook',
lineno=lineno+1, line=used_name, pkg=pkg)
diff --git
a/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/expected.json
b/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/expected.json
index 1dd73cac..9fbd6727 100644
---
a/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/expected.json
+++
b/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/expected.json
@@ -1,5 +1,7 @@
-{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "used_name": "prepare_locale",
"used_type": "function", "reserved_word": "prep", "reserved_type": "prefix",
"lineno": 5, "line": ""}
-{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "used_name": "DYNAMIC_DEPS",
"used_type": "variable", "reserved_word": "dyn", "reserved_type": "prefix",
"lineno": 6, "line": ""}
-{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "used_name": "_hook_prepare",
"used_type": "variable", "reserved_word": "hook", "reserved_type": "substring",
"lineno": 7, "line": ""}
-{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "used_name": "__ORIG_CC",
"used_type": "variable", "reserved_word": "__", "reserved_type": "prefix",
"lineno": 10, "line": ""}
-{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "used_name": "EBUILD_TEST",
"used_type": "variable", "reserved_word": "ebuild", "reserved_type":
"substring", "lineno": 12, "line": ""}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "prepare_locale",
"lineno": 6, "used_type": "function", "reserved_word": "prep", "reserved_type":
"prefix"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "DYNAMIC_DEPS",
"lineno": 7, "used_type": "variable", "reserved_word": "dyn", "reserved_type":
"prefix"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "_hook_prepare",
"lineno": 8, "used_type": "variable", "reserved_word": "hook", "reserved_type":
"substring"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "__ORIG_CC", "lineno":
11, "used_type": "variable", "reserved_word": "__", "reserved_type": "prefix"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "EBUILD_TEST",
"lineno": 13, "used_type": "variable", "reserved_word": "ebuild",
"reserved_type": "substring"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "post_src_unpack",
"lineno": 16, "used_type": "function", "reserved_word": "post_src_unpack",
"reserved_type": "phase hook"}
+{"__class__": "EbuildReservedName", "category": "EbuildReservedCheck",
"package": "EbuildReservedName", "version": "0", "line": "pre_src_test",
"lineno": 20, "used_type": "function", "reserved_word": "pre_src_test",
"reserved_type": "phase hook"}
diff --git
a/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/fix.patch
b/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/fix.patch
index 146e025e..3ce2b36a 100644
---
a/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/fix.patch
+++
b/testdata/data/repos/standalone/EbuildReservedCheck/EbuildReservedName/fix.patch
@@ -1,6 +1,7 @@
+diff -Naur
standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
fixed/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
---
standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
-+++
standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
-@@ -3,12 +3,11 @@ HOMEPAGE="https://github.com/pkgcore/pkgcheck";
++++ fixed/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
+@@ -3,20 +3,19 @@ HOMEPAGE="https://github.com/pkgcore/pkgcheck";
SLOT="0"
LICENSE="BSD"
@@ -17,3 +18,13 @@
EBUILD_SUCCESS_HOOKS="true"
-EBUILD_TEST="1"
REBUILD_ALL="1"
+
+-post_src_unpack() {
++my_post_src_unpack() {
+ echo "Larry was here"
+ }
+
+-pre_src_test() {
++my_pre_src_test() {
+ echo "Larry was even here"
+ }
diff --git
a/testdata/repos/standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
b/testdata/repos/standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
index 420f4dac..a0c8e201 100644
---
a/testdata/repos/standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
+++
b/testdata/repos/standalone/EbuildReservedCheck/EbuildReservedName/EbuildReservedName-0.ebuild
@@ -12,3 +12,11 @@ __ORIG_CC="STUB"
EBUILD_SUCCESS_HOOKS="true"
EBUILD_TEST="1"
REBUILD_ALL="1"
+
+post_src_unpack() {
+ echo "Larry was here"
+}
+
+pre_src_test() {
+ echo "Larry was even here"
+}
