Skip to content

Commit 21c9d56

Browse files
bitsandfoxesbitsandfoxes
authored
fix: Append builds for iOS respect CLI option changes (#2146)
1 parent 64a1ea1 commit 21c9d56

File tree

5 files changed

+132
-60
lines changed

5 files changed

+132
-60
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
### Fixes
66

7-
- When targeting Desktop Platforms, Sentry-CLI now respects the SDK's debug logging verbosity ([#2138](https://github.com/getsentry/sentry-unity/pull/2138))
7+
- When targeting iOS, the SDK now correctly updates the Sentry CLI options used for debug symbol upload when appending builds ([#2146](https://github.com/getsentry/sentry-unity/pull/2146))
8+
- When targeting Desktop Platforms, Sentry CLI now respects the SDK's debug logging verbosity ([#2138](https://github.com/getsentry/sentry-unity/pull/2138))
89
- When targeting iOS and setting `IgnoreCliErrors = true`, the Xcode build will now succeed even if the symbol upload itself failed. This is aimed to allow users to unblock themselves ([#2136](https://github.com/getsentry/sentry-unity/pull/2136))
910
- Sentry CLI no longer requires the 'Organisation' option, and they have been removed from the configuration window. If you're providing an Organisation right now, nothing changes. Fresh setups will have the option omitted ([#2137](https://github.com/getsentry/sentry-unity/pull/2137))
1011

‎src/Sentry.Unity.Editor.iOS/BuildPostProcess.cs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ internal static void AddSentryToXcodeProject(SentryUnityOptions? options,
5959

6060
if (IsNativeSupportEnabled(options, logger))
6161
{
62-
SetupSentry(options, cliOptions, logger, pathToProject);
62+
SetupSentry(options, logger, pathToProject);
6363
}
6464
else
6565
{
@@ -70,6 +70,8 @@ internal static void AddSentryToXcodeProject(SentryUnityOptions? options,
7070
SetupNoOpBridge(logger, pathToProject);
7171
}
7272

73+
SetupSentryCli(options, cliOptions, logger, pathToProject);
74+
7375
// We want to avoid users getting stuck on a cached built output.
7476
// This can happen if the user appends builds
7577
// - and toggles the `IosNativeSupportEnabled` from `true` to `false`
@@ -108,16 +110,13 @@ internal static void SetupNoOpBridge(IDiagnosticLogger logger, string pathToProj
108110
}
109111
}
110112

111-
internal static void SetupSentry(SentryUnityOptions options,
112-
SentryCliOptions? cliOptions,
113-
IDiagnosticLogger logger,
114-
string pathToProject)
113+
internal static void SetupSentry(SentryUnityOptions options, IDiagnosticLogger logger, string pathToProject)
115114
{
116115
logger.LogInfo("Attempting to add Sentry to the Xcode project.");
117116

118117
try
119118
{
120-
// The Sentry.xcframework ends in '~' to hide it from Unity. This prevents Unity from exporting it with the XCode build.
119+
// The Sentry.xcframework ends in '~' to hide it from Unity. This prevents Unity from exporting it with the Xcode build.
121120
// Ideally, we would let Unity copy this over but:
122121
// - Detection of `.xcframework` as datatype and non-folder happened in Unity 2021
123122
// - Without a `.meta` file we cannot opt-in embedding the framework
@@ -137,19 +136,6 @@ internal static void SetupSentry(SentryUnityOptions options,
137136
sentryXcodeProject.AddNativeOptions(options, NativeOptions.CreateFile);
138137
sentryXcodeProject.AddSentryToMain(options);
139138
}
140-
141-
if (cliOptions != null && cliOptions.IsValid(logger, EditorUserBuildSettings.development))
142-
{
143-
logger.LogInfo("Automatic symbol upload enabled. Adding script to build phase.");
144-
145-
SentryCli.CreateSentryProperties(pathToProject, cliOptions, options);
146-
SentryCli.SetupSentryCli(pathToProject, RuntimePlatform.OSXEditor);
147-
sentryXcodeProject.AddBuildPhaseSymbolUpload(cliOptions);
148-
}
149-
else if (options.Il2CppLineNumberSupportEnabled)
150-
{
151-
logger.LogWarning("The IL2CPP line number support requires the debug symbol upload to be enabled.");
152-
}
153139
}
154140
catch (Exception e)
155141
{
@@ -160,6 +146,38 @@ internal static void SetupSentry(SentryUnityOptions options,
160146
logger.LogInfo("Successfully added Sentry to the Xcode project.");
161147
}
162148

149+
internal static void SetupSentryCli(SentryUnityOptions options,
150+
SentryCliOptions? cliOptions,
151+
IDiagnosticLogger logger,
152+
string pathToProject)
153+
{
154+
if (cliOptions is null)
155+
{
156+
logger.LogWarning("No Sentry CLI options provided. You can create them by opening the Configuration Window at Tools -> Sentry");
157+
return;
158+
}
159+
160+
logger.LogInfo("Setting up Sentry CLI.");
161+
162+
if (options.Il2CppLineNumberSupportEnabled && cliOptions.IsValid(logger, EditorUserBuildSettings.development))
163+
{
164+
logger.LogWarning("The IL2CPP line number support requires the debug symbol upload to be enabled.");
165+
}
166+
167+
SentryCli.CreateSentryProperties(pathToProject, cliOptions, options);
168+
SentryCli.SetupSentryCli(pathToProject, RuntimePlatform.OSXEditor);
169+
170+
try
171+
{
172+
using var sentryXcodeProject = SentryXcodeProject.Open(pathToProject, logger);
173+
sentryXcodeProject.AddBuildPhaseSymbolUpload(cliOptions);
174+
}
175+
catch (Exception e)
176+
{
177+
logger.LogError(e, "Failed to add the debug symbol upload build phase.");
178+
}
179+
}
180+
163181
internal static void CopyFramework(string sourcePath, string targetPath, IDiagnosticLogger? logger)
164182
{
165183
if (!Directory.Exists(sourcePath))

‎src/Sentry.Unity.Editor.iOS/SentryXcodeProject.cs

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,36 @@ internal class SentryXcodeProject : IDisposable
2020
internal const string OptionsName = "SentryOptions.m";
2121
internal const string SymbolUploadPhaseName = "SymbolUpload";
2222

23+
internal const string SymbolUploadPropertyName = "SENTRY_CLI_UPLOAD_SYMBOLS";
24+
internal const string IncludeSourcesPropertyName = "SENTRY_CLI_INCLUDE_SOURCES";
25+
internal const string AllowFailurePropertyName = "SENTRY_CLI_ALLOW_FAILURE";
26+
internal const string PrintLogsPropertyName = "SENTRY_CLI_PRINT_LOGS";
27+
2328
internal static readonly string MainPath = Path.Combine("MainApp", "main.mm");
2429
private readonly string _optionsPath = Path.Combine("MainApp", OptionsName);
25-
private readonly string _uploadScript = @"
26-
export SENTRY_PROPERTIES=sentry.properties
27-
echo ""Uploading debug symbols and bcsymbolmaps.""
28-
echo ""Writing logs to './sentry-symbols-upload.log'""
29-
./{0} debug-files upload {1} $BUILT_PRODUCTS_DIR {2}
30-
";
31-
32-
internal const string LogAndPrintArg = "2>&1 | tee ./sentry-symbols-upload.log";
33-
internal const string LogOnlyArg = "2> ./sentry-symbols-upload.log";
30+
private readonly string _uploadScript = @"export SENTRY_PROPERTIES=sentry.properties
31+
if [ ""${" + SymbolUploadPropertyName + @"}"" == ""YES"" ]; then
32+
echo ""Uploading debug symbols and bcsymbolmaps.""
33+
echo ""Writing logs to './sentry-symbols-upload.log'""
34+
35+
ARGS=""--il2cpp-mapping""
36+
37+
if [ ""${" + IncludeSourcesPropertyName + @"}"" == ""YES"" ]; then
38+
ARGS=""$ARGS --include-sources""
39+
fi
40+
41+
if [ ""${" + AllowFailurePropertyName + @"}"" == ""YES"" ]; then
42+
ARGS=""$ARGS --allow-failure""
43+
fi
44+
45+
if [ ""${" + PrintLogsPropertyName + @"}"" == ""YES"" ]; then
46+
./" + SentryCli.SentryCliMacOS + @" debug-files upload $ARGS $BUILT_PRODUCTS_DIR 2>&1 | tee ./sentry-symbols-upload.log
47+
else
48+
./" + SentryCli.SentryCliMacOS + @" debug-files upload $ARGS $BUILT_PRODUCTS_DIR 2> ./sentry-symbols-upload.log
49+
fi
50+
else
51+
echo ""Sentry symbol upload has been disabled via build setting '" + SymbolUploadPropertyName + @"'.""
52+
fi";
3453

3554
private readonly IDiagnosticLogger? _logger = null;
3655
private readonly Type _pbxProjectType = null!; // Set in constructor or throws
@@ -160,37 +179,33 @@ internal void SetSearchPathBuildProperty(string path)
160179

161180
public void AddBuildPhaseSymbolUpload(SentryCliOptions sentryCliOptions)
162181
{
163-
_logger?.LogInfo("Adding automated debug symbol upload script to build phase.");
182+
_logger?.LogInfo("Setting up Sentry CLI build properties and upload script.");
164183

165-
if (MainTargetContainsSymbolUploadBuildPhase())
166-
{
167-
_logger?.LogDebug("Build phase '{0}' already added.", SymbolUploadPhaseName);
168-
return;
169-
}
184+
_pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) })
185+
.Invoke(_project, new object[] { _mainTargetGuid, SymbolUploadPropertyName, sentryCliOptions.UploadSymbols ? "YES" : "NO" });
170186

171-
var uploadDifArguments = "--il2cpp-mapping";
172-
if (sentryCliOptions.UploadSources)
173-
{
174-
uploadDifArguments += " --include-sources";
175-
}
187+
// Set additional upload properties
188+
_pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) })
189+
.Invoke(_project, new object[] { _mainTargetGuid, IncludeSourcesPropertyName, sentryCliOptions.UploadSources ? "YES" : "NO" });
190+
191+
_pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) })
192+
.Invoke(_project, new object[] { _mainTargetGuid, AllowFailurePropertyName, sentryCliOptions.IgnoreCliErrors ? "YES" : "NO" });
176193

177-
if (sentryCliOptions.IgnoreCliErrors)
194+
_pbxProjectType.GetMethod("SetBuildProperty", new[] { typeof(string), typeof(string), typeof(string) })
195+
.Invoke(_project, new object[] { _mainTargetGuid, PrintLogsPropertyName, sentryCliOptions.IgnoreCliErrors ? "NO" : "YES" });
196+
197+
if (MainTargetContainsSymbolUploadBuildPhase())
178198
{
179-
uploadDifArguments += " --allow-failure";
199+
_logger?.LogInfo("Success. Build phase '{0}' was already added.", SymbolUploadPhaseName);
200+
return;
180201
}
181202

182-
var uploadScript = string.Format(_uploadScript,
183-
SentryCli.SentryCliMacOS,
184-
uploadDifArguments,
185-
// Xcode parses the log-messages for 'error:', causing the phase to error with
186-
// 'Command PhaseScriptExecution emitted errors but did not return a nonzero exit code to indicate failure'
187-
// even with the '--allow-failure' arg. In that case, we're just logging to file instead.
188-
sentryCliOptions.IgnoreCliErrors ? LogOnlyArg : LogAndPrintArg);
203+
_logger?.LogDebug("Adding the upload script to {0}.", SymbolUploadPhaseName);
189204

190205
_pbxProjectType.GetMethod("AddShellScriptBuildPhase", new[] { typeof(string), typeof(string), typeof(string), typeof(string) })
191-
.Invoke(_project, new object[] { _mainTargetGuid, SymbolUploadPhaseName, "/bin/sh", uploadScript });
206+
.Invoke(_project, new object[] { _mainTargetGuid, SymbolUploadPhaseName, "/bin/sh", _uploadScript });
192207

193-
_logger?.LogDebug("Successfully added automated debug symbol upload script to build phase.");
208+
_logger?.LogInfo("Success. Added automated debug symbol upload script to build phase.");
194209
}
195210

