[PATCH v2] checkkconfigsymbols: resolve revisions before resetting the tree

[PATCH v2] checkkconfigsymbols: resolve revisions before resetting the tree

[Erkan Erdem]

The commit comparison resets the current branch to commit_a before
resolving commit_b. If the second revision is HEAD or the current branch
name, it then resolves to the first revision. For example, --diff
HEAD^..HEAD compares the parent with itself and silently misses newly
undefined symbols. The same problem affects --commit with the current
branch name.

Resolve and verify both revisions as commits before the first reset.
This keeps their meaning stable throughout the comparison and rejects
invalid endpoints before changing the working tree.

Keep lookup diagnostics separate from the resolved hashes. Use the
resolved range for --find too, since resetting also updates ORIG_HEAD.

Fixes: b1a3f243485f ("checkkconfigsymbols.py: make it Git aware")
Link: https://lore.kernel.org/20210901145212.478066-1-arielmarcovitch@gmail.com/
Assisted-by: LLM
Signed-off-by: Erkan Erdem <hexvalid@gmail.com>
---

Changes in v2, addressing Julian Braha's review:
- Submit this fix independently of the unrelated Kconfig input fix.
- No code changes from v1.
- Include the original Git-awareness author and contributors to the
  previous HEAD-reference fix in Cc.

v1: https://lore.kernel.org/all/20260905123237.40670-2-hexvalid@gmail.com/

An AI coding assistant found the issue, prepared the fix and changelog,
and ran the original verification. The same assistant prepared this
standalone v2 submission.

Validation:
- The code is unchanged from v1, which passed 15 actual-CLI cases in
  disposable Git repositories, including HEAD-relative ranges, branch
  names, hashes, tags, annotated tags, ambiguous names, --commit,
  --find, ORIG_HEAD and invalid or non-commit endpoints.
- Those checks verified HEAD, current branch, index and tracked file
  contents after every invocation.
- The script in mainline df2908090cda368b01ff43709f51890076c56157 still
  matches the original pre-fix file; this patch applies independently.

 scripts/checkkconfigsymbols.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
index 36c920e71..e0a1a631b 100755
--- a/scripts/checkkconfigsymbols.py
+++ b/scripts/checkkconfigsymbols.py
@@ -150,6 +150,13 @@ def print_undefined_symbols():
             undefined_a = {}
             undefined_b = {}
 
+        commit_a = execute(["git", "rev-parse", "--verify", commit_a + "^{commit}"],
+                           stderr=None).strip()
+        commit_b = execute(["git", "rev-parse", "--verify", commit_b + "^{commit}"],
+                           stderr=None).strip()
+        if args.diff:
+            args.diff = commit_a + ".." + commit_b
+
         # get undefined items before the commit
         reset(commit_a)
         undefined_a, _ = check_symbols(args.ignore)
@@ -223,10 +230,10 @@ def red(string):
     return "\033[31m%s\033[0m" % string if COLOR else string
 
 
-def execute(cmd):
+def execute(cmd, stderr=subprocess.STDOUT):
     """Execute %cmd and return stdout.  Exit in case of error."""
     try:
-        stdout = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=False)
+        stdout = subprocess.check_output(cmd, stderr=stderr, shell=False)
         stdout = stdout.decode(errors='replace')
     except subprocess.CalledProcessError as fail:
         exit(fail)

base-commit: 4d7d9486c04d917265f64c55bd23b2cc4fe7749c
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] checkkconfigsymbols: resolve revisions before resetting the tree

[Julian Braha]

On 9/7/26 11:37, Erkan Erdem wrote:
> The commit comparison resets the current branch to commit_a before
> resolving commit_b. If the second revision is HEAD or the current branch
> name, it then resolves to the first revision. For example, --diff
> HEAD^..HEAD compares the parent with itself and silently misses newly
> undefined symbols. The same problem affects --commit with the current
> branch name.
> 
> Resolve and verify both revisions as commits before the first reset.
> This keeps their meaning stable throughout the comparison and rejects
> invalid endpoints before changing the working tree.
> 
> Keep lookup diagnostics separate from the resolved hashes. Use the
> resolved range for --find too, since resetting also updates ORIG_HEAD.
> 
> Fixes: b1a3f243485f ("checkkconfigsymbols.py: make it Git aware")

While it's good to support this, I don't think the Fixes tag is
appropriate here, because this wasn't really a bug... it was just
unsupported functionality.

> Link: https://lore.kernel.org/20210901145212.478066-1-arielmarcovitch@gmail.com/

This commit added a check that will cause the script to exit if
'HEAD...' is passed to --commit. So it seems like this patch should also
remove those lines now.

Note that doing so would also effectively revert that commit, so in v3
it would be good to add:

'This reverts commit ...'

to your commit message.

- Julian Braha

Re: [PATCH v2] checkkconfigsymbols: resolve revisions before resetting the tree

[Nathan Chancellor]

On Tue, Sep 15, 2026 at 12:43:29PM +0100, Julian Braha wrote:
> On 9/7/26 11:37, Erkan Erdem wrote:
> > The same problem affects --commit with the current branch name.
> > 
> > Fixes: b1a3f243485f ("checkkconfigsymbols.py: make it Git aware")
> 
> While it's good to support this, I don't think the Fixes tag is
> appropriate here, because this wasn't really a bug... it was just
> unsupported functionality.

I guess it is because neither this change nor the below change did not
take care of disallowing the branch name with '--commit'. That said, I
do agree that I think a Fixes is a little superfluous here, I would
leave it off of future revisions.

> > Link: https://lore.kernel.org/20210901145212.478066-1-arielmarcovitch@gmail.com/
> 
> This commit added a check that will cause the script to exit if
> 'HEAD...' is passed to --commit. So it seems like this patch should also
> remove those lines now.

While I do agree that we should revert this change, which is

  d62d5aed3354 ("checkkconfigsymbols.py: Forbid passing 'HEAD' to --commit")

in the tree, I think it would be better done as a separate commit, whose
justification is that HEAD is now properly resolved as a result of this
change.

-- 
Cheers,
Nathan