Skip to content

Integration-test building without sentry SDK only on main branch #785

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

Merged
merged 2 commits into from
Jun 8, 2022

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Jun 7, 2022

I've originally made the change to ci.yml in #773, but to my surprise, it actually broke builds - suddenly some symbols weren't uploaded. Turns out this is also reproducible locally - the first build doesn't include *.dbg.so/*.sym.so symbols.

Update: the cause was that we relied on PlayerSettings.GetScriptingBackend(EditorUserBuildSettings.selectedBuildTargetGroup) but our Builder.cs didn't actually set the EditorUserBuildSettings.selectedBuildTargetGroup. So this wasn't really an issue for end-users, just the issue of our tests.

Before finally finding this out, I've thought it was the issue of Unity not putting the il2cpp symbols in the output folder, so I've originally collected these from Unity editor Data folder. I've reverted this change in the final version of the PR, but kept some refactorings that were originally needed to make it (and tests) work.

#skip-changelog

@vaind vaind added the Bug Something isn't working label Jun 7, 2022
@vaind vaind force-pushed the fix/first-build-symbol-upload branch from 596f80b to b039a35 Compare June 7, 2022 16:46
@vaind vaind force-pushed the fix/first-build-symbol-upload branch 2 times, most recently from 1dbf020 to b4b64f1 Compare June 8, 2022 06:26
it wasn't set during scripted builds...
@vaind vaind force-pushed the fix/first-build-symbol-upload branch from b4b64f1 to 1008fef Compare June 8, 2022 08:37
@vaind vaind removed the Bug Something isn't working label Jun 8, 2022
@vaind vaind marked this pull request as ready for review June 8, 2022 09:48
@vaind vaind enabled auto-merge (squash) June 8, 2022 10:09
@@ -229,6 +229,8 @@ jobs:
run: sudo pwsh ./test/Scripts.Integration.Test/integration-create-project.ps1 -UnityPath "${{ env.UNITY_PATH }}"

- name: Build without Sentry SDK
# This hasn't broken for many months, so disabling on PRs to speed up CI. And also to test a clean build with Sentry SDK included.
Copy link
Contributor

Choose a reason for hiding this comment

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

A surprise but a welcome one!

private string _symbolUploadTask = @"
// Credentials and project settings information are stored in the sentry.properties file
gradle.taskGraph.whenReady {{
gradle.taskGraph.allTasks[-1].doLast {{
println 'Uploading symbols to Sentry'
println 'Uploading symbols to Sentry. You can find the full log in ./Logs/sentry-symbols-upload.log (the file content may not be strictly sequential because it\'s a merge of two streams).'
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

uploadTaskFilter = Regex.Replace(uploadTaskFilter, @"\[", "\\[");

gradleBuildFile = Regex.Replace(gradleBuildFile, uploadTaskFilter, "");
var regex = new Regex(Regex.Escape(SymbolUploadTaskStartComment) + ".*" + Regex.Escape(SymbolUploadTaskEndComment), RegexOptions.Singleline);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing that!

@vaind vaind merged commit 859c2f7 into main Jun 8, 2022
@vaind vaind deleted the fix/first-build-symbol-upload branch June 8, 2022 15:10
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.

2 participants