196211
public void AddNativeOptions(SentryUnityOptions options, Action<string, SentryUnityOptions> nativeOptionFileCreation)

‎test/Sentry.Unity.Editor.iOS.Tests/BuildPostProcessorTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public void SetupSentry_CopiesFrameworkAndBridge()
174174
var options = new SentryUnityOptions();
175175
var testLogger = new TestLogger();
176176

177-
BuildPostProcess.SetupSentry(options, null, testLogger, _outputProjectPath);
177+
BuildPostProcess.SetupSentry(options, testLogger, _outputProjectPath);
178178

179179
var bridgePath = Path.Combine(_outputProjectPath, "Libraries", SentryPackageInfo.GetName(),
180180
SentryXcodeProject.BridgeName);

‎test/Sentry.Unity.Editor.iOS.Tests/SentryXcodeProjectTests.cs

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,38 +161,76 @@ public void AddBuildPhaseSymbolUpload_PhaseAlreadyAdded_LogsAndDoesNotAddAgain()
161161
Regex.Escape(SentryXcodeProject.SymbolUploadPhaseName)).Count;
162162

163163
Assert.IsTrue(_fixture.TestLogger.Logs.Any(log =>
164-
log.logLevel == SentryLevel.Debug &&
164+
log.logLevel == SentryLevel.Info &&
165165
log.message.Contains("already added."))); // Sanity check
166166
Assert.AreEqual(expectedBuildPhaseOccurence, actualBuildPhaseOccurence);
167167
}
168168

169169
[Test]
170-
public void AddBuildPhaseSymbolUpload_IgnoreCliErrorsFalse_UsesLogAndPrintArgs()
170+
public void AddBuildPhaseSymbolUpload_OptionsEnabled_SetsCorrectBuildProperties()
171171
{
172172
var xcodeProject = _fixture.GetSut();
173173
xcodeProject.ReadFromProjectFile();
174174

175175
var sentryCliOptions = ScriptableObject.CreateInstance<SentryCliOptions>();
176+
sentryCliOptions.UploadSymbols = true;
177+
sentryCliOptions.UploadSources = true;
178+
sentryCliOptions.IgnoreCliErrors = true;
179+
180+
xcodeProject.AddBuildPhaseSymbolUpload(sentryCliOptions);
181+
var projectString = xcodeProject.ProjectToString();
182+
183+
StringAssert.Contains($"{SentryXcodeProject.SymbolUploadPropertyName} = YES", projectString);
184+
StringAssert.Contains($"{SentryXcodeProject.IncludeSourcesPropertyName} = YES", projectString);
185+
StringAssert.Contains($"{SentryXcodeProject.AllowFailurePropertyName} = YES", projectString);
186+
StringAssert.Contains($"{SentryXcodeProject.PrintLogsPropertyName} = NO", projectString);
187+
}
188+
189+
[Test]
190+
public void AddBuildPhaseSymbolUpload_OptionsDisabled_SetsCorrectBuildProperties()
191+
{
192+
var xcodeProject = _fixture.GetSut();
193+
xcodeProject.ReadFromProjectFile();
194+
195+
var sentryCliOptions = ScriptableObject.CreateInstance<SentryCliOptions>();
196+
sentryCliOptions.UploadSymbols = false;
197+
sentryCliOptions.UploadSources = false;
176198
sentryCliOptions.IgnoreCliErrors = false;
177199

178200
xcodeProject.AddBuildPhaseSymbolUpload(sentryCliOptions);
201+
var projectString = xcodeProject.ProjectToString();
179202

180-
StringAssert.DoesNotContain("--allow-failure", xcodeProject.ProjectToString()); // sanity check
181-
StringAssert.Contains(SentryXcodeProject.LogAndPrintArg, xcodeProject.ProjectToString());
203+
StringAssert.Contains($"{SentryXcodeProject.SymbolUploadPropertyName} = NO", projectString);
204+
StringAssert.Contains($"{SentryXcodeProject.IncludeSourcesPropertyName} = NO", projectString);
205+
StringAssert.Contains($"{SentryXcodeProject.AllowFailurePropertyName} = NO", projectString);
206+
StringAssert.Contains($"{SentryXcodeProject.PrintLogsPropertyName} = YES", projectString);
182207
}
183208

