The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v1] perf python: Clean up and restructure setup.py
@ 2026-05-21 17:55 Ian Rogers
  2026-07-22 16:51 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2026-05-21 17:55 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Leo Yan, linux-perf-users, linux-kernel

Clean up and restructure the python setup script to resolve pylint
warnings, improve code quality, and increase robustness and
readability, targeting Python 3.9+ (the Linux kernel build minimum
Python version).

Changes:
- Restructure the script to use a `main()` function as the entry point,
  leaving only imports, classes, and pure functions at module level.
- Eliminate all global/module-level variables, making them local to
  `main()` or the respective classes/functions.
- Make `clang_has_option` a pure function by passing all necessary
  parameters explicitly.
- Extract clang compiler flag filtering into a new
  `filter_clang_options` helper function. This function uses a loop
  over a tuple of options, replacing ~30 lines of repetitive blocks
  and reducing branch/statement complexity in the main flow.
- Cleanly define attributes in `__init__` for `BuildExt` and `InstallLib`
  and read environment variables dynamically within the methods, removing
  their dependency on global variables.
- Replace legacy Popen with subprocess.run for safer process handling.
- Use list-based flag filtering (split, filter, re-join) instead of regex
  `re.sub` substitutions on space-separated compiler flag strings. This
  avoids boundary bugs and safely handles flags with arguments (e.g.
  `-fcf-protection=full`).
- Safely parse `CC` env var using `shlex.split` to handle quotes and pass
  compiler arguments as `list[str]` lists to helper functions, avoiding
  redundant string formatting and parsing.
- Remove unused `import re`.
- Rename setuptools command subclasses to PascalCase (BuildExt, InstallLib).
- Add type annotations to functions and methods.
- Add missing docstrings for module, functions, and classes.
- Split long lines to adhere to standard limits.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/setup.py | 253 +++++++++++++++++++++++++--------------
 1 file changed, 166 insertions(+), 87 deletions(-)

diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index b65b1792ca05..55ff7d8cba7e 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -1,108 +1,187 @@
-from os import getenv, path
-from subprocess import Popen, PIPE
-from re import sub
+"""Setup script for perf python extension.
+
+This script is used to build and install the perf python binding.
+It handles compiler-specific flags, especially for clang, and configures
+the setuptools Extension.
+"""
+
+import os
 import shlex
+import subprocess
+import sysconfig
 
-cc = getenv("CC")
-assert cc, "Environment variable CC not set"
+from setuptools import setup, Extension
+from setuptools.command.build_ext import build_ext as _build_ext
+from setuptools.command.install_lib import install_lib as _install_lib
 
-# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
-cc_tokens = cc.split()
-if len(cc_tokens) > 1:
-    cc = cc_tokens[0]
-    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
-else:
-    cc_options = ""
 
-# ignore optional stderr could be None as it is set to PIPE to avoid that.
-# mypy: disable-error-code="union-attr"
-cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
+def clang_has_option(cc: str, cc_args: list[str], src_feature_tests: str, option: str) -> bool:
+    """Check if clang supports a specific option.
 
-srctree = getenv('srctree')
-assert srctree, "Environment variable srctree, for the Linux sources, not set"
-src_feature_tests  = f'{srctree}/tools/build/feature'
+    Args:
+        cc: The compiler executable.
+        cc_args: Compiler arguments from CC environment variable.
+        src_feature_tests: Path to the feature tests directory.
+        option: The compiler option to check (e.g., "-mcet").
 
-def clang_has_option(option):
+    Returns:
+        True if the option is supported, False otherwise.
+    """
     error_substrings = (
         b"unknown argument",
         b"is not supported",
         b"unknown warning option"
     )
-    cmd = shlex.split(f"{cc} {cc_options} {option}") + [
+    cmd = [cc] + cc_args + [
+        option,
         "-o", "/dev/null",
-        path.join(src_feature_tests, "test-hello.c")
+        os.path.join(src_feature_tests, "test-hello.c")
     ]
-    cc_output = Popen(cmd, stderr=PIPE).stderr.readlines()
+    try:
+        res = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.DEVNULL, check=False)
+        cc_output = res.stderr.splitlines()
+    except OSError:
+        return False
     return not any(any(error in line for error in error_substrings) for line in cc_output)
 
