Skip to content

feat(repair): add --strip-level option for granular strip control#682

Open
taegyunkim wants to merge 5 commits into
pypa:mainfrom
taegyunkim:strip-levels
Open

feat(repair): add --strip-level option for granular strip control#682
taegyunkim wants to merge 5 commits into
pypa:mainfrom
taegyunkim:strip-levels

Conversation

@taegyunkim
Copy link
Copy Markdown

@taegyunkim taegyunkim commented Mar 24, 2026

Summary

Closes #491.

  • Adds a new --strip-level option to auditwheel repair with four levels: none (default), debug (strip -g), unneeded (strip --strip-unneeded), and all (strip -s)
  • Deprecates the existing --strip flag (maps to --strip-level=all for backward compatibility)
  • Validates conflicting --strip and --strip-level usage early, before any wheel processing begins

Motivation

The current --strip flag only supports strip -s (strip all symbols), which is too aggressive for many use cases. Users building wheels with debug symbols want the ability to strip only debug info (-g) or unneeded symbols (--strip-unneeded) while keeping the symbol table intact for debugging and profiling.

Design

  • StripLevel enum in repair.py maps each level to the corresponding strip CLI arguments
  • process_symbols() applies stripping to both vendored libraries and extension modules
  • Conflict guard and strip normalization happen at the top of repair_wheel(), before the pure-wheel early return
  • _get_strip_args(StripLevel.NONE) raises ValueError as a safety net (callers skip processing for NONE)

Test plan

  • Unit tests for argument parsing (TestStripLevelArgument)
  • Unit tests for execute() integration (TestStripLevelExecute) — verifies strip level is passed through, deprecation warning is emitted, and conflicting args error
  • Unit tests for repair_wheel() behavior (TestRepairWheelStripLevels) — verifies process_symbols is called/skipped correctly per level
  • Unit tests for _get_strip_args and process_symbols (TestGetStripArgs, TestProcessSymbols, TestStripSymbolsShim)
  • Conflict guard test on pure wheels (TestRepairWheelConflict)
  • All 200 unit tests pass

taegyunkim and others added 4 commits March 21, 2026 17:27
Add a --strip-level flag to auditwheel repair with four levels:
- none (default, no change in behavior)
- debug (strip -g: removes debug symbols only, preserves full backtraces)
- unneeded (strip --strip-unneeded: removes symbols not needed for relocation)
- all (strip -s: removes all symbols, equivalent to old --strip)

The existing --strip flag is deprecated in favour of --strip-level=all.
Both flags are mutually exclusive. The strip_symbols() function is kept
for backward API compatibility and delegates to the new process_symbols().

Closes pypa#491

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move conflict guard and strip normalization to top of repair_wheel(),
  before the pure-wheel early return, preventing partial wheel writes
- Resolve StripLevel.NONE vs Python None confusion: execute() now
  resolves --strip to strip_level=ALL before the loop and no longer
  passes the deprecated strip= parameter to repair_wheel()
- Fix zip_compression_level default from 0 (Z_NO_COMPRESSION) to
  zlib.Z_DEFAULT_COMPRESSION
- Make _get_strip_args(NONE) raise ValueError instead of returning []
  (which would invoke strip with no flags, stripping all symbols)
- Fix test mocks for Python 3.14 compatibility (MagicMock comparison),
  lazy import patch targets, dist-info assertion, and SBOM patching
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a --strip-level option to auditwheel repair exposing four granularities (none, debug, unneeded, all) mapped to the corresponding strip CLI flags, while keeping the legacy --strip flag as a deprecated alias for --strip-level=all. Conflicting use of the two options is rejected up front in execute(), and repair_wheel() is extended with a normalized strip_level parameter that drives a new process_symbols() helper.

Changes:

  • New StripLevel enum + process_symbols()/_get_strip_args() in repair.py; strip_symbols() kept as a thin compat shim.
  • --strip-level argparse option added and --strip deprecated; execute() validates and resolves both into a single strip_level before processing.
  • New unit tests covering the enum, arg parsing, execute() integration, repair_wheel() strip dispatch, and a conflict guard.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/auditwheel/repair.py Adds StripLevel, process_symbols, _get_strip_args; updates repair_wheel() signature and strip dispatch; keeps strip_symbols() as compat shim.
