Skip to content

feat: native crash backend as option for Windows#2692

Merged
bitsandfoxes merged 30 commits into
mainfrom
feat/windows-native-backend
Jun 3, 2026
Merged

feat: native crash backend as option for Windows#2692
bitsandfoxes merged 30 commits into
mainfrom
feat/windows-native-backend

Conversation

@bitsandfoxes
Copy link
Copy Markdown
Contributor

wip

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against ad1c58e

@bitsandfoxes bitsandfoxes changed the base branch from main to feat/macos-native May 22, 2026 14:05
@bitsandfoxes bitsandfoxes force-pushed the feat/windows-native-backend branch from 86d91fa to c933961 Compare May 28, 2026 14:18
Base automatically changed from feat/macos-native to main May 28, 2026 14:40
@bitsandfoxes bitsandfoxes marked this pull request as ready for review June 1, 2026 12:28
Comment thread build/native-sdks.targets
Comment on lines +183 to +189
<Target Name="EnsureWindowsNativeSDK"
BeforeTargets="BeforeBuild"
Condition="'$(MSBuildProjectName)' == 'Sentry.Unity'
And $([MSBuild]::IsOSPlatform('Windows'))
And !Exists('$(SentryWindowsNativeArtifactsDestination)sentry.dll')">
<CallTarget Targets="BuildWindowsNativeSDK" />
</Target>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The EnsureWindowsNativeSDK build target runs on every Windows build if sentry.dll is missing, even when the native backend is disabled, causing unnecessary build failures.
Severity: MEDIUM

Suggested Fix

Modify the condition for the EnsureWindowsNativeSDK target to also check if the native backend is enabled for the current build. This could be done by checking a property that indicates the selected backend, preventing the target from running when it's not needed.

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: build/native-sdks.targets#L183-L189

Potential issue: The MSBuild target `EnsureWindowsNativeSDK` in
`build/native-sdks.targets` is triggered on every Windows build where `sentry.dll` is
not present. The condition for this target only checks the OS and the existence of the
DLL, but not whether the native backend is actually enabled for the build. Consequently,
developers on Windows without the C++ build environment (CMake, submodules) will face a
build failure when running `dotnet build` on a fresh checkout, even if they don't intend
to use the native backend. This blocks development for a valid use case.

Did we get this right? 👍 / 👎 to inform future reviews.

@bitsandfoxes bitsandfoxes requested a review from a team June 2, 2026 09:55
Copy link
Copy Markdown
Member

@JoshuaMoelans JoshuaMoelans left a comment

Choose a reason for hiding this comment

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

lgtm!

one small q for the native backend in general; we have these three options (sentry_options_set_minidump_mode , sentry_options_set_crash_reporting_mode and sentry_options_set_crash_upload_mode) for the native backend, would it make sense to expose these in the Unity SDK too?

@bitsandfoxes
Copy link
Copy Markdown
Contributor Author

one small q for the native backend in general; we have these three options (sentry_options_set_minidump_mode , sentry_options_set_crash_reporting_mode and sentry_options_set_crash_upload_mode) for the native backend, would it make sense to expose these in the Unity SDK too?

Fair point. I'll follow up on this.

@bitsandfoxes bitsandfoxes merged commit 57304b6 into main Jun 3, 2026
158 of 160 checks passed
@bitsandfoxes bitsandfoxes deleted the feat/windows-native-backend branch June 3, 2026 10:13
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