-if cc_is_clang:
-    from sysconfig import get_config_vars
-    vars = get_config_vars()
+
+def filter_clang_options(cc: str, cc_args: list[str], src_feature_tests: str) -> None:
+    """Filter out unsupported clang options from sysconfig CFLAGS and OPT.
+
+    Args:
+        cc: The compiler executable.
+        cc_args: Compiler arguments from CC environment variable.
+        src_feature_tests: Path to the feature tests directory.
+    """
+    config_vars = sysconfig.get_config_vars()
     for var in ('CFLAGS', 'OPT'):
-        vars[var] = sub("-specs=[^ ]+", "", vars[var])
-        if not clang_has_option("-mcet"):
-            vars[var] = sub("-mcet", "", vars[var])
-        if not clang_has_option("-fcf-protection"):
-            vars[var] = sub("-fcf-protection", "", vars[var])
-        if not clang_has_option("-fstack-clash-protection"):
-            vars[var] = sub("-fstack-clash-protection", "", vars[var])
-        if not clang_has_option("-fstack-protector-strong"):
-            vars[var] = sub("-fstack-protector-strong", "", vars[var])
-        if not clang_has_option("-fno-semantic-interposition"):
-            vars[var] = sub("-fno-semantic-interposition", "", vars[var])
-        if not clang_has_option("-ffat-lto-objects"):
-            vars[var] = sub("-ffat-lto-objects", "", vars[var])
-        if not clang_has_option("-ftree-loop-distribute-patterns"):
-            vars[var] = sub("-ftree-loop-distribute-patterns", "", vars[var])
-        if not clang_has_option("-gno-variable-location-views"):
-            vars[var] = sub("-gno-variable-location-views", "", vars[var])
+        if var not in config_vars:
+            continue
+
+        # Split into individual flags to avoid regex boundary issues
+        flags = config_vars[var].split()
+
+        # Remove -specs=...
+        flags = [f for f in flags if not f.startswith("-specs=")]
+
+        options = (
+            "-mcet",
+            "-fcf-protection",
+            "-fstack-clash-protection",
+            "-fstack-protector-strong",
+            "-fno-semantic-interposition",
+            "-ffat-lto-objects",
+            "-ftree-loop-distribute-patterns",
+            "-gno-variable-location-views"
+        )
+        for option in options:
+            if not clang_has_option(cc, cc_args, src_feature_tests, option):
+                # Remove the option and any variant (e.g. -option=...)
+                flags = [f for f in flags if not f.startswith(option)]
+
+        # Re-join flags
+        config_vars[var] = " ".join(flags)
+
+
+class BuildExt(_build_ext):
+    """Custom build_ext command to set output directories."""
+
+    def __init__(self, *args, **kwargs):
+        self.build_lib = None
+        self.build_temp = None
+        super().__init__(*args, **kwargs)
+
+    def finalize_options(self) -> None:
+        _build_ext.finalize_options(self)
+        build_lib = os.getenv('PYTHON_EXTBUILD_LIB')
+        build_tmp = os.getenv('PYTHON_EXTBUILD_TMP')
+        if build_lib:
+            self.build_lib = build_lib
+        if build_tmp:
+            self.build_temp = build_tmp
 
-from setuptools import setup, Extension
 
-from setuptools.command.build_ext   import build_ext   as _build_ext
-from setuptools.command.install_lib import install_lib as _install_lib
+class InstallLib(_install_lib):
+    """Custom install_lib command to set output directory."""
 
-class build_ext(_build_ext):
-    def finalize_options(self):
-        _build_ext.finalize_options(self)
-        self.build_lib  = build_lib
-        self.build_temp = build_tmp
+    def __init__(self, *args, **kwargs):
+        self.build_dir = None
+        super().__init__(*args, **kwargs)
 
-class install_lib(_install_lib):
-    def finalize_options(self):
+    def finalize_options(self) -> None:
         _install_lib.finalize_options(self)
