Skip to content

fix(binary): drop intermediate static library search - #67

Merged
soimkim merged 2 commits into
mainfrom
fix/out-binary-search-paths
Sep 29, 2026
Merged

soimkim merged 2 commits into
mainfrom
fix/out-binary-search-paths

Conversation

@soimkim

@soimkim soimkim commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Stop searching obj/STATIC_LIBRARIES and out/soong/.intermediates for .a files. Those are link inputs, not binaries installed on the target.
  • Linked .so files and executables stay in the report through the existing system/ and apex/<module>/ searches.
  • Missing obj/STATIC_LIBRARIES no longer makes find print No such file or directory.

Test plan

  • Android 12 tree with obj/STATIC_LIBRARIES does not list lib*.a from that directory.
  • Android 17 generic_arm64 scan does not print find: .../obj/STATIC_LIBRARIES: No such file or directory.
  • Report still includes system/bin executables, system/lib64/*.so, and apex/<module>/ binaries.

Skip find on missing product directories and read target .a files
from out/soong when obj/STATIC_LIBRARIES does not exist.
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: fosslight/fosslight_android_scanner/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: f6f0d303-a081-4abd-a0c3-cb35cd5ca973

📥 Commits

Reviewing files that changed from the base of the PR and between 38d31aa and ef7ddb2.

📒 Files selected for processing (1)
  • src/fosslight_android/android_binary_analysis.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Binary discovery now adds search commands only for existing directories. Module lookup retains relative directories for paths under obj/STATIC_LIBRARIES.

Changes

Binary discovery

Layer / File(s) Summary
Search and module lookup
src/fosslight_android/android_binary_analysis.py
Search commands are added only when their directories exist. The dedicated obj/STATIC_LIBRARIES archive search is removed. Paths under that directory retain their relative directories during module lookup.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: bjk7119

Merge Risk: ⚪ Minimal · up to ef7dd

The change skips searches for missing directories, which avoids errors on newer Android builds. No concrete merge-blocking risk was found in the supplied changes.

Architecture Summary

Architecture risk: 🟡 Medium · up to ef7dd

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/fosslight_android/android_binary_analysis.py: Added _add_find_cmd, which appends a search command when its directory exists and otherwise logs that the path was skipped.
  • observed — Modified behavior in src/fosslight_android/android_binary_analysis.py: find_binaries_from_out_dir now builds its search-command list incrementally, adding system, root, top-level build-output, recursive build-output, and font searches only when their directories exist. The dedicated obj/STATIC_LIBRARIES archive search was removed.
  • observed — Modified behavior in src/fosslight_android/android_binary_analysis.py: Removed the special case that reduced obj/STATIC_LIBRARIES binary paths to basenames before module lookup; those paths now retain their relative directories.

Reliability and maintainability

  • inferred — Risk-relevant change factors for src: blast_radius_1; direct_dependents_1
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly states that the pull request removes intermediate static library searching. This matches the reported code changes.
Description check ✅ Passed The description accurately explains the removed searches, retained binary searches, missing-directory behavior, and test plan. It is directly related to the changeset.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Linked .so and executables are already collected from system and
apex. Static .a files are not installed on the target.
@soimkim soimkim changed the title fix(binary): search soong static libs when obj path is gone fix(binary): drop intermediate static library search Sep 29, 2026
@soimkim soimkim self-assigned this Sep 29, 2026
@soimkim soimkim added the enhancement New feature or request label Sep 29, 2026
@soimkim
soimkim merged commit 0fb74a0 into main Sep 29, 2026
6 of 7 checks passed
@soimkim
soimkim deleted the fix/out-binary-search-paths branch September 29, 2026 05:47
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.

1 participant