DAOS-18933 ci: fix PermissionError in pre-commit clang-format hook

[knard38]

Description

Fixes DAOS-18933 — the pre-commit clang-format hook crashes with a
PermissionError on any machine where the SCons WhereIs stub is active.

Problem

extra.py is used in two contexts:

1. Inside a SCons build — where SCons.Script.WhereIs is the real implementation that searches PATH.
2. Standalone, invoked by the pre-commit hook — where SCons replaces WhereIs with a no-op stub:

def WhereIs(path):
    """Fake WhereIs"""
    return ''

The previous guard only checked for None:

clang_exe = WhereIs('clang-format')
if clang_exe is None:   # '' slips through this check
    return None
subprocess.check_output([clang_exe, "-version"])  # runs ['', '-version'] → PermissionError

Because '' is not None, the guard was bypassed and subprocess.check_output
was called with an empty string as the executable, raising:

PermissionError: [Errno 13] Permission denied: ''

This hit every developer running git commit when the stub WhereIs was in effect.
The clang-format check was silently skipped, but the alarming traceback caused confusion.

Fix

• Fall back to shutil.which('clang-format') (Python stdlib, no SCons dependency)
  when WhereIs returns a falsy value.
• Replace the is None guard with a falsy guard (if not clang_exe) to cover both
  None and empty-string cases.

clang_exe = WhereIs('clang-format') or shutil.which('clang-format')
if not clang_exe:
    return None

shutil.which correctly searches os.environ['PATH'] in both contexts.

Changed files

File	Change
site_scons/site_tools/extra/extra.py	Add shutil import; use or shutil.which() fallback; replace is None with falsy guard

Steps for the author:

• Commit message follows the guidelines.
• Appropriate Features or Test-tag pragmas were used.
• Appropriate Functional Test Stages were run.
• At least two positive code reviews including at least one code owner from each category referenced in the PR.
• Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

• Gatekeeper requested (daos-gatekeeper added as a reviewer).

[github-actions]

Ticket title is 'Pre-commit clang-format hook crashes with PermissionError'
Status is 'Open'
https://daosio.atlassian.net/browse/DAOS-18933

[janekmi]

Why you decided to use both? Why not just replace the unreliable one with a reliable one?

[knard38]

SCons.Script ships a no-op stub for WhereIs that is active whenever the module is imported outside a full SCons build (i.e. the pre-commit hook context).

On my machine:

>>> from SCons.Script import WhereIs; WhereIs('clang-format')
''                          # stub always returns ''

>>> import shutil; shutil.which('clang-format')
'/usr/bin/clang-format'     # stdlib finds it correctly

Replacing WhereIs entirely would fix the crash but lose the SCons-environment-aware lookup during actual builds. The or keeps WhereIs when it works and falls back to shutil.which when the stub is in effect.

[janekmi]

It is a little messy. 😅

Another thought. Why not use shutil.which('clang-format') in the WhereIs stub?

[knard38]

It is a little messy. 😅

Another thought. Why not use shutil.which('clang-format') in the WhereIs stub?

Did not wanted to touch to the fake_scons stubs as I am not sure of their purpose.
I am going to investigate as I agree that it should be a better solution: I have to check that I am not breaking anything (test, etc.).