-        self.build_dir = build_lib
-
-
-cflags = getenv('CFLAGS', '').split()
-# switch off several checks (need to be at the end of cflags list)
-cflags += ['-fno-strict-aliasing', '-Wno-write-strings', '-Wno-unused-parameter', '-Wno-redundant-decls' ]
-if cc_is_clang:
-    cflags += ["-Wno-unused-command-line-argument" ]
-    if clang_has_option("-Wno-cast-function-type-mismatch"):
-        cflags += ["-Wno-cast-function-type-mismatch" ]
-else:
-    cflags += ['-Wno-cast-function-type' ]
-
-# The python headers have mixed code with declarations (decls after asserts, for instance)
-cflags += [ "-Wno-declaration-after-statement" ]
-
-src_perf  = f'{srctree}/tools/perf'
-build_lib = getenv('PYTHON_EXTBUILD_LIB')
-build_tmp = getenv('PYTHON_EXTBUILD_TMP')
-
-perf = Extension('perf',
-                 sources = [ src_perf + '/util/python.c' ],
-		         include_dirs = ['util/include'],
-		         extra_compile_args = cflags,
-                 )
-
-setup(name='perf',
-      version='0.1',
-      description='Interface with the Linux profiling infrastructure',
-      author='Arnaldo Carvalho de Melo',
-      author_email='acme@redhat.com',
-      license='GPLv2',
-      url='http://perf.wiki.kernel.org',
-      ext_modules=[perf],
-      cmdclass={'build_ext': build_ext, 'install_lib': install_lib})
+        build_lib = os.getenv('PYTHON_EXTBUILD_LIB')
+        if build_lib:
+            self.build_dir = build_lib
+
+
+def main() -> None:
+    """Main entry point for the setup script."""
+    cc_env = os.getenv("CC")
+    assert cc_env, "Environment variable CC not set"
+
+    # Safe parsing of CC environment variable which might contain options/quotes
+    cc_tokens = shlex.split(cc_env)
+    cc = cc_tokens[0]
+    cc_args = cc_tokens[1:]
+
+    # Run CC -v to check if it is clang.
+    try:
+        cc_info = subprocess.run(
+            [cc, "-v"], stderr=subprocess.PIPE, stdout=subprocess.DEVNULL, check=False
+        )
+        cc_is_clang = b"clang version" in cc_info.stderr
+    except OSError as e:
+        raise RuntimeError(f"Failed to execute compiler '{cc}': {e}") from e
+
+    srctree = os.getenv('srctree')
+    assert srctree, "Environment variable srctree, for the Linux sources, not set"
+    src_feature_tests = f'{srctree}/tools/build/feature'
+
+    if cc_is_clang:
+        filter_clang_options(cc, cc_args, src_feature_tests)
+
+    cflags = os.getenv('CFLAGS', '').split()
+    # switch off several checks (need to be at the end of cflags list)
+    cflags += [
+        '-fno-strict-aliasing',
+        '-Wno-write-strings',
+        '-Wno-unused-parameter',
+        '-Wno-redundant-decls'
+    ]
+    if cc_is_clang:
+        cflags += ["-Wno-unused-command-line-argument"]
+        if clang_has_option(
+            cc, cc_args, src_feature_tests, "-Wno-cast-function-type-mismatch"
+        ):
+            cflags += ["-Wno-cast-function-type-mismatch"]
+    else:
+        cflags += ['-Wno-cast-function-type']
+
+    # The python headers have mixed code with declarations (decls after asserts, for instance)
+    cflags += ["-Wno-declaration-after-statement"]
+
+    src_perf = f'{srctree}/tools/perf'
+
+    perf = Extension(
+        'perf',
+        sources=[os.path.join(src_perf, 'util/python.c')],
+        include_dirs=['util/include'],
+        extra_compile_args=cflags,
+    )
+
+    setup(
+        name='perf',
+        version='0.1',
+        description='Interface with the Linux profiling infrastructure',
+        author='Arnaldo Carvalho de Melo',
+        author_email='acme@redhat.com',
+        license='GPLv2',
+        url='http://perf.wiki.kernel.org',
+        ext_modules=[perf],
+        cmdclass={'build_ext': BuildExt, 'install_lib': InstallLib},
+    )
+
+
+if __name__ == '__main__':
+    main()
-- 
2.54.0.794.g4f17f83d09-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] perf python: Clean up and restructure setup.py
  2026-05-21 17:55 [PATCH v1] perf python: Clean up and restructure setup.py Ian Rogers
