[PATCH] devtools: fix symbol change check for non-lib patches

[PATCH] devtools: fix symbol change check for non-lib patches

[Ali Alnubani]

Initialize lib var per patch file and skip symbol handling when no
lib/drivers file header was seen. Avoids NameError exception when
processing patches that only touch docs or other paths.

Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
Cc: david.marchand@redhat.com

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 devtools/check-symbol-change.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/devtools/check-symbol-change.py b/devtools/check-symbol-change.py
index 1efeb82fcd..4cee4c765c 100755
--- a/devtools/check-symbol-change.py
+++ b/devtools/check-symbol-change.py
@@ -29,6 +29,7 @@
 symbols = {}
 
 for file in args.patch:
+    lib = None
     for ln in file.readlines():
         if file_header_regexp.match(ln):
             if file_header_regexp.match(ln).group(2) == "lib":
@@ -54,6 +55,8 @@
         else:
             continue
 
+        if lib is None:
+            continue
         if symbol not in symbols[lib]:
             symbols[lib][symbol] = {}
         added = ln[0] == "+"
-- 
2.53.0

Re: [PATCH] devtools: fix symbol change check for non-lib patches

[David Marchand]

On Thu, 5 Mar 2026 at 10:11, Ali Alnubani <alialnu@nvidia.com> wrote:
>
> Initialize lib var per patch file and skip symbol handling when no
> lib/drivers file header was seen. Avoids NameError exception when
> processing patches that only touch docs or other paths.
>
> Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
> Cc: david.marchand@redhat.com
>
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>

I don't think resetting the lib variable for each patch is enough,
this relies on the patch content order.

It would be more robust to catch all ^--- a/  lines (which marks the
start of a new file), then if the file is neither in drivers/ nor
lib/, skip any line until a new file is reached.


-- 
David Marchand

[PATCH v2] devtools: fix symbol change check for non-lib patches

[Ali Alnubani]

Handle patches that do not touch lib/drivers (e.g. doc-only) without
crashing. Treat every '--- a/' line as the start of a new file: if the
path is under lib/ or drivers/, set lib and process symbol lines;
otherwise set lib to None and ignore lines until the next file. This
avoids both NameError exceptions and misattributing export-like lines
from doc (or other) files to the previous lib when file order varies.

Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
Cc: david.marchand@redhat.com

Suggested-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---

v2:
Treat every '--- a/' as a new file: set lib only for paths under lib/
or drivers/, otherwise set lib to None and skip lines until the next
file, so behavior no longer depends on patch file order.

 devtools/check-symbol-change.py | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/devtools/check-symbol-change.py b/devtools/check-symbol-change.py
index 1efeb82fcd..a476ceae5e 100755
--- a/devtools/check-symbol-change.py
+++ b/devtools/check-symbol-change.py
@@ -29,17 +29,21 @@
 symbols = {}
 
 for file in args.patch:
+    lib = None
     for ln in file.readlines():
-        if file_header_regexp.match(ln):
-            if file_header_regexp.match(ln).group(2) == "lib":
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
-            elif file_header_regexp.match(ln).group(3) == "intel":
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3, 4))
+        if ln.startswith("--- a/"):
+            if file_header_regexp.match(ln):
+                m = file_header_regexp.match(ln)
+                if m.group(2) == "lib":
+                    lib = "/".join(m.group(2, 3))
+                elif m.group(3) == "intel":
+                    lib = "/".join(m.group(2, 3, 4))
+                else:
+                    lib = "/".join(m.group(2, 3))
+                if lib not in symbols:
+                    symbols[lib] = {}
             else:
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
-
-            if lib not in symbols:
-                symbols[lib] = {}
+                lib = None
             continue
 
         if export_exp_sym_regexp.match(ln):
@@ -54,6 +58,8 @@
         else:
             continue
 
+        if lib is None:
+            continue
         if symbol not in symbols[lib]:
             symbols[lib][symbol] = {}
         added = ln[0] == "+"
-- 
2.53.0

Re: [PATCH v2] devtools: fix symbol change check for non-lib patches

[David Marchand]

Hello Ali,

Just repeating what we discussed offlist for visibility.


On Thu, 5 Mar 2026 at 11:52, Ali Alnubani <alialnu@nvidia.com> wrote:
>
> Handle patches that do not touch lib/drivers (e.g. doc-only) without
> crashing. Treat every '--- a/' line as the start of a new file: if the
> path is under lib/ or drivers/, set lib and process symbol lines;
> otherwise set lib to None and ignore lines until the next file. This
> avoids both NameError exceptions and misattributing export-like lines
> from doc (or other) files to the previous lib when file order varies.
>
> Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
> Cc: david.marchand@redhat.com
>
> Suggested-by: David Marchand <david.marchand@redhat.com>

No need for a suggested-by, you caught the issue and I am just
reviewing the fix :-).


> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
>
> v2:
> Treat every '--- a/' as a new file: set lib only for paths under lib/
> or drivers/, otherwise set lib to None and skip lines until the next
> file, so behavior no longer depends on patch file order.
>
>  devtools/check-symbol-change.py | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/devtools/check-symbol-change.py b/devtools/check-symbol-change.py
> index 1efeb82fcd..a476ceae5e 100755
> --- a/devtools/check-symbol-change.py
> +++ b/devtools/check-symbol-change.py
> @@ -29,17 +29,21 @@
>  symbols = {}
>
>  for file in args.patch:
> +    lib = None
>      for ln in file.readlines():
> -        if file_header_regexp.match(ln):
> -            if file_header_regexp.match(ln).group(2) == "lib":
> -                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
> -            elif file_header_regexp.match(ln).group(3) == "intel":
> -                lib = "/".join(file_header_regexp.match(ln).group(2, 3, 4))
> +        if ln.startswith("--- a/"):

This line above must be updated to also catch +++ b/ or the subsequent
regexp won't be called in the "addition" case.


> +            if file_header_regexp.match(ln):
> +                m = file_header_regexp.match(ln)
> +                if m.group(2) == "lib":
> +                    lib = "/".join(m.group(2, 3))
> +                elif m.group(3) == "intel":
> +                    lib = "/".join(m.group(2, 3, 4))
> +                else:
> +                    lib = "/".join(m.group(2, 3))
> +                if lib not in symbols:
> +                    symbols[lib] = {}
>              else:
> -                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
> -
> -            if lib not in symbols:
> -                symbols[lib] = {}
> +                lib = None
>              continue
>
>          if export_exp_sym_regexp.match(ln):
> @@ -54,6 +58,8 @@
>          else:
>              continue
>
> +        if lib is None:
> +            continue

We can move this check before evaluating all export_* regexps.


>          if symbol not in symbols[lib]:
>              symbols[lib][symbol] = {}
>          added = ln[0] == "+"
> --
> 2.53.0
>


-- 
David Marchand

[PATCH v3] devtools: fix symbol change check for non-lib patches

[Ali Alnubani]

Handle patches that do not touch lib/drivers (e.g. doc-only) without
crashing. Treat every file header (--- a/ or +++ b/) as the start of a
new file: if the path is under lib/ or drivers/, set lib and process
symbol lines; otherwise set lib to None and ignore lines until the next
file. This avoids both NameError exceptions and misattributing
export-like lines from doc (or other) files to the previous lib when
file order varies.

Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
Cc: david.marchand@redhat.com

Suggested-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---

v2:
Treat every '--- a/' as a new file: set lib only for paths under lib/
or drivers/, otherwise set lib to None and skip lines until the next
file, so behavior no longer depends on patch file order.

v3:
Use a regex so both '--- a/' and '+++ b/' start a new file, and run 'if lib
is None: continue; right after the file-header block so non-lib lines are
skipped before any export regex is run.

 devtools/check-symbol-change.py | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/devtools/check-symbol-change.py b/devtools/check-symbol-change.py
index 1efeb82fcd..b1daa2934a 100755
--- a/devtools/check-symbol-change.py
+++ b/devtools/check-symbol-change.py
@@ -7,6 +7,7 @@
 import argparse
 import re
 
+new_file_regexp = re.compile(r"^(\-\-\-|\+\+\+) [ab]/")
 file_header_regexp = re.compile(r"^(\-\-\-|\+\+\+) [ab]/(lib|drivers)/([^/]+)/([^/]+)")
 # From eal_exports.h
 export_exp_sym_regexp = re.compile(r"^.RTE_EXPORT_EXPERIMENTAL_SYMBOL\(([^,]+),")
@@ -29,17 +30,24 @@
 symbols = {}
 
 for file in args.patch:
+    lib = None
     for ln in file.readlines():
-        if file_header_regexp.match(ln):
-            if file_header_regexp.match(ln).group(2) == "lib":
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
-            elif file_header_regexp.match(ln).group(3) == "intel":
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3, 4))
+        if new_file_regexp.match(ln):
+            if file_header_regexp.match(ln):
+                m = file_header_regexp.match(ln)
+                if m.group(2) == "lib":
+                    lib = "/".join(m.group(2, 3))
+                elif m.group(3) == "intel":
+                    lib = "/".join(m.group(2, 3, 4))
+                else:
+                    lib = "/".join(m.group(2, 3))
+                if lib not in symbols:
+                    symbols[lib] = {}
             else:
-                lib = "/".join(file_header_regexp.match(ln).group(2, 3))
+                lib = None
+            continue
 
-            if lib not in symbols:
-                symbols[lib] = {}
+        if lib is None:
             continue
 
         if export_exp_sym_regexp.match(ln):
-- 
2.53.0

Re: [PATCH v3] devtools: fix symbol change check for non-lib patches

[David Marchand]

On Mon, 9 Mar 2026 at 16:10, Ali Alnubani <alialnu@nvidia.com> wrote:
>
> Handle patches that do not touch lib/drivers (e.g. doc-only) without
> crashing. Treat every file header (--- a/ or +++ b/) as the start of a
> new file: if the path is under lib/ or drivers/, set lib and process
> symbol lines; otherwise set lib to None and ignore lines until the next
> file. This avoids both NameError exceptions and misattributing
> export-like lines from doc (or other) files to the previous lib when
> file order varies.
>
> Fixes: 1a0c104a7fa9 ("build: generate symbol maps")
>
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.


-- 
David Marchand