-
-
Notifications
You must be signed in to change notification settings - Fork 343
fix: support for shellScript in the Xcode 16 format #1018
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
fix: support for shellScript in the Xcode 16 format #1018
Conversation
|
@fortmarek @cschmatzler @asmitbm, please check when you can. Thank you! |
|
Hey all, any progress on this? /cc @pepicrft |
| isa = PBXCopyFilesBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| dstPath = ""; | ||
| dstSubfolderSpec = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this is another change in Xcode 26. I created an issue for it to tackle separately.
|
|
||
| /// Shell script. | ||
| public var shellScript: String? | ||
| public var shellScript: Either<String, [String]>? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change introduced by Xcode 26 is a breaking change, which explains why it's breaking here too, but I was wondering if we should take a different approach that wouldn't require creating a major version of tuist/XcodeProj. Thoughts @tuist/engineering ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the original issue, each newline represents a new element ... which makes me think, do we need to keep the representation 1-1? Or can we keep the type as String? and just handle this scenario when decoding/encoding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can preserve the format that we read from the project such that we can write back causing no diffs, then yeah. We can maybe have a private property that captures that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would first try if following the newline convention would be enough, but if needed, we could use a private property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will keep the String? as you mentioned @fortmarek. Pushing changes in a minute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Check again please @pepicrft @fortmarek.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I avoided to add the extra private property. I am not sure we need it. I am missing some use cases on when we are encoding the PBXShellScriptBuildPhase model most probably.
I completely missed this one. Thanks for the ping @ileitch Let's address figure out the best path for this first (cc @fortmarek ) |
fortmarek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! this looks good 😌
|
Thanks a lot @michaelversus 🚀. Release in progress... |
|
@all-contributors add @michaelversus for code |
|
I've put up a pull request to add @michaelversus! 🎉 |
Resolves #921
Short description 📝
This MR fixes an issue with projects that use the new format, where for build phases, a shell script is an array of strings instead of a string.
Solution 📦
I am deserializing the script with a fallback approach.
If we fail deserializing using the
Stringtype we are trying the[String]type to ensure that I will not break old xcode version formatImplementation 👩💻👨💻
shellScriptproperty ofPBXShellScriptBuildPhase)PBXShellScriptBuildPhaseTests)