Skip to content

Conversation

@pepicrft
Copy link
Owner

This PR adds comprehensive build utilities to build Xcode schemes for iOS Simulator with code signing disabled and identify launchable products.

Changes

New Module: app/src/xcode/build.rs

  • build_scheme() - Builds an Xcode scheme for iOS Simulator with signing disabled
  • Automatic product detection (apps, frameworks, libraries, tests, extensions)
  • get_launchable_products() - Filters build products to return only launchable apps
  • Comprehensive error handling and async execution

New API Endpoints

  • POST /api/xcode/build - Build a scheme for simulator
  • POST /api/xcode/launchable-products - Get launchable products from build directory

Implementation Details

  • Uses xcodebuild with appropriate flags for simulator builds
  • Disables code signing (CODE_SIGN_IDENTITY="", CODE_SIGNING_REQUIRED=NO, CODE_SIGNING_ALLOWED=NO)
  • Outputs to /tmp/plasma-build derived data path
  • Detects and categorizes all build product types
  • Includes unit tests for product filtering logic

Testing

  • All existing tests continue to pass
  • New unit tests added for product filtering
  • Build compiles successfully without errors

Added comprehensive build utilities to build Xcode schemes for iOS Simulator with code signing disabled and identify launchable products.

Features:
- Build schemes for iOS Simulator with signing disabled
- Detect and categorize build products (apps, frameworks, libraries, tests)
- Filter launchable products (apps) from build artifacts
- API endpoints for building and querying build products

New API endpoints:
- POST /api/xcode/build - Build a scheme for simulator
- POST /api/xcode/launchable-products - Get launchable products from build dir

Implementation includes error handling, async execution, and unit tests.
Added Server-Sent Events (SSE) support to stream xcodebuild output in real-time to the frontend.

Changes:
- Added build_scheme_stream() function that streams build output line by line
- New BuildEvent enum with Started, Output, Completed, and Error variants
- SSE endpoint at POST /api/xcode/build/stream for real-time build monitoring
- Dependencies: async-stream, tokio-stream, futures for stream handling

The stream emits JSON events:
- started: When build begins
- output: Each line of build output (stdout/stderr)
- completed: When build finishes with products list
- error: If build process fails

This enables the frontend to show live build progress instead of waiting for the entire build to complete.
Add integration tests for the build functionality using the fixture project:
- test_build_scheme_with_fixture: Tests direct build_scheme() function
- test_build_scheme_stream_with_fixture: Tests streaming build events
- test_build_endpoint_with_fixture: Tests /api/xcode/build endpoint

Tests are macOS-only using #[cfg(target_os = "macos")] and handle both
successful and failed builds gracefully.
Copy link

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 introduces comprehensive build utilities for Xcode schemes, enabling iOS Simulator builds with code signing disabled. The implementation adds both synchronous and streaming build APIs, automatic product detection and categorization, and filtering of launchable applications.

Key changes:

  • New build.rs module with build_scheme() and build_scheme_stream() functions for building Xcode schemes
  • Automatic detection and categorization of build products (apps, frameworks, libraries, tests, extensions)
  • Three new API endpoints: /api/xcode/build, /api/xcode/build/stream, and /api/xcode/launchable-products

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
app/Cargo.toml Adds async streaming dependencies (tokio-stream, futures, async-stream)
app/src/xcode/mod.rs Exports new build module and its public API
app/src/xcode/build.rs Core build utilities with product detection, error handling, and streaming support
app/src/routes/xcode.rs New HTTP endpoints for building schemes and retrieving launchable products
app/src/routes/mod.rs Registers three new Xcode build-related routes
app/tests/xcode_integration_tests.rs Integration tests for build functionality with real Xcode fixtures

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

});
}

let build_dir = "/tmp/plasma-build/Build/Products/Debug-iphonesimulator".to_string();
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The hardcoded build directory path /tmp/plasma-build/Build/Products/Debug-iphonesimulator is used in multiple places throughout the codebase. This creates a maintainability issue if the path ever needs to be changed. Consider extracting this as a constant at the module level, such as const DEFAULT_BUILD_DIR: &str = "/tmp/plasma-build/Build/Products/Debug-iphonesimulator";

Copilot uses AI. Check for mistakes.
let path = Path::new(&request.path);

match xcode::build_scheme(path, &request.scheme).await {
Ok(result) => (StatusCode::OK, Json(serde_json::to_value(result).unwrap())).into_response(),
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The unwrap() call will panic if serialization fails. While serialization of the BuildResult is unlikely to fail, for consistency with error handling patterns in the rest of the code, consider using map_err to convert to a 500 Internal Server Error, or at least use expect() with a descriptive message.

Suggested change
Ok(result) => (StatusCode::OK, Json(serde_json::to_value(result).unwrap())).into_response(),
Ok(result) => match serde_json::to_value(result) {
Ok(value) => (StatusCode::OK, Json(value)).into_response(),
Err(error) => (
StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({ "error": error.to_string() })),
)
.into_response(),
},

Copilot uses AI. Check for mistakes.
Comment on lines 66 to 70
Ok(products) => (
StatusCode::OK,
Json(serde_json::to_value(products).unwrap()),
)
.into_response(),
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The unwrap() call will panic if serialization fails. Consider using map_err to handle potential serialization errors gracefully, or at least use expect() with a descriptive message for clarity.

Suggested change
Ok(products) => (
StatusCode::OK,
Json(serde_json::to_value(products).unwrap()),
)
.into_response(),
Ok(products) => match serde_json::to_value(products) {
Ok(value) => (
StatusCode::OK,
Json(value),
)
.into_response(),
Err(error) => (
StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({ "error": format!("Failed to serialize products: {}", error) })),
)
.into_response(),
},

