Skip to content
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

Alphabetically sort schemes and provide an API to hide them #3598

Merged
merged 11 commits into from
Oct 27, 2021

Conversation

pepicrft
Copy link
Contributor

@pepicrft pepicrft commented Oct 26, 2021

Short description 📝

Xcode projects have a file, xcschememanagement.plist that provides additional configuration for schemes. In particular, it allows specifying the order of the scheme, and whether they are visible in the schemes dropdown menu.
This PR adds a new API, Scheme(hidden: true) to hide them, and adjusts the generation logic to sort them alphabetically.

I've also taken the opportunity to diff new and old schemes and delete the ones that are no longer necessary. I noticed while doing tuist cache warm and tuist focus that some of the schemes necessary for warming remained in the focused project.

Checklist ✅

  • The code architecture and patterns are consistent with the rest of the codebase.
  • The changes have been tested following the documented guidelines.
  • The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
  • In case the PR introduces changes that affect users, the documentation has been updated.
  • In case the PR introduces changes that affect how the cache artifact is generated starting from the TuistGraph.Target, the Constants.cacheVersion has been updated.

@pepicrft
Copy link
Contributor Author

Note: Fixing and adding tests is pending

Pedro Piñera Buendía and others added 3 commits October 26, 2021 17:45
Co-authored-by: Daniele Formichelli <df@bendingspoons.com>
Co-authored-by: Daniele Formichelli <df@bendingspoons.com>
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @pepibumur!

while testing I noticed I was getting generation errors 🤔

$ tuist generate
Generating workspace Workspace.xcworkspace
Generating project MainApp
Generating project Framework1
Generating project Framework2
We received an error that we couldn't handle:
    - Localized description: The file “xcschememanagement.plist” doesn’t exist.
    - Error: Error Domain=NSCocoaErrorDomain Code=4 "The file “xcschememanagement.plist” doesn’t exist." UserInfo={NSFilePath=/path/to/tuist/projects/tuist/fixtures/ios_app_with_custom_scheme/Frameworks/Framework1/Framework1.xcodeproj/xcuserdata/kwridan.xcuserdatad/xcschemes/xcschememanagement.plist, NSUnderlyingError=0x7f851b510fa0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}

...

I'll take a closer look to also remind myself how it works 😅

Comment on lines 70 to 74
// If we don't delete the schemes that we no longer need they'll remain as leftovers and show up
// on the schemes dropdown menu
try Set(currentSchemes).subtracting(writtenSchemes).forEach { schemeToDelete in
try FileHandler.shared.delete(schemeToDelete)
}
Copy link
Collaborator

@kwridan kwridan Oct 26, 2021

Choose a reason for hiding this comment

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

There are two set of schemes Tuist writes to today,

  • Shared schemes
    • They live in xcshareddata/xcschemes/
    • Those are fully controlled by Tuist
  • Non-shared schemes (aka User schemes)
    • They live in xcuserdata/<username>.xcuserdatad/xcschemes/
    • Those are written to by Tuist however they aren't fully managed by it
    • Users can create their own local schemes that are preserved between generations
      • Examples of this include custom launch parameters / arguments / etc... which aren't enabled by default for the entire team

We need to be mindful of not wiping schemes/data from xcuserdata - by design we previously didn't remove user schemes on regeneration to persevere this

Currently XcodeProj.XCSharedData takes care of replacing the entire xcshareddata alleviating the need for us to keep tabs on which schemes to add/remove.

I had a quick check and indeed there is the issue you described about some lingering shared schemes, I believe those are Workspace schemes (within Workspace.xcworkspace/xcshareddata/xcschemes/) which we had to manually write to in Tuist as XCWorkspace doesn't have an XCSharedData like XcodeProj does - we can safely replace the contents of xcshareddata for shared schemes (possibly replace the entire directory or deleting and re-creating it, worth a sanity check to see if Xcode tolerates it, will check this too)

- Improved naming of variables / methods to clarify which schemes are being written at which stage
  - When writing projects, XcodeProj deals with writing shared schemes vs Tuist writes user schmes
  - When writing workspaces, Tuist writes both shared and user schemes (XcodeProj doesn't offer an option for this)
- Add support for clearing shared workspace schemes
- Fixed missing propagation of `hidden` property
- Fixed issue where scheme management for projects didn't list all schemes
- Updated tests for which schemes are replaces vs preserved
@kwridan
Copy link
Collaborator

kwridan commented Oct 27, 2021

Having re-read the code I realised there were a few non obvious things (well done past me 😅)

The main bits were

  • When writing projects, XcodeProj deals with writing shared schemes vs Tuist writes user schemes
  • When writing workspaces, Tuist writes both shared and user schemes
  • The enriching method did more than enrich, it also swapped out the scheme descriptors 🙈

I pushed a few fixes and improvements - hope this helps!

@pepicrft
Copy link
Contributor Author

Thanks a lot for reviewing this one @kwridan & @danyf90 and pushing some commits to it. By working on this I realized how much context I lost in some areas of the codebase.

@pepicrft pepicrft merged commit 46c4730 into main Oct 27, 2021
@pepicrft pepicrft deleted the scheme-improvements branch October 27, 2021 17:58
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