Skip to content
This repository was archived by the owner on Apr 6, 2022. It is now read-only.

Commit 9bd5767

Browse files
ulfalizergalak
authored andcommitted
check_compliance.py: Avoid running 'git' before _main()
Running 'git rev-parse --show-toplevel' to set GIT_TOP before _main() prevents any errors from it from being reported to GitHub, because the GitHub connection hasn't been initialized yet. If there's an error, the error reporting is currently broken as well, because git() calls err(), which is defined at the end of the file. Initialize GIT_TOP in _main() instead of at the top level, and use a magic "<repo>" string to refer to the top-level repository directory when setting the 'ComplianceTest.path_hint' class variables instead. Discovered while working on some unrelated error reporting improvements. Also move the ZEPHYR_BASE initialization to a better spot, next to other global variables. Signed-off-by: Ulf Magnusson <[email protected]>
1 parent afa81c3 commit 9bd5767

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

scripts/check_compliance.py

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828

2929
logger = None
3030

31+
# This ends up as None when we're not running in a Zephyr tree
32+
ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE')
33+
3134

3235
def git(*args, cwd=None):
3336
# Helper for running a Git command. Returns the rstrip()ed stdout output.
@@ -59,13 +62,6 @@ def git(*args, cwd=None):
5962
return stdout.rstrip()
6063

6164

62-
# This ends up as None when we're not running in a Zephyr tree
63-
ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE')
64-
65-
# The absolute path of the top-level git directory
66-
GIT_TOP = git("rev-parse", "--show-toplevel")
67-
68-
6965
def get_shas(refspec):
7066
"""
7167
Returns the list of Git SHAs for 'refspec'.
@@ -103,6 +99,12 @@ class ComplianceTest:
10399
path_hint:
104100
The path the test runs itself in. This is just informative and used in
105101
the message that gets printed when running the test.
102+
103+
The magic string "<git-top>" refers to the top-level repository
104+
directory. This avoids running 'git' to find the top-level directory
105+
before main() runs (class variable assignments run when the 'class ...'
106+
statement runs). That avoids swallowing errors, because main() reports
107+
them to GitHub.
106108
"""
107109
def __init__(self):
108110
self.case = MyCase(self.name)
@@ -195,7 +197,7 @@ class CheckPatch(ComplianceTest):
195197
"""
196198
name = "checkpatch"
197199
doc = "https://docs.zephyrproject.org/latest/contribute/#coding-style"
198-
path_hint = GIT_TOP
200+
path_hint = "<git-top>"
199201

200202
def run(self):
201203
# Default to Zephyr's checkpatch if ZEPHYR_BASE is set
@@ -580,7 +582,7 @@ class Codeowners(ComplianceTest):
580582
"""
581583
name = "Codeowners"
582584
doc = "https://help.github.com/articles/about-code-owners/"
583-
path_hint = GIT_TOP
585+
path_hint = "<git-top>"
584586

585587
def ls_owned_files(self, codeowners):
586588
"""Returns an OrderedDict mapping git patterns from the CODEOWNERS file
@@ -712,7 +714,7 @@ class Nits(ComplianceTest):
712714
"""
713715
name = "Nits"
714716
doc = "https://docs.zephyrproject.org/latest/contribute/#coding-style"
715-
path_hint = GIT_TOP
717+
path_hint = "<git-top>"
716718

717719
def run(self):
718720
# Loop through added/modified files
@@ -823,7 +825,7 @@ class GitLint(ComplianceTest):
823825
"""
824826
name = "Gitlint"
825827
doc = "https://docs.zephyrproject.org/latest/contribute/#commit-guidelines"
826-
path_hint = GIT_TOP
828+
path_hint = "<git-top>"
827829

828830
def run(self):
829831
# By default gitlint looks for .gitlint configuration only in
@@ -847,7 +849,7 @@ class PyLint(ComplianceTest):
847849
"""
848850
name = "pylint"
849851
doc = "https://www.pylint.org/"
850-
path_hint = GIT_TOP
852+
path_hint = "<git-top>"
851853

852854
def run(self):
853855
# Path to pylint configuration file
@@ -1012,7 +1014,7 @@ class Identity(ComplianceTest):
10121014
doc = "https://docs.zephyrproject.org/latest/contribute/#commit-guidelines"
10131015
# git rev-list and git log don't depend on the current (sub)directory
10141016
# unless explicited
1015-
path_hint = GIT_TOP
1017+
path_hint = "<git-top>"
10161018

10171019
def run(self):
10181020
for shaidx in get_shas(COMMIT_RANGE):
@@ -1296,6 +1298,11 @@ def _main(args):
12961298
# The "real" main(), which is wrapped to catch exceptions and report them
12971299
# to GitHub. Returns the number of test failures.
12981300

1301+
# The absolute path of the top-level git directory. Initialize it here so
1302+
# that issues running Git can be reported to GitHub.
1303+
global GIT_TOP
1304+
GIT_TOP = git("rev-parse", "--show-toplevel")
1305+
12991306
# The commit range passed in --commit, e.g. "HEAD~3"
13001307
global COMMIT_RANGE
13011308
COMMIT_RANGE = args.commits
@@ -1344,8 +1351,8 @@ def _main(args):
13441351

13451352
test = testcase()
13461353
try:
1347-
print("Running {:16} tests in {} ..."
1348-
.format(test.name, test.path_hint))
1354+
print(f"Running {test.name:16} tests in "
1355+
f"{GIT_TOP if test.path_hint == '<git-top>' else test.path_hint} ...")
13491356
test.run()
13501357
except EndTest:
13511358
pass

0 commit comments

Comments
 (0)