-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add debug symbols upload #59
Merged
Merged
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
d7eb257
Add CLI tools
tustanivsky 4d7e4f9
Add debug symbols upload for iOS, Mac and Linux
tustanivsky 38d5f0a
Add debug symbols upload script for Windows
tustanivsky fd2267b
Fix errors with debug symbols upload on Linux
tustanivsky 7733ae7
Remove redundant script
tustanivsky 22e4db0
Update location of symbols upload scripts
tustanivsky 7023aea
Update .gitignore
tustanivsky 6e7930a
Fix packaging issue on Linux
tustanivsky 4e94707
Add plugin binaries symbols upload
tustanivsky efb1f73
Merge branch 'main' into feature/debug-symbols-upload
tustanivsky fdb6571
Remove Sentry CLI binaries
tustanivsky 9e9b91f
Fix download script
tustanivsky 414a8fa
Fix workflow
tustanivsky ebd3620
Fix script name
tustanivsky 6dffe37
Fix workflow once again
tustanivsky 83670c4
Fix empty line
tustanivsky af65898
Fix typo
tustanivsky c8a69b0
Fix CLI pathj
tustanivsky 16058cf
Remo redundant message
tustanivsky 9281127
Fix CLI download sript
tustanivsky e7ca053
Remo user credentials for debug symbols upload from plugin settings
tustanivsky 5a91498
Update debug symbols upload script for win
tustanivsky eeebbe1
Fix regex
tustanivsky 0c3d486
Update changelog
tustanivsky 9ed8deb
Merge branch 'main' into feature/debug-symbols-upload
tustanivsky 3a0d8a1
Merge branch 'main' into feature/debug-symbols-upload
vaind 42bc2f3
chore: rename cli download script
vaind fc428cd
use sentry.properties as a relative path
vaind 231dff6
fix: android debug symbol upload
vaind 4fa854f
Fix issue with reading sentry.properties file path from settings on Mac
tustanivsky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,3 +6,5 @@ | |
; /README.txt | ||
; /Extras/... | ||
; /Binaries/ThirdParty/*.dll | ||
|
||
/Scripts/... |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
param([string] $TargetPlatform,[string] $TargetName,[string] $TargetType,[string] $ProjectPath,[string] $PluginPath) | ||
|
||
$ProjectBinariesPath = "$ProjectPath\Binaries\$TargetPlatform" | ||
$PluginBinariesPath = "$PluginPath\Source\ThirdParty\$TargetPlatform" | ||
$ConfigPath = "$ProjectPath\Config" | ||
|
||
Write-Host "Sentry: Start debug symbols upload" | ||
|
||
If ($TargetType -eq "Editor") | ||
{ | ||
Write-Host "Sentry: Automatic symbols upload is not required for Editor target. Terminating..." | ||
Exit | ||
} | ||
|
||
If ($TargetPlatform -eq "Win64") | ||
{ | ||
Set-Variable -Name "CliExec" -Value "$PluginPath\Source\ThirdParty\CLI\sentry-cli-Windows-x86_64.exe" | ||
} | ||
Else | ||
{ | ||
Write-Host "Sentry: Unexpected platform $TargetPlatform. Terminating..." | ||
Exit | ||
} | ||
|
||
function ParseIniFile | ||
{ | ||
param([parameter(Mandatory = $true)] [string] $filePath) | ||
|
||
$anonymous = "NoSection" | ||
|
||
$ini = @{} | ||
switch -regex -file $filePath | ||
{ | ||
"^\[(.+)\]$" # Section | ||
{ | ||
$section = $matches[1] | ||
$ini[$section] = @{} | ||
$CommentCount = 0 | ||
} | ||
|
||
"^(;.*)$" # Comment | ||
{ | ||
if (!($section)) | ||
{ | ||
$section = $anonymous | ||
$ini[$section] = @{} | ||
} | ||
$value = $matches[1] | ||
$CommentCount = $CommentCount + 1 | ||
$name = "Comment" + $CommentCount | ||
$ini[$section][$name] = $value | ||
} | ||
|
||
"(.+?)\s*=\s*(.*)" # Key | ||
{ | ||
if (!($section)) | ||
{ | ||
$section = $anonymous | ||
$ini[$section] = @{} | ||
} | ||
$name,$value = $matches[1..2] | ||
$ini[$section][$name] = $value | ||
} | ||
} | ||
|
||
return $ini | ||
} | ||
|
||
$ConfigIni = ParseIniFile "$ConfigPath\DefaultEngine.ini" | ||
$SentrySettingsSection = "/Script/Sentry.SentrySettings" | ||
|
||
$UploadSymbols = $ConfigIni.$SentrySettingsSection.UploadSymbolsAutomatically | ||
|
||
If (("$UploadSymbols" -eq "") -or ("$UploadSymbols" -eq "False")) | ||
{ | ||
Write-Host "Sentry: Automatic symbols upload is disabled in plugin settings. Terminating..." | ||
Exit | ||
} | ||
|
||
Write-Host "Sentry: Parse project settings" | ||
|
||
$SentryProjectName = $ConfigIni.$SentrySettingsSection.ProjectName | ||
$SentryProjectOrg = $ConfigIni.$SentrySettingsSection.OrganisationName | ||
$SentryAuthToken = $ConfigIni.$SentrySettingsSection.AuthToken | ||
|
||
Write-Host "Sentry: Upload started" | ||
|
||
& $CliExec upload-dif --include-sources --log-level info --org $SentryProjectOrg --project $SentryProjectName --auth-token $SentryAuthToken $ProjectBinariesPath $PluginBinariesPath | ||
|
||
Write-Host "Sentry: Upload finished" |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
#!/bin/bash | ||
|
||
export targetPlatform=$1 | ||
export targetName=$2 | ||
export targetType=$3 | ||
export projectPath=$4 | ||
export pluginPath=$5 | ||
|
||
PROJECT_BINARIES_PATH=$projectPath/Binaries/$targetPlatform | ||
PLUGIN_BINARIES_PATH=$pluginPath/Source/ThirdParty/$targetPlatform | ||
CONFIG_PATH=$projectPath/Config | ||
|
||
echo "Sentry: Start debug symbols upload" | ||
|
||
if [ $targetType = "Editor" ]; then | ||
echo "Sentry: Automatic symbols upload is not required for Editor target. Terminating..." | ||
exit | ||
fi | ||
|
||
if [ $targetPlatform = "IOS" ] || [ $targetPlatform = "Mac" ]; then | ||
SENTRY_CLI_EXEC=$pluginPath/Source/ThirdParty/CLI/sentry-cli-Darwin-universal | ||
elif [ $targetPlatform = "Linux" ]; then | ||
SENTRY_CLI_EXEC=$pluginPath/Source/ThirdParty/CLI/sentry-cli-Linux-x86_64 | ||
else | ||
echo "Sentry: Unexpected platform ${targetPlatform}. Terminating..." | ||
exit | ||
fi | ||
|
||
UPLOAD_SYMBOLS=$(awk -F "=" '/UploadSymbolsAutomatically/ {print $2}' ${CONFIG_PATH}/DefaultEngine.ini) | ||
|
||
if [ -z $UPLOAD_SYMBOLS ]; then | ||
echo "Sentry: Automatic symbols upload is disabled in plugin settings. Terminating..." | ||
exit | ||
fi | ||
|
||
if [ $UPLOAD_SYMBOLS != "True" ]; then | ||
echo "Sentry: Automatic symbols upload is disabled in plugin settings. Terminating..." | ||
exit | ||
fi | ||
|
||
echo "Sentry: parse project settings" | ||
|
||
PROJECT_NAME=$(awk -F "=" '/ProjectName/ {print $2}' ${CONFIG_PATH}/DefaultEngine.ini) | ||
ORG_NAME=$(awk -F "=" '/OrganisationName/ {print $2}' ${CONFIG_PATH}/DefaultEngine.ini) | ||
AUTH_TOKEN=$(awk -F "=" '/AuthToken/ {print $2}' ${CONFIG_PATH}/DefaultEngine.ini) | ||
|
||
echo "Sentry: Copy user credentials config file template to home directory" | ||
cp $pluginPath/Resources/sentry.properties $PROJECT_BINARIES_PATH/sentry.properties | ||
|
||
sed -i.backup 's/your-project/'$PROJECT_NAME'/g' $PROJECT_BINARIES_PATH/sentry.properties | ||
sed -i.backup 's/your-org/'$ORG_NAME'/g' $PROJECT_BINARIES_PATH/sentry.properties | ||
sed -i.backup 's/YOUR_AUTH_TOKEN/'$AUTH_TOKEN'/g' $PROJECT_BINARIES_PATH/sentry.properties | ||
|
||
export SENTRY_PROPERTIES=$PROJECT_BINARIES_PATH/sentry.properties | ||
|
||
chmod +x $SENTRY_CLI_EXEC | ||
|
||
$SENTRY_CLI_EXEC upload-dif --include-sources $PROJECT_BINARIES_PATH $PLUGIN_BINARIES_PATH |
This file contains 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
Binary file not shown.
Binary file not shown.
Binary file not shown.
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.
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.
Is this file normally under version control? If so, we shouldn't use it for sentry CLI configuration (a.k.a
sentry.properties
) as that's supposed to be excluded from git due to the auth-token.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.
Yes,
DefaultEngine.ini
is an essential part of any UE project that keeps all of its settings (including plugins) and usually it's under source control. Though I don't think it can cause any issues for plugin users since they are likely to take care of securing their project config files. Or did I get it wrong?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.
Even if it's in a private repo, unless it's a single-person project, it's best not to commit auth tokens into source control. Additionally, this disqualifies any open-source games.
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'd suggest sticking to a single
sentry.propertiies
file, regardless of the platformThere 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.
Don't we have the exact same issue with Unity then? AFAIK there auth token is stored in a separate asset (
SentryCliOptions
scriptible object) which apparently could be a part of the game as wellThere 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.
yes, we use the sentry.properties file on windows without an issue with Unity
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.
Practical illustration why we shouldn't store the settings in the same file as everything else - from the in-repo sample PR (#64):
How can we move that to a different location so the "shared" file doesn't change?
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.
We can replace user credentials here with a path to
sentry.properties
file which won't be publicly accessible. Later we can add some editor tool that will automate the creation of this file. What do you think?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.
sounds good to me
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.
Alright, no more user credentials in
DefaultEngine.ini
. Now in order to obtain auth token and other stuff for Sentry CLI path tosentry.properties
file has to be provided firs. Apparently not an ideal solution since it may vary on different machines, though at least we've removed the risk of compromising sensitive data