Skip to content

Conversation

@kraenhansen
Copy link
Collaborator

Merging this PR will:

  • Introduce a new --strip option to allow the stripping of debug information from Android and Apple libraries.

@kraenhansen kraenhansen self-assigned this Oct 23, 2025
@kraenhansen kraenhansen requested a review from Copilot October 23, 2025 13:25
@kraenhansen kraenhansen added the enhancement New feature or request label Oct 23, 2025
Copy link
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

This PR adds a --strip option to cmake-rn for removing debug symbols from built Android and Apple native libraries, reducing binary size in production builds.

Key Changes:

  • Added --strip CLI option to enable debug symbol stripping
  • Implemented platform-specific stripping logic using strip (Apple) and llvm-strip (Android)
  • Refactored Android NDK path resolution into reusable helper functions

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/cmake-rn/src/cli.ts Adds --strip CLI option definition
packages/cmake-rn/src/platforms/apple.ts Implements stripping for Apple platforms using strip -rSTx command
packages/cmake-rn/src/platforms/android.ts Implements stripping for Android using NDK's llvm-strip and refactors NDK path helpers
.changeset/great-kings-clean.md Documents the new feature in the changeset

Comment on lines 246 to 250
const outputPath = path.join(buildPath, "out");
assert(
fs.existsSync(outputPath),
`Expected output file in ${outputPath}`,
);
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The outputPath variable is computed but never used. The assertion checks for 'out' directory existence, but this appears unrelated to the stripping operation. Either remove this unused code or clarify its purpose if it's meant to validate something about the strip operation.

Suggested change
const outputPath = path.join(buildPath, "out");
assert(
fs.existsSync(outputPath),
`Expected output file in ${outputPath}`,
);

Copilot uses AI. Check for mistakes.
const outputPath = path.join(buildPath, "out");
assert(
fs.existsSync(outputPath),
`Expected output file in ${outputPath}`,
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

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

The error message says 'Expected output file' but the check is for a directory named 'out'. The message should say 'Expected output directory' to accurately reflect what's being validated.

Suggested change
`Expected output file in ${outputPath}`,
`Expected output directory at ${outputPath}`,

Copilot uses AI. Check for mistakes.
@kraenhansen kraenhansen force-pushed the kh/strip-debug-symbols branch from ba50b37 to 22952b9 Compare October 23, 2025 13:46
@kraenhansen kraenhansen merged commit 5c9321b into main Oct 23, 2025
6 checks passed
@kraenhansen kraenhansen deleted the kh/strip-debug-symbols branch October 23, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants