fix: Append builds for iOS respect CLI option changes
#2146
+132
−60
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bitsandfoxes
commented
May 12, 2025
Comment on lines
-141
to
-152
| if (cliOptions != null && cliOptions.IsValid(logger, EditorUserBuildSettings.development)) | ||
| { | ||
| logger.LogInfo("Automatic symbol upload enabled. Adding script to build phase."); | ||
|
|
||
| SentryCli.CreateSentryProperties(pathToProject, cliOptions, options); | ||
| SentryCli.SetupSentryCli(pathToProject, RuntimePlatform.OSXEditor); | ||
| sentryXcodeProject.AddBuildPhaseSymbolUpload(cliOptions); | ||
| } | ||
| else if (options.Il2CppLineNumberSupportEnabled) | ||
| { | ||
| logger.LogWarning("The IL2CPP line number support requires the debug symbol upload to be enabled."); | ||
| } |
There was a problem hiding this comment.
I moved this out of SetupSentry into SetupSentryCli.
SetupSentry is responsible for adding the framework and modifying the Xcode project to have the options and bridge set.
SetupSentryCli adds the executable, writes the .properties and adds the script to invoke the symbol upload.
bitsandfoxes
commented
May 12, 2025
Comment on lines
+162
to
+165
| if (options.Il2CppLineNumberSupportEnabled && cliOptions.IsValid(logger, EditorUserBuildSettings.development)) | ||
| { | ||
| logger.LogWarning("The IL2CPP line number support requires the debug symbol upload to be enabled."); | ||
| } |
There was a problem hiding this comment.
We want to log configuration that is allowed but that we already know might not yield the expected result. I.e. the user has LineNumberSupport enabled but skips the symbol upload.
bitsandfoxes
commented
May 12, 2025
Comment on lines
+188
to
+203
| _pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) }) | ||
| .Invoke(_project, new object[] { _mainTargetGuid, IncludeSourcesPropertyName, sentryCliOptions.UploadSources ? "YES" : "NO" }); | ||
|
|
||
| _pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) }) | ||
| .Invoke(_project, new object[] { _mainTargetGuid, AllowFailurePropertyName, sentryCliOptions.IgnoreCliErrors ? "YES" : "NO" }); | ||
|
|
||
| if (sentryCliOptions.IgnoreCliErrors) | ||
| _pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) }) | ||
| .Invoke(_project, new object[] { _mainTargetGuid, PrintLogsPropertyName, sentryCliOptions.IgnoreCliErrors ? "NO" : "YES" }); | ||
|
|
||
| if (MainTargetContainsSymbolUploadBuildPhase()) | ||
| { | ||
| uploadDifArguments += " --allow-failure"; | ||
| _logger?.LogInfo("Success. Build phase '{0}' was already added.", SymbolUploadPhaseName); | ||
| return; | ||
| } | ||
|
|
||
| var uploadScript = string.Format(_uploadScript, | ||
| SentryCli.SentryCliMacOS, | ||
| uploadDifArguments, | ||
| // Xcode parses the log-messages for 'error:', causing the phase to error with | ||
| // 'Command PhaseScriptExecution emitted errors but did not return a nonzero exit code to indicate failure' | ||
| // even with the '--allow-failure' arg. In that case, we're just logging to file instead. | ||
| sentryCliOptions.IgnoreCliErrors ? LogOnlyArg : LogAndPrintArg); | ||
| _logger?.LogDebug("Adding the upload script to {0}.", SymbolUploadPhaseName); |
There was a problem hiding this comment.
These properties can get overwritten via Unity's PBXProject. The shellscript itself cannot. Attempting to overwrite the script causes unexpected behaviour like adding it twice.
Instead, the properties get set (and overwritten) during the Unity export and read by the script during the Xcode build
vaind
approved these changes
May 12, 2025
Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1973
There are a couple of limitations with the PBXProject API, namely the ability to remove or overwrite an existing shell script in the build phase. When building via
appendthis caused CLI options to not get copied over and respected by Xcode.Instead of hardcoding arguments the shell script is now controlled by build settings that can be modified when building.