@ 2026-07-22 16:51 ` Ian Rogers
  2026-07-23  5:32   ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2026-07-22 16:51 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Leo Yan, linux-perf-users, linux-kernel

On Thu, May 21, 2026 at 10:55 AM Ian Rogers <irogers@google.com> wrote:
>
> Clean up and restructure the python setup script to resolve pylint
> warnings, improve code quality, and increase robustness and
> readability, targeting Python 3.9+ (the Linux kernel build minimum
> Python version).
>
> Changes:
> - Restructure the script to use a `main()` function as the entry point,
>   leaving only imports, classes, and pure functions at module level.
> - Eliminate all global/module-level variables, making them local to
>   `main()` or the respective classes/functions.
> - Make `clang_has_option` a pure function by passing all necessary
>   parameters explicitly.
> - Extract clang compiler flag filtering into a new
>   `filter_clang_options` helper function. This function uses a loop
>   over a tuple of options, replacing ~30 lines of repetitive blocks
>   and reducing branch/statement complexity in the main flow.
> - Cleanly define attributes in `__init__` for `BuildExt` and `InstallLib`
>   and read environment variables dynamically within the methods, removing
>   their dependency on global variables.
> - Replace legacy Popen with subprocess.run for safer process handling.
> - Use list-based flag filtering (split, filter, re-join) instead of regex
>   `re.sub` substitutions on space-separated compiler flag strings. This
>   avoids boundary bugs and safely handles flags with arguments (e.g.
>   `-fcf-protection=full`).
> - Safely parse `CC` env var using `shlex.split` to handle quotes and pass
>   compiler arguments as `list[str]` lists to helper functions, avoiding
>   redundant string formatting and parsing.
> - Remove unused `import re`.
> - Rename setuptools command subclasses to PascalCase (BuildExt, InstallLib).
> - Add type annotations to functions and methods.
> - Add missing docstrings for module, functions, and classes.
> - Split long lines to adhere to standard limits.
>
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Ian Rogers <irogers@google.com>

Ping.

Thanks,
Ian

> ---
>  tools/perf/util/setup.py | 253 +++++++++++++++++++++++++--------------
>  1 file changed, 166 insertions(+), 87 deletions(-)
>
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index b65b1792ca05..55ff7d8cba7e 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -1,108 +1,187 @@
> -from os import getenv, path
> -from subprocess import Popen, PIPE
> -from re import sub
> +"""Setup script for perf python extension.
> +
> +This script is used to build and install the perf python binding.
> +It handles compiler-specific flags, especially for clang, and configures
> +the setuptools Extension.
> +"""
> +
> +import os
>  import shlex
> +import subprocess
> +import sysconfig
>
> -cc = getenv("CC")
> -assert cc, "Environment variable CC not set"
> +from setuptools import setup, Extension
> +from setuptools.command.build_ext import build_ext as _build_ext
> +from setuptools.command.install_lib import install_lib as _install_lib
>
> -# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
> -cc_tokens = cc.split()
> -if len(cc_tokens) > 1:
> -    cc = cc_tokens[0]
> -    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
> -else:
> -    cc_options = ""
>
> -# ignore optional stderr could be None as it is set to PIPE to avoid that.
> -# mypy: disable-error-code="union-attr"
> -cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
> +def clang_has_option(cc: str, cc_args: list[str], src_feature_tests: str, option: str) -> bool:
> +    """Check if clang supports a specific option.
>
> -srctree = getenv('srctree')
> -assert srctree, "Environment variable srctree, for the Linux sources, not set"
> -src_feature_tests  = f'{srctree}/tools/build/feature'
> +    Args:
> +        cc: The compiler executable.
> +        cc_args: Compiler arguments from CC environment variable.
> +        src_feature_tests: Path to the feature tests directory.
> +        option: The compiler option to check (e.g., "-mcet").
>
> -def clang_has_option(option):
> +    Returns:
> +        True if the option is supported, False otherwise.
> +    """
>      error_substrings = (
>          b"unknown argument",
>          b"is not supported",
>          b"unknown warning option"
>      )
> -    cmd = shlex.split(f"{cc} {cc_options} {option}") + [
> +    cmd = [cc] + cc_args + [
> +        option,
>          "-o", "/dev/null",
> -        path.join(src_feature_tests, "test-hello.c")
> +        os.path.join(src_feature_tests, "test-hello.c")
>      ]
> -    cc_output = Popen(cmd, stderr=PIPE).stderr.readlines()
> +    try:
> +        res = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.DEVNULL, check=False)
> +        cc_output = res.stderr.splitlines()
> +    except OSError:
> +        return False
>      return not any(any(error in line for error in error_substrings) for line in cc_output)
>
> -if cc_is_clang:
> -    from sysconfig import get_config_vars
> -    vars = get_config_vars()
> +
> +def filter_clang_options(cc: str, cc_args: list[str], src_feature_tests: str) -> None:
> +    """Filter out unsupported clang options from sysconfig CFLAGS and OPT.
> +
> +    Args:
> +        cc: The compiler executable.
> +        cc_args: Compiler arguments from CC environment variable.
> +        src_feature_tests: Path to the feature tests directory.
> +    """
> +    config_vars = sysconfig.get_config_vars()
>      for var in ('CFLAGS', 'OPT'):
> -        vars[var] = sub("-specs=[^ ]+", "", vars[var])
> -        if not clang_has_option("-mcet"):
> -            vars[var] = sub("-mcet", "", vars[var])
> -        if not clang_has_option("-fcf-protection"):
> -            vars[var] = sub("-fcf-protection", "", vars[var])
> -        if not clang_has_option("-fstack-clash-protection"):
> -            vars[var] = sub("-fstack-clash-protection", "", vars[var])
> -        if not clang_has_option("-fstack-protector-strong"):
> -            vars[var] = sub("-fstack-protector-strong", "", vars[var])
> -        if not clang_has_option("-fno-semantic-interposition"):
> -            vars[var] = sub("-fno-semantic-interposition", "", vars[var])
> -        if not clang_has_option("-ffat-lto-objects"):
> -            vars[var] = sub("-ffat-lto-objects", "", vars[var])
> -        if not clang_has_option("-ftree-loop-distribute-patterns"):
> -            vars[var] = sub("-ftree-loop-distribute-patterns", "", vars[var])
> -        if not clang_has_option("-gno-variable-location-views"):
> -            vars[var] = sub("-gno-variable-location-views", "", vars[var])
> +        if var not in config_vars:
> +            continue
> +
> +        # Split into individual flags to avoid regex boundary issues
> +        flags = config_vars[var].split()
> +
> +        # Remove -specs=...
> +        flags = [f for f in flags if not f.startswith("-specs=")]
> +
> +        options = (
> +            "-mcet",
> +            "-fcf-protection",
> +            "-fstack-clash-protection",
> +            "-fstack-protector-strong",
> +            "-fno-semantic-interposition",
> +            "-ffat-lto-objects",
> +            "-ftree-loop-distribute-patterns",
> +            "-gno-variable-location-views"
> +        )
> +        for option in options:
> +            if not clang_has_option(cc, cc_args, src_feature_tests, option):
> +                # Remove the option and any variant (e.g. -option=...)
> +                flags = [f for f in flags if not f.startswith(option)]
> +
> +        # Re-join flags
> +        config_vars[var] = " ".join(flags)
> +
> +
> +class BuildExt(_build_ext):
> +    """Custom build_ext command to set output directories."""
> +
> +    def __init__(self, *args, **kwargs):
> +        self.build_lib = None
> +        self.build_temp = None
> +        super().__init__(*args, **kwargs)
> +
> +    def finalize_options(self) -> None:
> +        _build_ext.finalize_options(self)
> +        build_lib = os.getenv('PYTHON_EXTBUILD_LIB')
> +        build_tmp = os.getenv('PYTHON_EXTBUILD_TMP')
> +        if build_lib:
> +            self.build_lib = build_lib
> +        if build_tmp:
> +            self.build_temp = build_tmp
>
> -from setuptools import setup, Extension
>
> -from setuptools.command.build_ext   import build_ext   as _build_ext
> -from setuptools.command.install_lib import install_lib as _install_lib
> +class InstallLib(_install_lib):
> +    """Custom install_lib command to set output directory."""
>
> -class build_ext(_build_ext):
> -    def finalize_options(self):
> -        _build_ext.finalize_options(self)
> -        self.build_lib  = build_lib
> -        self.build_temp = build_tmp
> +    def __init__(self, *args, **kwargs):
> +        self.build_dir = None
> +        super().__init__(*args, **kwargs)
>
> -class install_lib(_install_lib):
> -    def finalize_options(self):
> +    def finalize_options(self) -> None:
>          _install_lib.finalize_options(self)
> -        self.build_dir = build_lib
> -
> -
> -cflags = getenv('CFLAGS', '').split()
> -# switch off several checks (need to be at the end of cflags list)
> -cflags += ['-fno-strict-aliasing', '-Wno-write-strings', '-Wno-unused-parameter', '-Wno-redundant-decls' ]
> -if cc_is_clang:
> -    cflags += ["-Wno-unused-command-line-argument" ]
> -    if clang_has_option("-Wno-cast-function-type-mismatch"):
> -        cflags += ["-Wno-cast-function-type-mismatch" ]
> -else:
> -    cflags += ['-Wno-cast-function-type' ]
> -
> -# The python headers have mixed code with declarations (decls after asserts, for instance)
> -cflags += [ "-Wno-declaration-after-statement" ]
> -
> -src_perf  = f'{srctree}/tools/perf'
> -build_lib = getenv('PYTHON_EXTBUILD_LIB')
> -build_tmp = getenv('PYTHON_EXTBUILD_TMP')
> -
> -perf = Extension('perf',
> -                 sources = [ src_perf + '/util/python.c' ],
> -                        include_dirs = ['util/include'],
> -                        extra_compile_args = cflags,
> -                 )
> -
> -setup(name='perf',
> -      version='0.1',
> -      description='Interface with the Linux profiling infrastructure',
> -      author='Arnaldo Carvalho de Melo',
> -      author_email='acme@redhat.com',
> -      license='GPLv2',
> -      url='http://perf.wiki.kernel.org',
> -      ext_modules=[perf],
> -      cmdclass={'build_ext': build_ext, 'install_lib': install_lib})
> +        build_lib = os.getenv('PYTHON_EXTBUILD_LIB')
> +        if build_lib:
> +            self.build_dir = build_lib
> +
> +
> +def main() -> None:
> +    """Main entry point for the setup script."""
> +    cc_env = os.getenv("CC")
> +    assert cc_env, "Environment variable CC not set"
> +
> +    # Safe parsing of CC environment variable which might contain options/quotes
> +    cc_tokens = shlex.split(cc_env)
> +    cc = cc_tokens[0]
> +    cc_args = cc_tokens[1:]
> +
> +    # Run CC -v to check if it is clang.
> +    try:
> +        cc_info = subprocess.run(
> +            [cc, "-v"], stderr=subprocess.PIPE, stdout=subprocess.DEVNULL, check=False
> +        )
> +        cc_is_clang = b"clang version" in cc_info.stderr
> +    except OSError as e:
> +        raise RuntimeError(f"Failed to execute compiler '{cc}': {e}") from e
> +
> +    srctree = os.getenv('srctree')
> +    assert srctree, "Environment variable srctree, for the Linux sources, not set"
> +    src_feature_tests = f'{srctree}/tools/build/feature'
> +
> +    if cc_is_clang:
> +        filter_clang_options(cc, cc_args, src_feature_tests)
> +
> +    cflags = os.getenv('CFLAGS', '').split()
> +    # switch off several checks (need to be at the end of cflags list)
> +    cflags += [
> +        '-fno-strict-aliasing',
> +        '-Wno-write-strings',
> +        '-Wno-unused-parameter',
> +        '-Wno-redundant-decls'
> +    ]
> +    if cc_is_clang:
> +        cflags += ["-Wno-unused-command-line-argument"]
> +        if clang_has_option(
> +            cc, cc_args, src_feature_tests, "-Wno-cast-function-type-mismatch"
> +        ):
> +            cflags += ["-Wno-cast-function-type-mismatch"]
> +    else:
> +        cflags += ['-Wno-cast-function-type']
> +
> +    # The python headers have mixed code with declarations (decls after asserts, for instance)
> +    cflags += ["-Wno-declaration-after-statement"]
> +
> +    src_perf = f'{srctree}/tools/perf'
> +
> +    perf = Extension(
> +        'perf',
> +        sources=[os.path.join(src_perf, 'util/python.c')],
> +        include_dirs=['util/include'],
> +        extra_compile_args=cflags,
> +    )
> +
> +    setup(
> +        name='perf',
> +        version='0.1',
> +        description='Interface with the Linux profiling infrastructure',
> +        author='Arnaldo Carvalho de Melo',
> +        author_email='acme@redhat.com',
> +        license='GPLv2',
> +        url='http://perf.wiki.kernel.org',
> +        ext_modules=[perf],
> +        cmdclass={'build_ext': BuildExt, 'install_lib': InstallLib},
> +    )
> +
> +
> +if __name__ == '__main__':
> +    main()
> --
> 2.54.0.794.g4f17f83d09-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] perf python: Clean up and restructure setup.py
  2026-07-22 16:51 ` Ian Rogers
