fix: cleanup stale native sdk during build - #2793
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { | ||
| logger.LogDebug("Removing stale Sentry artifact from prior build: '{0}'", staleDsym); | ||
| Directory.Delete(staleDsym, recursive: true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: The cleanup logic for Sentry.dylib.dSYM fails because it checks for a directory with Directory.Exists(), but the artifact is actually a file.
Severity: MEDIUM
Suggested Fix
Update the cleanup logic to handle both files and directories. First, check if a directory exists at the path with Directory.Exists() and delete it recursively if it does. Then, add a check using File.Exists() and use File.Delete() if a file is found at the same path. This ensures both stale directories and files are properly removed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/Sentry.Unity.Editor/Native/BuildPostProcess.cs#L269-L273
Potential issue: The cleanup logic for stale build artifacts incorrectly assumes that
`Sentry.dylib.dSYM` is a directory. The build script actually creates this artifact as a
single file. The code uses `Directory.Exists()` to check for its presence, which returns
`false` for a file. As a result, when switching macOS backends between builds, the stale
`Sentry.dylib.dSYM` file from a previous build is not removed, which could lead to build
failures or resolution issues.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 35f83af. Configure here.
| { | ||
| logger.LogDebug("Removing stale Sentry artifact from prior build: '{0}'", staleDsym); | ||
| Directory.Delete(staleDsym, recursive: true); | ||
| } |
There was a problem hiding this comment.
Stale dSYM treated as directory
Medium Severity
CleanupStaleMacOSArtifacts removes Sentry.dylib.dSYM only when Directory.Exists is true, but the Cocoa backend stores that artifact as a file (the DWARF binary copied by build-cocoa-sdk.ps1). GetNativePluginArtifact then copies that file into Contents/MacOS, so after switching backends the stale dSYM is never deleted. The new test creates a directory and therefore misses this failure mode.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 35f83af. Configure here.
|
got gaslit.. |
1 participant
Fixes an issue when switching native backend from cocoa to native or the other way around.