Copilot uses AI. Check for mistakes.
Comment on lines 392 to 394
assert_eq!(
build_result.build_dir,
"/tmp/plasma-build/Build/Products/Debug-iphonesimulator"
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The hardcoded build directory path /tmp/plasma-build/Build/Products/Debug-iphonesimulator appears in the test assertions. While this matches the implementation, it creates tight coupling between tests and implementation details. If the build directory path changes, these tests will need to be updated. Consider using a constant or extracting the expected path from the build result for more maintainable tests.

Copilot uses AI. Check for mistakes.
Comment on lines 451 to 453
assert_eq!(
build_dir,
"/tmp/plasma-build/Build/Products/Debug-iphonesimulator"
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The hardcoded build directory path /tmp/plasma-build/Build/Products/Debug-iphonesimulator is duplicated in the test assertion. This creates tight coupling between tests and implementation details. Consider using a constant or asserting that the build_dir is non-empty rather than checking the exact path.

Suggested change
assert_eq!(
build_dir,
"/tmp/plasma-build/Build/Products/Debug-iphonesimulator"
assert!(
!build_dir.is_empty(),
"Build directory should not be empty"

Copilot uses AI. Check for mistakes.
}
Ok(None) => {},
Err(e) => {
yield Err(BuildError::ParseError(e.to_string()));
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

In the stderr reading loop, when an error occurs (line 298-300), the error is yielded but the loop continues (no break statement). This differs from the stdout error handling (lines 286-289) which breaks the loop. This inconsistency could lead to continued attempts to read from a failed stderr stream. Consider adding a break statement after yielding the error on line 300 for consistent error handling.

Suggested change
yield Err(BuildError::ParseError(e.to_string()));
yield Err(BuildError::ParseError(e.to_string()));
break;

Copilot uses AI. Check for mistakes.
Comment on lines 156 to 160
if file_name_str.contains("UITests") {
(ProductType::UITest, false)
} else {
(ProductType::UnitTest, false)
}
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The logic to distinguish between UITest and UnitTest products relies on checking if the filename contains "UITests" (case-sensitive). This is fragile as it depends on naming conventions that may not always be followed. For example, a test bundle named "MyUiTests.xctest" or "Myuitests.xctest" would be incorrectly classified as UnitTest. Consider making this check case-insensitive or using additional heuristics to properly categorize test types.

Copilot uses AI. Check for mistakes.
Comment on lines 145 to 165
let (product_type, is_launchable) = if file_name_str.ends_with(".app") {
(ProductType::Application, true)
} else if file_name_str.ends_with(".framework") {
(ProductType::Framework, false)
} else if file_name_str.ends_with(".a") {
(ProductType::StaticLibrary, false)
} else if file_name_str.ends_with(".dylib") {
(ProductType::DynamicLibrary, false)
} else if file_name_str.ends_with(".bundle") {
(ProductType::Bundle, false)
} else if file_name_str.ends_with(".xctest") {
if file_name_str.contains("UITests") {
(ProductType::UITest, false)
} else {
(ProductType::UnitTest, false)
}
} else if file_name_str.ends_with(".appex") {
(ProductType::AppExtension, false)
} else {
continue;
};
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The product type detection logic in find_build_products function (lines 145-165) lacks unit test coverage. While there are integration tests that exercise this code, the logic for distinguishing between different product types (especially the UITest vs UnitTest distinction based on filename) should have dedicated unit tests to ensure the categorization works correctly for various file naming patterns.

Copilot uses AI. Check for mistakes.
Comment on lines 93 to 94
.arg("-derivedDataPath")
.arg("/tmp/plasma-build");
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The hardcoded derived data path /tmp/plasma-build is duplicated in multiple locations. This should be extracted as a constant to ensure consistency and ease of maintenance. The same path appears on lines 94, 250, and is used to construct the build directory on lines 111 and 307.

Copilot uses AI. Check for mistakes.
Comment on lines 103 to 104
Err(_) => {
let error_json = json!({"type": "error", "message": "Stream error"}).to_string();
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The unwrap_or_else with a default empty JSON object is appropriate, but the error case on line 103 silently swallows the build error details. Consider including the error message in the SSE event so clients can distinguish between serialization errors and actual build errors, such as including the error string in the error event data.

Suggested change
Err(_) => {
let error_json = json!({"type": "error", "message": "Stream error"}).to_string();
Err(error) => {
let error_json = json!({
"type": "error",
"message": error.to_string()
})
.to_string();

Copilot uses AI. Check for mistakes.
- Extract hardcoded paths as constants (DERIVED_DATA_PATH, BUILD_PRODUCTS_DIR)
- Make UITest detection case-insensitive
- Add break statement in stderr error handling for consistency
- Replace unwrap() calls with proper error handling in routes
- Add error logging for stream serialization failures
- Remove hardcoded /tmp/plasma-build path
- Use xcodebuild -showBuildSettings to discover build directory
- Parse CONFIGURATION_BUILD_DIR from build settings
- Update tests to check for DerivedData in build path
- Benefits: follows Xcode conventions, enables build caching

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
#[serde(rename_all = "lowercase")]
pub enum ProductType {
Copy link
Owner Author

Choose a reason for hiding this comment

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

We only want to fetch apps, so we can ignore all the other cases.

- Remove all product types except Application
- Simplify find_build_products to only detect .app files
- Update tests to reflect simplified product detection
- All build products are now launchable applications
- Remove ProductType enum since we only detect applications
- Remove is_launchable field from BuildProduct (all products are apps)
- Simplify BuildProduct to only have name and path fields
- Update get_launchable_products to simply clone the input
- Update all tests to reflect simplified structure
@pepicrft pepicrft merged commit de550ca into main Dec 25, 2025
11 checks passed
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