184209
[Test]
185-
public void AddBuildPhaseSymbolUpload_IgnoreCliErrorsTrue_UsesLogOnlyArgs()
210+
public void AddBuildPhaseSymbolUpload_UploadScriptContainsPropertyReferences()
186211
{
187212
var xcodeProject = _fixture.GetSut();
188213
xcodeProject.ReadFromProjectFile();
189214

190215
var sentryCliOptions = ScriptableObject.CreateInstance<SentryCliOptions>();
191-
sentryCliOptions.IgnoreCliErrors = true;
216+
xcodeProject.AddBuildPhaseSymbolUpload(sentryCliOptions);
217+
var projectString = xcodeProject.ProjectToString();
192218

219+
StringAssert.Contains($"${{{SentryXcodeProject.SymbolUploadPropertyName}}}", projectString);
220+
StringAssert.Contains($"${{{SentryXcodeProject.IncludeSourcesPropertyName}}}", projectString);
221+
StringAssert.Contains($"${{{SentryXcodeProject.AllowFailurePropertyName}}}", projectString);
222+
StringAssert.Contains($"${{{SentryXcodeProject.PrintLogsPropertyName}}}", projectString);
223+
}
224+
225+
[Test]
226+
public void AddBuildPhaseSymbolUpload_ScriptContainsSentryCliPath()
227+
{
228+
var xcodeProject = _fixture.GetSut();
229+
xcodeProject.ReadFromProjectFile();
230+
231+
var sentryCliOptions = ScriptableObject.CreateInstance<SentryCliOptions>();
193232
xcodeProject.AddBuildPhaseSymbolUpload(sentryCliOptions);
194233

195-
StringAssert.Contains("--allow-failure", xcodeProject.ProjectToString());
196-
StringAssert.Contains(SentryXcodeProject.LogOnlyArg, xcodeProject.ProjectToString());
234+
StringAssert.Contains(SentryCli.SentryCliMacOS, xcodeProject.ProjectToString());
197235
}
198236
}

0 commit comments

Comments
 (0)