From 48ec06f5d64683273b8fdbe997dac1d4667cd2d4 Mon Sep 17 00:00:00 2001 From: Trevor Elkins Date: Thu, 16 Oct 2025 11:11:06 -0400 Subject: [PATCH] Add todo labels --- src/launchpad/api/update_api_models.py | 4 +- src/launchpad/artifact_processor.py | 6 +- .../parsers/android/dex/dex_method_parser.py | 10 +- src/launchpad/parsers/apple/macho_parser.py | 4 +- src/launchpad/size/analyzers/apple.py | 4 +- src/launchpad/size/hermes/utils.py | 2 +- .../insights/android/image_optimization.py | 2 +- .../apple/localized_strings_minify.py | 2 +- src/launchpad/size/models/apple.py | 3 +- src/launchpad/size/treemap/treemap_builder.py | 1 - src/launchpad/size/utils/apple_bundle_size.py | 2 +- src/launchpad/utils/apple/apple_strip.py | 2 +- src/launchpad/utils/apple/swift_demangle.py | 136 ------------------ tests/integration/test_swift_demangle.py | 85 ----------- 14 files changed, 17 insertions(+), 246 deletions(-) delete mode 100644 src/launchpad/utils/apple/swift_demangle.py delete mode 100644 tests/integration/test_swift_demangle.py diff --git a/src/launchpad/api/update_api_models.py b/src/launchpad/api/update_api_models.py index aee83a1e..d6d7ec6c 100644 --- a/src/launchpad/api/update_api_models.py +++ b/src/launchpad/api/update_api_models.py @@ -46,7 +46,7 @@ class AppleAppInfo(BaseModel): main_binary_uuid: Optional[str] = None profile_expiration_date: Optional[str] = None certificate_expiration_date: Optional[str] = None - # TODO: add "date_built" field once exposed in 'AppleAppInfo' + # TODO(EME-423): add "date_built" field once exposed in 'AppleAppInfo' class UpdateData(BaseModel): @@ -63,4 +63,4 @@ def serialize_datetime(self, dt: datetime | None) -> str | None: """Serialize datetime objects to ISO format strings for JSON compatibility.""" return dt.isoformat() if dt is not None else None - # TODO: add "date_built" and custom android fields + # TODO(EME-423): add "date_built" and custom android fields diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index e476e863..c03af7b9 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -252,7 +252,7 @@ def _do_distribution( with apk.raw_file() as f: self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) else: - # TODO: Should call _update_artifact_error here once we + # TODO(EME-422): Should call _update_artifact_error here once we # support setting errors just for build. logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id} (project: {project_id}, org: {organization_id})") @@ -429,7 +429,7 @@ def _get_artifact_type(artifact: Artifact) -> ArtifactType: apple_app_info = None if isinstance(app_info, AppleAppInfo): - # TODO: add "date_built" field once exposed in 'AppleAppInfo' + # TODO(EME-423): add "date_built" field once exposed in 'AppleAppInfo' apple_app_info = AppleAppInfoModel( is_simulator=app_info.is_simulator, codesigning_type=app_info.codesigning_type, @@ -440,7 +440,7 @@ def _get_artifact_type(artifact: Artifact) -> ArtifactType: profile_expiration_date=app_info.profile_expiration_date, certificate_expiration_date=app_info.certificate_expiration_date, ) - # TODO: add "date_built" and custom android fields + # TODO(EME-423): add "date_built" and custom android fields update_data = UpdateData( app_name=app_info.name, diff --git a/src/launchpad/parsers/android/dex/dex_method_parser.py b/src/launchpad/parsers/android/dex/dex_method_parser.py index abbc2a3d..f1eaa3d3 100644 --- a/src/launchpad/parsers/android/dex/dex_method_parser.py +++ b/src/launchpad/parsers/android/dex/dex_method_parser.py @@ -1,11 +1,5 @@ from launchpad.parsers.android.dex.dex_mapping import DexMapping -from launchpad.parsers.android.dex.types import ( - AccessFlag, - Annotation, - DexFileHeader, - Method, - Prototype, -) +from launchpad.parsers.android.dex.types import AccessFlag, Annotation, DexFileHeader, Method, Prototype from launchpad.parsers.buffer_wrapper import BufferWrapper @@ -59,7 +53,7 @@ def parse(self) -> Method: prototype=self._prototype, access_flags=self._access_flags, annotations=self._annotations, - parameters=[], # TODO: Implement when needed in future + parameters=[], # TODO(EME-424): Implement when needed in future ) def get_size(self) -> int: diff --git a/src/launchpad/parsers/apple/macho_parser.py b/src/launchpad/parsers/apple/macho_parser.py index deefaefa..47aa1a04 100644 --- a/src/launchpad/parsers/apple/macho_parser.py +++ b/src/launchpad/parsers/apple/macho_parser.py @@ -72,7 +72,7 @@ def get_header_size(self) -> int: # Mach-O header is typically at the beginning # Size varies by architecture but 32 bytes is common for 64-bit header_size = 32 - # TODO: implement proper header size, seems hard to do with LIEF + # TODO(EME-425): implement proper header size, seems hard to do with LIEF return header_size @sentry_sdk.trace @@ -290,7 +290,7 @@ def find_symbol(addr: int) -> lief.Symbol | None: if sym: symbols.append(sym) else: - # TODO: there are some addresses that are not in the symbols list + # TODO(EME-426): there are some addresses that are not in the symbols list # but are present in the FUNCTION_STARTS section. for now we can just # add a placeholder symbol name. symbols.append(f"__mod_init_func_{count}") diff --git a/src/launchpad/size/analyzers/apple.py b/src/launchpad/size/analyzers/apple.py index 1dc4e403..49a5f37a 100644 --- a/src/launchpad/size/analyzers/apple.py +++ b/src/launchpad/size/analyzers/apple.py @@ -231,7 +231,7 @@ def analyze(self, artifact: AppleArtifact) -> AppleAnalysisResults: alternate_icons_optimization=self._generate_insight_with_tracing( AlternateIconsOptimizationInsight, insights_input, "alternate_icons_optimization" ), - # TODO: enable audio/video compression insights once we handle ffmpeg + # TODO(EME-427): enable audio/video compression insights once we handle ffmpeg # audio_compression=self._generate_insight_with_tracing( # AudioCompressionInsight, insights_input, "audio_compression" # ), @@ -349,7 +349,7 @@ def _get_profile_type(self, profile_data: dict[str, Any]) -> Tuple[str, str]: # Check certificate type developer_certs = profile_data.get("DeveloperCertificates", []) if developer_certs: - # TODO: Parse DER certificate to check if it's a development certificate + # TODO(EME-428): Parse DER certificate to check if it's a development certificate # For now, default to development if we have a certificate return "development", profile_name diff --git a/src/launchpad/size/hermes/utils.py b/src/launchpad/size/hermes/utils.py index be62fb2f..75d7e772 100644 --- a/src/launchpad/size/hermes/utils.py +++ b/src/launchpad/size/hermes/utils.py @@ -16,7 +16,7 @@ def make_hermes_reports(file_path: Path) -> Dict[str, HermesReport]: for hermes_file in hermes_files: report = make_hermes_report(hermes_file) if report is not None: - # TODO: Add absolute path support to FileInfo so we can use the absolute path here + # TODO(EME-429): Add absolute path support to FileInfo so we can use the absolute path here reports[str(hermes_file.relative_to(file_path))] = report return reports diff --git a/src/launchpad/size/insights/android/image_optimization.py b/src/launchpad/size/insights/android/image_optimization.py index efeac253..6af8a21c 100644 --- a/src/launchpad/size/insights/android/image_optimization.py +++ b/src/launchpad/size/insights/android/image_optimization.py @@ -25,7 +25,7 @@ def generate(self, input: InsightsInput) -> WebPOptimizationInsightResult | None if file_info.full_path.name.endswith(".9.png"): continue - # TODO: verify that the file is actually an image + # TODO(EME-430): verify that the file is actually an image original_size = get_file_size(Path(file_info.full_path)) with tempfile.NamedTemporaryFile(suffix=".webp", delete=True) as tmp_webp: diff --git a/src/launchpad/size/insights/apple/localized_strings_minify.py b/src/launchpad/size/insights/apple/localized_strings_minify.py index 2f3758f1..bf447b81 100644 --- a/src/launchpad/size/insights/apple/localized_strings_minify.py +++ b/src/launchpad/size/insights/apple/localized_strings_minify.py @@ -50,7 +50,7 @@ def generate(self, input: InsightsInput) -> LocalizedStringCommentsInsightResult total_savings = 0 for file_info in input.file_analysis.files: - # TODO: look into InfoPlist.strings + # TODO(EME-431): look into InfoPlist.strings if not file_info.path.endswith(".strings") or file_info.path.endswith("InfoPlist.strings"): continue diff --git a/src/launchpad/size/models/apple.py b/src/launchpad/size/models/apple.py index f37e210e..aa01f23a 100644 --- a/src/launchpad/size/models/apple.py +++ b/src/launchpad/size/models/apple.py @@ -125,8 +125,7 @@ class MachOBinaryAnalysis: segments: List[SegmentInfo] load_commands: List[LoadCommandInfo] swift_metadata: SwiftMetadata | None = None - # TODO(telkins): try to remove the lief types from this model - # it's only working by coincidence right now + # TODO(EME-432): remove lief types from this model so it's safe to use after the binary is closed symbol_info: SymbolInfo | None = None header_size: int = 0 dyld_info: DyldInfo | None = None diff --git a/src/launchpad/size/treemap/treemap_builder.py b/src/launchpad/size/treemap/treemap_builder.py index a2e9df4e..3199adf2 100644 --- a/src/launchpad/size/treemap/treemap_builder.py +++ b/src/launchpad/size/treemap/treemap_builder.py @@ -39,7 +39,6 @@ def __init__( filesystem_block_size: int | None = None, # Optional presentation tweak: collapse one-child directory chains (off by default) compress_paths: bool = False, - # TODO: Move iOS-specific logic out of constructor binary_analysis_map: Dict[str, MachOBinaryAnalysis] | None = None, class_definitions: list[ClassDefinition] | None = None, hermes_reports: Dict[str, HermesReport] | None = None, diff --git a/src/launchpad/size/utils/apple_bundle_size.py b/src/launchpad/size/utils/apple_bundle_size.py index 54721f83..c7b7be1f 100644 --- a/src/launchpad/size/utils/apple_bundle_size.py +++ b/src/launchpad/size/utils/apple_bundle_size.py @@ -186,5 +186,5 @@ def _zip_metadata_size_for_bundle(bundle_url: Path) -> int: def _get_extra_code_signature_size(bundle_url: Path) -> int: """Calculate additional space needed for code signature.""" - # TODO: Implement actual code signature size calculation + # TODO(EME-433): Implement actual code signature size calculation return 0 diff --git a/src/launchpad/utils/apple/apple_strip.py b/src/launchpad/utils/apple/apple_strip.py index bd6adf3c..55d7ab89 100644 --- a/src/launchpad/utils/apple/apple_strip.py +++ b/src/launchpad/utils/apple/apple_strip.py @@ -43,7 +43,7 @@ def strip( return result def _get_strip_path(self) -> str: - # TODO: eventually remove this and wire this up to our deps tool + # TODO(EME-434): eventually remove this and wire this up to our deps tool system = platform.system() if system == "Darwin": diff --git a/src/launchpad/utils/apple/swift_demangle.py b/src/launchpad/utils/apple/swift_demangle.py deleted file mode 100644 index 99389de5..00000000 --- a/src/launchpad/utils/apple/swift_demangle.py +++ /dev/null @@ -1,136 +0,0 @@ -import os -import re -import subprocess -import tempfile -import uuid - -from typing import Dict, List - -from launchpad.utils.logging import get_logger - -logger = get_logger(__name__) - - -class SwiftDemangler: - """A class to demangle Swift symbol names using the swift-demangle tool.""" - - def __init__(self, remangle: bool = False): - """ - Initialize the NameDemangler. - - Args: - remangle: Whether to use the --remangle-new flag - """ - self.remangle = remangle - self.queue: List[str] = [] - - def add_name(self, name: str) -> None: - """ - Add a name to the demangling queue. - - Args: - name: The mangled name to demangle - """ - self.queue.append(name) - - def demangle_all(self) -> Dict[str, str]: - """ - Demangle all names in the queue. - - Returns: - A dictionary mapping original names to their demangled versions - """ - if not self.queue: - return {} - - names = self.queue.copy() - self.queue.clear() - results: Dict[str, str] = {} - - # Process in chunks to avoid ENOBUFS error - chunk_size = 500 - - for i in range(0, len(names), chunk_size): - chunk = names[i : i + chunk_size] - chunk_results = self._demangle_chunk(chunk) - results.update(chunk_results) - - return results - - def _demangle_chunk(self, names: List[str]) -> Dict[str, str]: - if not names: - logger.warning("No names to demangle") - return {} - - binary_path = self._get_binary_path() - results: Dict[str, str] = {} - - # Create a temporary file for the names - with tempfile.NamedTemporaryFile( - mode="w", prefix=f"swift-demangle-{uuid.uuid4()}-", suffix=".txt" - ) as temp_file: - temp_file.write("\n".join(names)) - temp_file.flush() - - try: - # Build the command - if self.remangle: - command = f"{binary_path} --remangle-new < {temp_file.name}" - else: - command = f"{binary_path} < {temp_file.name}" - - # Execute the command - result = subprocess.run(command, shell=True, capture_output=True, text=True, check=True) - - output_lines = result.stdout.split("\n") - - # Map results back to original names - for i, name in enumerate(names): - results[name] = output_lines[i].strip() if i < len(output_lines) else name - - return results - - except subprocess.CalledProcessError as error: - # Handle partial output and errors - partial_output = error.stdout or "" - stdout_lines = partial_output.split("\n") - error_message = error.stderr or "" - - # Try to extract the failed name from the error message - match = re.search(r"Error: unable to de-mangle (.+)", error_message) - if match and match.group(1): - failed_name = match.group(1).strip() - try: - failed_index = names.index(failed_name) - except ValueError: - logger.error(f"Could not find demangled name error: {error}") - return results - - # Add successful results before the failure - for i in range(failed_index): - results[names[i]] = stdout_lines[i].strip() if i < len(stdout_lines) else names[i] - - # Recursively process the remainder - remainder = names[failed_index + 1 :] - remainder_results = self._demangle_chunk(remainder) - results.update(remainder_results) - - else: - logger.error(f"Demangler error did not match regex: {error}") - - return results - - def _get_binary_path(self) -> str: - platform = os.name - - if platform == "posix": # macOS and Linux - if os.uname().sysname == "Darwin": # macOS - return "xcrun swift-demangle" - elif os.uname().sysname == "Linux": # Linux - # TODO: Implement Linux swift-demangle binary - # For now, raise an error as mentioned in the requirements - raise RuntimeError("Linux swift-demangle binary not yet available") - else: - raise RuntimeError(f"Unsupported POSIX platform: {os.uname().sysname}") - else: - raise RuntimeError(f"Unsupported platform: {platform}") diff --git a/tests/integration/test_swift_demangle.py b/tests/integration/test_swift_demangle.py deleted file mode 100644 index 9ae6b997..00000000 --- a/tests/integration/test_swift_demangle.py +++ /dev/null @@ -1,85 +0,0 @@ -import os - -import pytest - -from launchpad.utils.apple.swift_demangle import SwiftDemangler - - -def is_darwin() -> bool: - """Check if running on macOS.""" - return os.name == "posix" and os.uname().sysname == "Darwin" - - -@pytest.mark.skipif(not is_darwin(), reason="swift-demangle is _currently_ only available on macOS") -class TestSwiftDemangler: - """Integration test cases for the SwiftDemangler class.""" - - def test_init(self): - """Test SwiftDemangler initialization.""" - demangler = SwiftDemangler(remangle=True) - assert demangler.remangle is True - assert demangler.queue == [] - - def test_add_name(self): - """Test adding names to the queue.""" - demangler = SwiftDemangler() - demangler.add_name("_$s3foo3barBaz") - demangler.add_name("_$s3foo3quxQux") - - assert demangler.queue == ["_$s3foo3barBaz", "_$s3foo3quxQux"] - - def test_demangle_all_empty_queue(self): - """Test demangle_all with empty queue.""" - demangler = SwiftDemangler() - result = demangler.demangle_all() - assert result == {} - - def test_demangle_all_success(self): - """Test successful demangling with real swift-demangle.""" - demangler = SwiftDemangler() - demangler.add_name( - "_$s6Sentry0A14OnDemandReplayC8addFrame33_70FE3B80E922CEF5576FF378226AFAE1LL5image9forScreenySo7UIImageC_SSSgtF" - ) - demangler.add_name( - "_$s6Sentry0A18UserFeedbackWidgetC18RootViewControllerC6config6buttonAeA0abC13ConfigurationC_AA0abcd6ButtonF0Ctcfc" - ) - - result = demangler.demangle_all() - - assert len(result) == 2 - assert ( - "_$s6Sentry0A14OnDemandReplayC8addFrame33_70FE3B80E922CEF5576FF378226AFAE1LL5image9forScreenySo7UIImageC_SSSgtF" - in result - ) - assert ( - "_$s6Sentry0A18UserFeedbackWidgetC18RootViewControllerC6config6buttonAeA0abC13ConfigurationC_AA0abcd6ButtonF0Ctcfc" - in result - ) - assert ( - result[ - "_$s6Sentry0A14OnDemandReplayC8addFrame33_70FE3B80E922CEF5576FF378226AFAE1LL5image9forScreenySo7UIImageC_SSSgtF" - ] - == "Sentry.SentryOnDemandReplay.(addFrame in _70FE3B80E922CEF5576FF378226AFAE1)(image: __C.UIImage, forScreen: Swift.String?) -> ()" - ) - assert ( - result[ - "_$s6Sentry0A18UserFeedbackWidgetC18RootViewControllerC6config6buttonAeA0abC13ConfigurationC_AA0abcd6ButtonF0Ctcfc" - ] - == "Sentry.SentryUserFeedbackWidget.RootViewController.init(config: Sentry.SentryUserFeedbackConfiguration, button: Sentry.SentryUserFeedbackWidgetButtonView) -> Sentry.SentryUserFeedbackWidget.RootViewController" - ) - - def test_demangle_all_chunked_processing(self): - """Test that chunked processing works with many names.""" - demangler = SwiftDemangler() - - # Add more than 500 names to test chunking - for i in range(600): - demangler.add_name(f"_$s3foo3bar{i}Baz") - - result = demangler.demangle_all() - - # Should process all names - assert len(result) == 600 - # All names should be processed (even if some fail, they should be in the result) - for i in range(600): - assert f"_$s3foo3bar{i}Baz" in result