Skip to content

Comments

Migrate action query from text to protobuf parsing#40

Merged
rockbruno merged 10 commits intospotify:mainfrom
sean7218:sean7218/action-query-proto
Aug 18, 2025
Merged

Migrate action query from text to protobuf parsing#40
rockbruno merged 10 commits intospotify:mainfrom
sean7218:sean7218/action-query-proto

Conversation

@sean7218
Copy link
Contributor

@sean7218 sean7218 commented Aug 11, 2025

Summary

This PR modernizes the action query functionality by migrating from plain text parsing to protocol buffer (protobuf) parsing for Bazel action query results. This change improves reliability, type safety, and maintainability of compiler argument extraction.

📝 Key Changes

  • Replaced text-based parsing with protobuf parsing in BazelTargetAquerier
  • Added --output proto flag to all aquery commands to generate protobuf output
  • Simplified argument processing logic in CompilerArgumentsProcessor by leveraging structured data
  • Updated test resources with binary protobuf files (aquery.pb and aquery_objc.pb)
  • Enhanced command runner with dedicated bazelActionQuery method for protobuf data handling

✨ Benefits

  1. Improved Reliability: Eliminates fragile text parsing that could break with format changes
  2. Type Safety: Leverages Swift's type system with protobuf-generated structs
  3. Better Performance: More efficient binary parsing vs. text processing
  4. Enhanced Maintainability: Cleaner, more readable code with structured data access
  5. Future-Proof: Protobuf format is stable and versioned by Bazel

🧪 Testing

  • Updated all test expectations to use protobuf output format
  • Added binary test resources for both Swift and Objective-C action queries
  • Maintained existing test coverage while improving test data accuracy

  - append command string with "--output proto"
  - added CompilerArgError for target not found when processing compiler arguments
  - added back skip `swiftc` and `worker` arguments
  - added aquery.pb and aquery_objc.pb as mocking data
  - updated expected aquery by append "--output proto"
@sean7218 sean7218 requested a review from a team as a code owner August 11, 2025 02:58
@sean7218 sean7218 force-pushed the sean7218/action-query-proto branch from 387063a to 1beb0cb Compare August 11, 2025 03:09
Copy link
Member

@rockbruno rockbruno left a comment

Choose a reason for hiding this comment

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

Nice! Just some smaller points:

@sean7218 sean7218 force-pushed the sean7218/action-query-proto branch from ba38037 to cc8f3d9 Compare August 14, 2025 10:53
@sean7218 sean7218 requested a review from rockbruno August 16, 2025 04:30
Copy link
Member

@rockbruno rockbruno left a comment

Choose a reason for hiding this comment

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

Works perfectly. Thanks Sean!

@rockbruno rockbruno merged commit 2909a82 into spotify:main Aug 18, 2025
1 check passed
@sean7218 sean7218 deleted the sean7218/action-query-proto branch August 18, 2025 11:07
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.

2 participants