@ 2026-07-23  5:32   ` Namhyung Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2026-07-23  5:32 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
	Adrian Hunter, James Clark, Leo Yan, linux-perf-users,
	linux-kernel

On Wed, Jul 22, 2026 at 09:51:34AM -0700, Ian Rogers wrote:
> On Thu, May 21, 2026 at 10:55 AM Ian Rogers <irogers@google.com> wrote:
> >
> > Clean up and restructure the python setup script to resolve pylint
> > warnings, improve code quality, and increase robustness and
> > readability, targeting Python 3.9+ (the Linux kernel build minimum
> > Python version).
> >
> > Changes:
> > - Restructure the script to use a `main()` function as the entry point,
> >   leaving only imports, classes, and pure functions at module level.
> > - Eliminate all global/module-level variables, making them local to
> >   `main()` or the respective classes/functions.
> > - Make `clang_has_option` a pure function by passing all necessary
> >   parameters explicitly.
> > - Extract clang compiler flag filtering into a new
> >   `filter_clang_options` helper function. This function uses a loop
> >   over a tuple of options, replacing ~30 lines of repetitive blocks
> >   and reducing branch/statement complexity in the main flow.
> > - Cleanly define attributes in `__init__` for `BuildExt` and `InstallLib`
> >   and read environment variables dynamically within the methods, removing
> >   their dependency on global variables.
> > - Replace legacy Popen with subprocess.run for safer process handling.
> > - Use list-based flag filtering (split, filter, re-join) instead of regex
> >   `re.sub` substitutions on space-separated compiler flag strings. This
> >   avoids boundary bugs and safely handles flags with arguments (e.g.
> >   `-fcf-protection=full`).
> > - Safely parse `CC` env var using `shlex.split` to handle quotes and pass
> >   compiler arguments as `list[str]` lists to helper functions, avoiding
> >   redundant string formatting and parsing.
> > - Remove unused `import re`.
> > - Rename setuptools command subclasses to PascalCase (BuildExt, InstallLib).
> > - Add type annotations to functions and methods.
> > - Add missing docstrings for module, functions, and classes.
> > - Split long lines to adhere to standard limits.
> >
> > Assisted-by: Antigravity:gemini-3.5-flash
> > Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Ping.

Sorry for the delay.  Can you please resend the series?

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-23  5:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 17:55 [PATCH v1] perf python: Clean up and restructure setup.py Ian Rogers
2026-07-22 16:51 ` Ian Rogers
2026-07-23  5:32   ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox