Skip to content

Use xcworkspace instead of xcodeproj if path is xcworkpace #384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ThomasDutartre
Copy link

@ThomasDutartre ThomasDutartre commented Jul 17, 2025

Checklist

  • I've read and followed the Contribution Guidelines
  • step.yml and README.md is updated with the changes (if needed)

Version

Requires a PATCH version update

Context

Fix multiple workspace project handling issues where:

  1. Commands used -project instead of -workspace for workspace-based projects, causing build latency due to different build cache usage
  2. Build settings parser overwriting main target settings with secondary target values in multi-target schemes
  3. Missing fallback logic when workspace scheme commands fail for secondary targets

With the official step :
image

With our fork :
image

Tested with NotificationService extensions

Changes

Core Architecture

  • Workspace-first logic: Always use -workspace and -scheme commands when workspace is present
  • Consistent build cache usage: Prevents cache invalidation that occurs when mixing -project and -workspace commands
  • Main vs Secondary target handling: Differentiate between main target (uses main scheme) and secondary targets (use target-specific schemes or fallback)
  • Robust fallback mechanism: If workspace scheme fails for secondary targets, fallback to project target commands

Build Settings Parser

  • Add overwrite parameter to parseBuildSettings() and RunAndReturnSettings()
  • Set overwrite=false in all calls to preserve first target in scheme build targets list
  • Prevents settings overwriting in multi-target schemes where later targets would override first target values

Entitlements Handling

  • Fallback logic for entitlements similar to build settings (workspace scheme → project target)
  • Enhanced entitlements resolution for both main and secondary targets

Code Signing Logic

  • Enhanced logging with debug-level details (only visible with --verbose)
  • Target-specific provisioning for main and secondary targets
  • Comprehensive error handling with detailed trace information

Investigation details

Build Cache Issue

Root cause of massive build latency: When working with .xcworkspace files, using -project commands instead of -workspace commands causes Xcode to use different build caches. This results in:

  • Cache invalidation: Xcode treats -project and -workspace builds as separate build contexts
  • Redundant compilation: Files that were already compiled for the workspace get recompiled when using -project
  • Massive time loss: What should be a quick incremental build becomes a near-full rebuild

Solution: Enforce -workspace and -scheme commands consistently when a workspace is present.

Build Settings Overwriting Issue

The main issue was in parseBuildSettings() where duplicate keys were being overwritten. In multi-target schemes, xcodebuild -showBuildSettings outputs settings for all targets sequentially:

Build settings for action build and target "MainApp":
    PRODUCT_BUNDLE_IDENTIFIER = com.company.app
    DEVELOPMENT_TEAM = ABC123

Build settings for action build and target "NotificationService":
    PRODUCT_BUNDLE_IDENTIFIER = com.company.app.notifications
    DEVELOPMENT_TEAM = XYZ789

The parser was keeping the last occurrence (last target in scheme build targets list), but we need the first occurrence (first target in scheme build targets list) for correct bundle ID resolution.

Current implementation: Added overwrite parameter and set overwrite=false in all calls to keep first target in scheme build targets list instead of last target, fixing the issue.

Overwrite Parameter Debate

Current approach: Added overwrite parameter and changed behavior by setting overwrite=false everywhere
Question: Should we remove this parameter and always use overwrite=false?

Arguments for removing the parameter:

  • Simpler API, less configuration
  • First target in scheme build targets list is almost always the correct one for settings resolution
  • Overwriting with later targets' values is rarely desired
  • We already changed the behavior, so no real backward compatibility benefit

Arguments for keeping the parameter:

  • Flexibility for edge cases where someone might want the old behavior (overwrite=true)
  • Explicit control over parsing behavior
  • Future-proofing for different parsing strategies

Need feedback on this design decision 🤔

Next Steps

go-xcode Library Update: Once we have feedback on the overwrite parameter strategy, I will update the upstream go-xcode library with the final implementation:

  • If we decide to remove the parameter: Update go-xcode with overwrite=false hardcoded
  • If we decide to keep the parameter: Update go-xcode with the flexible parameter approach

This ensures consistency between the step implementation and the underlying library.

Decisions

  • Workspace-only enforcement: When workspace is provided, never use -project commands
  • Target differentiation: Main target uses main scheme, secondary targets use target-specific schemes with fallback
  • Conservative approach: Added overwrite parameter for safety (open to removing it)
  • Comprehensive logging: Debug-level logs for troubleshooting, clean output for users
  • Fallback resilience: Multiple fallback strategies for robust workspace handling

Testing

NotificationService extensions: Confirmed working with proper provisioning profiles
Multi-target schemes: Bundle ID resolution works correctly
Fallback scenarios: Secondary targets handled gracefully when scheme commands fail

@sarthak-mishra-swiggy
Copy link

We tried this based on the Bitrise support team's recommendation. While it successfully resolves packages and correctly uses the -scheme flag with the .xcworkspace, it fails when handling nested schemes. For example, our project includes schemes like Notification and App Extension, but when resolving build settings for these child schemes, it continues to use the parent app scheme's name. This results in a certificate mismatch.

@ThomasDutartre
Copy link
Author

We tried this based on the Bitrise support team's recommendation. While it successfully resolves packages and correctly uses the -scheme flag with the .xcworkspace, it fails when handling nested schemes. For example, our project includes schemes like Notification and App Extension, but when resolving build settings for these child schemes, it continues to use the parent app scheme's name. This results in a certificate mismatch.

Thank you for the feedback, i will try to handle this case

@ThomasDutartre ThomasDutartre marked this pull request as draft July 30, 2025 10:29
@ThomasDutartre ThomasDutartre force-pushed the main branch 2 times, most recently from 7a291bf to 8ac649b Compare July 30, 2025 19:15
Add parameter to not overwrite buildsettings
@ThomasDutartre
Copy link
Author

Can you test again @sarthak-mishra-swiggy ? 🙏

I need your opinions about this PR, what do you think about the context and the answer ? Technically it could probably be improved, but i need to know if you agree with the original problem
@godrei @viktorbenei @ofalvai @BirmacherAkos

@ThomasDutartre ThomasDutartre marked this pull request as ready for review July 31, 2025 10:03
@lpusok lpusok self-assigned this Jul 31, 2025
@lpusok lpusok self-requested a review July 31, 2025 13:03
@sarthak-mishra-swiggy
Copy link

sarthak-mishra-swiggy commented Jul 31, 2025

It worked! Appreciate you quickly fixing this!

@sarthak-mishra-swiggy
Copy link

#370 It has already been reported here. So the issue is surely pretty relevant.

@ThomasDutartre
Copy link
Author

It worked! Appreciate you quickly fixing this!

Nice ! Can you share the step time ? One with the "official" step, and the other with this PR 🙏

@sarthak-mishra-swiggy
Copy link

Umm didn't notice a difference in the step time, are we expecting some time based gains with this PR as well?

@ThomasDutartre
Copy link
Author

Umm didn't notice a difference in the step time, are we expecting some time based gains with this PR as well?

On our project, we are gaining 35% of build time

@sarthak-mishra-swiggy
Copy link

@lpusok Any updates on this? Our cache is massively bloated due to this...

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.

3 participants