[PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[Ricardo Ribalda]

If the git.Repo object's scope extends to the Python interpreter's
shutdown phase, its destructor may fail due to the interpreter's state.

Exception ignored in: <function Git.AutoInterrupt.__del__ at 0x7f1941dd5620>
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/git/cmd.py", line 565, in __del__
  File "/usr/lib/python3/dist-packages/git/cmd.py", line 546, in _terminate
  File "/usr/lib/python3.13/subprocess.py", line 2227, in terminate
ImportError: sys.meta_path is None, Python is likely shutting down

Make repo a variable of the function read_spdxdata() and scan_git_tree()
to limit the scope of git.Repo and ensure proper resource management.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v2:
- Make repo a local variable
- Link to v1: https://lore.kernel.org/r/20250225-spx-v1-1-e935b27eb80d@chromium.org
---
 scripts/spdxcheck.py | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py
index 8d608f61bf371647e7ca0129f583e94e535b6193..d5c4c37f1b068486af28110261f74c67301618a9 100755
--- a/scripts/spdxcheck.py
+++ b/scripts/spdxcheck.py
@@ -45,7 +45,9 @@ class dirinfo(object):
                 self.files.append(fname)
 
 # Read the spdx data from the LICENSES directory
-def read_spdxdata(repo):
+def read_spdxdata():
+    repo = git.Repo(os.getcwd())
+    assert not repo.bare
 
     # The subdirectories of LICENSES in the kernel source
     # Note: exceptions needs to be parsed as last directory.
@@ -295,7 +297,15 @@ def exclude_file(fpath):
             return True
     return False
 
-def scan_git_tree(tree, basedir, dirdepth):
+def scan_git_tree(basedir, dirdepth):
+    repo = git.Repo(os.getcwd())
+    tree = repo.head.commit.tree
+
+    basedir = basedir.strip('/')
+    if basedir != '.':
+        for p in basedir.split('/'):
+            tree = tree[p]
+
     parser.set_dirinfo(basedir, dirdepth)
     for el in tree.traverse():
         if not os.path.isfile(el.path):
@@ -306,11 +316,6 @@ def scan_git_tree(tree, basedir, dirdepth):
         with open(el.path, 'rb') as fd:
             parser.parse_lines(fd, args.maxlines, el.path)
 
-def scan_git_subtree(tree, path, dirdepth):
-    for p in path.strip('/').split('/'):
-        tree = tree[p]
-    scan_git_tree(tree, path.strip('/'), dirdepth)
-
 def read_exclude_file(fname):
     rules = []
     if not fname:
@@ -348,12 +353,8 @@ if __name__ == '__main__':
         sys.exit(1)
 
     try:
-        # Use git to get the valid license expressions
-        repo = git.Repo(os.getcwd())
-        assert not repo.bare
-
         # Initialize SPDX data
-        spdx = read_spdxdata(repo)
+        spdx = read_spdxdata()
 
         # Initialize the parser
         parser = id_parser(spdx)
@@ -389,14 +390,13 @@ if __name__ == '__main__':
                     if os.path.isfile(p):
                         parser.parse_lines(open(p, 'rb'), args.maxlines, p)
                     elif os.path.isdir(p):
-                        scan_git_subtree(repo.head.reference.commit.tree, p,
-                                         args.depth)
+                        scan_git_tree(p, args.depth)
                     else:
                         sys.stderr.write('path %s does not exist\n' %p)
                         sys.exit(1)
             else:
                 # Full git tree scan
-                scan_git_tree(repo.head.commit.tree, '.', args.depth)
+                scan_git_tree('.', args.depth)
 
             ndirs = len(parser.spdx_dirs)
             dirsok = 0

---
base-commit: d082ecbc71e9e0bf49883ee4afd435a77a5101b6
change-id: 20250225-spx-382cf543370e

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[Gon Solo]

Am Wed, Apr 09, 2025 at 08:04:19PM +0000 schrieb Ricardo Ribalda:
> Changes in v2:
> - Make repo a local variable
> - Link to v1: https://lore.kernel.org/r/20250225-spx-v1-1-e935b27eb80d@chromium.org

This is not necessary as it was Python's fault and is fixed by 3.13.3
which came out yesterday. I just checked.

Re: [PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[Ricardo Ribalda]

On Wed, 9 Apr 2025 at 22:40, Gon Solo <gonsolo@gmail.com> wrote:
>
> Am Wed, Apr 09, 2025 at 08:04:19PM +0000 schrieb Ricardo Ribalda:
> > Changes in v2:
> > - Make repo a local variable
> > - Link to v1: https://lore.kernel.org/r/20250225-spx-v1-1-e935b27eb80d@chromium.org
>
> This is not necessary as it was Python's fault and is fixed by 3.13.3
> which came out yesterday. I just checked.

It will take some time before this reaches all distributions. This
patch is relatively simple.

I might be biased, but I think the benefits outweigh the cons.

>


-- 
Ricardo Ribalda

Re: [PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[Gon Solo]

> > This is not necessary as it was Python's fault and is fixed by 3.13.3
> > which came out yesterday. I just checked.
> 
> It will take some time before this reaches all distributions. This
> patch is relatively simple.
> 
> I might be biased, but I think the benefits outweigh the cons.

3.13.3 is in Arch core-testing and Debian unstable, Debian stable's 3.11
is not affected by this bug. But I agree, it's annoying to be hit by
it. :/

g

Re: [PATCH v2] scripts/spdxcheck: Limit the scope of git.Repo

[Greg Kroah-Hartman]

On Thu, Apr 10, 2025 at 08:21:09AM +0200, Gon Solo wrote:
> > > This is not necessary as it was Python's fault and is fixed by 3.13.3
> > > which came out yesterday. I just checked.
> > 
> > It will take some time before this reaches all distributions. This
> > patch is relatively simple.
> > 
> > I might be biased, but I think the benefits outweigh the cons.
> 
> 3.13.3 is in Arch core-testing and Debian unstable, Debian stable's 3.11
> is not affected by this bug. But I agree, it's annoying to be hit by
> it. :/

As this is a bug in python, let's not work around it in our scripts but
rather rely on the proper fix in python instead.

thanks,

greg k-h