src/auditwheel/main_repair.py Adds --strip-level CLI option, deprecates --strip, validates conflicts, resolves strip_level and forwards it to repair_wheel().
tests/unit/test_strip_levels.py Unit tests for StripLevel, _get_strip_args, process_symbols, and the strip_symbols shim.
tests/unit/test_repair_strip_levels.py Tests repair_wheel() strip-level dispatch, backwards compat for strip=True, and the conflict guard.
tests/unit/test_main_repair_strip_levels.py Tests CLI parsing, deprecation warning, conflict error path, and forwarding of strip_level from execute() to repair_wheel().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wheel_files: list[Path] = args.WHEEL_FILE

# Validate and resolve strip arguments once, before processing any wheel.
if args.STRIP and args.STRIP_LEVEL != "none":
Comment on lines +167 to +172
if args.STRIP:
warnings.warn(
"The --strip option is deprecated. Use --strip-level=all instead.",
DeprecationWarning,
stacklevel=2,
)
Comment thread src/auditwheel/repair.py
Comment on lines +170 to +183
def process_symbols(
libraries: Iterable[Path],
strip_level: StripLevel,
) -> None:
"""Process symbols in libraries according to the given strip level."""
libraries_list = list(libraries)

if not libraries_list or strip_level == StripLevel.NONE:
return

strip_args = _get_strip_args(strip_level)
for lib in libraries_list:
logger.info("Stripping symbols from %s (level: %s)", lib, strip_level.value)
check_call(["strip", *strip_args, str(lib)])
Copy link
Copy Markdown
Member

@mayeut mayeut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your pull request,
Can you please address my inline comments as well as copilot ones ?

Comment thread src/auditwheel/repair.py
Comment on lines +51 to +53
strip: bool | None = None,
strip_level: StripLevel | None = None,
zip_compression_level: int = zlib.Z_DEFAULT_COMPRESSION,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auditwheel is a CLI tool as mentioned in the README. Although the modules do not follow the conventional naming for this, there's no public python API, everything is private. Please do not keep the compatibility shims in the python code.

Suggested change
strip: bool | None = None,
strip_level: StripLevel | None = None,
zip_compression_level: int = zlib.Z_DEFAULT_COMPRESSION,
strip_level: StripLevel,
zip_compression_level: int,

Comment thread src/auditwheel/repair.py
Comment on lines +55 to +64
# Validate and normalize strip arguments before doing any work.
if strip is not None and strip_level is not None:
msg = "Cannot specify both 'strip' and 'strip_level' parameters"
raise ValueError(msg)

if strip is True:
strip_level = StripLevel.ALL
elif strip_level is None:
strip_level = StripLevel.NONE

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the reason above, this is not needed without compatibility in mind.

Suggested change
# Validate and normalize strip arguments before doing any work.
if strip is not None and strip_level is not None:
msg = "Cannot specify both 'strip' and 'strip_level' parameters"
raise ValueError(msg)
if strip is True:
strip_level = StripLevel.ALL
elif strip_level is None:
strip_level = StripLevel.NONE

Comment thread src/auditwheel/repair.py
Comment on lines +155 to +156
extensions = list(external_refs_by_fn.keys())
all_libraries = list(itertools.chain(libs_to_process, extensions))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to enforce lists, iterables should be enough

Suggested change
extensions = list(external_refs_by_fn.keys())
all_libraries = list(itertools.chain(libs_to_process, extensions))
extensions =external_refs_by_fn.keys()
all_libraries = itertools.chain(libs_to_process, extensions)

Comment thread src/auditwheel/repair.py
libs_to_process = [path for (_, path) in soname_map.values()]
extensions = list(external_refs_by_fn.keys())
all_libraries = list(itertools.chain(libs_to_process, extensions))
process_symbols(all_libraries, strip_level)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
process_symbols(all_libraries, strip_level)
strip_symbols(all_libraries, strip_level)

Comment thread src/auditwheel/repair.py
return output_wheel


def process_symbols(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def process_symbols(
def strip_symbols(

Comment thread src/auditwheel/repair.py
Comment on lines +204 to +208


def strip_symbols(libraries: Iterable[Path]) -> None:
for lib in libraries:
logger.info("Stripping symbols from %s", lib)
check_call(["strip", "-s", lib])
"""Legacy function for backward compatibility. Use process_symbols instead."""
process_symbols(libraries, StripLevel.ALL)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def strip_symbols(libraries: Iterable[Path]) -> None:
for lib in libraries:
logger.info("Stripping symbols from %s", lib)
check_call(["strip", "-s", lib])
"""Legacy function for backward compatibility. Use process_symbols instead."""
process_symbols(libraries, StripLevel.ALL)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow repair to use strip with --strip-debug instead of only allowing --strip-all

3 participants